Skip to content
65 changes: 14 additions & 51 deletions contracts/src/consensus/ConsensusV1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
error ValidatorAlreadyResigned();
error BellowMinValidators();
error NoActiveValidators();
error InsufficientActiveValidators(uint256 available, uint256 required);

error BlsKeyAlreadyRegistered();
error BlsKeyIsInvalid();
Expand Down Expand Up @@ -346,7 +347,6 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {

_minValidators = n;

_shuffle();
_deleteRoundValidators();

_roundValidatorsHead = address(0);
Expand Down Expand Up @@ -385,11 +385,16 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
}
}

if (_roundValidatorsCount == 0) {
revert NoActiveValidators();
// Prepare temp array. Used when _roundValidatorsCount < _minValidators
// A round must consist of exactly `_minValidators` DISTINCT validators. With fewer
// eligible active validators, padding the round by repeating a validator across slots
// would orphan those slots: the consensus signed message excludes validatorIndex, so a
// duplicated validator's single signature cannot fill its extra slots, the round can
// never reach the +2/3 threshold, and consensus halts. Fail loudly instead.
if (_roundValidatorsCount < _minValidators) {
revert InsufficientActiveValidators(_roundValidatorsCount, _minValidators);
}

// Prepare temp array. Used when _roundValidatorsCount < _minValidators
address next = _roundValidatorsHead;
address[] memory tmpValidators = new address[](_roundValidatorsCount);

Expand All @@ -399,13 +404,14 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
}
_shuffleMem(tmpValidators);

// Fill round & _roundValidators
// Fill round & _roundValidators with `_minValidators` distinct validators (guaranteed
// available by the check above) — no slot is ever duplicated.
RoundValidator[] storage round = _rounds.push();
delete _roundValidators;
_roundValidators = new address[](_minValidators);

for (uint256 i = 0; i < _minValidators; i++) {
address addr = tmpValidators[i % _roundValidatorsCount];
address addr = tmpValidators[i];
_roundValidators[i] = addr;
round.push(RoundValidator({addr: addr, voteBalance: _validatorsData[addr].voteBalance}));
}
Expand Down Expand Up @@ -523,49 +529,6 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
return result;
}

// Internal functions
function _shuffle() internal {
uint256 n = _activeValidators.length;
if (n == 0) {
return;
}

for (uint256 i = n - 1; i > 0; i--) {
// Get a random index between 0 and i (inclusive)
uint256 j = uint256(keccak256(abi.encodePacked(block.timestamp, i))) % (i + 1);

if (i == j) {
continue; // No need to swap if indices are the same
}

/* Swap example
i = 0; j = 2;

Initial state
A B C
A:0 B:1 C:2

Array SWAP
C B A
A:0 B:1 C:2

Index SWAP
C B A
A:2 B:1 C:0
*/

// Swap elements at index i and j
address addrA = _activeValidators[i];
address addrB = _activeValidators[j];

_activeValidators[i] = _activeValidators[j];
_activeValidators[j] = addrA;

_activeValidatorIndex[addrA] = j;
_activeValidatorIndex[addrB] = i;
}
}

function _shuffleMem(address[] memory array) internal view {
uint256 n = array.length;
if (n == 0) {
Expand Down Expand Up @@ -698,8 +661,8 @@ contract ConsensusV1 is UUPSUpgradeable, OwnableUpgradeable {
_voters[voter.prev].next = address(0);
_votersTail = voter.prev;
} else if (_votersHead == msg.sender) {
_voters[_votersTail].prev = address(0);
_votersHead = _voters[_votersHead].next;
_voters[voter.next].prev = address(0);
_votersHead = voter.next;
} else {
_voters[voter.prev].next = voter.next;
_voters[voter.next].prev = voter.prev;
Expand Down
29 changes: 19 additions & 10 deletions contracts/test/consensus/Consensus-CalculateTop.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ contract ConsensusTest is Base {
registerValidator(address(2));
resignValidator(addr);

// Only one eligible (non-resigned) validator remains; a round of 2 would have required
// duplicating it across slots, which is now rejected instead of silently produced.
vm.expectRevert(abi.encodeWithSelector(ConsensusV1.InsufficientActiveValidators.selector, 1, 2));
consensus.calculateRoundValidators(2);
ConsensusV1.Validator[] memory validators = consensus.getRoundValidators();
assertEq(validators.length, 2);
assertEq(validators[0].addr, address(2));
assertEq(validators[1].addr, address(2)); // Second validator is duplicated
}

// Inverted order
Expand All @@ -72,11 +71,8 @@ contract ConsensusTest is Base {
registerValidator(address(2));
resignValidator(address(2));

vm.expectRevert(abi.encodeWithSelector(ConsensusV1.InsufficientActiveValidators.selector, 1, 2));
consensus.calculateRoundValidators(2);
ConsensusV1.Validator[] memory validators = consensus.getRoundValidators();
assertEq(validators.length, 2);
assertEq(validators[0].addr, addr);
assertEq(validators[1].addr, addr); // Second validator is duplicated
}

function test_should_ignore_validators_without_bls_public_key() public {
Expand All @@ -85,11 +81,24 @@ contract ConsensusTest is Base {
registerValidator(addr);
consensus.addValidator(address(2), new bytes(0), false);

// address(2) has no BLS key so it is not eligible; only one eligible validator remains.
vm.expectRevert(abi.encodeWithSelector(ConsensusV1.InsufficientActiveValidators.selector, 1, 2));
consensus.calculateRoundValidators(2);
}

function test_should_exclude_resigned_validator_and_form_distinct_round() public {
registerValidator(address(1));
registerValidator(address(2));
registerValidator(address(3));
resignValidator(address(2));

// Two eligible validators remain (1 and 3); the round is formed from DISTINCT validators,
// excluding the resigned one and never duplicating a slot.
consensus.calculateRoundValidators(2);
ConsensusV1.Validator[] memory validators = consensus.getRoundValidators();
assertEq(validators.length, 2);
assertEq(validators[0].addr, addr);
assertEq(validators[1].addr, addr); // Second validator is duplicated
assertTrue(validators[0].addr != validators[1].addr); // no duplicate slot
assertTrue(validators[0].addr != address(2) && validators[1].addr != address(2)); // resigned excluded
}

function test_consensus_sortedValidators_sameVoteCounts() public {
Expand Down
12 changes: 4 additions & 8 deletions contracts/test/consensus/Consensus-ValidatorResignation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,12 @@ contract ConsensusTest is Base {

assertEq(consensus.validatorsCount(), 3);

// Act - higher value
// Act - value higher than the active validator count must revert (no duplicate-slot padding).
// _minValidators is left unchanged because the whole call reverts.
vm.expectRevert(abi.encodeWithSelector(ConsensusV1.InsufficientActiveValidators.selector, 3, 5));
consensus.calculateRoundValidators(5);

// Test
vm.startPrank(addr);
vm.expectRevert(ConsensusV1.BellowMinValidators.selector);
consensus.resignValidator();
vm.stopPrank();

// Act - same value
// Act - same value as the active count sets _minValidators = 3
consensus.calculateRoundValidators(3);

// Test
Expand Down
57 changes: 57 additions & 0 deletions contracts/test/consensus/Consensus-Vote.sol
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,63 @@ contract ConsensusTest is Base {
assertEq(allVoters.length, 0);
}

function test_unvote_head_with_multiple_remaining() public {
// Regression: removing the voter-list HEAD while >=2 voters remain must keep the list
// consistent. The previous code cleared the TAIL's prev pointer instead of the new head's,
// corrupting the list so a later tail removal made getVotes read out of bounds (panic 0x32).
registerValidator(address(1));
registerValidator(address(2));
registerValidator(address(3));

address voterA = address(11); // head
address voterB = address(12);
address voterC = address(13); // tail
vm.deal(voterA, 100 ether);
vm.deal(voterB, 100 ether);
vm.deal(voterC, 100 ether);

vm.prank(voterA);
consensus.vote(address(1));
vm.prank(voterB);
consensus.vote(address(2));
vm.prank(voterC);
consensus.vote(address(3));

assertEq(consensus.getVotesCount(), 3);

// Unvote the HEAD while B and C remain (the previously-buggy branch).
vm.prank(voterA);
consensus.unvote();

assertEq(consensus.getVotesCount(), 2);
ConsensusV1.VoteResult[] memory voters = consensus.getVotes(address(0), 10);
assertEq(voters.length, 2);
assertEq(voters[0].voter, voterB);
assertEq(voters[1].voter, voterC);

// Unvote the TAIL next. With the old bug this corrupted the list and the getVotes below
// reverted with array out-of-bounds.
vm.prank(voterC);
consensus.unvote();

assertEq(consensus.getVotesCount(), 1);
voters = consensus.getVotes(address(0), 10);
assertEq(voters.length, 1);
assertEq(voters[0].voter, voterB);

// A subsequent vote must remain reachable from the head.
address voterD = address(14);
vm.deal(voterD, 100 ether);
vm.prank(voterD);
consensus.vote(address(1));

assertEq(consensus.getVotesCount(), 2);
voters = consensus.getVotes(address(0), 10);
assertEq(voters.length, 2);
assertEq(voters[0].voter, voterB);
assertEq(voters[1].voter, voterD);
}

function test_multiple_voted_different_validators() public {
// Assert voters
assertEq(consensus.getVotesCount(), 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe<{
});

it("#generate - should return generated data", async ({ generator, mnemonicGenerator }) => {
const validatorsCount = 10;
const validatorsCount = 53;
assert.object(
await generator.generate(
mnemonicGenerator.generate(),
Expand Down
2,406 changes: 1,203 additions & 1,203 deletions packages/core/bin/config/devnet/core/crypto.json

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions packages/core/bin/config/devnet/core/genesis-wallet.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"address": "0xBDE46e6fa010CC68340A31D4DF539fF779df4474",
"address": "0x9628976Da3e4eb2573b98c36f5752019E20AB988",
"consensusKeys": {
"compressed": true,
"privateKey": "3c832fc0f6f219c68c0c5eaf4e6c9291cf776405b66fcee1ad6a4e780efc186b",
"publicKey": "af1cdd86ecb1ca690868af4b55eb6d1f1836f689a6d94c3305c0babb6a3a9cd6685e9c74e77a5eb6b3a6e3c757d4c1ac"
"privateKey": "21db9070adcdb295c96152a10cb810e3fde94a487ae62d0c3677c07336792695",
"publicKey": "849f445b903e3a93e04a8f3848616b24c4e9c686a8c1fc2bfcd1774fddaf01f661aa10d70c729d09626ebfe13faed434"
},
"keys": {
"compressed": true,
"privateKey": "9bfff9d0ca04017f5c134c6bfa7d0a1e19e50461cb9c0dac1a0da4f572159176",
"publicKey": "03c52c510d53fd1a303ce3f31b177cb7cf8921d7a6955c752aefffd01595757389"
"privateKey": "42a435b591099e257d598ae703fa53ed54a1fa701a6ca69d3cad9af511c9d405",
"publicKey": "02237debf0ab2795d5f49f99032c2289059494da7339d79fd823b15e5efc0fad41"
},
"passphrase": "spoon grunt burden actress episode broken theme huge retreat tool boil huge learn wrong dinosaur risk ocean coyote smile clarify guess protect same light"
"passphrase": "clump mushroom glimpse kitchen surprise leaf host forum portion say wrong soccer erase antique ginger bachelor velvet hover promote energy gauge select adapt method"
}
Loading
Loading