Skip to content

Commit 1230c1c

Browse files
cryptoAtwillcryptoAtwill
cryptoAtwill
and
cryptoAtwill
authored
fix: submit bottom up checkpoint performs signature check (#1338)
Co-authored-by: cryptoAtwill <willes.lau@protocol.ai>
1 parent 626c14b commit 1230c1c

7 files changed

Lines changed: 135 additions & 63 deletions

File tree

‎contracts/contracts/errors/IPCErrors.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ error MissingActivityCommitment();
8383
error ValidatorAlreadyClaimed();
8484
error InvalidActivityProof();
8585
error NotOwner();
86+
error SignatureAddressesNotSorted();
87+
error DuplicateValidatorSignaturesFound();
8688

8789
enum InvalidXnetMessageReason {
8890
Sender,

‎contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-License-Identifier: MIT OR Apache-2.0
22
pragma solidity ^0.8.23;
33

4-
import {InvalidBatchEpoch, MaxMsgsPerBatchExceeded, InvalidSignatureErr, BottomUpCheckpointAlreadySubmitted, CannotSubmitFutureCheckpoint, InvalidCheckpointEpoch} from "../errors/IPCErrors.sol";
4+
import {InvalidBatchEpoch, MaxMsgsPerBatchExceeded, InvalidSignatureErr, DuplicateValidatorSignaturesFound, SignatureAddressesNotSorted, BottomUpCheckpointAlreadySubmitted, CannotSubmitFutureCheckpoint, InvalidCheckpointEpoch} from "../errors/IPCErrors.sol";
55
import {IGateway} from "../interfaces/IGateway.sol";
66
import {BottomUpCheckpoint, BottomUpMsgBatch, BottomUpMsgBatchInfo} from "../structs/CrossNet.sol";
77
import {Validator, ValidatorSet} from "../structs/Subnet.sol";
@@ -66,6 +66,19 @@ contract SubnetActorCheckpointingFacet is SubnetActorModifiers, ReentrancyGuard,
6666
bytes32 hash,
6767
bytes[] memory signatures
6868
) public view {
69+
for (uint256 i = 1; i < signatories.length; ) {
70+
if (signatories[i] < signatories[i - 1]) {
71+
revert SignatureAddressesNotSorted();
72+
}
73+
if (signatories[i] == signatories[i - 1]) {
74+
revert DuplicateValidatorSignaturesFound();
75+
}
76+
77+
unchecked {
78+
i++;
79+
}
80+
}
81+
6982
// This call reverts if at least one of the signatories (validator) is not in the active validator set.
7083
uint256[] memory collaterals = s.validatorSet.getTotalPowerOfValidators(signatories);
7184
uint256 activeCollateral = s.validatorSet.getTotalActivePower();

‎contracts/test/helpers/TestUtils.sol‎

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@ library TestUtils {
3636
Vm vm
3737
) internal returns (uint256[] memory validatorKeys, address[] memory addresses, uint256[] memory weights) {
3838
validatorKeys = new uint256[](4);
39-
validatorKeys[0] = 100;
40-
validatorKeys[1] = 200;
41-
validatorKeys[2] = 300;
42-
validatorKeys[3] = 400;
39+
for (uint i = 0; i < 4; i++) {
40+
validatorKeys[i] = getPrivateKey(i);
41+
}
4342

4443
addresses = new address[](4);
4544
addresses[0] = vm.addr(validatorKeys[0]);
@@ -64,9 +63,9 @@ library TestUtils {
6463
Vm vm
6564
) internal returns (uint256[] memory validatorKeys, address[] memory addresses, uint256[] memory weights) {
6665
validatorKeys = new uint256[](3);
67-
validatorKeys[0] = 100;
68-
validatorKeys[1] = 200;
69-
validatorKeys[2] = 300;
66+
for (uint i = 0; i < 3; i++) {
67+
validatorKeys[i] = getPrivateKey(i);
68+
}
7069

7170
addresses = new address[](3);
7271
addresses[0] = vm.addr(validatorKeys[0]);
@@ -97,15 +96,52 @@ library TestUtils {
9796
addr = address(uint160(uint256(keccak256(dataSubset))));
9897
}
9998

99+
/// The dummy private keys generated offchain so that the addresses of the private keys
100+
/// are sorted in ascending order.
101+
function getPrivateKey(uint256 idx) internal pure returns (uint256 privateKey) {
102+
if (idx == 0) {
103+
return 0xaf2be542934f2283d6cb21ee8c80495ab76d63b1071b75276a4a5e7673e4dae6;
104+
}
105+
if (idx == 1) {
106+
return 0xb423cb058c1bd6b4494364de8bcbfd89d534ba709dd6de06dc7e7884dafca7b3;
107+
}
108+
if (idx == 2) {
109+
return 0xc0df835826416ebdfe6372000466c8c5fe1013f8d40448281ab572d11f4aee50;
110+
}
111+
if (idx == 3) {
112+
return 0xf731c1d1a68c1d060817b50dc3d56d2cdfb35a1636ad99ec9c3352e617bb907b;
113+
}
114+
if (idx == 4) {
115+
return 0x3f339e466f1946264212212866a929837cd565423bf1d4b3818870021f2ec12e;
116+
}
117+
if (idx == 5) {
118+
return 0x1ec4b12edb4056925ccc5687ca77bdd002de7af60de2a4be27f318b0a73df2fa;
119+
}
120+
if (idx == 6) {
121+
return 0x072e51399be0cb21e3c50b076d8af6bb35f019a8d3d629b06e0d0c9e440921df;
122+
}
123+
if (idx == 7) {
124+
return 0x8127a61993cac5dfa6f56d440f010e185fe7d308ddfd012a8966c7f353d123de;
125+
}
126+
if (idx == 8) {
127+
return 0x19d47f8e12c86e270438ed1caa284e2f1ffdd0a99420b2b220ea70647799a60f;
128+
}
129+
if (idx == 9) {
130+
return 0x98374b780974d9c11465be371871b6354fb537f2f24a6b985dbd4375b3f558a8;
131+
}
132+
revert("more than 10 validators not supported");
133+
}
134+
100135
function newValidator(
101-
uint256 key
136+
uint256 idx
102137
) internal pure returns (address addr, uint256 privKey, bytes memory validatorKey) {
103-
privKey = key;
104-
bytes memory pubkey = derivePubKeyBytes(key);
105-
validatorKey = deriveValidatorPubKeyBytes(key);
138+
privKey = getPrivateKey(idx);
139+
bytes memory pubkey = derivePubKeyBytes(privKey);
140+
validatorKey = deriveValidatorPubKeyBytes(privKey);
106141
addr = address(uint160(uint256(keccak256(pubkey))));
107142
}
108143

144+
/// Generate a list of validators whose addresses are arranged in ascending order.
109145
function newValidators(
110146
uint256 n
111147
) internal pure returns (address[] memory validators, uint256[] memory privKeys, bytes[] memory validatorKeys) {
@@ -114,7 +150,7 @@ library TestUtils {
114150
privKeys = new uint256[](n);
115151

116152
for (uint i = 0; i < n; i++) {
117-
(address addr, uint256 key, bytes memory validatorKey) = newValidator(100 + i);
153+
(address addr, uint256 key, bytes memory validatorKey) = newValidator(i);
118154
validators[i] = addr;
119155
validatorKeys[i] = validatorKey;
120156
privKeys[i] = key;
@@ -123,19 +159,6 @@ library TestUtils {
123159
return (validators, privKeys, validatorKeys);
124160
}
125161

126-
function derivePubKey(uint8 seq) internal pure returns (address addr, bytes memory data) {
127-
data = new bytes(65);
128-
data[1] = bytes1(seq);
129-
130-
// use data[1:] for the hash
131-
bytes memory dataSubset = new bytes(data.length - 1);
132-
for (uint i = 1; i < data.length; i++) {
133-
dataSubset[i - 1] = data[i];
134-
}
135-
136-
addr = address(uint160(uint256(keccak256(dataSubset))));
137-
}
138-
139162
function ensureBytesEqual(bytes memory _a, bytes memory _b) internal pure {
140163
require(_a.length == _b.length, "bytes len not equal");
141164
require(keccak256(_a) == keccak256(_b), "bytes not equal");

‎contracts/test/integration/GatewayDiamond.t.sol‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ contract GatewayActorDiamondTest is Test, IntegrationTestBase, SubnetWithNativeT
567567
}
568568

569569
function testGatewayDiamond_Single_Funding() public {
570-
(address validatorAddress, , bytes memory publicKey) = TestUtils.newValidator(100);
570+
(address validatorAddress, , bytes memory publicKey) = TestUtils.newValidator(0);
571571

572572
join(validatorAddress, publicKey);
573573

@@ -688,7 +688,7 @@ contract GatewayActorDiamondTest is Test, IntegrationTestBase, SubnetWithNativeT
688688
}
689689

690690
function testGatewayDiamond_Fund_Works_ReactivatedSubnet() public {
691-
(address validatorAddress, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
691+
(address validatorAddress, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
692692
assert(validatorAddress == vm.addr(privKey));
693693

694694
join(validatorAddress, publicKey);

‎contracts/test/integration/SubnetActorDiamond.t.sol‎

Lines changed: 52 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,10 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
112112

113113
// create genesis validators
114114
uint256 numGenesisValidators = 3;
115-
uint256 startingPrivateKey = 100;
116115
gwConstructorParams.genesisValidators = new Validator[](numGenesisValidators);
117116

118117
for (uint256 i = 0; i < numGenesisValidators; i++) {
119-
(address validator, , bytes memory publicKey) = TestUtils.newValidator(startingPrivateKey + i);
118+
(address validator, , bytes memory publicKey) = TestUtils.newValidator(i);
120119
gwConstructorParams.genesisValidators[i] = Validator({addr: validator, weight: 100, metadata: publicKey});
121120
}
122121

@@ -147,8 +146,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
147146

148147
/// @notice Testing the basic join, stake, leave lifecycle of validators
149148
function testSubnetActorDiamond_BasicLifeCycle() public {
150-
(address validator1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(100);
151-
(address validator2, uint256 privKey2, bytes memory publicKey2) = TestUtils.newValidator(101);
149+
(address validator1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(0);
150+
(address validator2, uint256 privKey2, bytes memory publicKey2) = TestUtils.newValidator(1);
152151

153152
// total collateral in the gateway
154153
uint256 collateral = 0;
@@ -427,7 +426,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
427426
}
428427

429428
function testSubnetActorDiamond_Bootstrap_Node() public {
430-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
429+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
431430

432431
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE);
433432
vm.prank(validator);
@@ -463,7 +462,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
463462
}
464463

465464
function testSubnetActorDiamond_Leave_NotValidator() public {
466-
(address validator, , ) = TestUtils.newValidator(100);
465+
(address validator, , ) = TestUtils.newValidator(0);
467466

468467
// non-empty subnet can't be killed
469468
vm.prank(validator);
@@ -472,9 +471,9 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
472471
}
473472

474473
function testSubnetActorDiamond_Leave_Subnet() public {
475-
(address validator1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(100);
476-
(address validator2, uint256 privKey2, bytes memory publicKey2) = TestUtils.newValidator(101);
477-
(address validator3, uint256 privKey3, bytes memory publicKey3) = TestUtils.newValidator(102);
474+
(address validator1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(0);
475+
(address validator2, uint256 privKey2, bytes memory publicKey2) = TestUtils.newValidator(1);
476+
(address validator3, uint256 privKey3, bytes memory publicKey3) = TestUtils.newValidator(2);
478477

479478
vm.deal(validator1, DEFAULT_MIN_VALIDATOR_STAKE);
480479
vm.deal(validator2, 3 * DEFAULT_MIN_VALIDATOR_STAKE);
@@ -513,7 +512,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
513512
}
514513

515514
function testSubnetActorDiamond_Kill_NotBootstrappedSubnet() public {
516-
(address validator1, , ) = TestUtils.newValidator(100);
515+
(address validator1, , ) = TestUtils.newValidator(0);
517516

518517
// not bootstrapped subnet can't be killed
519518
vm.expectRevert(SubnetNotBootstrapped.selector);
@@ -670,7 +669,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
670669
saDiamond.checkpointer().validateActiveQuorumSignatures(validators, hash, signatures);
671670
}
672671

673-
function testSubnetActorDiamond_validateActiveQuorumSignatures_InvalidSignatory() public {
672+
function testSubnetActorDiamond_validateActiveQuorumSignatures_DuplicatedValidators() public {
674673
(uint256[] memory keys, address[] memory validators, ) = TestUtils.getThreeValidators(vm);
675674
bytes[] memory pubKeys = new bytes[](3);
676675
bytes[] memory signatures = new bytes[](3);
@@ -690,11 +689,35 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
690689
saDiamond.manager().join{value: 10}(pubKeys[i], 10);
691690
}
692691

693-
// swap validators to trigger `InvalidSignatory` error;
694-
address a;
695-
a = validators[0];
692+
signatures[0] = signatures[1];
696693
validators[0] = validators[1];
697-
validators[1] = a;
694+
695+
vm.expectRevert(abi.encodeWithSelector(DuplicateValidatorSignaturesFound.selector));
696+
saDiamond.checkpointer().validateActiveQuorumSignatures(validators, hash0, signatures);
697+
}
698+
699+
function testSubnetActorDiamond_validateActiveQuorumSignatures_InvalidSignatory() public {
700+
(uint256[] memory keys, address[] memory validators, ) = TestUtils.getThreeValidators(vm);
701+
bytes[] memory pubKeys = new bytes[](3);
702+
bytes[] memory signatures = new bytes[](3);
703+
704+
bytes32 hash = keccak256(abi.encodePacked("test"));
705+
bytes32 hash0 = keccak256(abi.encodePacked("test1"));
706+
707+
for (uint256 i = 0; i < 3; i++) {
708+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(keys[i], hash);
709+
710+
// create incorrect signature using `vv`
711+
signatures[i] = abi.encodePacked(r, s, v);
712+
713+
pubKeys[i] = TestUtils.deriveValidatorPubKeyBytes(keys[i]);
714+
vm.deal(validators[i], 10 gwei);
715+
vm.prank(validators[i]);
716+
saDiamond.manager().join{value: 10}(pubKeys[i], 10);
717+
}
718+
719+
// use validator 1's signature for validator 0 to trigger `InvalidSignatory` error;
720+
signatures[0] = signatures[1];
698721

699722
vm.expectRevert(
700723
abi.encodeWithSelector(InvalidSignatureErr.selector, MultisignatureChecker.Error.InvalidSignatory)
@@ -1466,8 +1489,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
14661489
}
14671490

14681491
function test_second_validator_can_join() public {
1469-
(address validatorAddress1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(101);
1470-
(address validatorAddress2, , bytes memory publicKey2) = TestUtils.newValidator(102);
1492+
(address validatorAddress1, uint256 privKey1, bytes memory publicKey1) = TestUtils.newValidator(1);
1493+
(address validatorAddress2, , bytes memory publicKey2) = TestUtils.newValidator(2);
14711494

14721495
join(validatorAddress1, publicKey1);
14731496

@@ -1728,7 +1751,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
17281751

17291752
saDiamond.manager().setFederatedPower(validators, publicKeys, powers);
17301753

1731-
confirmChange(validators[2], privKeys[2], validators[1], privKeys[1]);
1754+
confirmChange(validators[1], privKeys[1], validators[2], privKeys[2]);
17321755

17331756
require(saDiamond.getter().isActiveValidator(validators[0]), "not active validator 0");
17341757
require(saDiamond.getter().isActiveValidator(validators[1]), "not active validator 1");
@@ -1867,8 +1890,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
18671890
// Tests for collateral token
18681891
// ----------------------------
18691892
function testSubnetActorDiamond_CollateralERC20_SupplyERC20_RegisteredInGateway() public {
1870-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
1871-
(address validator2, , ) = TestUtils.newValidator(101);
1893+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
1894+
(address validator2, , ) = TestUtils.newValidator(1);
18721895

18731896
// a bit of gas for execution, should not be needed
18741897
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE - 100);
@@ -2005,8 +2028,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
20052028
}
20062029

20072030
function testSubnetActorDiamond_CollateralERC20_SupplyERC20_SameToken_RegisteredInGateway() public {
2008-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
2009-
(address validator2, , ) = TestUtils.newValidator(101);
2031+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
2032+
(address validator2, , ) = TestUtils.newValidator(1);
20102033

20112034
// a bit of gas for execution, should not be needed
20122035
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE - 100);
@@ -2128,8 +2151,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
21282151
}
21292152

21302153
function testSubnetActorDiamond_CollateralERC20_SupplyNative_RegisteredInGateway() public {
2131-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
2132-
(address validator2, , ) = TestUtils.newValidator(101);
2154+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
2155+
(address validator2, , ) = TestUtils.newValidator(1);
21332156

21342157
// a bit of gas for execution, should not be needed
21352158
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE - 100);
@@ -2232,8 +2255,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
22322255
}
22332256

22342257
function testSubnetActorDiamond_CollateralNative_SupplyNative_RegisteredInGateway() public {
2235-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
2236-
(address validator2, , ) = TestUtils.newValidator(101);
2258+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
2259+
(address validator2, , ) = TestUtils.newValidator(1);
22372260

22382261
// a bit of gas for execution, should not be needed
22392262
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE * 10);
@@ -2303,8 +2326,8 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
23032326
}
23042327

23052328
function testSubnetActorDiamond_CollateralNative_SupplyERC20_RegisteredInGateway() public {
2306-
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(100);
2307-
(address validator2, , ) = TestUtils.newValidator(101);
2329+
(address validator, uint256 privKey, bytes memory publicKey) = TestUtils.newValidator(0);
2330+
(address validator2, , ) = TestUtils.newValidator(1);
23082331

23092332
// a bit of gas for execution, should not be needed
23102333
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE * 10);
@@ -2745,7 +2768,7 @@ contract SubnetActorDiamondTest is Test, IntegrationTestBase {
27452768

27462769
saDiamond.manager().setValidatorGater(address(gater));
27472770

2748-
(address validator, , bytes memory publicKey) = TestUtils.newValidator(100);
2771+
(address validator, , bytes memory publicKey) = TestUtils.newValidator(0);
27492772

27502773
vm.deal(validator, DEFAULT_MIN_VALIDATOR_STAKE * 3);
27512774
vm.prank(validator);

‎fendermint/testing/contract-test/tests/staking/machine.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ impl StateMachine for StakingMachine {
298298
signatures.push((*addr, signature.into()));
299299
}
300300

301+
signatures.sort_by_key(|(addr, _)| *addr);
302+
301303
system
302304
.subnet
303305
.try_submit_checkpoint(

0 commit comments

Comments
 (0)