EVM audit (#500)

* Bridge testnet (#448) (#449)

* reporter module

* use legacy dec for tips and commision rates

* re-add max commision rate check

* chore: mv legacy_dec.go to types/legacy_dec.go

* minor refactor for clarity

* checkpoint to pull main

* use LgeacyDec for withdraws

* remove old BigUint to LegacyDec fcns

* linting

* add test

* linting

* separate blobstream testnet child contract

---------

Co-authored-by: danflo27 <[email protected]>
Co-authored-by: akrem <[email protected]>
(cherry picked from commit e800173)

Co-authored-by: tkernell <[email protected]>

* feat: snapshot limit param set by gov (#453) (#455)

* add more timestamps for tracking

* cleanup after review

* snapshot limit

* fix tests

* lint

* remove log

(cherry picked from commit 0efe1cc)

Co-authored-by: tkernell <[email protected]>

* fix oracle domain separator

* add retrieveData function

* use ECDSA tryRecover for key malleability prevention

* R09 - use solidity 0.8.19

* R08 - import specific contracts

* R07 - named mappings

* R06 - emit events

* R05 - use constants

* R04 - remove unused code

* L02 - check guardian reset timestamp

* L01 - prevent dust amount lost

* L03 - getter limit values consider minting

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 341a33c)

evm/contracts/bridge/BlobstreamO.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
-import "./ECDSA.sol";
+import {ECDSA} from "./ECDSA.sol";
 import "./Constants.sol";
 
 struct OracleAttestationData {
@@ -45,8 +45,10 @@ contract BlobstreamO is ECDSA {
     uint256 public validatorTimestamp; /// Timestamp of the block where validator set is updated.
     address public deployer; /// Address that deployed the contract.
     bool public initialized; /// True if the contract is initialized.
+    uint256 public constant MS_PER_SECOND = 1000; // factor to convert milliseconds to seconds
 
     /*Events*/
+    event GuardianResetValidatorSet(uint256 _powerThreshold, uint256 _validatorTimestamp, bytes32 _validatorSetHash);
     event ValidatorSetUpdated(uint256 _powerThreshold, uint256 _validatorTimestamp, bytes32 _validatorSetHash);
 
     /*Errors*/
@@ -55,7 +57,6 @@ contract BlobstreamO is ECDSA {
     error InvalidPowerThreshold();
     error InvalidSignature();
     error MalformedCurrentValidatorSet();
-    error NotConsensusValue();
     error NotDeployer();
     error NotGuardian();
     error StaleValidatorSet();
@@ -110,12 +111,16 @@ contract BlobstreamO is ECDSA {
         if (msg.sender != guardian) {
             revert NotGuardian();
         }
-        if (block.timestamp - (validatorTimestamp / 1000) < unbondingPeriod) {
+        if (block.timestamp - (validatorTimestamp / MS_PER_SECOND) < unbondingPeriod) {
             revert ValidatorSetNotStale();
         }
+        if (_validatorTimestamp <= validatorTimestamp) {
+            revert ValidatorTimestampMustIncrease();
+        }
         powerThreshold = _powerThreshold;
         validatorTimestamp = _validatorTimestamp;
         lastValidatorSetCheckpoint = _validatorSetCheckpoint;
+        emit GuardianResetValidatorSet(_powerThreshold, _validatorTimestamp, _validatorSetCheckpoint);
     }
 
     /// @notice This updates the validator set by checking that the validators
@@ -233,7 +238,7 @@ contract BlobstreamO is ECDSA {
         bytes32 _digest,
         uint256 _powerThreshold
     ) internal view {
-        if (block.timestamp - (validatorTimestamp / 1000) > unbondingPeriod) {
+        if (block.timestamp - (validatorTimestamp / MS_PER_SECOND) > unbondingPeriod) {
             revert StaleValidatorSet();
         }
         uint256 _cumulativePower = 0;
@@ -289,7 +294,11 @@ contract BlobstreamO is ECDSA {
         Signature calldata _sig
     ) internal pure returns (bool) {
         _digest = sha256(abi.encodePacked(_digest));
-        return _signer == ecrecover(_digest, _sig.v, _sig.r, _sig.s);
+        (address _recovered, RecoverError error, ) = tryRecover(_digest, _sig.v, _sig.r, _sig.s);
+        if (error != RecoverError.NoError) {
+            revert InvalidSignature();
+        }
+        return _signer == _recovered;
     }
 }
 

evm/contracts/bridge/Constants.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0
-pragma solidity ^0.8.22;
+pragma solidity 0.8.19;
 
-/// @dev bytes32 encoding of the string "tellorNewReport"
+/// @dev bytes32 encoding of the string "tellorCurrentAttestation"
 bytes32 constant NEW_REPORT_ATTESTATION_DOMAIN_SEPARATOR =
     0x74656c6c6f7243757272656e744174746573746174696f6e0000000000000000;
 

evm/contracts/bridge/ECDSA.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts (last updated v5.0.0) (utils/cryptography/ECDSA.sol)
-pragma solidity ^0.8.20;
+pragma solidity 0.8.19;
 
 /**
  * @dev Elliptic Curve Digital Signature Algorithm (ECDSA) operations.

evm/contracts/interfaces/IBridgeProxy.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 interface IBridgeProxy {
     function pauseBridge() external;

evm/contracts/interfaces/ITellorMaster.sol

@@ -5,6 +5,7 @@ interface ITellorMaster {
     function allowance(address owner, address spender) external view returns (uint256);
     function approve(address spender, uint256 amount) external returns (bool);
     function balanceOf(address account) external view returns (uint256);
+    function getUintVar(bytes32 _data) external view returns (uint256);
     function mintToOracle() external;
     function totalSupply() external view returns (uint256);
     function transfer(address recipient, uint256 amount) external returns (bool);

evm/contracts/testing/BridgeProxyMock.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 import { IBridgeProxy } from "../interfaces/IBridgeProxy.sol";
 

evm/contracts/testing/SimpleLayerUser.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 import "../interfaces/IBlobstreamO.sol";
 

evm/contracts/testing/TellorPlayground.sol

@@ -31,6 +31,7 @@ contract TellorPlayground {
     mapping(bytes32 => uint256[]) public voteRounds; // mapping of vote identifier hashes to an array of dispute IDs
     mapping(address => mapping(address => uint256)) private _allowances;
     mapping(address => uint256) private _balances;
+    mapping(bytes32 => uint256) public uintVars;
 
     uint256 public stakeAmount;
     uint256 public constant timeBasedReward = 5e17; // time based reward for a reporter for successfully submitting a value
@@ -42,7 +43,6 @@ contract TellorPlayground {
     string private _symbol;
     uint8 private _decimals;
     address public oracleMintRecipient;
-    uint256 public lastReleaseTimeDao;
     address public deployer;
 
     // Structs
@@ -63,7 +63,7 @@ contract TellorPlayground {
         _symbol = "TRBP";
         _decimals = 18;
         token = address(this);
-        lastReleaseTimeDao = block.timestamp;
+        uintVars[keccak256("_LAST_RELEASE_TIME_DAO")] = block.timestamp;
         deployer = msg.sender;
     }
 
@@ -144,9 +144,9 @@ contract TellorPlayground {
             return;
         }
         uint256 _releasedAmount = (146.94 ether *
-            (block.timestamp - lastReleaseTimeDao)) /
+            (block.timestamp - uintVars[keccak256("_LAST_RELEASE_TIME_DAO")])) /
             86400;
-        lastReleaseTimeDao = block.timestamp;
+        uintVars[keccak256("_LAST_RELEASE_TIME_DAO")] = block.timestamp;
         uint256 _stakingRewards = (_releasedAmount * 2) / 100;
         _mint(oracleMintRecipient, _releasedAmount - _stakingRewards);
         _mint(address(this), _stakingRewards);
@@ -529,6 +529,15 @@ contract TellorPlayground {
         return timestamps[_queryId][_index];
     }
 
+    /**
+     * @dev Returns the value of a uint variable
+     * @param _data the key of the variable
+     * @return uint256 the value of the variable
+     */
+    function getUintVar(bytes32 _data) external view returns (uint256) {
+        return uintVars[_data];
+    }
+
     /**
      * @dev Returns an array of voting rounds for a given vote
      * @param _hash is the identifier hash for a vote

evm/contracts/testing/TestTokenBridge.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.22;
+pragma solidity 0.8.19;
 
 import { TokenBridge } from "../token-bridge/TokenBridge.sol";
 

evm/contracts/testing/forking/SimpleSchelling.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 import {IBridgeProxy} from "../../interfaces/IBridgeProxy.sol";
 

evm/contracts/token-bridge/LayerTransition.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 import { ITellorFlex } from "../interfaces/ITellorFlex.sol";
 import { ITellorMaster } from "../interfaces/ITellorMaster.sol";
@@ -83,6 +83,14 @@ contract LayerTransition {
         return tellorFlex.isInDispute(_queryId, _timestamp);
     }
 
+    /// @notice This forwards retrieveData calls to the old tellorFlex
+    /// @param _queryId being requested
+    /// @param _timestamp to retrieve data/value from
+    /// @return bytes value for timestamp submitted
+    function retrieveData(bytes32 _queryId, uint256 _timestamp) external view returns(bytes memory) {
+        return tellorFlex.retrieveData(_queryId, _timestamp);
+    }
+
     /// @notice This returns a big number.  Necessary for upgrading the contract
     function verify() external pure returns (uint256) {
         return 9999;

evm/contracts/token-bridge/TokenBridge.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.22;
+pragma solidity 0.8.19;
 
 import "../interfaces/IBlobstreamO.sol";
 import { LayerTransition } from "./LayerTransition.sol";
@@ -22,14 +22,16 @@ contract TokenBridge is LayerTransition{
     uint256 public withdrawLimitUpdateTime; // last time the withdraw limit was updated
     uint256 public withdrawLimitRecord; // amount you can withdraw per limit period
     uint256 public constant DEPOSIT_LIMIT_DENOMINATOR = 100e18 / 20e18; // 100/depositLimitPercentage
+    uint256 public constant MS_PER_SECOND = 1000; // factor to convert milliseconds to seconds
     uint256 public constant PAUSE_PERIOD = 21 days; // time period guardian can pause bridge, only once
     uint256 public constant PAUSE_TRIBUTE_AMOUNT = 10000 ether; // amount of tokens burned to pause bridge
-    uint256 public constant TWELVE_HOUR_UPDATE_INTERVAL = 12 hours; // deposit and withdraw limits update interval
+    uint256 public constant TOKEN_DECIMAL_PRECISION_MULTIPLIER = 1e12; // multiplier to convert from loya to 1e18
+    uint256 public constant TWELVE_HOUR_CONSTANT = 12 hours; // deposit and withdraw limits update interval
     uint256 public constant WITHDRAW_LIMIT_DENOMINATOR = 100e18 / 5e18; // 100/withdrawLimitPercentage
 
-    mapping(uint256 => bool) public withdrawClaimed; // withdraw id => claimed status
-    mapping(address => uint256) public tokensToClaim; // recipient => extra amount to claim
-    mapping(uint256 => DepositDetails) public deposits; // deposit id => deposit details
+    mapping(uint256 depositId => DepositDetails) public deposits; // deposit id => deposit details
+    mapping(address recipient => uint256 extraAmountToClaim) public tokensToClaim; // recipient => extra amount to claim
+    mapping(uint256 withdrawId => bool claimed) public withdrawClaimed; // withdraw id => claimed status
 
     struct DepositDetails {
         address sender;
@@ -87,6 +89,7 @@ contract TokenBridge is LayerTransition{
     /// @param _layerRecipient your cosmos address on layer (don't get it wrong!!)
     function depositToLayer(uint256 _amount, uint256 _tip, string memory _layerRecipient) external {
         require(_amount > 0.1 ether, "TokenBridge: amount must be greater than 0.1 tokens");
+        require(_amount % TOKEN_DECIMAL_PRECISION_MULTIPLIER == 0, "TokenBridge: amount must be divisible by 1e12");
         require(_amount <= _refreshDepositLimit(_amount), "TokenBridge: amount exceeds deposit limit for time period");
         require(_tip <= _amount, "TokenBridge: tip must be less than or equal to amount");
         if (_tip > 0) {
@@ -132,13 +135,13 @@ contract TokenBridge is LayerTransition{
         require(bridgeState != BridgeState.PAUSED, "TokenBridge: bridge is paused");
         require(_attestData.queryId == keccak256(abi.encode("TRBBridge", abi.encode(false, _depositId))), "TokenBridge: invalid queryId");
         require(!withdrawClaimed[_depositId], "TokenBridge: withdraw already claimed");
-        require(block.timestamp - (_attestData.report.timestamp / 1000) > 12 hours, "TokenBridge: premature attestation");
-        require(block.timestamp - (_attestData.attestationTimestamp / 1000) < 12 hours, "TokenBridge: attestation too old");
+        require(block.timestamp - (_attestData.report.timestamp / MS_PER_SECOND) > TWELVE_HOUR_CONSTANT, "TokenBridge: premature attestation");
+        require(block.timestamp - (_attestData.attestationTimestamp / MS_PER_SECOND) < TWELVE_HOUR_CONSTANT, "TokenBridge: attestation too old");
         bridge.verifyOracleData(_attestData, _valset, _sigs);
         require(_attestData.report.aggregatePower >= bridge.powerThreshold(), "Report aggregate power must be greater than or equal to _minimumPower");
         withdrawClaimed[_depositId] = true;    
         (address _recipient, string memory _layerSender,uint256 _amountLoya,) = abi.decode(_attestData.report.value, (address, string, uint256, uint256));
-        uint256 _amountConverted = _amountLoya * 1e12; 
+        uint256 _amountConverted = _amountLoya * TOKEN_DECIMAL_PRECISION_MULTIPLIER; 
         uint256 _withdrawLimit = _refreshWithdrawLimit(_amountConverted);
         if(_withdrawLimit < _amountConverted){
             tokensToClaim[_recipient] = tokensToClaim[_recipient] + (_amountConverted - _withdrawLimit);
@@ -153,8 +156,8 @@ contract TokenBridge is LayerTransition{
     /// @notice returns the amount of tokens that can be deposited in the current 12 hour period
     /// @return amount of tokens that can be deposited
     function depositLimit() external view returns (uint256) {
-        if (block.timestamp - depositLimitUpdateTime > TWELVE_HOUR_UPDATE_INTERVAL) {
-            return token.balanceOf(address(this)) / DEPOSIT_LIMIT_DENOMINATOR;
+        if (block.timestamp - depositLimitUpdateTime > TWELVE_HOUR_CONSTANT) {
+            return (token.balanceOf(address(this)) + _getMintAmount()) / DEPOSIT_LIMIT_DENOMINATOR;
         }
         else{
             return depositLimitRecord;
@@ -164,19 +167,28 @@ contract TokenBridge is LayerTransition{
     /// @notice returns the withdraw limit
     /// @return amount of tokens that can be withdrawn
     function withdrawLimit() external view returns (uint256) {
-        if (block.timestamp - withdrawLimitUpdateTime > TWELVE_HOUR_UPDATE_INTERVAL) {
-            return token.balanceOf(address(this)) / WITHDRAW_LIMIT_DENOMINATOR;
+        if (block.timestamp - withdrawLimitUpdateTime > TWELVE_HOUR_CONSTANT) {
+            return (token.balanceOf(address(this)) + _getMintAmount()) / WITHDRAW_LIMIT_DENOMINATOR;
         }
         else{
             return withdrawLimitRecord;
         }
     }
 
     /* Internal Functions */
+    /// @notice returns the amount of tokens pending to be minted to this contract
+    /// @return amount of tokens pending to be minted
+    function _getMintAmount() internal view returns (uint256) {
+        uint256 _releasedAmount = (146.94 ether *
+            (block.timestamp - token.getUintVar(keccak256("_LAST_RELEASE_TIME_DAO")))) /
+            86400;
+        return _releasedAmount;
+    }
+
     /// @notice refreshes the deposit limit every 12 hours so no one can spam layer with new tokens
     /// @return max amount of tokens that can be deposited
     function _refreshDepositLimit(uint256 _amount) internal returns (uint256) {
-        if (block.timestamp - depositLimitUpdateTime > TWELVE_HOUR_UPDATE_INTERVAL) {
+        if (block.timestamp - depositLimitUpdateTime > TWELVE_HOUR_CONSTANT) {
             uint256 _tokenBalance = token.balanceOf(address(this));
             if (_tokenBalance < _amount) {
                 token.mintToOracle();
@@ -192,7 +204,7 @@ contract TokenBridge is LayerTransition{
     /// @param _amount of tokens to withdraw
     /// @return max amount of tokens that can be withdrawn
     function _refreshWithdrawLimit(uint256 _amount) internal returns (uint256) {
-        if (block.timestamp - withdrawLimitUpdateTime > TWELVE_HOUR_UPDATE_INTERVAL) {
+        if (block.timestamp - withdrawLimitUpdateTime > TWELVE_HOUR_CONSTANT) {
             uint256 _tokenBalance = token.balanceOf(address(this));
             if (_tokenBalance < _amount) {
                 token.mintToOracle();
@@ -203,4 +215,4 @@ contract TokenBridge is LayerTransition{
         } 
         return withdrawLimitRecord;
     }
-}
+}

evm/hardhat.config.js

@@ -10,7 +10,7 @@ module.exports = {
   solidity: {
     compilers: [
       {
-        version: "0.8.22",
+        version: "0.8.19",
         settings: {
           optimizer: {
             enabled: true,

evm/test/BlobstreamOFunctionTestsHH.js

@@ -73,7 +73,14 @@ describe("Blobstream - Function Tests", async function () {
         await h.expectThrow(blobstream.connect(guardian).guardianResetValidatorSet(newThreshold, newValTimestamp, newValCheckpoint));
         await h.advanceTime(UNBONDING_PERIOD + 1)
         await h.expectThrow(blobstream.guardianResetValidatorSet(newThreshold, newValTimestamp, newValCheckpoint));//not guardian
+        oldValTimestamp = await blobstream.validatorTimestamp()
+        await h.expectThrow(blobstream.connect(guardian).guardianResetValidatorSet(newThreshold, oldValTimestamp, newValCheckpoint));
+        blocky = await h.getBlock()
+        newValTimestamp = (blocky.timestamp - 1) * 1000
         await blobstream.connect(guardian).guardianResetValidatorSet(newThreshold, newValTimestamp, newValCheckpoint);
+        assert.equal(await blobstream.validatorTimestamp(), newValTimestamp)
+        assert.equal(await blobstream.lastValidatorSetCheckpoint(), newValCheckpoint)
+        assert.equal(await blobstream.powerThreshold(), newThreshold)
     })
     it("updateValidatorSet", async function () {
         newValAddrs = [val1.address, val2.address, val3.address]