SNO-521: Check single validator signature in testSubmit* (paritytech#867

)

* Require a single proof in submitInitial* calls

* Skip handover when testing stale commitment

* s/_proofs/proofs/g

* Remove extra proof

* Remove explicit any

* Check that proof index is in bitfield

core/packages/contracts/scripts/ffiWrapper.ts

@@ -1,6 +1,6 @@
 #!/usr/bin/env node
 import { ValidatorSet, createRandomSubset, readSetBits } from "./helpers"
-import { ethers } from "ethers"
+import { BigNumber, ethers } from "ethers"
 import fs from "fs"
 import type { BeefyClient } from "../types/BeefyClient"
 import { accounts } from "./wallets"
@@ -45,9 +45,8 @@ if (command == "GenerateInitialSet") {
     } else {
         validatorSet = new ValidatorSet(validatorSetID, validatorSetSize)
     }
-    const validatorProof = validatorSet.createSignatureProof(subset[0], commitHash)
     const finalBitfieldLength = parseInt(process.argv[3])
-    let finalBitfield: any = []
+    let finalBitfield: BigNumber[] = []
     for (let i = 0; i < finalBitfieldLength; i++) {
         finalBitfield.push(ethers.BigNumber.from(process.argv[4 + i]))
     }
@@ -58,15 +57,13 @@ if (command == "GenerateInitialSet") {
         `${encoder.encode(
             [
                 "bytes32",
-                "tuple(uint8 v, bytes32 r, bytes32 s, uint256 index,address account,bytes32[] proof)",
                 "tuple(uint8 v, bytes32 r, bytes32 s, uint256 index,address account,bytes32[] proof)[]",
                 "bytes32[]",
                 "tuple(uint8 version,uint32 parentNumber,bytes32 parentHash,uint64 nextAuthoritySetID,uint32 nextAuthoritySetLen,bytes32 nextAuthoritySetRoot,bytes32 parachainHeadsRoot)",
                 "uint256",
             ],
             [
                 validatorSet.root,
-                validatorProof,
                 validatorFinalProofs,
                 mmrLeafProofs,
                 mmrLeaf,

core/packages/contracts/src/BeefyClient.sol

@@ -196,21 +196,34 @@ contract BeefyClient is Ownable {
      * @dev Begin submission of commitment that was signed by the current validator set
      * @param commitmentHash contains the commitmentHash signed by the validators
      * @param bitfield a bitfield claiming which validators have signed the commitment
+     * @param proof a proof that a single validator from currentValidatorSet has signed the commitmentHash
      */
-    function submitInitial(bytes32 commitmentHash, uint256[] calldata bitfield) external payable {
-        doSubmitInitial(currentValidatorSet, commitmentHash, bitfield);
+    function submitInitial(bytes32 commitmentHash, uint256[] calldata bitfield, ValidatorProof calldata proof) external payable {
+        doSubmitInitial(currentValidatorSet, commitmentHash, bitfield, proof);
     }
 
     /**
      * @dev Begin submission of commitment that was signed by the next validator set
      * @param commitmentHash contains the commitmentHash signed by the validators
      * @param bitfield a bitfield claiming which validators have signed the commitment
+     * @param proof a proof that a single validator from nextValidatorSet has signed the commitmentHash
      */
-    function submitInitialWithHandover(bytes32 commitmentHash, uint256[] calldata bitfield) external payable {
-        doSubmitInitial(nextValidatorSet, commitmentHash, bitfield);
+    function submitInitialWithHandover(bytes32 commitmentHash, uint256[] calldata bitfield, ValidatorProof calldata proof) external payable {
+        doSubmitInitial(nextValidatorSet, commitmentHash, bitfield, proof);
     }
 
-    function doSubmitInitial(ValidatorSet memory vset, bytes32 commitmentHash, uint256[] calldata bitfield) internal {
+    function doSubmitInitial(ValidatorSet memory vset, bytes32 commitmentHash, uint256[] calldata bitfield, ValidatorProof calldata proof) internal {
+        // 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)) {
+            revert InvalidValidatorProof();
+        }
+
+        // Check if validatorSignature is correct, ie. check if it matches
+        // the signature of senderPublicKey on the commitmentHash
+        if (ECDSA.recover(commitmentHash, proof.v, proof.r, proof.s) != proof.account) {
+            revert InvalidSignature();
+        }
+
         // 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 - (vset.length - 1) / 3) {

core/packages/contracts/test/BeefyClient.t.sol

@@ -60,14 +60,14 @@ contract BeefyClientTest is Test {
         for (uint256 i = 0; i < finalBitfield.length; i++) {
             inputs[i + 4] = Strings.toString(finalBitfield[i]);
         }
-        BeefyClient.ValidatorProof[] memory _proofs;
-        (root,, _proofs, mmrLeafProofs, mmrLeaf, leafProofOrder) = abi.decode(
+        BeefyClient.ValidatorProof[] memory proofs;
+        (root, proofs, mmrLeafProofs, mmrLeaf, leafProofOrder) = abi.decode(
             vm.ffi(inputs),
-            (bytes32, BeefyClient.ValidatorProof, BeefyClient.ValidatorProof[], bytes32[], BeefyClient.MMRLeaf, uint256)
+            (bytes32, BeefyClient.ValidatorProof[], bytes32[], BeefyClient.MMRLeaf, uint256)
         );
         // Cache finalValidatorProofs to storage in order to reuse in submitFinal later
-        for (uint256 i = 0; i < _proofs.length; i++) {
-            finalValidatorProofs.push(_proofs[i]);
+        for (uint256 i = 0; i < proofs.length; i++) {
+            finalValidatorProofs.push(proofs[i]);
         }
         console.log("current validator's merkle root is: %s", Strings.toHexString(uint256(root), 32));
     }
@@ -83,7 +83,7 @@ contract BeefyClientTest is Test {
     function testSubmit() public {
         initialize(setId);
 
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
 
         // mine random delay blocks
         vm.roll(block.number + randaoCommitDelay);
@@ -102,7 +102,7 @@ contract BeefyClientTest is Test {
     function testSubmitFailInvalidSignature() public {
         initialize(setId);
 
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
 
         // mine random delay blocks
         vm.roll(block.number + randaoCommitDelay);
@@ -123,7 +123,7 @@ contract BeefyClientTest is Test {
     function testSubmitFailValidatorNotInBitfield() public {
         initialize(setId);
 
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
 
         // mine random delay blocks
         vm.roll(block.number + randaoCommitDelay);
@@ -145,7 +145,7 @@ contract BeefyClientTest is Test {
         // first round of submit should be fine
         testSubmit();
 
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
         vm.roll(block.number + randaoCommitDelay);
         vm.prevrandao(bytes32(uint256(difficulty)));
         beefyClient.commitPrevRandao(commitHash);
@@ -158,7 +158,7 @@ contract BeefyClientTest is Test {
     function testSubmitFailWithInvalidBitfield() public {
         initialize(setId);
 
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
 
         vm.roll(block.number + randaoCommitDelay);
 
@@ -175,7 +175,7 @@ contract BeefyClientTest is Test {
 
     function testSubmitFailWithoutPrevRandao() public {
         initialize(setId);
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
         BeefyClient.Commitment memory commitment = BeefyClient.Commitment(blockNumber, setId, payload);
         // reverted without commit PrevRandao
         vm.expectRevert(BeefyClient.PrevRandaoNotCaptured.selector);
@@ -184,7 +184,7 @@ contract BeefyClientTest is Test {
 
     function testSubmitFailForPrevRandaoTooEarlyOrTooLate() public {
         initialize(setId);
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
         // reverted for commit PrevRandao too early
         vm.expectRevert(BeefyClient.WaitPeriodNotOver.selector);
         beefyClient.commitPrevRandao(commitHash);
@@ -197,7 +197,7 @@ contract BeefyClientTest is Test {
 
     function testSubmitFailForPrevRandaoCapturedMoreThanOnce() public {
         initialize(setId);
-        beefyClient.submitInitial(commitHash, bitfield);
+        beefyClient.submitInitial(commitHash, bitfield, finalValidatorProofs[0]);
         vm.roll(block.number + randaoCommitDelay);
         vm.prevrandao(bytes32(uint256(difficulty)));
         beefyClient.commitPrevRandao(commitHash);
@@ -210,7 +210,7 @@ contract BeefyClientTest is Test {
         //initialize with previous set
         initialize(setId - 1);
 
-        beefyClient.submitInitialWithHandover(commitHash, bitfield);
+        beefyClient.submitInitialWithHandover(commitHash, bitfield, finalValidatorProofs[0]);
 
         vm.roll(block.number + randaoCommitDelay);
 
@@ -229,7 +229,7 @@ contract BeefyClientTest is Test {
         //initialize with previous set
         initialize(setId - 1);
 
-        beefyClient.submitInitialWithHandover(commitHash, bitfield);
+        beefyClient.submitInitialWithHandover(commitHash, bitfield, finalValidatorProofs[0]);
 
         BeefyClient.Commitment memory commitment = BeefyClient.Commitment(blockNumber, setId, payload);
 
@@ -240,9 +240,9 @@ contract BeefyClientTest is Test {
     }
 
     function testSubmitWithHandoverFailStaleCommitment() public {
-        testSubmitWithHandover();
+        testSubmit();
 
-        beefyClient.submitInitialWithHandover(commitHash, bitfield);
+        beefyClient.submitInitialWithHandover(commitHash, bitfield, finalValidatorProofs[0]);
 
         vm.roll(block.number + randaoCommitDelay);