Skip to content

Commit ef231f8

Browse files
jannikluhnclaude
jannikluhn
and
claude
committed
feat(keyper-set-manager): validate DKG contract pairing on addKeyperSet
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>
1 parent 8c43092 commit ef231f8

6 files changed

Lines changed: 113 additions & 13 deletions

File tree

‎src/common/KeyperSetManager.sol‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import "openzeppelin/contracts/utils/math/Math.sol";
55
import "./KeyperSet.sol";
66
import "./RestrictedPausable.sol";
77
import "./intf/IKeyperSetManager.sol";
8+
import "./intf/IDKGContract.sol";
89

910
error AlreadyDeactivated();
11+
error DKGContractNotSet();
12+
error DKGContractManagerMismatch();
1013

1114
contract KeyperSetManager is RestrictedPausable, IKeyperSetManager {
1215
struct KeyperSetData {
@@ -43,6 +46,18 @@ contract KeyperSetManager is RestrictedPausable, IKeyperSetManager {
4346
if (!KeyperSet(keyperSetContract).isFinalized()) {
4447
revert KeyperSetNotFinalized();
4548
}
49+
// The Keyper Set must designate a DKG Contract that itself references
50+
// this manager. This is the registration-time complement to the
51+
// runtime _checkDKGContract guard in DKGContract: without it, a Keyper
52+
// Set paired with the wrong (or no) DKG Contract registers cleanly but
53+
// can never run DKG, since every submission would revert WrongDKGContract.
54+
address dkgContract = IKeyperSet(keyperSetContract).getDKGContract();
55+
if (dkgContract == address(0)) {
56+
revert DKGContractNotSet();
57+
}
58+
if (IDKGContract(dkgContract).keyperSetManager() != address(this)) {
59+
revert DKGContractManagerMismatch();
60+
}
4661
keyperSets.push(KeyperSetData(activationBlock, keyperSetContract));
4762
KeyperSet keyperSet = KeyperSet(keyperSetContract);
4863
emit KeyperSetAdded(

‎test/DKGContract.t.sol‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,19 @@ contract DKGContractTest is Test {
727727
ks.addMembers(members);
728728
ks.setThreshold(2);
729729
ks.setPublisher(address(dkgContract));
730-
// dkgContract left as address(0) — intentional mismatch
730+
// Point the Keyper Set at a *different* DKGContract that is still bound
731+
// to this KeyperSetManager. Registration accepts it (its
732+
// keyperSetManager matches the manager), but it is not the dkgContract
733+
// the keypers submit to, so the runtime _checkDKGContract guard rejects
734+
// every submission with WrongDKGContract. A zero DKGContract can no
735+
// longer reach this guard: addKeyperSet rejects it at registration.
736+
DKGContract otherDKG = new DKGContract(
737+
PHASE_LENGTH,
738+
DKG_LEAD_LENGTH,
739+
address(keyperSetManager),
740+
address(keyBroadcastContract)
741+
);
742+
ks.setDKGContract(address(otherDKG));
731743
ks.setFinalized();
732744
uint64 activation = ACTIVATION_BLOCK_0 * 10;
733745
vm.prank(dao);
@@ -787,16 +799,4 @@ contract DKGContractTest is Test {
787799
vm.expectRevert(DKGContract.WrongDKGContract.selector);
788800
dkgContract.submitSuccessVote(ksi, 0, 0, EON_KEY_A);
789801
}
790-
791-
function testSubmitDealingRevertsWhenDKGContractIsZero() public {
792-
// address(0) is an explicit mismatch — no special case.
793-
(, uint64 ksi) = _makeKeyperSetWithWrongDKGContract();
794-
uint64 activation = ACTIVATION_BLOCK_0 * 10;
795-
uint64 dealingBlock = activation - DKG_LEAD_LENGTH;
796-
vm.roll(dealingBlock);
797-
bytes[] memory evals = new bytes[](0);
798-
vm.prank(keyper0);
799-
vm.expectRevert(DKGContract.WrongDKGContract.selector);
800-
dkgContract.submitDealing(ksi, 0, 0, hex"01", evals);
801-
}
802802
}

‎test/ECIESKeyRegistry.t.sol‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import "forge-std/Test.sol";
55
import "../src/common/ECIESKeyRegistry.sol";
66
import "../src/common/KeyperSet.sol";
77
import "../src/common/KeyperSetManager.sol";
8+
import "../src/common/DKGContract.sol";
89
import "../src/common/intf/IKeyperSetManager.sol";
910

1011
contract ECIESKeyRegistryTest is Test {
@@ -31,13 +32,21 @@ contract ECIESKeyRegistryTest is Test {
3132

3233
registry = new ECIESKeyRegistry(address(keyperSetManager));
3334

35+
DKGContract dkgContract = new DKGContract(
36+
1,
37+
1,
38+
address(keyperSetManager),
39+
address(0)
40+
);
41+
3442
keyperSet0 = new KeyperSet();
3543
address[] memory members = new address[](3);
3644
members[0] = keyper0;
3745
members[1] = keyper1;
3846
members[2] = keyper2;
3947
keyperSet0.addMembers(members);
4048
keyperSet0.setThreshold(2);
49+
keyperSet0.setDKGContract(address(dkgContract));
4150
keyperSet0.setFinalized();
4251
vm.prank(dao);
4352
keyperSetManager.addKeyperSet(ACTIVATION_BLOCK_0, address(keyperSet0));

‎test/EonKeyPublish.t.sol‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ pragma solidity ^0.8.22;
33

44
import "forge-std/Test.sol";
55
import "../src/common/EonKeyPublish.sol";
6+
import "../src/common/DKGContract.sol";
67

78
contract EonKeyPublishTest is Test {
89
KeyperSetManager public manager;
910
EonKeyPublish public eonKeyPublish;
1011
KeyperSet public keyperSet0;
1112
KeyperSet public keyperSet;
1213
KeyBroadcastContract public broadcastContract;
14+
DKGContract public dkgContract;
1315
address public initializer;
1416
address public dao;
1517

@@ -23,8 +25,16 @@ contract EonKeyPublishTest is Test {
2325
vm.prank(initializer);
2426
manager.initialize(dao, address(420));
2527
broadcastContract = new KeyBroadcastContract(address(manager));
28+
dkgContract = new DKGContract(
29+
1,
30+
1,
31+
address(manager),
32+
address(broadcastContract)
33+
);
2634
keyperSet = new KeyperSet();
35+
keyperSet.setDKGContract(address(dkgContract));
2736
keyperSet0 = new KeyperSet();
37+
keyperSet0.setDKGContract(address(dkgContract));
2838
keyperSet0.setFinalized();
2939
vm.prank(dao);
3040
uint64 eon = 1;
@@ -58,6 +68,7 @@ contract EonKeyPublishTest is Test {
5868
);
5969
ks.setPublisher(address(publisher));
6070
ks.addMembers(members);
71+
ks.setDKGContract(address(dkgContract));
6172
ks.setFinalized();
6273
vm.prank(dao);
6374
manager.addKeyperSet(uint64(10), address(ks));

‎test/KeyBroadcastContract.t.sol‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import "forge-std/Test.sol";
55
import "../src/common/KeyBroadcastContract.sol";
66
import "../src/common/KeyperSetManager.sol";
77
import "../src/common/KeyperSet.sol";
8+
import "../src/common/DKGContract.sol";
89

910
contract MockPublisher is EonKeyPublisher {
1011
function eonKeyConfirmed(bytes memory) external pure returns (bool) {
@@ -41,15 +42,23 @@ contract KeyBroadcastTest is Test {
4142
keyBroadcastContract = new KeyBroadcastContract(
4243
address(keyperSetManager)
4344
);
45+
DKGContract dkgContract = new DKGContract(
46+
1,
47+
1,
48+
address(keyperSetManager),
49+
address(keyBroadcastContract)
50+
);
4451
keyperSet0 = new KeyperSet();
4552
keyperSet0.setPublisher(address(publisher0));
53+
keyperSet0.setDKGContract(address(dkgContract));
4654
keyperSet0.setFinalized();
4755

4856
vm.prank(dao);
4957
keyperSetManager.addKeyperSet(100, address(keyperSet0));
5058

5159
keyperSet1 = new KeyperSet();
5260
keyperSet1.setPublisher(address(publisher1));
61+
keyperSet1.setDKGContract(address(dkgContract));
5362
keyperSet1.setFinalized();
5463

5564
vm.prank(dao);

‎test/KeyperSetManager.t.sol‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,22 @@ pragma solidity ^0.8.20;
44
import "forge-std/Test.sol";
55
import "../src/common/KeyperSetManager.sol";
66
import "../src/common/KeyperSet.sol";
7+
import "../src/common/DKGContract.sol";
8+
import "../src/common/KeyBroadcastContract.sol";
79

810
contract KeyperSetManagerTest is Test {
911
KeyperSetManager public keyperSetManager;
12+
KeyBroadcastContract public keyBroadcastContract;
13+
DKGContract public dkgContract;
1014
KeyperSet public members0;
1115
KeyperSet public members1;
1216
address public owner;
1317
address public dao;
1418
address public sequencer;
1519

20+
uint64 constant PHASE_LENGTH = 10;
21+
uint64 constant DKG_LEAD_LENGTH = 40;
22+
1623
function setUp() public {
1724
owner = vm.addr(42);
1825
dao = address(420);
@@ -21,12 +28,31 @@ contract KeyperSetManagerTest is Test {
2128
keyperSetManager = new KeyperSetManager(owner);
2229
vm.prank(owner);
2330
keyperSetManager.initialize(dao, sequencer);
31+
keyBroadcastContract = new KeyBroadcastContract(
32+
address(keyperSetManager)
33+
);
34+
dkgContract = deployDKGContract(address(keyperSetManager));
2435
members0 = new KeyperSet();
36+
members0.setDKGContract(address(dkgContract));
2537
members0.setFinalized();
2638
members1 = new KeyperSet();
39+
members1.setDKGContract(address(dkgContract));
2740
members1.setFinalized();
2841
}
2942

43+
// Deploys a DKGContract bound to the given KeyperSetManager. The
44+
// KeyBroadcastContract address is irrelevant to addKeyperSet validation,
45+
// which only reads back DKGContract.keyperSetManager().
46+
function deployDKGContract(address manager) internal returns (DKGContract) {
47+
return
48+
new DKGContract(
49+
PHASE_LENGTH,
50+
DKG_LEAD_LENGTH,
51+
manager,
52+
address(keyBroadcastContract)
53+
);
54+
}
55+
3056
function testGetNumKeyperSets() public {
3157
assertEq(keyperSetManager.getNumKeyperSets(), 0);
3258
vm.prank(dao);
@@ -56,6 +82,36 @@ contract KeyperSetManagerTest is Test {
5682
keyperSetManager.addKeyperSet(0, address(ks));
5783
}
5884

85+
function testAddKeyperSetRevertsOnZeroDKGContract() public {
86+
KeyperSet ks = new KeyperSet();
87+
ks.setFinalized();
88+
vm.expectRevert(DKGContractNotSet.selector);
89+
vm.prank(dao);
90+
keyperSetManager.addKeyperSet(1000, address(ks));
91+
}
92+
93+
function testAddKeyperSetRevertsOnMismatchedDKGContractManager() public {
94+
// A DKGContract bound to a different KeyperSetManager must be rejected.
95+
KeyperSetManager otherManager = new KeyperSetManager(owner);
96+
DKGContract mismatchedDKG = deployDKGContract(address(otherManager));
97+
KeyperSet ks = new KeyperSet();
98+
ks.setDKGContract(address(mismatchedDKG));
99+
ks.setFinalized();
100+
vm.expectRevert(DKGContractManagerMismatch.selector);
101+
vm.prank(dao);
102+
keyperSetManager.addKeyperSet(1000, address(ks));
103+
}
104+
105+
function testAddKeyperSetAcceptsMatchingDKGContract() public {
106+
KeyperSet ks = new KeyperSet();
107+
ks.setDKGContract(address(dkgContract));
108+
ks.setFinalized();
109+
vm.prank(dao);
110+
keyperSetManager.addKeyperSet(1000, address(ks));
111+
assertEq(keyperSetManager.getNumKeyperSets(), 1);
112+
assertEq(keyperSetManager.getKeyperSetAddress(0), address(ks));
113+
}
114+
59115
function testAddKeyperSetRequiresIncreasingActivationBlock() public {
60116
vm.prank(dao);
61117
keyperSetManager.addKeyperSet(1000, address(members0));

0 commit comments

Comments
 (0)