Add additional checks to consensus contracts

.changeset/nine-poems-burn.md

@@ -0,0 +1,5 @@
+---
+"@cartesi/rollups": minor
+---
+
+Make `Authority` and `Quorum` validate last processed block number

.changeset/petite-numbers-read.md

@@ -0,0 +1,5 @@
+---
+"@cartesi/rollups": major
+---
+
+Avoid conflicting claims in Quorum

.changeset/sweet-tools-taste.md

@@ -0,0 +1,5 @@
+---
+"@cartesi/rollups": patch
+---
+
+Bump solc from 0.8.23 to 0.8.29

foundry.toml

@@ -1,6 +1,6 @@
 [profile.default]
 libs = []
-solc_version = "0.8.23"
+solc_version = "0.8.29"
 via_ir = true
 optimizer = true
 

src/consensus/AbstractConsensus.sol

@@ -1,7 +1,7 @@
 // (c) Cartesi and individual authors (see AUTHORS)
 // SPDX-License-Identifier: Apache-2.0 (see LICENSE)
 
-pragma solidity ^0.8.8;
+pragma solidity ^0.8.26;
 
 import {ERC165} from "@openzeppelin-contracts-5.2.0/utils/introspection/ERC165.sol";
 import {IERC165} from "@openzeppelin-contracts-5.2.0/utils/introspection/IERC165.sol";
@@ -12,7 +12,7 @@ import {IConsensus} from "./IConsensus.sol";
 /// @notice Abstract implementation of IConsensus
 abstract contract AbstractConsensus is IConsensus, ERC165 {
     /// @notice The epoch length
-    uint256 private immutable _epochLength;
+    uint256 internal immutable _epochLength;
 
     /// @notice Indexes accepted claims by application contract address.
     mapping(address => mapping(bytes32 => bool)) private _validOutputsMerkleRoots;
@@ -51,10 +51,30 @@ abstract contract AbstractConsensus is IConsensus, ERC165 {
             || super.supportsInterface(interfaceId);
     }
 
+    /// @notice Validate a last processed block number.
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    function _validateLastProcessedBlockNumber(uint256 lastProcessedBlockNumber)
+        internal
+        view
+    {
+        require(
+            lastProcessedBlockNumber % _epochLength == (_epochLength - 1),
+            NotEpochFinalBlock(lastProcessedBlockNumber, _epochLength)
+        );
+        {
+            uint256 upperBound = block.number;
+            require(
+                lastProcessedBlockNumber < upperBound,
+                NotPastBlock(lastProcessedBlockNumber, upperBound)
+            );
+        }
+    }
+
     /// @notice Accept a claim.
     /// @param appContract The application contract address
     /// @param lastProcessedBlockNumber The number of the last processed block
     /// @param outputsMerkleRoot The output Merkle root hash
+    /// @dev Marks the outputsMerkleRoot as valid.
     /// @dev Emits a `ClaimAccepted` event.
     function _acceptClaim(
         address appContract,

src/consensus/IConsensus.sol

@@ -38,12 +38,31 @@ interface IConsensus is IOutputsMerkleRootValidator {
     /// @param appContract The application contract address
     /// @param lastProcessedBlockNumber The number of the last processed block
     /// @param outputsMerkleRoot The outputs Merkle root
+    /// @dev For each application and lastProcessedBlockNumber,
+    /// there can be at most one accepted claim.
     event ClaimAccepted(
         address indexed appContract,
         uint256 lastProcessedBlockNumber,
         bytes32 outputsMerkleRoot
     );
 
+    /// @notice The claim contains the number of a block that is not
+    /// at the end of an epoch (its modulo epoch length is not epoch length - 1).
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    /// @param epochLength The epoch length
+    error NotEpochFinalBlock(uint256 lastProcessedBlockNumber, uint256 epochLength);
+
+    /// @notice The claim contains the number of a block in the future
+    /// (it is greater or equal to the current block number).
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    /// @param currentBlockNumber The number of the current block
+    error NotPastBlock(uint256 lastProcessedBlockNumber, uint256 currentBlockNumber);
+
+    /// @notice A claim for that application and epoch was already submitted by the validator.
+    /// @param appContract The application contract address
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    error NotFirstClaim(address appContract, uint256 lastProcessedBlockNumber);
+
     /// @notice Submit a claim to the consensus.
     /// @param appContract The application contract address
     /// @param lastProcessedBlockNumber The number of the last processed block

src/consensus/authority/Authority.sol

@@ -3,6 +3,7 @@
 
 pragma solidity ^0.8.8;
 
+import {BitMaps} from "@openzeppelin-contracts-5.2.0/utils/structs/BitMaps.sol";
 import {IERC165} from "@openzeppelin-contracts-5.2.0/utils/introspection/IERC165.sol";
 import {Ownable} from "@openzeppelin-contracts-5.2.0/access/Ownable.sol";
 
@@ -15,6 +16,12 @@ import {IOwnable} from "../../access/IOwnable.sol";
 /// @dev This contract inherits from OpenZeppelin's `Ownable` contract.
 ///      For more information on `Ownable`, please consult OpenZeppelin's official documentation.
 contract Authority is IAuthority, AbstractConsensus, Ownable {
+    using BitMaps for BitMaps.BitMap;
+
+    /// @notice Epochs with a submitted (and accepted) claim, per application.
+    /// @dev Epochs are stored in bitmap structure by their number (last processed block number / epoch length).
+    mapping(address => BitMaps.BitMap) _validatedEpochs;
+
     /// @param initialOwner The initial contract owner
     /// @param epochLength The epoch length
     /// @dev Reverts if the epoch length is zero.
@@ -29,10 +36,23 @@ contract Authority is IAuthority, AbstractConsensus, Ownable {
         uint256 lastProcessedBlockNumber,
         bytes32 outputsMerkleRoot
     ) external override onlyOwner {
+        _validateLastProcessedBlockNumber(lastProcessedBlockNumber);
+
+        uint256 epochNumber = lastProcessedBlockNumber / _epochLength;
+
+        BitMaps.BitMap storage bitmap = _validatedEpochs[appContract];
+
+        require(
+            !bitmap.get(epochNumber), NotFirstClaim(appContract, lastProcessedBlockNumber)
+        );
+
         emit ClaimSubmitted(
             msg.sender, appContract, lastProcessedBlockNumber, outputsMerkleRoot
         );
+
         _acceptClaim(appContract, lastProcessedBlockNumber, outputsMerkleRoot);
+
+        bitmap.set(epochNumber);
     }
 
     /// @inheritdoc Ownable

src/consensus/quorum/IQuorum.sol

@@ -27,6 +27,27 @@ interface IQuorum is IConsensus {
     /// @dev Invalid IDs map to address zero.
     function validatorById(uint256 id) external view returns (address);
 
+    /// @notice Get the number of validators in favor of any claim in a given epoch.
+    /// @param appContract The application contract address
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    /// @return Number of validators in favor of any claim in the epoch
+    function numOfValidatorsInFavorOfAnyClaimInEpoch(
+        address appContract,
+        uint256 lastProcessedBlockNumber
+    ) external view returns (uint256);
+
+    /// @notice Check whether a validator is in favor of any claim in a given epoch.
+    /// @param appContract The application contract address
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    /// @param id The ID of the validator
+    /// @return Whether validator is in favor of any claim in the epoch
+    /// @dev Assumes the provided ID is valid.
+    function isValidatorInFavorOfAnyClaimInEpoch(
+        address appContract,
+        uint256 lastProcessedBlockNumber,
+        uint256 id
+    ) external view returns (bool);
+
     /// @notice Get the number of validators in favor of a claim.
     /// @param appContract The application contract address
     /// @param lastProcessedBlockNumber The number of the last processed block

src/consensus/quorum/Quorum.sol

@@ -36,6 +36,12 @@ contract Quorum is IQuorum, AbstractConsensus {
         BitMaps.BitMap inFavorById;
     }
 
+    /// @notice Votes indexed by application contract address,
+    /// and last processed block number.
+    /// @dev See the `numOfValidatorsInFavorOfAnyClaimInEpoch`
+    /// and `isValidatorInFavorOfAnyClaimInEpoch` functions.
+    mapping(address => mapping(uint256 => Votes)) private _allVotes;
+
     /// @notice Votes indexed by application contract address,
     /// last processed block number and outputs Merkle root.
     /// @dev See the `numOfValidatorsInFavorOf` and `isValidatorInFavorOf` functions.
@@ -68,14 +74,32 @@ contract Quorum is IQuorum, AbstractConsensus {
         uint256 id = _validatorId[msg.sender];
         require(id > 0, "Quorum: caller is not validator");
 
+        _validateLastProcessedBlockNumber(lastProcessedBlockNumber);
+
         emit ClaimSubmitted(
             msg.sender, appContract, lastProcessedBlockNumber, outputsMerkleRoot
         );
 
         Votes storage votes =
             _getVotes(appContract, lastProcessedBlockNumber, outputsMerkleRoot);
 
+        Votes storage allVotes = _getAllVotes(appContract, lastProcessedBlockNumber);
+
+        // Skip storage changes if validator already voted
+        // for the same exact claim before
         if (!votes.inFavorById.get(id)) {
+            // Revert if validator has submitted another claim for the same epoch
+            require(
+                !allVotes.inFavorById.get(id),
+                NotFirstClaim(appContract, lastProcessedBlockNumber)
+            );
+
+            // Register vote (for any claim in the epoch)
+            allVotes.inFavorById.set(id);
+            ++allVotes.inFavorCount;
+
+            // Register vote (for the specific claim)
+            // and accept the claim if a majority has been reached
             votes.inFavorById.set(id);
             if (++votes.inFavorCount == 1 + _numOfValidators / 2) {
                 _acceptClaim(appContract, lastProcessedBlockNumber, outputsMerkleRoot);
@@ -95,6 +119,21 @@ contract Quorum is IQuorum, AbstractConsensus {
         return _validatorById[id];
     }
 
+    function numOfValidatorsInFavorOfAnyClaimInEpoch(
+        address appContract,
+        uint256 lastProcessedBlockNumber
+    ) external view override returns (uint256) {
+        return _getAllVotes(appContract, lastProcessedBlockNumber).inFavorCount;
+    }
+
+    function isValidatorInFavorOfAnyClaimInEpoch(
+        address appContract,
+        uint256 lastProcessedBlockNumber,
+        uint256 id
+    ) external view override returns (bool) {
+        return _getAllVotes(appContract, lastProcessedBlockNumber).inFavorById.get(id);
+    }
+
     function numOfValidatorsInFavorOf(
         address appContract,
         uint256 lastProcessedBlockNumber,
@@ -115,6 +154,18 @@ contract Quorum is IQuorum, AbstractConsensus {
             .get(id);
     }
 
+    /// @notice Get a `Votes` structure from storage from a given epoch.
+    /// @param appContract The application contract address
+    /// @param lastProcessedBlockNumber The number of the last processed block
+    /// @return The `Votes` structure related to all claims in a given epoch
+    function _getAllVotes(address appContract, uint256 lastProcessedBlockNumber)
+        internal
+        view
+        returns (Votes storage)
+    {
+        return _allVotes[appContract][lastProcessedBlockNumber];
+    }
+
     /// @notice Get a `Votes` structure from storage from a given claim.
     /// @param appContract The application contract address
     /// @param lastProcessedBlockNumber The number of the last processed block

test/consensus/authority/Authority.t.sol

@@ -118,24 +118,102 @@ contract AuthorityTest is Test, ERC165Test, OwnableTest {
         authority.submitClaim(appContract, lastProcessedBlockNumber, claim);
     }
 
-    function testSubmitClaim(
+    function testSubmitClaimNotFirstClaim(
         address owner,
         uint256 epochLength,
         address appContract,
-        uint256 lastProcessedBlockNumber,
-        bytes32 claim
+        uint256 epochNumber,
+        uint256 blockNumber,
+        bytes32 claim,
+        bytes32 claim2
     ) public {
         vm.assume(owner != address(0));
-        vm.assume(epochLength > 0);
+        vm.assume(epochLength >= 1);
 
         IAuthority authority = new Authority(owner, epochLength);
 
+        blockNumber = bound(blockNumber, epochLength, type(uint256).max);
+        epochNumber = bound(epochNumber, 0, (blockNumber / epochLength) - 1);
+        uint256 lastProcessedBlockNumber = epochNumber * epochLength + (epochLength - 1);
+
+        vm.roll(blockNumber);
+
         _expectClaimEvents(authority, owner, appContract, lastProcessedBlockNumber, claim);
 
         vm.prank(owner);
         authority.submitClaim(appContract, lastProcessedBlockNumber, claim);
 
         assertTrue(authority.isOutputsMerkleRootValid(appContract, claim));
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IConsensus.NotFirstClaim.selector, appContract, lastProcessedBlockNumber
+            )
+        );
+        vm.prank(owner);
+        authority.submitClaim(appContract, lastProcessedBlockNumber, claim2);
+    }
+
+    function testSubmitClaimNotEpochFinalBlock(
+        address owner,
+        uint256 epochLength,
+        address appContract,
+        uint256 epochNumber,
+        uint256 blockNumber,
+        uint256 blocksAfterEpochStart,
+        bytes32 claim
+    ) public {
+        vm.assume(owner != address(0));
+        vm.assume(epochLength >= 2);
+
+        IAuthority authority = new Authority(owner, epochLength);
+
+        blocksAfterEpochStart = bound(blocksAfterEpochStart, 0, epochLength - 2);
+        blockNumber = bound(blockNumber, epochLength, type(uint256).max);
+        epochNumber = bound(epochNumber, 0, (blockNumber / epochLength) - 1);
+        uint256 lastProcessedBlockNumber =
+            epochNumber * epochLength + blocksAfterEpochStart;
+
+        vm.roll(blockNumber);
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IConsensus.NotEpochFinalBlock.selector,
+                lastProcessedBlockNumber,
+                epochLength
+            )
+        );
+        vm.prank(owner);
+        authority.submitClaim(appContract, lastProcessedBlockNumber, claim);
+    }
+
+    function testSubmitClaimNotPastBlock(
+        address owner,
+        uint256 epochLength,
+        address appContract,
+        uint256 epochNumber,
+        uint256 blockNumber,
+        bytes32 claim
+    ) public {
+        vm.assume(owner != address(0));
+        vm.assume(epochLength >= 1);
+
+        IAuthority authority = new Authority(owner, epochLength);
+
+        uint256 maxEpochNumber = (type(uint256).max - (epochLength - 1)) / epochLength;
+        epochNumber = bound(epochNumber, 0, maxEpochNumber);
+        uint256 lastProcessedBlockNumber = epochNumber * epochLength + (epochLength - 1);
+        blockNumber = bound(blockNumber, 0, lastProcessedBlockNumber);
+
+        vm.roll(blockNumber);
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IConsensus.NotPastBlock.selector, lastProcessedBlockNumber, blockNumber
+            )
+        );
+        vm.prank(owner);
+        authority.submitClaim(appContract, lastProcessedBlockNumber, claim);
     }
 
     function testIsOutputsMerkleRootValid(