Minor improvements for BEEFY light client (#993)

contracts/src/Verification.sol

@@ -11,11 +11,11 @@ library Verification {
     /// @dev Merkle proof for parachain header finalized by BEEFY
     /// Reference: https://github.com/paritytech/polkadot/blob/09b61286da11921a3dda0a8e4015ceb9ef9cffca/runtime/rococo/src/lib.rs#L1312
     struct HeadProof {
-        /// @dev The leaf index of the parachain being proven
+        // The leaf index of the parachain being proven
         uint256 pos;
-        /// @dev The number of leaves in the merkle tree
+        // The number of leaves in the merkle tree
         uint256 width;
-        /// @dev The proof items
+        // The proof items
         bytes32[] proof;
     }
 
@@ -54,15 +54,15 @@ library Verification {
 
     /// @dev A chain of proofs
     struct Proof {
-        /// @dev The parachain header containing the message commitment as a digest item
+        // The parachain header containing the message commitment as a digest item
         ParachainHeader header;
-        /// @dev The proof used to generate a merkle root of parachain heads
+        // The proof used to generate a merkle root of parachain heads
         HeadProof headProof;
-        /// @dev The MMR leaf to be proven
+        // The MMR leaf to be proven
         MMRLeafPartial leafPartial;
-        /// @dev The MMR leaf prove
+        // The MMR leaf prove
         bytes32[] leafProof;
-        /// @dev The order in which proof items should be combined
+        // The order in which proof items should be combined
         uint256 leafProofOrder;
     }
 

contracts/src/utils/Bitfield.sol

@@ -74,11 +74,8 @@ library Bitfield {
 
         bitfield = new uint256[](arrayLength);
 
-        for (uint256 i = 0; i < bitsToSet.length;) {
+        for (uint256 i = 0; i < bitsToSet.length; i++) {
             set(bitfield, bitsToSet[i]);
-            unchecked {
-                ++i;
-            }
         }
 
         return bitfield;

contracts/src/utils/MMRProof.sol

@@ -25,11 +25,8 @@ library MMRProof {
         }
 
         bytes32 acc = leafHash;
-        for (uint256 i = 0; i < proof.length;) {
+        for (uint256 i = 0; i < proof.length; i++) {
             acc = hashPairs(acc, proof[i], (proofOrder >> i) & 1);
-            unchecked {
-                i++;
-            }
         }
         return root == acc;
     }

contracts/src/utils/Math.sol

@@ -95,7 +95,9 @@ library Math {
     function saturatingAdd(uint16 a, uint16 b) internal pure returns (uint16) {
         unchecked {
             uint16 c = a + b;
-            if (c < a) return 0xFFFF;
+            if (c < a) {
+                return 0xFFFF;
+            }
             return c;
         }
     }
@@ -105,7 +107,9 @@ library Math {
      */
     function saturatingSub(uint256 a, uint256 b) internal pure returns (uint256) {
         unchecked {
-            if (b > a) return 0;
+            if (b > a) {
+                return 0;
+            }
             return a - b;
         }
     }

contracts/src/utils/Counter.sol → contracts/src/utils/Uint16Array.sol

@@ -29,40 +29,43 @@ pragma solidity 0.8.22;
  * above table counter XX is at logical index 22. It will convert to a physical index of 1 in the array and
  * then to bit-index 128 to 143 of uint256[1].
  */
-library Counter {
+library Uint16Array {
+    struct Array {
+        uint256[] data;
+    }
     /**
      * @dev Creates a new counter which can store at least `length` counters.
      * @param length The amount of counters.
      */
-    function createCounter(uint256 length) internal pure returns (uint256[] memory) {
-        // create space for length counters and round up if needed.
-        uint256 counterLength = length / 16 + (length % 16 == 0 ? 0 : 1);
-        // Counters are returned zeroed.
-        return new uint256[](counterLength);
+
+    function create(uint256 length) internal pure returns (Array memory) {
+        // create space for `length` elements and round up if needed.
+        uint256 bufferLength = length / 16 + (length % 16 == 0 ? 0 : 1);
+        return Array({data: new uint256[](bufferLength)});
     }
 
     /**
      * @dev Gets the counter at the logical index
-     * @param self The counter array.
+     * @param self The array.
      * @param index The logical index.
      */
-    function get(uint256[] storage self, uint256 index) internal view returns (uint16) {
+    function get(Array storage self, uint256 index) internal view returns (uint16) {
         // Right-shift the index by 4. This truncates the first 4 bits (bit-index) leaving us with the index
         // into the array.
         uint256 element = index >> 4;
         // Mask out the first 4 bytes of the logical index to give us the bit-index.
         uint8 inside = uint8(index) & 0x0F;
         // find the element in the array, shift until its bit index and mask to only take the first 16 bits.
-        return uint16((self[element] >> (16 * inside)) & 0xFFFF);
+        return uint16((self.data[element] >> (16 * inside)) & 0xFFFF);
     }
 
     /**
      * @dev Sets the counter at the logical index.
-     * @param self The counter array.
+     * @param self The array.
      * @param index The logical index of the counter in the array.
      * @param value The value to set the counter to.
      */
-    function set(uint256[] storage self, uint256 index, uint16 value) internal {
+    function set(Array storage self, uint256 index, uint16 value) internal {
         // Right-shift the index by 4. This truncates the first 4 bits (bit-index) leaving us with the index
         // into the array.
         uint256 element = index >> 4;
@@ -73,6 +76,6 @@ library Counter {
         // Shift the value to the bit index.
         uint256 shiftedValue = uint256(value) << (16 * inside);
         // Take the element, apply the zero mask to clear the existing value, and then apply the shifted value with bitwise or.
-        self[element] = self[element] & zero | shiftedValue;
+        self.data[element] = self.data[element] & zero | shiftedValue;
     }
 }

contracts/test/BeefyClient.t.sol

@@ -704,4 +704,8 @@ contract BeefyClientTest is Test {
         result = beefyClient.computeNumRequiredSignatures_public(1, 0, 0);
         assertEq(1, result, "C");
     }
+
+    function testStorageToStorageCopies() public {
+        beefyClient.copyCounters();
+    }
 }

contracts/test/Counter.t.sol → contracts/test/Uint16Array.t.sol

@@ -4,12 +4,12 @@ pragma solidity 0.8.22;
 import "forge-std/Test.sol";
 import "forge-std/console.sol";
 
-import {Counter} from "../src/utils/Counter.sol";
+import {Uint16Array} from "../src/utils/Uint16Array.sol";
 
-contract CounterTest is Test {
-    using Counter for uint256[];
+contract Uint16ArrayTest is Test {
+    using Uint16Array for Uint16Array.Array;
 
-    uint256[] counters;
+    Uint16Array.Array counters;
 
     function setUp() public {
         delete counters;
@@ -18,26 +18,26 @@ contract CounterTest is Test {
     function testCounterCreatedInitializationRoundsUp() public {
         // 33 uint16s will require 3 uint256s
         uint256[] memory expected = new uint256[](3);
-        counters = Counter.createCounter(33);
-        assertEq(counters, expected);
+        counters = Uint16Array.create(33);
+        assertEq(counters.data, expected);
     }
 
     function testCounterWithLengthNotMultipleOf16() public {
         // 33 uint16s will require 3 uint256s
         uint256[] memory expected = new uint256[](3);
         expected[2] = 1;
 
-        counters = Counter.createCounter(33);
+        counters = Uint16Array.create(33);
         counters.set(32, counters.get(32) + 1);
-        assertEq(counters, expected);
+        assertEq(counters.data, expected);
     }
 
     function testCounterCreatedAsZeroed() public {
         uint256[] memory expected = new uint256[](2);
-        counters = Counter.createCounter(16);
-        counters[0] = 0xABABABAB;
-        counters = Counter.createCounter(32);
-        assertEq(counters, expected);
+        counters = Uint16Array.create(16);
+        counters.data[0] = 0xABABABAB;
+        counters = Uint16Array.create(32);
+        assertEq(counters.data, expected);
     }
 
     function testCounterSet() public {
@@ -46,23 +46,23 @@ contract CounterTest is Test {
         // Manually set the 16th index to 2.
         expected[1] = 2;
 
-        counters = Counter.createCounter(32);
+        counters = Uint16Array.create(32);
         counters.set(16, 2);
 
-        assertEq(counters, expected);
+        assertEq(counters.data, expected);
     }
 
     function testCounterGet() public {
-        counters = Counter.createCounter(32);
+        counters = Uint16Array.create(32);
 
         // Manually set the 16th index to 2.
-        counters[1] = 2;
+        counters.data[1] = 2;
 
         assertEq(counters.get(16), 2);
     }
 
     function testCounterGetAndSetAlongEntireRange() public {
-        counters = Counter.createCounter(32);
+        counters = Uint16Array.create(32);
         for (uint16 index = 0; index < 32; index++) {
             // Should be zero as the initial value.
             uint16 value = counters.get(index);
@@ -93,7 +93,7 @@ contract CounterTest is Test {
     }
 
     function testCounterGetAndSetWithTwoIterations() public {
-        counters = Counter.createCounter(300);
+        counters = Uint16Array.create(300);
         uint256 index = 0;
         uint16 value = 11;
         counters.set(index, value);

contracts/test/mocks/BeefyClientMock.sol

@@ -2,10 +2,11 @@
 pragma solidity 0.8.22;
 
 import {BeefyClient} from "../../src/BeefyClient.sol";
-import {Counter} from "../../src/utils/Counter.sol";
+import {Uint16Array} from "../../src/utils/Uint16Array.sol";
+import "forge-std/console.sol";
 
 contract BeefyClientMock is BeefyClient {
-    using Counter for uint256[];
+    using Uint16Array for Uint16Array.Array;
 
     constructor(uint256 randaoCommitDelay, uint256 randaoCommitExpiration, uint256 minNumRequiredSignatures)
         BeefyClient(
@@ -36,17 +37,40 @@ contract BeefyClientMock is BeefyClient {
         ValidatorSet calldata _nextValidatorSet
     ) external {
         latestBeefyBlock = _initialBeefyBlock;
-        currentValidatorSet = _initialValidatorSet;
-        currentValidatorSetCounters = Counter.createCounter(currentValidatorSet.length);
-        nextValidatorSet = _nextValidatorSet;
-        nextValidatorSetCounters = Counter.createCounter(nextValidatorSet.length);
+        currentValidatorSet.id = _initialValidatorSet.id;
+        currentValidatorSet.length = _initialValidatorSet.length;
+        currentValidatorSet.root = _initialValidatorSet.root;
+        currentValidatorSet.usageCounters = Uint16Array.create(currentValidatorSet.length);
+        nextValidatorSet.id = _nextValidatorSet.id;
+        nextValidatorSet.length = _nextValidatorSet.length;
+        nextValidatorSet.root = _nextValidatorSet.root;
+        nextValidatorSet.usageCounters = Uint16Array.create(nextValidatorSet.length);
+        console.log(currentValidatorSet.usageCounters.data.length);
+    }
+
+    // Used to verify integrity of storage to storage copies
+    function copyCounters() external {
+        currentValidatorSet.usageCounters = Uint16Array.create(1000);
+        for (uint256 i = 0; i < 1000; i++) {
+            currentValidatorSet.usageCounters.set(i, 5);
+        }
+        nextValidatorSet.usageCounters = Uint16Array.create(800);
+        for (uint256 i = 0; i < 800; i++) {
+            nextValidatorSet.usageCounters.set(i, 7);
+        }
+
+        // Perform the copy
+        currentValidatorSet = nextValidatorSet;
+
+        assert(currentValidatorSet.usageCounters.data.length == nextValidatorSet.usageCounters.data.length);
+        assert(currentValidatorSet.usageCounters.get(799) == 7);
     }
 
     function getValidatorCounter(bool next, uint256 index) public view returns (uint16) {
         if (next) {
-            return nextValidatorSetCounters.get(index);
+            return nextValidatorSet.usageCounters.get(index);
         }
-        return currentValidatorSetCounters.get(index);
+        return currentValidatorSet.usageCounters.get(index);
     }
 
     function computeNumRequiredSignatures_public(

docs/architecture/verification/polkadot/README.md

@@ -18,11 +18,11 @@ In collaboration with W3F, we have designed a protocol where the light client ne
 
 In the EVM there is no cryptographically secure source of randomness. Instead, we make our update protocol crypto-economically secure through an interactive update protocol. In this protocol, a candidate commitment is verified over 3 transactions. At a high level it works like this:
 
-1. `submitInitial` - In the [first transaction](../../../../contracts/src/BeefyClient.sol#L233), the relayer submits the commitment, a randomly selected validator signature, and an initial bitfield claiming which validators have signed the commitment.
+1. `submitInitial` - In the first transaction, the relayer submits the commitment, a randomly selected validator signature, and an initial bitfield claiming which validators have signed the commitment.
 2. The relayer must then wait [MAX\_SEED\_LOOKAHEAD](https://eth2book.info/bellatrix/part3/config/preset/#max\_seed\_lookahead) blocks.
-3. `commitPrevRandao` - The relayer submits a [second transaction](../../../../contracts/src/BeefyClient.sol#L288) to reveal and commit to a random seed, derived from Ethereum's [RANDAO](https://eips.ethereum.org/EIPS/eip-4399).
-4. The relayer [requests](../../../../contracts/src/BeefyClient.sol#L404) from the light client a bitfield with $$N$$randomly chosen validators sampled from the initial bitfield.​
-5. `submitFinal` - The relayer sends a [third transaction](../../../../contracts/src/BeefyClient.sol#L320) with signatures for all the validators specified in the final bitfield
+3. `commitPrevRandao` - The relayer submits a second transaction to reveal and commit to a random seed, derived from Ethereum's [RANDAO](https://eips.ethereum.org/EIPS/eip-4399).
+4. The relayer requests from the light client a bitfield with $$N$$randomly chosen validators sampled from the initial bitfield.​
+5. `submitFinal` - The relayer sends a third and final transaction with signatures for all the validators specified in the final bitfield
 6. The light client verifies all validator signatures in the third transaction to ensure:
    1. The provided validators are in the current validator set
    2. The provided validators are in the final bitfield
@@ -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#L182-L187)
-* [Dynamic signature calculation.](../../../../contracts/src/BeefyClient.sol#L437)
-* [Python implementation of required signatures.](../../../../scripts/beefy\_signature\_sampling.py#L9)
+* [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)
 
 ## Message Verification