Merge pull request #58 from AztecProtocol/pw/full-math

Pw/full math

Author: PhilWindle

.circleci/config.yml

@@ -22,7 +22,7 @@ jobs:
             yarn setup:foundry
             yarn compile:typechain
       - run:
-          name: Test Contracts No Element
+          name: Test Contracts
           shell: /bin/bash
           command: |
             yarn config set script-shell /bin/bash

package.json

@@ -10,12 +10,10 @@
     "clean": "rm -rf ./cache ./dest ./out ./artifacts ./typechain-types ./client-dest",
     "test": "yarn test:contracts && yarn test:client",
     "test:client": "yarn compile:typechain && jest test",
-    "test:element": "forge test --fork-block-number 14000000 --match-contract Element -vvv --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c",
-    "test:set": "forge test --fork-block-number 14000000 --match-contract Set -vvv --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c",
+    "test:pinned": "forge test --fork-block-number 14000000 --match-contract 'Element|Set' -vvv --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c",
     "test:contracts:block": "forge test --fork-block-number",
     "cast": "cast",
-    "test:contracts": "forge test --no-match-contract 'Element|Set' -vvv && yarn test:element && yarn test:set",
-    "test:contracts:noelement": "forge test --no-match-contract Element -vvv",
+    "test:contracts": "forge test --no-match-contract 'Element|Set' -vvv && yarn test:pinned",
     "build": "yarn clean && yarn compile:typechain && yarn compile:client-dest"
   },
   "dependencies": {
@@ -29,7 +27,7 @@
     "rootDir": "./src"
   },
   "name": "@aztec/bridge-clients",
-  "version": "0.1.32",
+  "version": "0.1.33",
   "description": "This repo contains the solidity files and typescript helper class for all of the Aztec Connect Bridge Contracts",
   "repository": "[email protected]:AztecProtocol/aztec-connect-bridges.git",
   "license": "MIT",

src/bridges/element/ElementBridge.sol

@@ -12,6 +12,7 @@ import {IERC20Permit, IERC20} from '../../interfaces/IERC20Permit.sol';
 import {IWrappedPosition} from './interfaces/IWrappedPosition.sol';
 import {IRollupProcessor} from '../../interfaces/IRollupProcessor.sol';
 import {MinHeap} from './MinHeap.sol';
+import {FullMath} from '../uniswapv3/libraries/FullMath.sol';
 
 import {IDefiBridge} from '../../interfaces/IDefiBridge.sol';
 
@@ -50,6 +51,9 @@ contract ElementBridge is IDefiBridge {
     error UNREGISTERED_POOL();
     error UNREGISTERED_POSITION();
     error UNREGISTERED_PAIR();
+    error INVALID_TOKEN_BALANCE_RECEIVED();
+    error INVALID_CHANGE_IN_BALANCE();
+    error RECEIVED_LESS_THAN_LIMIT();
 
     /*----------------------------------------
       STRUCTS
@@ -144,21 +148,21 @@ contract ElementBridge is IDefiBridge {
     // 48 hours in seconds, usd for calculating speeedbump expiries
     uint256 internal constant _FORTY_EIGHT_HOURS = 172800;
 
-    uint256 internal constant MAX_INT = 2**256 - 1;
+    uint256 internal constant MAX_UINT = type(uint256).max;
 
     uint256 internal constant MIN_GAS_FOR_CHECK_AND_FINALISE = 50000;
     uint256 internal constant MIN_GAS_FOR_FUNCTION_COMPLETION = 5000;
     uint256 internal constant MIN_GAS_FOR_FAILED_INTERACTION = 20000;
     uint256 internal constant MIN_GAS_FOR_EXPIRY_REMOVAL = 25000;
 
     // event emitted on every successful convert call
-    event Convert(uint256 indexed nonce, uint256 totalInputValue);
+    event LogConvert(uint256 indexed nonce, uint256 totalInputValue);
 
     // event emitted on every attempt to finalise, successful or otherwise
-    event Finalise(uint256 indexed nonce, bool success, string message);
+    event LogFinalise(uint256 indexed nonce, bool success, string message);
 
     // event emitted on wvery newly configured pool
-    event PoolAdded(address poolAddress, address wrappedPositionAddress, uint64 expiry);
+    event LogPoolAdded(address poolAddress, address wrappedPositionAddress, uint64 expiry);
 
     /**
      * @dev Constructor
@@ -320,7 +324,7 @@ contract ElementBridge is IDefiBridge {
         }
 
         // verify with Element that the provided contracts are registered
-        validatePositionAndPoolAddresses(wrappedPositionAddress, poolAddress);
+        validatePositionAndPoolAddressesWithElementRegistry(wrappedPositionAddress, poolAddress);
 
         // we store the pool information against a hash of the asset and expiry
         uint256 assetExpiryHash = hashAssetAndExpiry(poolSpec.underlyingAsset, trancheExpiry);
@@ -338,9 +342,9 @@ contract ElementBridge is IDefiBridge {
         // initialising the expiry -> nonce mapping here like this reduces a chunk of gas later when we start to add interactions for this expiry
         uint256[] storage nonces = expiryToNonce[trancheExpiry];
         if (nonces.length == 0) {
-            expiryToNonce[trancheExpiry].push(MAX_INT);
+            expiryToNonce[trancheExpiry].push(MAX_UINT);
         }
-        emit PoolAdded(poolAddress, wrappedPositionAddress, trancheExpiry);
+        emit LogPoolAdded(poolAddress, wrappedPositionAddress, trancheExpiry);
     }
 
     /**
@@ -375,7 +379,7 @@ contract ElementBridge is IDefiBridge {
     * @param wrappedPosition address of a wrapped position contract
     * @param pool address of a balancer pool contract
      */
-    function validatePositionAndPoolAddresses(address wrappedPosition, address pool) internal {
+    function validatePositionAndPoolAddressesWithElementRegistry(address wrappedPosition, address pool) internal {
         IDeploymentValidator validator = IDeploymentValidator(elementDeploymentValidatorAddress);
         if (!validator.checkPoolValidation(pool)) {
             revert UNREGISTERED_POOL();
@@ -458,26 +462,7 @@ contract ElementBridge is IDefiBridge {
         // approve the transfer of tokens to the balancer address
         ERC20(inputAssetA.erc20Address).approve(balancerAddress, totalInputValue);
         // execute the swap on balancer
-        address inputAsset = inputAssetA.erc20Address;
-
-        uint256 principalTokensAmount = IVault(balancerAddress).swap(
-            IVault.SingleSwap({
-                poolId: pool.poolId,
-                kind: IVault.SwapKind.GIVEN_IN,
-                assetIn: IAsset(inputAsset),
-                assetOut: IAsset(pool.trancheAddress),
-                amount: totalInputValue,
-                userData: '0x00'
-            }),
-            IVault.FundManagement({
-                sender: address(this), // the bridge has already received the tokens from the rollup so it owns totalInputValue of inputAssetA
-                fromInternalBalance: false,
-                recipient: payable(address(this)),
-                toInternalBalance: false
-            }),
-            totalInputValue,
-            block.timestamp
-        );
+        uint256 principalTokensAmount = exchangeAssetForTrancheTokens(inputAssetA.erc20Address, pool, totalInputValue);
         // store the tranche that underpins our interaction, the expiry and the number of received tokens against the nonce
         Interaction storage newInteraction = interactions[interactionNonce];
         newInteraction.expiry = trancheExpiry;
@@ -486,17 +471,65 @@ contract ElementBridge is IDefiBridge {
         newInteraction.quantityPT = principalTokensAmount;
         newInteraction.trancheAddress = pool.trancheAddress;
         // add the nonce and expiry to our expiry heap
-        addNonceAndExpiry(interactionNonce, trancheExpiry);
+        addNonceAndExpiryToNonceMapping(interactionNonce, trancheExpiry);
         // increase our tranche account deposits and holdings
         // other members are left as their initial values (all zeros)
         TrancheAccount storage trancheAccount = trancheAccounts[newInteraction.trancheAddress];
         trancheAccount.numDeposits++;
         trancheAccount.quantityTokensHeld += newInteraction.quantityPT;
-        emit Convert(interactionNonce, totalInputValue);
+        emit LogConvert(interactionNonce, totalInputValue);
         finaliseExpiredInteractions(MIN_GAS_FOR_FUNCTION_COMPLETION);       
         // we need to get here with MIN_GAS_FOR_FUNCTION_COMPLETION gas to exit.
     }
 
+    /** 
+    * @dev Function to exchange the input asset for tranche tokens on Balancer
+    * @param inputAsset the address of the asset we want to swap
+    * @param pool storage struct containing details of the pool we wish to use for the swap
+    * @param inputQuantity the quantity of the input asset we wish to swap
+    * @return quantityReceived amount of tokens recieved
+    */
+    function exchangeAssetForTrancheTokens(address inputAsset, Pool storage pool, uint256 inputQuantity) internal returns (uint256 quantityReceived) {
+        IVault.SingleSwap memory singleSwap = IVault.SingleSwap({
+            poolId: pool.poolId, // the id of the pool we want to use
+            kind: IVault.SwapKind.GIVEN_IN, // We are exchanging a given number of input tokens
+            assetIn: IAsset(inputAsset), // the input asset for the swap
+            assetOut: IAsset(pool.trancheAddress), // the tranche token address as the output asset
+            amount: inputQuantity, // the total amount of input asset we wish to swap
+            userData: '0x00' // set to 0 as per the docs, this is unused in current balancer pools
+        });
+        IVault.FundManagement memory fundManagement = IVault.FundManagement({
+            sender: address(this), // the bridge has already received the tokens from the rollup so it owns totalInputValue of inputAssetA
+            fromInternalBalance: false,
+            recipient: payable(address(this)), // we want the output tokens transferred back to us
+            toInternalBalance: false
+        });
+
+        uint256 trancheTokenQuantityBefore = ERC20(pool.trancheAddress).balanceOf(address(this));
+        quantityReceived = IVault(balancerAddress).swap(
+            singleSwap,
+            fundManagement,
+            inputQuantity, // we won't accept less than 1 output token per input token
+            block.timestamp
+        );
+
+        uint256 trancheTokenQuantityAfter = ERC20(pool.trancheAddress).balanceOf(address(this));
+        // ensure we haven't lost tokens!
+        if (trancheTokenQuantityAfter < trancheTokenQuantityBefore) {
+            revert INVALID_CHANGE_IN_BALANCE();
+        }
+        // change in balance must be >= 0 here
+        uint256 changeInBalance = trancheTokenQuantityAfter - trancheTokenQuantityBefore;
+        // ensure the change in balance matches that reported to us
+        if (changeInBalance != quantityReceived) {
+            revert INVALID_TOKEN_BALANCE_RECEIVED();
+        }
+        // ensure we received at least the limit we placed
+        if (quantityReceived < inputQuantity) {
+            revert RECEIVED_LESS_THAN_LIMIT();
+        }
+    }
+
     /**
      * @dev Function to attempt finalising of as many interactions as possible within the specified gas limit
      * Continue checking for and finalising interactions until we expend the available gas
@@ -510,7 +543,7 @@ contract ElementBridge is IDefiBridge {
         while (gasleft() > gasLoopCondition) {
             // check the heap to see if we can finalise an expired transaction
             // we provide a gas floor to the function which will enable us to leave this function without breaching our gasFloor
-            (bool expiryAvailable, uint256 nonce) = checkNextExpiry(ourGasFloor);
+            (bool expiryAvailable, uint256 nonce) = checkForNextInteractionToFinalise(ourGasFloor);
             if (!expiryAvailable) {
                 break;
             }
@@ -570,7 +603,7 @@ contract ElementBridge is IDefiBridge {
         if (trancheAccount.numDeposits == 0) {
             // shouldn't be possible, this means we have had no deposits against this tranche
             setInteractionAsFailure(interaction, interactionNonce, 'NO_DEPOSITS_FOR_TRANCHE');
-            popInteraction(interaction, interactionNonce);
+            popInteractionFromNonceMapping(interaction, interactionNonce);
             return (0, 0, false);
         }
 
@@ -586,12 +619,12 @@ contract ElementBridge is IDefiBridge {
             } catch Error(string memory errorMessage) {
                 setInteractionAsFailure(interaction, interactionNonce, errorMessage);
                 trancheAccount.redemptionStatus = TrancheRedemptionStatus.REDEMPTION_FAILED;
-                popInteraction(interaction, interactionNonce);
+                popInteractionFromNonceMapping(interaction, interactionNonce);
                 return (0, 0, false);
             } catch {
                 setInteractionAsFailure(interaction, interactionNonce, 'UNKNOWN_ERROR_FROM_TRANCHE_WITHDRAW');
                 trancheAccount.redemptionStatus = TrancheRedemptionStatus.REDEMPTION_FAILED;
-                popInteraction(interaction, interactionNonce);
+                popInteractionFromNonceMapping(interaction, interactionNonce);
                 return (0, 0, false);
             }
         }
@@ -606,7 +639,9 @@ contract ElementBridge is IDefiBridge {
             amountToAllocate = trancheAccount.quantityAssetRedeemed / trancheAccount.numDeposits;
         } else {
             // apportion the output asset based on the interaction's holding of the principle token
-            amountToAllocate = (trancheAccount.quantityAssetRedeemed * interaction.quantityPT) / trancheAccount.quantityTokensHeld;
+            // protects against phantom overflow in the operation of
+            // amountToAllocate = (trancheAccount.quantityAssetRedeemed * interaction.quantityPT) / trancheAccount.quantityTokensHeld;
+            amountToAllocate = FullMath.mulDiv(trancheAccount.quantityAssetRedeemed, interaction.quantityPT, trancheAccount.quantityTokensHeld);
         }
         // numDeposits and numFinalised are uint32 types, so easily within range for an int256
         int256 numRemainingInteractionsForTranche = int256(uint256(trancheAccount.numDeposits)) - int256(uint256(trancheAccount.numFinalised));
@@ -622,11 +657,11 @@ contract ElementBridge is IDefiBridge {
         // approve the transfer of funds back to the rollup contract
         ERC20(outputAssetA.erc20Address).approve(rollupProcessor, amountToAllocate);
         interaction.finalised = true;
-        popInteraction(interaction, interactionNonce);
+        popInteractionFromNonceMapping(interaction, interactionNonce);
         outputValueA = amountToAllocate;
         outputValueB = 0;
         interactionCompleted = true;
-        emit Finalise(interactionNonce, interactionCompleted, '');
+        emit LogFinalise(interactionNonce, interactionCompleted, '');
     }
 
     /**
@@ -641,7 +676,7 @@ contract ElementBridge is IDefiBridge {
         string memory message
     ) internal {
         interaction.failed = true;
-        emit Finalise(interactionNonce, false, message);
+        emit LogFinalise(interactionNonce, false, message);
     }
 
     /**
@@ -650,12 +685,12 @@ contract ElementBridge is IDefiBridge {
      * @param expiry The expiry of the interaction to be added
      * @return expiryAdded Flag specifying whether the interactions expiry was added to the heap
      */
-    function addNonceAndExpiry(uint256 nonce, uint64 expiry) internal returns (bool expiryAdded) {
+    function addNonceAndExpiryToNonceMapping(uint256 nonce, uint64 expiry) internal returns (bool expiryAdded) {
         // get the set of nonces already against this expiry
-        // check for the MAX_INT placeholder nonce that exists to reduce gas costs at this point in the code
+        // check for the MAX_UINT placeholder nonce that exists to reduce gas costs at this point in the code
         expiryAdded = false;
         uint256[] storage nonces = expiryToNonce[expiry];
-        if (nonces.length == 1 && nonces[0] == MAX_INT) {
+        if (nonces.length == 1 && nonces[0] == MAX_UINT) {
             nonces[0] = nonce;
         } else {
             nonces.push(nonce);
@@ -674,7 +709,7 @@ contract ElementBridge is IDefiBridge {
      * @param interactionNonce The nonce of the interaction to be removed
      * @return expiryRemoved Flag specifying whether the interactions expiry was removed from the heap
      */
-    function popInteraction(Interaction storage interaction, uint256 interactionNonce) internal returns (bool expiryRemoved) {
+    function popInteractionFromNonceMapping(Interaction storage interaction, uint256 interactionNonce) internal returns (bool expiryRemoved) {
         uint256[] storage nonces = expiryToNonce[interaction.expiry];
         if (nonces.length == 0) {
             return (false);
@@ -706,7 +741,7 @@ contract ElementBridge is IDefiBridge {
      * @return expiryAvailable Flag specifying whether an expiry is available to be finalised
      * @return nonce The next interaction nonce to be finalised
      */
-    function checkNextExpiry(uint256 gasFloor)
+    function checkForNextInteractionToFinalise(uint256 gasFloor)
         internal
         returns (
             bool expiryAvailable,
@@ -728,7 +763,7 @@ contract ElementBridge is IDefiBridge {
         uint256 minGasForLoop = (gasFloor + MIN_GAS_FOR_FAILED_INTERACTION);
         while (nonces.length > 0 && gasleft() >= minGasForLoop) {
             uint256 nextNonce = nonces[nonces.length - 1];
-            if (nextNonce == MAX_INT) {
+            if (nextNonce == MAX_UINT) {
                 // this shouldn't happen, this value is the placeholder for reducing gas costs on convert
                 // we just need to pop and continue
                 nonces.pop();

src/bridges/element/MinHeap.sol

@@ -10,10 +10,10 @@ library MinHeap {
     error HEAP_EMPTY();
 
     // maximum value of uint64. used as initial value in pre-allocated array 
-    uint64 internal constant MAX_INT = 2**64 - 1;
+    uint64 internal constant MAX_INT = type(uint64).max;
 
     /**
-     * @dev Encapsulates the underlyding data structure used to manage the heap
+     * @dev Encapsulates the underlying data structure used to manage the heap
      *
      * @param heap the array of values contained within the heap
      * @param heapSize the current size of the heap, usually different to the size of underlying array so tracked seperately
@@ -64,7 +64,7 @@ library MinHeap {
      * @param value the value to be removed
      */
     function remove(MinHeapData storage self, uint64 value) internal {
-        removeExpiryFromHeap(self, value);
+        removeValueFromHeap(self, value);
     }
 
     /**
@@ -110,8 +110,6 @@ library MinHeap {
     function addToHeap(MinHeapData storage self, uint64 value) private {
         // standard min-heap insertion
         // push to the end of the heap and sift up.
-        // there is a high probability that the expiry being added will remain where it is
-        // so this operation will end up being O(1)
         if (self.heapSize == self.heap.length) {
             self.heap.push(value);       
         } else {
@@ -170,7 +168,7 @@ library MinHeap {
      * @param self reference to the underlying data structure of the heap
      * @param value the value to be removed
      */
-    function removeExpiryFromHeap(MinHeapData storage self, uint64 value) private {
+    function removeValueFromHeap(MinHeapData storage self, uint64 value) private {
         uint256 index = 0;
         while (index < self.heapSize && self.heap[index] != value) {
             ++index;