diff --git a/contracts/src/BeefyClient.sol b/contracts/src/BeefyClient.sol index 48ebf4b66..9e689e00f 100644 --- a/contracts/src/BeefyClient.sol +++ b/contracts/src/BeefyClient.sol @@ -412,7 +412,9 @@ contract BeefyClient { bytes32 newMMRRoot = ensureProvidesMMRRoot(commitment); if (is_next_session) { - if (leaf.nextAuthoritySetID != nextValidatorSet.id + 1) { + // The id for candidate nextValidatorSet should be greater than the current + // nextValidatorSet id + if (leaf.nextAuthoritySetID <= nextValidatorSet.id) { revert InvalidMMRLeaf(); } bool leafIsValid = MMRProof.verifyLeafProof( @@ -566,7 +568,9 @@ contract BeefyClient { verifyFiatShamirCommitment(commitmentHash, bitfield, vset, proofs); if (is_next_session) { - if (leaf.nextAuthoritySetID != nextValidatorSet.id + 1) { + // The id for candidate nextValidatorSet should be greater than the current + // nextValidatorSet id + if (leaf.nextAuthoritySetID <= nextValidatorSet.id) { revert InvalidMMRLeaf(); } bool leafIsValid = MMRProof.verifyLeafProof( diff --git a/contracts/test/BeefyClient.t.sol b/contracts/test/BeefyClient.t.sol index 99935825a..eeaf36094 100644 --- a/contracts/test/BeefyClient.t.sol +++ b/contracts/test/BeefyClient.t.sol @@ -683,6 +683,39 @@ contract BeefyClientTest is Test { ); } + /// @dev Candidate nextValidatorSet ID must be > current nextValidatorSet.id; revert when less. + function testSubmitFailWithInvalidMMRLeafWhenNextAuthoritySetIdLessThanCurrent() public { + //initialize with previous set (nextValidatorSet.id = setId) + BeefyClient.Commitment memory commitment = initialize(setId - 1); + + beefyClient.submitInitial(commitment, bitfield, finalValidatorProofs[0]); + + vm.roll(block.number + randaoCommitDelay); + vm.prevrandao(bytes32(uint256(prevRandao))); + beefyClient.commitPrevRandao(commitHash); + createFinalProofs(); + + // nextValidatorSet.id == setId; set leaf to less than that + mmrLeaf.nextAuthoritySetID = uint64(setId > 0 ? setId - 1 : 0); + vm.expectRevert(BeefyClient.InvalidMMRLeaf.selector); + beefyClient.submitFinal( + commitment, bitfield, finalValidatorProofs, mmrLeaf, mmrLeafProofs, leafProofOrder + ); + } + + /// @dev FiatShamir path: candidate nextValidatorSet ID must be > current; revert when less. + function testSubmitFiatShamirFailWithInvalidMMRLeafWhenNextAuthoritySetIdLessThanCurrent() + public + { + BeefyClient.Commitment memory commitment = initialize(setId - 1); + + mmrLeaf.nextAuthoritySetID = uint64(setId > 0 ? setId - 1 : 0); + vm.expectRevert(BeefyClient.InvalidMMRLeaf.selector); + beefyClient.submitFiatShamir( + commitment, bitfield, fiatShamirValidatorProofs, mmrLeaf, mmrLeafProofs, leafProofOrder + ); + } + function testSubmitFailWithInvalidMMRLeafProof() public { //initialize with previous set BeefyClient.Commitment memory commitment = initialize(setId - 1); diff --git a/contracts/test/BeefyClientAdvanced.t.sol b/contracts/test/BeefyClientAdvanced.t.sol index 4f3353514..03906f878 100644 --- a/contracts/test/BeefyClientAdvanced.t.sol +++ b/contracts/test/BeefyClientAdvanced.t.sol @@ -341,6 +341,52 @@ contract BeefyClientAdvancedTest is Test { assertEq(beefyClient.latestBeefyBlock(), uint64(1), "beefy block updated"); } + /// @dev Candidate nextValidatorSet ID may be more than nextValidatorSet.id + 1 (e.g. skip a set). + function testFiatShamirCommitWithNextValidatorSetIdMoreThanPlusOne() public { + // nextValidatorSet.id == VSET_ID + 1; use leaf.nextAuthoritySetID = VSET_ID + 3 (> id + 1) + BeefyClient.MMRLeaf memory leaf; + leaf.version = 0; + leaf.parentNumber = 0; + leaf.parentHash = bytes32(0); + leaf.nextAuthoritySetID = VSET_ID + 3; + leaf.nextAuthoritySetLen = uint32(VSET_LEN); + leaf.nextAuthoritySetRoot = keccak256(abi.encodePacked("next-authority-root")); + leaf.parachainHeadsRoot = bytes32(0); + + bytes memory encodedLeaf = bytes.concat( + ScaleCodec.encodeU8(leaf.version), + ScaleCodec.encodeU32(leaf.parentNumber), + leaf.parentHash, + ScaleCodec.encodeU64(leaf.nextAuthoritySetID), + ScaleCodec.encodeU32(leaf.nextAuthoritySetLen), + leaf.nextAuthoritySetRoot, + leaf.parachainHeadsRoot + ); + bytes32 leafHash = keccak256(encodedLeaf); + + (bytes32 mmrRoot, bytes32[] memory leafProof, uint256 leafProofOrder) = + MerkleLib.buildMerkleWithTargetLeaf(16, 3, leafHash); + + (BeefyClient.Commitment memory commitment, bytes32 commitmentHash) = + _buildCommitment(1, VSET_ID + 1, mmrRoot); + + uint256 quorum = beefyClient.computeQuorum_public(VSET_LEN); + uint256[] memory bitfield = new uint256[](Bitfield.containerLength(VSET_LEN)); + for (uint256 i = 0; i < quorum; i++) { + Bitfield.set(bitfield, i); + } + + (, BeefyClient.ValidatorProof[] memory finalProofs) = _generateFiatShamirProofs( + commitment, commitmentHash, bitfield, FIAT_SHAMIR_REQUIRED_SIGNATURES + ); + + beefyClient.submitFiatShamir( + commitment, bitfield, finalProofs, leaf, leafProof, leafProofOrder + ); + assertEq(beefyClient.latestMMRRoot(), mmrRoot, "MMR root updated"); + assertEq(beefyClient.latestBeefyBlock(), uint64(1), "beefy block updated"); + } + // ---------------------- Helpers ---------------------- function _buildCommitment(uint32 blockNumber, uint64 validatorSetID, bytes32 mmrRoot)