audit: airdrop.sol

Author: vnavascues

TODO_VN.md

@@ -0,0 +1,32 @@
+# TODO
+
+## Repo
+
+### Audits
+
+- Missing previous audit reports (if any).
+
+### Dependencies
+
+- Adopt Soldeer to pin dependencies (e.g., OZ) by version.
+- Remove unused dependencies.
+- Adopt latest forge
+
+### Pre-commit pipeline:
+
+- Enforce `forge lint` before code is commited. Prettify any non-Solidity code.
+
+### CI
+
+- Consider running tests grouped by scope.
+
+### Contracts
+
+Upgradeability:
+
+- Consider using https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades due to OZ's UUPS.
+
+Uncompilable:
+
+- src/EnsoMint.sol
+- src/EnsoStaking.sol

foundry.lock

@@ -0,0 +1,11 @@
+{
+  "lib/forge-std": {
+    "rev": "77041d2ce690e692d6e03cc812b57d1ddaa4d505"
+  },
+  "lib/openzeppelin-contracts": {
+    "rev": "e4f70216d759d8e6a64144a9e1f7bbeed78e7079"
+  },
+  "lib/openzeppelin-contracts-upgradeable": {
+    "rev": "60b305a8f3ff0c7688f02ac470417b6bbf1c4d27"
+  }
+}

src/Airdrop.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-only
 pragma solidity ^0.8.20;
 
+// @audit-info 2-step ownership
 import { Ownable } from "openzeppelin-contracts/access/Ownable.sol";
 import { IERC20 } from "openzeppelin-contracts/token/ERC20/IERC20.sol";
 import { MerkleProof } from "openzeppelin-contracts/utils/cryptography/MerkleProof.sol";
@@ -16,6 +17,10 @@ contract Airdrop is Ownable {
     IERC20 public immutable token;
     bytes32 public immutable root;
     uint256 public immutable expiration;
+    // @audit consider using a `mapping(uint256 bucket => uint256 bitmap) bucketBitmap` instead of
+    // `mapping(bytes32 leafHash => bool isClaimed)` for gas efficiency
+    // https://github.com/Uniswap/merkle-distributor/blob/master/contracts/MerkleDistributor.sol
+    // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/structs/BitMaps.sol
     mapping(bytes32 => bool) public claimed;
 
     constructor(address _token, bytes32 _root, uint256 _expiration, address _owner) Ownable(_owner) {
@@ -24,10 +29,13 @@ contract Airdrop is Ownable {
         expiration = _expiration;
     }
 
+    // @audit ideally hash twice to prevent any possible "node as leaf" attack, where the `hash(address, amount)` equals
+    // to an existing node hash
     function getLeafHash(address to, uint256 amount) public pure returns (bytes32) {
         return keccak256(abi.encode(to, amount));
     }
 
+    // @audit `to` instead of `msg.sender` => tax consequences. Consider validating `to == msg.sender`
     function claim(bytes32[] memory proof, address to, uint256 amount) external {
         if (block.timestamp > expiration) revert AirdropExpired();
 
@@ -40,11 +48,13 @@ contract Airdrop is Ownable {
 
         token.transfer(to, amount);
 
+        // @audit-info Claimed, past participle
         emit Claim(to, amount);
     }
 
     function sweep(address to) external onlyOwner {
         if (block.timestamp <= expiration) revert AirdropNotExpired();
         token.transfer(to, token.balanceOf(address(this)));
+        // @audit-info emit Swept event
     }
 }

src/EnsoMint.sol

@@ -2,8 +2,9 @@
 pragma solidity ^0.8.20;
 
 import { EnsoValidatorWallet } from "./EnsoValidatorWallet.sol";
-import { Ownable } from "openzeppelin-contracts/access/Ownable.sol";
+
 import { MintToken } from "./interfaces/MintToken.sol";
+import { Ownable } from "openzeppelin-contracts/access/Ownable.sol";
 
 contract EnsoMint is Ownable {
     MintToken public token;
@@ -36,11 +37,17 @@ contract EnsoMint is Ownable {
         emit ValidatorAdded(address(validatorWallet));
     }
 
-    function issueFunds(uint64 timestamp, uint256 totalShares, ValidatorIssuance[] calldata issuance) external onlyOwner {
+    function issueFunds(
+        uint64 timestamp,
+        uint256 totalShares,
+        ValidatorIssuance[] calldata issuance
+    )
+        external
+        onlyOwner
+    {
         uint256 period = timestamp - lastTimestamp;
         uint256 totalFunds = tokensPerSecond * period;
         lastTimestamp = timestamp;
-        
 
         uint256 shareCount;
         for (uint256 i; i < issuance.length; i++) {
@@ -53,15 +60,17 @@ contract EnsoMint is Ownable {
             token.mint(validator, validatorShare);
             uint256 stakersShare = amount - validatorShare;
             token.mint(address(this), stakersShare);
-            token.approve(address(staking), stakersShare);
-            staking.issueRewards(validator, stakersShare);
+            // TODO: unfinished implementation
+            // @audit undeclared identifier `staking`
+            // token.approve(address(staking), stakersShare);
+            // staking.issueRewards(validator, stakersShare);
         }
-        if (totalShares += shareCount) revert IncorrectTotalShares(totalShares, shareCount);
-        emit FundsIssued(totalFunds, period);
+        // if (totalShares += shareCount) revert IncorrectTotalShares(totalShares, shareCount);
+        // emit FundsIssued(totalFunds, period);
     }
 
     function updateTokensPerSecond(uint256 amount) external onlyOwner {
         tokensPerSecond = amount;
         emit TokensPerSecondUpdated(amount);
     }
-}
+}

src/EnsoStaking.sol renamed to src/EnsoStaking._sol

@@ -4,8 +4,8 @@ pragma solidity ^0.8.20;
 // Based on OpenZeppelin's VestingWallet & VestingWalletCliff contracts:
 // https://github.com/OpenZeppelin/openzeppelin-contracts/tree/master/contracts/finance
 
-import { SafeERC20, IERC20 } from "openzeppelin-contracts/token/ERC20/utils/SafeERC20.sol";
 import { Ownable } from "openzeppelin-contracts/access/Ownable.sol";
+import { IERC20, SafeERC20 } from "openzeppelin-contracts/token/ERC20/utils/SafeERC20.sol";
 
 contract EnsoVestingWallet is Ownable {
     using SafeERC20 for IERC20;
@@ -16,7 +16,7 @@ contract EnsoVestingWallet is Ownable {
     uint256 public released;
     bool public revoked;
     bool public immutable revocable;
-    
+
     IERC20 private immutable _token;
     address private immutable _revoker;
     uint64 private immutable _start;
@@ -28,7 +28,16 @@ contract EnsoVestingWallet is Ownable {
     error NotRevocable();
     error NotRevoker(address sender, address revoker);
 
-    constructor(IERC20 token, address revoker, address beneficiary, uint64 startTimestamp, uint64 durationSeconds, uint64 cliffSeconds) Ownable(beneficiary) {
+    constructor(
+        IERC20 token,
+        address revoker,
+        address beneficiary,
+        uint64 startTimestamp,
+        uint64 durationSeconds,
+        uint64 cliffSeconds
+    )
+        Ownable(beneficiary)
+    {
         if (cliffSeconds > durationSeconds) {
             revert InvalidCliffDuration(cliffSeconds, durationSeconds);
         }
@@ -60,10 +69,11 @@ contract EnsoVestingWallet is Ownable {
      * @dev Getter for the end timestamp.
      */
     function end() public view returns (uint256) {
+        // @audit return _start + _duration?
         return start() + duration();
     }
 
-     /**
+    /**
      * @dev Getter for the cliff timestamp.
      */
     function cliff() public view virtual returns (uint256) {
@@ -87,7 +97,8 @@ contract EnsoVestingWallet is Ownable {
     /**
      * @dev Getter for the amount of releasable tokens.
      */
-    function releasable() public view  returns (uint256) {
+    function releasable() public view returns (uint256) {
+        // @audit return `_vestingSchedule(...)`?
         return vestedAmount(uint64(block.timestamp)) - released;
     }
 
@@ -122,6 +133,7 @@ contract EnsoVestingWallet is Ownable {
         if (!revocable) revert NotRevocable();
         if (msg.sender != _revoker) revert NotRevoker(msg.sender, _revoker);
         release(); // first, release funds beneficiary is entitled to up to this point
+        // @audit CEI pattern violated, create a private `_relase()` that skips checking `revoked`
         revoked = true;
         uint256 amount = _token.balanceOf(address(this));
         _token.safeTransfer(receiver, amount);
@@ -132,10 +144,7 @@ contract EnsoVestingWallet is Ownable {
      * @dev Implementation of the vesting formula. This returns the amount vested, as a function of time, for
      * an asset given its total historical allocation. Returns 0 if the {cliff} timestamp is not met.
      */
-    function _vestingSchedule(
-        uint256 totalAllocation,
-        uint64 timestamp
-    ) internal view returns (uint256) {
+    function _vestingSchedule(uint256 totalAllocation, uint64 timestamp) internal view returns (uint256) {
         if (revoked || timestamp < cliff()) {
             return 0;
         } else if (timestamp >= end()) {
@@ -144,4 +153,4 @@ contract EnsoVestingWallet is Ownable {
             return (totalAllocation * (timestamp - start())) / duration();
         }
     }
-}
+}