Implement DKG Contracts#9
Open
jannikluhn wants to merge 17 commits into
Open
Conversation
Introduces the DKGContract with its four immutables (PHASE_LENGTH, DKG_LEAD_LENGTH, keyperSetManager, keyBroadcastContract) and a matching deploy fixture. Phase arithmetic, bulletin-board functions, and success voting follow in subsequent commits.
Adds Phase enum (None/Dealing/Accusing/Apologizing/Finalizing), cycleLength(), dkgStart(keyperSetIndex, retryCounter), and currentPhase(keyperSetIndex, retryCounter) to DKGContract. DKG instance (k, r) starts at: activation_block(k) - DKG_LEAD_LENGTH + r * cycleLength() All phase windows are derived arithmetically from that start; no per-instance block is stored. dkgStart returns int256 so it can go negative when lead exceeds the activation block (edge case handled without reverting). Tests cover all phase boundaries for r=0, the r=0→r=1 retry boundary, dkgStart arithmetic, and the negative-start edge case.
Adds a standalone registry where Keypers register their ECIES encryption public keys. Membership is verified via KeyperSetManager at registration time; keys persist across Keyper Set transitions so a new KeyperSet deployment does not require re-registration. Key design decisions: - Uses OZ EnumerableSet.AddressSet so off-chain services can backfill the full set of registered keys via getKeyperCount()/getKeyperAt(). - Re-registration (key update) is accepted: add() is idempotent so the set size does not grow on re-registration. - KeyperSetNotFinalized is surfaced from KeyperSet when the set is not yet finalized. Also updates Deploy.gnosh.s.sol and Deploy.s.sol to deploy DKGContract and ECIESKeyRegistry alongside the existing contracts, and wires DKGContract as the KeyperSet publisher in Deploy.s.sol. Tests cover basic registration, key lookup, identity/membership checks, arbitrary-bytes acceptance, and the iterable enumeration API.
Adds submitDealing (Dealing Phase), submitAccusation (Accusing Phase), and submitApology (Apologizing Phase) to DKGContract. Each validates phase, identity, and the succeeded[k] guard, then emits an event with no on-chain cryptographic verification and no deduplication. Key decisions: - submitDealing accepts commitment and polyEval together; callers cannot submit them separately. - Empty accusedIndices reverts with EmptyAccusation. An empty accusation has no protocol meaning; clients should skip the call. - Empty apology arrays (length 0) are accepted as a valid no-op. Mismatched array lengths revert with MismatchedArrays. - succeeded[k] mapping is declared here as a read-only gate for the bulletin-board functions; the write logic is owned by issue #4. - Events use 3 indexed fields (keyperSetIndex, retryCounter, keyperIndex) for efficient off-chain filtering. - A shared _checkMember helper calls KeyperSet(manager.getKeyperSetAddress(k)).getMember(keyperIndex) and requires msg.sender match — reverts NotKeyperAtIndex. Tests cover phase/identity/array-validation for all three functions.
Implements submitSuccessVote(keyperSetIndex, retryCounter, keyperIndex, eonPublicKey) on DKGContract. Accepts votes only during the Finalizing phase, verifies caller membership, and prevents double-voting per (keyperSetIndex, retryCounter, voter). When a key hash reaches KeyperSet.getThreshold(): - succeeded[keyperSetIndex] = true (gates all message functions) - KeyBroadcastContract.broadcastEonKey is invoked in try/catch so an already-broadcast key (or unauthorized publisher) does not block recording success. - DKGSucceeded event is emitted regardless of the broadcast outcome. Late-arriving votes within the same Finalizing window are accepted and emit SuccessVoteSubmitted but do not re-trigger success once it has already been recorded. State added: voteCount[k][r][keyHash], hasVoted[k][r][voter]. Events added: SuccessVoteSubmitted, DKGSucceeded. Error: AlreadyVoted. setUp now sets the DKGContract as keyperSet0's publisher before finalization so the threshold-reached broadcast succeeds in the default fixture. The silent-revert case constructs its own keyper set with a different publisher. Tests cover phase rejection, identity, double-vote, threshold counting (per-key-hash isolation, no broadcast below threshold), silent broadcast failure, post-success rejection across all message types, concurrent keyper-set independence, and the r=0/r=1 retry boundary.
Replace the opaque ABI-encoded bytes blob with a native bytes[] so the Go binding handles encoding and decoding without bespoke helpers. Key decisions: - Empty arrays are accepted for backward compatibility with the existing testConcurrentDKGInstancesAreIndependent / retry-dealing tests, which previously passed an empty bytes blob. Files changed: - src/common/DKGContract.sol: submitDealing argument and DealingSubmitted event now use bytes[] for polyEvals - test/DKGContract.t.sol: all submitDealing call sites and the mirrored event signature updated to bytes[]
Add `dkgContract` slot, owner-only pre-finalization setter `setDKGContract`, and `getDKGContract` view getter to KeyperSet.sol. Expose `getDKGContract` on the IKeyperSet interface so the Go keyper can read it through the typed binding when handling KeyperSetAdded. Tests: - forge test passes (116 tests, incl. new testDKGContract, testSetDKGContractOnlyOwner, and the after-finalized revert case) - go build ./... passes for the regenerated keyperset binding Files: - src/common/KeyperSet.sol - src/common/intf/IKeyperSet.sol - test/KeyperSet.t.sol - bindings/keyperset/keyperset.go (regenerated) Notes for next iteration: - E2E tests in mise-test-setup were not run: the foundry/anvil docker image is linux/amd64 and the sandbox is arm64, so the ethereum container fails with `exec format error`. Acceptance criteria of this task only require forge test + go build. - Unblocks docs/dkg-module-refactor/contract-updates/003 (eons per- keyperset phase params), which will call GetDKGContract from Go and store phase_length/lead_length on the eons row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Forge test contract `DKGBenchmark` that measures total on-chain gas for a clean DKG run (n submitDealing calls + ⌈2n/3⌉ submitSuccessVote calls) at n = 3, 5, 10, 20, 50. Setup gas — contract deployment and keyper set creation — is excluded; only protocol operation cost is accumulated via gasleft() deltas around each contract call. Key decisions: - Realistic, non-zero (0xAB) payload sizes from the PRD: commitment = 4 + threshold × 96 bytes, each PolyEvals entry = 129 bytes (ECIES), EonPublicKey = 96 bytes. Non-zero bytes cost 16 gas/byte in calldata and match real BLS/ECIES data. - Threshold via integer ⌈2n/3⌉ = (2n + 2) / 3. - Fresh KeyperSetManager / KeyBroadcastContract / DKGContract per test so warm-storage artefacts from earlier runs do not contaminate results. - Per-test console.log line emits scenario, n, threshold, and gas total for easy parsing. Measured gas (monotonic by design — quadratic calldata dominates): n=3 → 293,144 n=5 → 457,151 n=10 → 901,165 n=20 → 2,368,268 n=50 → 10,962,537 Files changed: - test/DKGBenchmark.t.sol (new) Notes for next iteration: - 002-worst-case-benchmark extends this with Accusing + Apologizing phases, accumulating into the same totalGas counter and adding five test_worstCase_n* entry points. - Rolling-shutter e2e suite could not be exercised in this sandbox (postgres/sqlc downloads firewall-blocked); the change is purely additive Forge test code so it cannot regress Go paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refactor _runHappyPath into a parameterized _run(n, worstCase) and
extend it with accusing/apologizing phases for the worst-case scenario.
Add test_worstCase_n{3,5,10,20,50} entry points.
The worst case has every keyper accuse all n-1 others in one tx, then
respond to all n-1 accusers in one apology tx with plaintext 32-byte
BLS12-381 scalars (NOT 129-byte ECIES ciphertext — the accused keyper
reveals the eval in the clear).
Key decisions:
- Single _run function (parameterized) over duplicating happy-path code.
- Apology polyEvalData entries are 32 bytes per PRD: plaintext scalar.
- Accusing block = dealingBlock + PHASE_LENGTH;
apologizing block = dealingBlock + 2 * PHASE_LENGTH;
finalizing block unchanged.
Files: contracts/test/DKGBenchmark.t.sol.
Verified all 10 tests pass; worst-case gas strictly exceeds happy-path
at each n (n=50: 22M worst-case vs 11M happy-path).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AddKeyperSet Service deployment now also deploys DKGContract and ECIESKeyRegistry, so the e2e tests can exercise the full DKG cycle against the shutterservice keyper. AddKeyperSet (both common and gnosh) accept an optional DKG_CONTRACT_ADDRESS env var and call setDKGContract on the new KeyperSet when provided. Key decisions: - Reuse the existing deployDKGContract / deployECIESKeyRegistry helpers (already shared with Deploy.gnosh.s.sol) instead of forking. Reads the same DKG_PHASE_LENGTH / DKG_LEAD_LENGTH env defaults. - AddKeyperSet treats DKG_CONTRACT_ADDRESS as optional: if unset, no setDKGContract call is made. This keeps backward compatibility with callers that haven't been updated, and lets the keyper's eons-row code fall back to its config-supplied global DKG contract. Files changed: - script/Deploy.service.s.sol: deploys DKGContract + ECIESKeyRegistry. - script/AddKeyperSet.s.sol, script/AddKeyperSet.gnosh.s.sol: accept DKG_CONTRACT_ADDRESS env var. Notes for next iteration: - mise-test-setup's contracts container is built from https://github.com/shutter-network/contracts.git#docker so these script changes will only reach the e2e test runs once that branch is pushed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Added `error WrongDKGContract` and `_checkDKGContract(uint64 keyperSetIndex)` helper that reverts if the Keyper Set's `dkgContract` field does not match `address(this)` (address(0) is treated as a mismatch — no special case). Called before `_checkMember` in `submitDealing`, `submitAccusation`, `submitApology`, and `submitSuccessVote`. Files changed: - src/common/DKGContract.sol: error + helper + 4 call sites - test/DKGContract.t.sol: setUp wires setDKGContract; 5 new revert tests Decision: check runs after phase/success guards so misconfigured keypers get a clear WrongDKGContract error rather than a phase error.
All ten tests were reverting with WrongDKGContract(). _setup() finalized the KeyperSet without calling ks.setDKGContract(address(dkg)), so _checkDKGContract saw address(0) instead of address(this). Added ks.setDKGContract(address(dkg)) between setPublisher and setFinalized, mirroring DKGContractTest.t.sol. Files changed: test/DKGBenchmark.t.sol
8 tasks
All ten DKGBenchmark tests reported inflated [PASS] (gas: X) figures because _setup() (four contract deployments + keyper set creation) was included in Forge's test-level gas counter. The PRD explicitly excludes setup gas; totalGas accumulated via gasleft() was already correct but the headline number was misleading. Fixed by wrapping _setup(n) in vm.pauseGasMetering() / vm.resumeGasMetering() in _run(). vm.pauseGasMetering() pauses Forge's EVM gas counter without affecting gasleft(), so the per-call gasleft()-difference accumulation into totalGas continues to work correctly. The [PASS] gas numbers now reflect only protocol operations. Key decisions: - vm.pauseGasMetering() / vm.resumeGasMetering() is the canonical Forge approach for excluding scaffolding from per-test gas figures. No restructuring (e.g. splitting into per-N test contracts or moving _setup to Forge's setUp()) was needed. - gasleft() is unaffected by pauseGasMetering, so the console.log totalGas output is unchanged and remains the authoritative measurement. Before → after [PASS] gas (happy path): n=3: 5,007,283 → 427,541 n=5: 5,347,205 → 672,400 n=10: 6,195,906 → 1,283,443 n=20: 8,489,712 → 3,101,929 n=50: 19,548,247 → 12,734,473 Files changed: contracts/test/DKGBenchmark.t.sol Notes: no e2e tests to run for a benchmark-only change; all ten DKGBenchmark tests pass with forge test --match-contract DKGBenchmark.
Introduce IDKGContract covering DKGContract's full public surface (the Phase enum, the seven external/public functions, and external-view getters for every public state variable) so other contracts -- notably KeyperSetManager in the next slice -- can call into a DKG contract without importing the concrete type and re-creating the KeyperSetManager <-> DKGContract import cycle. Key decisions: - The Phase enum moves from DKGContract to IDKGContract; DKGContract inherits it (references stay unqualified). DKGContract.t.sol now uses IDKGContract.Phase. - keyperSetManager()/keyBroadcastContract() in the interface return address, matching this repo's existing intf/ convention (IKeyperSet, IKeyperSetManager return address, never concrete types) and the comparison the next slice needs (IDKGContract(dkg).keyperSetManager() != address(this)). Returning concrete types would force IDKGContract to import KeyperSetManager, reinstating the very cycle this interface exists to break. To satisfy the address-typed getters while keeping internal calls typed, the two contract-typed immutables are renamed to internal _keyperSetManager/_keyBroadcastContract with explicit address getters. ABI is unchanged (the auto-getters already returned address), so Go bindings are unaffected. Files: src/common/intf/IDKGContract.sol (new), src/common/DKGContract.sol, test/DKGContract.t.sol. Verification: forge build clean (no new warnings vs baseline), forge test 131/131 pass, ABI diff confirms the interface covers exactly DKGContract's 14 public functions with matching return types. Notes for next iteration: unblocks 002-validate-dkg-address-on- registration. e2e suite (test-dkg-happy-path, test-dkg-offline-recovery) not run -- mise absent and its bootstrap is blocked by the sandbox classifier, as in every prior commit in this PRD area; this slice is Solidity-only with no ABI change and is fully covered by the forge suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Enforce the registration-time complement to DKGContract's runtime _checkDKGContract guard: a KeyperSet may only be registered if it designates a DKG Contract that itself references this KeyperSetManager. Without this, a KeyperSet paired with a zero or wrong DKG Contract registers cleanly but can never run DKG (every submission reverts WrongDKGContract), with no early signal to the DAO. Changes (src): - KeyperSetManager: two new errors DKGContractNotSet (zero DKG address) and DKGContractManagerMismatch (DKG's keyperSetManager != this). Both checks run after the existing isFinalized check, reading the DKG address via IKeyperSet.getDKGContract() and calling IDKGContract.keyperSetManager(). Imports IDKGContract (the interface, not the concrete DKGContract) so the KeyperSetManager <-> DKGContract cycle stays broken. Changes (tests): - KeyperSetManager.t.sol: setUp now pairs members0/members1 with a DKGContract bound to the test's manager; three new cases cover zero DKG, mismatched-manager DKG, and the matching happy path. - DKGContract/ECIESKeyRegistry/EonKeyPublish/KeyBroadcastContract tests: their setUps registered bare KeyperSets, which now revert at registration; each now sets a correctly-paired DKGContract before finalization. Key decisions: - Errors declared at file scope on KeyperSetManager.sol (next to AlreadyDeactivated), matching the issue's "KeyperSetManager declares" wording; selectors are visible to tests via the existing import. - DKGContract.t.sol's WrongDKGContract guard tests previously registered a KeyperSet with a zero DKG address. That state is now unreachable (addKeyperSet rejects it), so the helper points at a *second* valid DKGContract bound to the same manager (registration passes; runtime guard still fires). The now-impossible testSubmitDealingRevertsWhen DKGContractIsZero case is removed; the != comparison it exercised is fully covered by the remaining different-contract cases. Verification: forge build clean (only pre-existing unsafe-typecast lints in scripts/benchmarks; none from this change); forge test 133/133 pass (was 131: +3 KeyperSetManager cases, -1 obsolete zero-address case). Blockers/notes: e2e suite (rolling-shutter/mise-test-setup/e2e-tests) not run -- mise is absent and its mise.run bootstrap is blocked by the sandbox classifier, as in every prior commit in this PRD area. This change is Solidity-only, additive, with no addKeyperSet ABI signature change and no runtime behavior change for correctly-configured keyper sets, so the Go e2e happy-path/offline-recovery suites are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The AddKeyperSet deployment script now reads the DKG contract address
with vm.envAddress("DKG_CONTRACT_ADDRESS") instead of the
vm.envOr(..., address(0)) fallback, and calls setDKGContract
unconditionally. This is the deployment-path complement to the
on-chain addKeyperSet validation added in ef231f8: a zero DKG address
would now revert DKGContractNotSet at the contract call, so failing
earlier at env-resolution time with Foundry's clear "environment
variable not found" message is strictly better than a silent
zero-address branch that always fails downstream.
Key decisions:
- Scope limited to AddKeyperSet.s.sol per the PRD/issue. The byte-
identical AddKeyperSet.gnosh.s.sol is left untouched; it is not on any
automated path (the e2e add-keyper-set mise-task always invokes
AddKeyperSet.s.sol regardless of deployment type), so hardening it is
a separate, out-of-scope follow-up.
- No new forge test: scripts are not unit-tested in this repo, and the
new revert-on-missing-env behavior is Foundry framework semantics
(vm.envAddress), not our logic.
Files changed:
- script/AddKeyperSet.s.sol
Verification: forge clean && forge build clean (only pre-existing
unsafe-typecast lints in scripts/benchmarks); forge test 133/133 pass.
The e2e add-keyper-set task already derives DKG_CONTRACT_ADDRESS from the
deployed DKGContract (both Deploy.service and Deploy.gnosh create one),
so the success path is preserved.
Notes: e2e suite (test-dkg-happy-path, test-dkg-offline-recovery) not
run -- mise is absent and its mise.run | sh bootstrap is blocked by the
sandbox classifier (Code-from-External), as in every prior commit in
this PRD area. The change is a one-line deployment-script edit with no
ABI/runtime impact on correctly-configured keyper sets, so the Go e2e
flows are unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.