From 39fe6ef9f244dbcddc465be0da261bf06d603383 Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Wed, 17 Sep 2025 15:27:16 +0200 Subject: [PATCH 01/10] improve quorum check --- contracts/src/BeefyClient.sol | 2 +- contracts/src/utils/Bitfield.sol | 63 ++++++++++++++- contracts/test/Bitfield.t.sol | 99 ++++++++++++++++++++++-- contracts/test/mocks/BitfieldWrapper.sol | 4 + 4 files changed, 161 insertions(+), 7 deletions(-) diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index eed2cb166..97ca1b74d 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -289,7 +289,7 @@ contract BeefyClient { // For the initial submission, the supplied bitfield should claim that more than // two thirds of the validator set have sign the commitment - if (Bitfield.countSetBits(bitfield) < computeQuorum(vset.length)) { + if (Bitfield.countSetBits(bitfield, vset.length) < computeQuorum(vset.length)) { revert NotEnoughClaims(); } diff --git a/contracts/src/utils/Bitfield.sol b/contracts/src/utils/Bitfield.sol index 0e14ed40f..292f065af 100644 --- a/contracts/src/utils/Bitfield.sol +++ b/contracts/src/utils/Bitfield.sol @@ -112,6 +112,67 @@ library Bitfield { } } + /** + * @notice Calculates the number of set bits in the first `maxBits` bits of the bitfield. + * This is a bounded variant of `countSetBits` that only counts bits within the specified range. + * + * @dev Example usage: + * If a bitfield has bits set at positions [0, 5, 10, 256, 300]: + * - countSetBits(bitfield, 11) returns 3 (bits 0, 5, 10) + * - countSetBits(bitfield, 257) returns 4 (bits 0, 5, 10, 256) + * - countSetBits(bitfield, 1000) returns 5 (all bits) + * + * @param self The bitfield to count bits in + * @param maxBits The maximum number of bits to count (counting from bit 0) + * @return count The number of set bits in the first `maxBits` positions + */ + function countSetBits(uint256[] memory self, uint256 maxBits) + internal + pure + returns (uint256) + { + if (maxBits == 0 || self.length == 0) { + return 0; + } + + unchecked { + uint256 count = 0; + uint256 fullElements = maxBits / 256; + uint256 remainingBits = maxBits % 256; + + // Count bits in full 256-bit elements + for (uint256 i = 0; i < fullElements && i < self.length; i++) { + uint256 x = self[i]; + x = (x & M1) + ((x >> 1) & M1); //put count of each 2 bits into those 2 bits + x = (x & M2) + ((x >> 2) & M2); //put count of each 4 bits into those 4 bits + x = (x & M4) + ((x >> 4) & M4); //put count of each 8 bits into those 8 bits + x = (x & M8) + ((x >> 8) & M8); //put count of each 16 bits into those 16 bits + x = (x & M16) + ((x >> 16) & M16); //put count of each 32 bits into those 32 bits + x = (x & M32) + ((x >> 32) & M32); //put count of each 64 bits into those 64 bits + x = (x & M64) + ((x >> 64) & M64); //put count of each 128 bits into those 128 bits + x = (x & M128) + ((x >> 128) & M128); //put count of each 256 bits into those 256 bits + count += x; + } + + // Count bits in the partial element (if any) + if (remainingBits > 0 && fullElements < self.length) { + uint256 mask = (ONE << remainingBits) - 1; + uint256 x = self[fullElements] & mask; + x = (x & M1) + ((x >> 1) & M1); + x = (x & M2) + ((x >> 2) & M2); + x = (x & M4) + ((x >> 4) & M4); + x = (x & M8) + ((x >> 8) & M8); + x = (x & M16) + ((x >> 16) & M16); + x = (x & M32) + ((x >> 32) & M32); + x = (x & M64) + ((x >> 64) & M64); + x = (x & M128) + ((x >> 128) & M128); + count += x; + } + + return count; + } + } + function isSet(uint256[] memory self, uint256 index) internal pure returns (bool) { uint256 element = index >> 8; return self[element].bit(uint8(index)) == 1; @@ -136,7 +197,7 @@ library Bitfield { if (length == 0) { return 0; } - + assembly { mstore(0x00, seed) mstore(0x20, iteration) diff --git a/contracts/test/Bitfield.t.sol b/contracts/test/Bitfield.t.sol index 2d7f8d8ac..e1ac5893b 100644 --- a/contracts/test/Bitfield.t.sol +++ b/contracts/test/Bitfield.t.sol @@ -31,23 +31,112 @@ contract BitfieldTest is Test { assertEq(30, counter); assertEq(Bitfield.countSetBits(finalBitfield), counter); } - + function testBitfieldWithZeroLength() public { BitfieldWrapper bw = new BitfieldWrapper(); - + // Empty bitfield with zero length uint256[] memory emptyBits; emptyBits = new uint256[](0); - + // This should create a valid bitfield with 0 length uint256[] memory initialBitField = bw.createBitfield(emptyBits, 0); - + // When length is 0, subsample should handle it gracefully without infinite loop // Since we're asking for 0 bits, it should return an empty bitfield uint256[] memory finalBitfield = bw.subsample(67, initialBitField, 0, 0); - + // Ensure the returned bitfield has the expected length and no set bits assertEq(finalBitfield.length, initialBitField.length); assertEq(Bitfield.countSetBits(finalBitfield), 0); } + + function testBoundedCountSetBits() public { + BitfieldWrapper bw = new BitfieldWrapper(); + + // Create a bitfield with some known set bits + // Set bits at positions: 0, 5, 10, 255, 256, 300, 500 + uint256[] memory bitsToSet = new uint256[](7); + bitsToSet[0] = 0; + bitsToSet[1] = 5; + bitsToSet[2] = 10; + bitsToSet[3] = 255; + bitsToSet[4] = 256; + bitsToSet[5] = 300; + bitsToSet[6] = 500; + + uint256[] memory bitfield = bw.createBitfield(bitsToSet, 600); + + // Test counting first 1 bit (should find bit 0) + assertEq(bw.countSetBits(bitfield, 1), 1); + + // Test counting first 6 bits (should find bit 0 and 5) + assertEq(bw.countSetBits(bitfield, 6), 2); + + // Test counting first 11 bits (should find bits 0, 5, 10) + assertEq(bw.countSetBits(bitfield, 11), 3); + + // Test counting first 256 bits (should find bits 0, 5, 10, 255) + assertEq(bw.countSetBits(bitfield, 256), 4); + + // Test counting first 257 bits (should find bits 0, 5, 10, 255, 256) + assertEq(bw.countSetBits(bitfield, 257), 5); + + // Test counting first 301 bits (should find bits 0, 5, 10, 255, 256, 300) + assertEq(bw.countSetBits(bitfield, 301), 6); + + // Test counting all bits (should find all 7 bits) + assertEq(bw.countSetBits(bitfield, 600), 7); + + // Test counting more than available bits (should still find all 7) + assertEq(bw.countSetBits(bitfield, 1000), 7); + } + + function testBoundedCountSetBitsEdgeCases() public { + BitfieldWrapper bw = new BitfieldWrapper(); + + // Test with empty bitfield + uint256[] memory emptyBits = new uint256[](0); + uint256[] memory emptyBitfield = bw.createBitfield(emptyBits, 0); + assertEq(bw.countSetBits(emptyBitfield, 10), 0); + + // Test with maxBits = 0 + uint256[] memory someBits = new uint256[](2); + someBits[0] = 1; + someBits[1] = 100; + uint256[] memory bitfield = bw.createBitfield(someBits, 200); + assertEq(bw.countSetBits(bitfield, 0), 0); + + // Test with all bits set in first 256 positions + uint256[] memory allFirstBits = new uint256[](256); + for (uint256 i = 0; i < 256; i++) { + allFirstBits[i] = i; + } + uint256[] memory fullBitfield = bw.createBitfield(allFirstBits, 512); + + // Should count exactly 256 bits when maxBits = 256 + assertEq(bw.countSetBits(fullBitfield, 256), 256); + + // Should count exactly 100 bits when maxBits = 100 + assertEq(bw.countSetBits(fullBitfield, 100), 100); + } + + function testBoundedCountSetBitsVsUnbounded() public { + BitfieldWrapper bw = new BitfieldWrapper(); + + string memory json = + vm.readFile(string.concat(vm.projectRoot(), "/test/data/beefy-validator-set.json")); + uint32 setSize = uint32(json.readUint(".validatorSetSize")); + uint256[] memory bitSetArray = json.readUintArray(".participants"); + + uint256[] memory bitfield = bw.createBitfield(bitSetArray, setSize); + + // When maxBits >= total bitfield size, bounded should equal unbounded + assertEq(bw.countSetBits(bitfield, setSize), Bitfield.countSetBits(bitfield)); + assertEq(bw.countSetBits(bitfield, setSize + 100), Bitfield.countSetBits(bitfield)); + + // Bounded count should be <= unbounded count + assertTrue(bw.countSetBits(bitfield, setSize / 2) <= Bitfield.countSetBits(bitfield)); + assertTrue(bw.countSetBits(bitfield, 10) <= Bitfield.countSetBits(bitfield)); + } } diff --git a/contracts/test/mocks/BitfieldWrapper.sol b/contracts/test/mocks/BitfieldWrapper.sol index 9993dc8b0..cba26cadf 100644 --- a/contracts/test/mocks/BitfieldWrapper.sol +++ b/contracts/test/mocks/BitfieldWrapper.sol @@ -19,4 +19,8 @@ contract BitfieldWrapper { { return Bitfield.subsample(seed, prior, n, length); } + + function countSetBits(uint256[] memory self, uint256 maxBits) public pure returns (uint256) { + return Bitfield.countSetBits(self, maxBits); + } } From a6a6a1055fbc7f66dfd1fd1dd8b8de2cef652dd3 Mon Sep 17 00:00:00 2001 From: Ron Date: Sun, 21 Sep 2025 17:27:54 +0800 Subject: [PATCH 02/10] Add a check for bitfield length (#2) --- contracts/src/BeefyClient.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index 97ca1b74d..e4f385f2c 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -272,6 +272,10 @@ contract BeefyClient { revert InvalidCommitment(); } + if (bitfield.length != (vset.length + 255) / 256) { + revert InvalidBitfieldLength(); + } + // Check if merkle proof is valid based on the validatorSetRoot and if proof is included in bitfield if ( !isValidatorInSet(vset, proof.account, proof.index, proof.proof) From 868711e3b5892af5615d05275c799a340dd6455b Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 13:51:17 +0200 Subject: [PATCH 03/10] Improve validation routes --- contracts/src/BeefyClient.sol | 41 ++++++++++++++++------------- contracts/src/utils/Bitfield.sol | 44 +++++++++++++++++--------------- contracts/test/BeefyClient.t.sol | 2 +- contracts/test/Bitfield.t.sol | 2 +- 4 files changed, 48 insertions(+), 41 deletions(-) diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index e4f385f2c..03c3a96bb 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -13,7 +13,8 @@ import {ScaleCodec} from "./utils/ScaleCodec.sol"; /** * @title BeefyClient * - * High-level documentation at https://docs.snowbridge.network/architecture/verification/polkadot + * The BEEFY protocol is defined in https://eprint.iacr.org/2025/057.pdf. Higher level documentation + * is available at https://docs.snowbridge.network/architecture/verification/polkadot. * * To submit new commitments, relayers must call the following methods sequentially: * 1. submitInitial: Setup the session for the interactive submission @@ -189,9 +190,8 @@ contract BeefyClient { uint256 public immutable randaoCommitExpiration; /** - * @dev Minimum number of signatures required to validate a new commitment. This parameter - * is calculated based on `randaoCommitExpiration`. See ~/scripts/beefy_signature_sampling.py - * for the calculation. + * @dev The lower bound on the number of signatures required to validate a new commitment. Note + * that the final number of signatures is calculated dynamically. */ uint256 public immutable minNumRequiredSignatures; @@ -256,14 +256,13 @@ contract BeefyClient { revert StaleCommitment(); } - ValidatorSetState storage vset; + ValidatorSetState storage vset = currentValidatorSet; uint16 signatureUsageCount; if (commitment.validatorSetID == currentValidatorSet.id) { signatureUsageCount = currentValidatorSet.usageCounters.get(proof.index); currentValidatorSet.usageCounters.set( proof.index, signatureUsageCount.saturatingAdd(1) ); - vset = currentValidatorSet; } else if (commitment.validatorSetID == nextValidatorSet.id) { signatureUsageCount = nextValidatorSet.usageCounters.get(proof.index); nextValidatorSet.usageCounters.set(proof.index, signatureUsageCount.saturatingAdd(1)); @@ -272,14 +271,15 @@ contract BeefyClient { revert InvalidCommitment(); } - if (bitfield.length != (vset.length + 255) / 256) { - revert InvalidBitfieldLength(); + if (bitfield.length != Bitfield.containerLength(vset.length)) { + revert InvalidBitfield(); } // Check if merkle proof is valid based on the validatorSetRoot and if proof is included in bitfield if ( !isValidatorInSet(vset, proof.account, proof.index, proof.proof) - || !Bitfield.isSet(bitfield, proof.index) + || proof.index >= vset.length + || !Bitfield.isSet(bitfield, proof.index) ) { revert InvalidValidatorProof(); } @@ -365,16 +365,18 @@ contract BeefyClient { validateTicket(ticketID, commitment, bitfield); bool is_next_session = false; - ValidatorSetState storage vset; + ValidatorSetState storage vset = currentValidatorSet; if (commitment.validatorSetID == nextValidatorSet.id) { is_next_session = true; vset = nextValidatorSet; - } else if (commitment.validatorSetID == currentValidatorSet.id) { - vset = currentValidatorSet; - } else { + } else if (commitment.validatorSetID != currentValidatorSet.id) { revert InvalidCommitment(); } + if (bitfield.length != Bitfield.containerLength(vset.length)) { + revert InvalidBitfield(); + } + verifyCommitment(commitmentHash, ticketID, bitfield, vset, proofs); bytes32 newMMRRoot = ensureProvidesMMRRoot(commitment); @@ -448,7 +450,7 @@ contract BeefyClient { revert InvalidBitfield(); } return Bitfield.subsample( - ticket.prevRandao, bitfield, ticket.numRequiredSignatures, ticket.validatorSetLen + ticket.prevRandao, bitfield, ticket.validatorSetLen, ticket.numRequiredSignatures ); } @@ -492,11 +494,14 @@ contract BeefyClient { } /** - * @dev Calculates 2/3 majority required for quorum for a given number of validators. + * @dev Calculates majority required for quorum for a given number of validators. * @param numValidators The number of validators in the validator set. */ function computeQuorum(uint256 numValidators) internal pure returns (uint256) { - return numValidators - (numValidators - 1) / 3; + if (numValidators > 3) { + return numValidators - (numValidators - 1) / 3; + } + return numValidators; } /** @@ -518,13 +523,13 @@ contract BeefyClient { // Generate final bitfield indicating which validators need to be included in the proofs. uint256[] memory finalbitfield = - Bitfield.subsample(ticket.prevRandao, bitfield, numRequiredSignatures, vset.length); + Bitfield.subsample(ticket.prevRandao, bitfield, vset.length, numRequiredSignatures); for (uint256 i = 0; i < proofs.length; i++) { ValidatorProof calldata proof = proofs[i]; // Check that validator is in bitfield - if (!Bitfield.isSet(finalbitfield, proof.index)) { + if (proof.index >= vset.length || !Bitfield.isSet(finalbitfield, proof.index)) { revert InvalidValidatorProof(); } diff --git a/contracts/src/utils/Bitfield.sol b/contracts/src/utils/Bitfield.sol index 292f065af..10b207fe0 100644 --- a/contracts/src/utils/Bitfield.sol +++ b/contracts/src/utils/Bitfield.sol @@ -31,41 +31,41 @@ library Bitfield { uint256 internal constant ONE = uint256(1); /** - * @notice Core subsampling algorithm. Draws a random number, derives an index in the bitfield, and sets the bit if it is in the `prior` and not - * yet set. Repeats that `n` times. + * @dev Core subsampling algorithm. Draws a random number, derives an index in the bitfield, + * and sets the bit if it is in the `priorBitfield` and not yet set. Repeats that `n` times. * @param seed Source of randomness for selecting validator signatures. - * @param prior Bitfield indicating which validators claim to have signed the commitment. - * @param n Number of unique bits in prior that must be set in the result. Must be <= number of set bits in `prior`. - * @param length Length of the bitfield prior to draw bits from. Must be <= prior.length * 256. + * @param priorBitfield Bitfield indicating which validators claim to have signed the commitment. + * @param priorBitfieldSize Number of bits in priorBitfield Must be <= priorBitfield.length * 256. + * @param n Number of unique bits in priorBitfield that must be set in the output. + * Must be <= number of set bits in priorBitfield. */ - function subsample(uint256 seed, uint256[] memory prior, uint256 n, uint256 length) - internal - pure - returns (uint256[] memory bitfield) - { - bitfield = new uint256[](prior.length); + function subsample( + uint256 seed, + uint256[] memory priorBitfield, + uint256 priorBitfieldSize, + uint256 n + ) internal pure returns (uint256[] memory outputBitfield) { + outputBitfield = new uint256[](priorBitfield.length); uint256 found = 0; for (uint256 i = 0; found < n;) { - uint256 index = makeIndex(seed, i, length); + uint256 index = makeIndex(seed, i, priorBitfieldSize); - // require randomly selected bit to be set in prior and not yet set in bitfield - if (!isSet(prior, index) || isSet(bitfield, index)) { + // require randomly selected bit to be set in priorBitfield and not yet set in bitfield + if (!isSet(priorBitfield, index) || isSet(outputBitfield, index)) { unchecked { i++; } continue; } - set(bitfield, index); + set(outputBitfield, index); unchecked { found++; i++; } } - - return bitfield; } /** @@ -76,10 +76,7 @@ library Bitfield { pure returns (uint256[] memory bitfield) { - // Calculate length of uint256 array based on rounding up to number of uint256 needed - uint256 arrayLength = (length + 255) / 256; - - bitfield = new uint256[](arrayLength); + bitfield = new uint256[](containerLength(length)); for (uint256 i = 0; i < bitsToSet.length; i++) { set(bitfield, bitsToSet[i]); @@ -204,4 +201,9 @@ library Bitfield { index := mod(keccak256(0x00, 0x40), length) } } + + // Calculate length of uint256 bitfield array based on rounding up to number of uint256 needed + function containerLength(uint256 length) internal pure returns (uint256) { + return (length + 255) / 256; + } } diff --git a/contracts/test/BeefyClient.t.sol b/contracts/test/BeefyClient.t.sol index d377e2202..040568c5e 100644 --- a/contracts/test/BeefyClient.t.sol +++ b/contracts/test/BeefyClient.t.sol @@ -148,7 +148,7 @@ contract BeefyClientTest is Test { console.log("print initialBitField"); printBitArray(bitfield); prevRandao = uint32(vm.envOr("PREV_RANDAO", prevRandao)); - finalBitfield = Bitfield.subsample(prevRandao, bitfield, numRequiredSignatures, setSize); + finalBitfield = Bitfield.subsample(prevRandao, bitfield, setSize, numRequiredSignatures); console.log("print finalBitField"); printBitArray(finalBitfield); diff --git a/contracts/test/Bitfield.t.sol b/contracts/test/Bitfield.t.sol index e1ac5893b..bf384282f 100644 --- a/contracts/test/Bitfield.t.sol +++ b/contracts/test/Bitfield.t.sol @@ -20,7 +20,7 @@ contract BitfieldTest is Test { uint256[] memory bitSetArray = json.readUintArray(".participants"); uint256[] memory initialBitField = bw.createBitfield(bitSetArray, setSize); - uint256[] memory finalBitfield = bw.subsample(67, initialBitField, 30, setSize); + uint256[] memory finalBitfield = bw.subsample(67, initialBitField, setSize, 30); uint256 counter = 0; for (uint256 i = 0; i < bitSetArray.length; i++) { From fca1fd9691c7cb58a5b7ba6846d90120a0ef910a Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 14:36:18 +0200 Subject: [PATCH 04/10] refactor --- contracts/src/BeefyClient.sol | 21 ++++++++------------- contracts/src/utils/Bitfield.sol | 8 ++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index 03c3a96bb..58de3ba64 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -271,14 +271,9 @@ contract BeefyClient { revert InvalidCommitment(); } - if (bitfield.length != Bitfield.containerLength(vset.length)) { - revert InvalidBitfield(); - } - // Check if merkle proof is valid based on the validatorSetRoot and if proof is included in bitfield if ( !isValidatorInSet(vset, proof.account, proof.index, proof.proof) - || proof.index >= vset.length || !Bitfield.isSet(bitfield, proof.index) ) { revert InvalidValidatorProof(); @@ -291,6 +286,10 @@ contract BeefyClient { revert InvalidSignature(); } + if (bitfield.length != Bitfield.containerLength(vset.length)) { + revert InvalidBitfield(); + } + // For the initial submission, the supplied bitfield should claim that more than // two thirds of the validator set have sign the commitment if (Bitfield.countSetBits(bitfield, vset.length) < computeQuorum(vset.length)) { @@ -373,10 +372,6 @@ contract BeefyClient { revert InvalidCommitment(); } - if (bitfield.length != Bitfield.containerLength(vset.length)) { - revert InvalidBitfield(); - } - verifyCommitment(commitmentHash, ticketID, bitfield, vset, proofs); bytes32 newMMRRoot = ensureProvidesMMRRoot(commitment); @@ -528,13 +523,13 @@ contract BeefyClient { for (uint256 i = 0; i < proofs.length; i++) { ValidatorProof calldata proof = proofs[i]; - // Check that validator is in bitfield - if (proof.index >= vset.length || !Bitfield.isSet(finalbitfield, proof.index)) { + // Check that validator is actually in a validator set + if (!isValidatorInSet(vset, proof.account, proof.index, proof.proof)) { revert InvalidValidatorProof(); } - // Check that validator is actually in a validator set - if (!isValidatorInSet(vset, proof.account, proof.index, proof.proof)) { + // Check that validator is in bitfield + if (!Bitfield.isSet(finalbitfield, proof.index)) { revert InvalidValidatorProof(); } diff --git a/contracts/src/utils/Bitfield.sol b/contracts/src/utils/Bitfield.sol index 10b207fe0..21bc03e23 100644 --- a/contracts/src/utils/Bitfield.sol +++ b/contracts/src/utils/Bitfield.sol @@ -7,6 +7,9 @@ import {Bits} from "./Bits.sol"; library Bitfield { using Bits for uint256; + + error InvalidSamplingParams(); + /** * @dev Constants used to efficiently calculate the hamming weight of a bitfield. See * https://en.wikipedia.org/wiki/Hamming_weight#Efficient_implementation for an explanation of those constants. @@ -45,6 +48,11 @@ library Bitfield { uint256 priorBitfieldSize, uint256 n ) internal pure returns (uint256[] memory outputBitfield) { + if (priorBitfield.length != Bitfield.containerLength(priorBitfieldSize) + || n > countSetBits(priorBitfield, priorBitfieldSize)) { + revert InvalidSamplingParams(); + } + outputBitfield = new uint256[](priorBitfield.length); uint256 found = 0; From 89e8529cc8e0397641b38885eaaa1f3c0a1cf41a Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 15:17:20 +0200 Subject: [PATCH 05/10] refactor --- contracts/src/BeefyClient.sol | 10 +++------- contracts/test/BeefyClient.t.sol | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index 58de3ba64..48a4510cd 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -207,7 +207,6 @@ contract BeefyClient { error InvalidValidatorProof(); error InvalidValidatorProofLength(); error CommitmentNotRelevant(); - error NotEnoughClaims(); error PrevRandaoAlreadyCaptured(); error PrevRandaoNotCaptured(); error StaleCommitment(); @@ -286,14 +285,11 @@ contract BeefyClient { revert InvalidSignature(); } - if (bitfield.length != Bitfield.containerLength(vset.length)) { - revert InvalidBitfield(); - } - // For the initial submission, the supplied bitfield should claim that more than // two thirds of the validator set have sign the commitment - if (Bitfield.countSetBits(bitfield, vset.length) < computeQuorum(vset.length)) { - revert NotEnoughClaims(); + if (bitfield.length != Bitfield.containerLength(vset.length) + || Bitfield.countSetBits(bitfield, vset.length) < computeQuorum(vset.length)) { + revert InvalidBitfield(); } tickets[createTicketID(msg.sender, commitmentHash)] = Ticket({ diff --git a/contracts/test/BeefyClient.t.sol b/contracts/test/BeefyClient.t.sol index 040568c5e..c027f4134 100644 --- a/contracts/test/BeefyClient.t.sol +++ b/contracts/test/BeefyClient.t.sol @@ -775,7 +775,7 @@ contract BeefyClientTest is Test { uint256[] memory initialBits = absentBitfield; Bitfield.set(initialBits, finalValidatorProofs[0].index); printBitArray(initialBits); - vm.expectRevert(BeefyClient.NotEnoughClaims.selector); + vm.expectRevert(BeefyClient.InvalidBitfield.selector); beefyClient.submitInitial(commitment, initialBits, finalValidatorProofs[0]); } From 545d0386a0fba36e39a676d24d7f1b92eb5972b1 Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 16:12:17 +0200 Subject: [PATCH 06/10] rename parameter --- contracts/src/utils/Bitfield.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/src/utils/Bitfield.sol b/contracts/src/utils/Bitfield.sol index 21bc03e23..8ce828e72 100644 --- a/contracts/src/utils/Bitfield.sol +++ b/contracts/src/utils/Bitfield.sol @@ -211,7 +211,7 @@ library Bitfield { } // Calculate length of uint256 bitfield array based on rounding up to number of uint256 needed - function containerLength(uint256 length) internal pure returns (uint256) { - return (length + 255) / 256; + function containerLength(uint256 bitfieldSize) internal pure returns (uint256) { + return (bitfieldSize + 255) / 256; } } From 8c4940507dff9b8beee48aa6e5b78944d410189a Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 16:17:26 +0200 Subject: [PATCH 07/10] Improve documentation --- docs/architecture/verification/polkadot/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/architecture/verification/polkadot/README.md b/docs/architecture/verification/polkadot/README.md index 48e618981..aa6662165 100644 --- a/docs/architecture/verification/polkadot/README.md +++ b/docs/architecture/verification/polkadot/README.md @@ -1,6 +1,6 @@ # Polkadot -We use Polkadot’s[ BEEFY](https://github.com/paritytech/grandpa-bridge-gadget/blob/master/docs/walkthrough.md) gadget to implement an efficient light client that only needs to verify a very small subset of relay chain validator signatures. BEEFY is live on Rococo, and is awaiting deployment on Kusama and Polkadot. +We use Polkadot’s [BEEFY](https://eprint.iacr.org/2025/057.pdf) protocol to implement an efficient light client that only needs to verify a very small subset of relay chain validator signatures. BEEFY is live on Rococo, and is awaiting deployment on Kusama and Polkadot. Fundamentally, the BEEFY light client allows the bridge to prove that a specified parachain header was finalized by the relay chain. @@ -31,7 +31,7 @@ In the EVM there is no cryptographically secure source of randomness. Instead, w ## Signature Sampling -The choice $$N$$ is described by the [formal analysis of signature sampling from W3F](https://hackmd.io/c6STzrvfQGyN2P2rVmTmoA). It consists of the following variables. +The choice $$N$$ is described by the [formal analysis of signature sampling from W3F](https://eprint.iacr.org/2025/057.pdf). It consists of the following variables. $$ N = \lceil log_2(R * V * \frac{1}{S} *(75+E)*172.8)\rceil + 1 + 2 \lceil log_2(C) \rceil @@ -48,9 +48,9 @@ $$ From the list above 1 and 2 are known in the light client and can be calculated on-chain. Variables 3, 4.1, and 4.2 are not known by the light client and are instead calculated off-chain and set as a minimum number of required signatures during the initialization of the light client. This minimum is immutable for the life time of the light client. -* [Minimum required signatures](../../../../contracts/src/BeefyClient.sol#L185-L190) -* [Dynamic signature calculation](../../../../contracts/src/BeefyClient.sol#L444) -* [Python implementation of required signatures](../../../../scripts/beefy\_signature\_sampling.py#L9) +## Slashing of BEEFY validators who produce equivocations + +The BEEFY protocol includes a mechanism to slash validators who commit fraud. For example, the entire set of active validators could maliciously sign a fraudulent commitment, and collude with a relayer to submit it to our light client. We have equivocation fishermen that can detect such activity and submit [proof-of-equivocation](https://docs.rs/pallet-beefy/latest/pallet_beefy/struct.EquivocationOffence.html) back to the Polkadot relay chain. This is vital to ensuring that all BEEFY validators act honestly. ## Message Verification From 14b39e95dd06227d722d1e931014df1ffb9a475e Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 16:29:24 +0200 Subject: [PATCH 08/10] improve docs again --- docs/architecture/verification/polkadot/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/verification/polkadot/README.md b/docs/architecture/verification/polkadot/README.md index aa6662165..4afa4cd61 100644 --- a/docs/architecture/verification/polkadot/README.md +++ b/docs/architecture/verification/polkadot/README.md @@ -50,7 +50,7 @@ From the list above 1 and 2 are known in the light client and can be calculated ## Slashing of BEEFY validators who produce equivocations -The BEEFY protocol includes a mechanism to slash validators who commit fraud. For example, the entire set of active validators could maliciously sign a fraudulent commitment, and collude with a relayer to submit it to our light client. We have equivocation fishermen that can detect such activity and submit [proof-of-equivocation](https://docs.rs/pallet-beefy/latest/pallet_beefy/struct.EquivocationOffence.html) back to the Polkadot relay chain. This is vital to ensuring that all BEEFY validators act honestly. +The BEEFY protocol includes a mechanism to slash validators who commit fraud. For example, a subset of active validators could maliciously sign a fraudulent commitment, and collude with a relayer to submit it to our light client. However, we have an equivocation fisherman that can detect such activity and submit [proof-of-equivocation](https://docs.rs/pallet-beefy/latest/pallet_beefy/struct.EquivocationOffence.html) back to the Polkadot relay chain, whereupon these validators will be slashed 50% of their stake. ## Message Verification From fda9e39adf7f61aece8f75e51883ebc623cf6dc4 Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 21 Sep 2025 16:41:09 +0200 Subject: [PATCH 09/10] whitespace --- contracts/src/utils/Bitfield.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/src/utils/Bitfield.sol b/contracts/src/utils/Bitfield.sol index 8ce828e72..0e1f9bc45 100644 --- a/contracts/src/utils/Bitfield.sol +++ b/contracts/src/utils/Bitfield.sol @@ -7,7 +7,6 @@ import {Bits} from "./Bits.sol"; library Bitfield { using Bits for uint256; - error InvalidSamplingParams(); /** From 902dd566d7c647a478e1f346521220a16e2743c9 Mon Sep 17 00:00:00 2001 From: Ron Date: Sun, 21 Sep 2025 22:43:14 +0800 Subject: [PATCH 10/10] More tests (#1572) --- contracts/test/Bitfield.t.sol | 30 +++++++++++++++++++++++- contracts/test/mocks/BitfieldWrapper.sol | 4 ++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/contracts/test/Bitfield.t.sol b/contracts/test/Bitfield.t.sol index bf384282f..9eb66346e 100644 --- a/contracts/test/Bitfield.t.sol +++ b/contracts/test/Bitfield.t.sol @@ -11,6 +11,8 @@ import {stdJson} from "forge-std/StdJson.sol"; contract BitfieldTest is Test { using stdJson for string; + uint256 public constant SEED = 2954466101346023252933346884990731083400112195551952331583346342070284928184; + function testBitfieldSubsampling() public { BitfieldWrapper bw = new BitfieldWrapper(); @@ -20,7 +22,7 @@ contract BitfieldTest is Test { uint256[] memory bitSetArray = json.readUintArray(".participants"); uint256[] memory initialBitField = bw.createBitfield(bitSetArray, setSize); - uint256[] memory finalBitfield = bw.subsample(67, initialBitField, setSize, 30); + uint256[] memory finalBitfield = bw.subsample(SEED, initialBitField, setSize, 30); uint256 counter = 0; for (uint256 i = 0; i < bitSetArray.length; i++) { @@ -139,4 +141,30 @@ contract BitfieldTest is Test { assertTrue(bw.countSetBits(bitfield, setSize / 2) <= Bitfield.countSetBits(bitfield)); assertTrue(bw.countSetBits(bitfield, 10) <= Bitfield.countSetBits(bitfield)); } + + function testBitfieldSubsamplingWithInvalidParams() public { + BitfieldWrapper bw = new BitfieldWrapper(); + + string memory json = + vm.readFile(string.concat(vm.projectRoot(), "/test/data/beefy-validator-set.json")); + uint32 setSize = uint32(json.readUint(".validatorSetSize")); + + uint256 length = (setSize+255) / 256; + uint256 N = 26; + uint256[] memory initialBitField = new uint256[](length); + for (uint256 i = 0; i < N; i++) { + Bitfield.set(initialBitField, i); + } + + uint256[] memory finalBitfield = bw.subsample(SEED, initialBitField, setSize, N); + assertEq(Bitfield.countSetBits(finalBitfield), N); + + // Test setSize overflow + vm.expectRevert(Bitfield.InvalidSamplingParams.selector); + finalBitfield = bw.subsample(SEED, initialBitField, setSize * 2, N); + + // Test N overflow + vm.expectRevert(Bitfield.InvalidSamplingParams.selector); + finalBitfield = bw.subsample(SEED, initialBitField, setSize, N+1); + } } diff --git a/contracts/test/mocks/BitfieldWrapper.sol b/contracts/test/mocks/BitfieldWrapper.sol index cba26cadf..25120fdbb 100644 --- a/contracts/test/mocks/BitfieldWrapper.sol +++ b/contracts/test/mocks/BitfieldWrapper.sol @@ -12,12 +12,12 @@ contract BitfieldWrapper { return Bitfield.createBitfield(bitsToSet, length); } - function subsample(uint256 seed, uint256[] memory prior, uint256 n, uint256 length) + function subsample(uint256 seed, uint256[] memory prior, uint256 length, uint256 n) public pure returns (uint256[] memory bitfield) { - return Bitfield.subsample(seed, prior, n, length); + return Bitfield.subsample(seed, prior, length, n); } function countSetBits(uint256[] memory self, uint256 maxBits) public pure returns (uint256) {