Limit gas when doing twin transfers (#694)

* limit gas on twin transfers

* dynamically adjust gas instead of a constant limit

* transfer only if enough gas left

* fix tests

* Merge branch 'main' into limit-gas-twin-transfer

---------

Co-authored-by: Mischa <mischa@mmt.me.uk>

contracts/domain/BosonConstants.sol

@@ -114,6 +114,8 @@ string constant EXCHANGE_ALREADY_EXISTS = "Exchange already exists";
 string constant INVALID_RANGE_LENGTH = "Range length is too large or zero";
 
 // Revert Reasons: Twin related
+uint256 constant SINGLE_TWIN_RESERVED_GAS = 140000;
+uint256 constant MINIMAL_RESIDUAL_GAS = 180000;
 string constant NO_SUCH_TWIN = "No such twin";
 string constant NO_TRANSFER_APPROVED = "No transfer approved";
 string constant TWIN_TRANSFER_FAILED = "Twin could not be transferred";

contracts/mock/Foreign1155.sol

@@ -25,3 +25,16 @@ contract Foreign1155 is ERC1155Upgradeable {
         selfdestruct(payable(msg.sender));
     }
 }
+
+/*
+ * @title Foreign1155 that consumes all gas when transfer is called
+ *
+ * @notice Mock ERC-(1155) for Unit Testing
+ */
+contract Foreign1155GasTheft is Foreign1155 {
+    function safeTransferFrom(address, address, uint256, uint256, bytes memory) public virtual override {
+        while (true) {
+            // consume all gas
+        }
+    }
+}

contracts/mock/Foreign20.sol

@@ -213,3 +213,16 @@ contract Foreign20TransferFromReturnFalse is Foreign20 {
         return false;
     }
 }
+
+/*
+ * @title Foreign20 that consumes all gas when transfer is called
+ *
+ * @notice Mock ERC-(20) for Unit Testing
+ */
+contract Foreign20GasTheft is Foreign20 {
+    function transferFrom(address, address, uint256) public virtual override returns (bool) {
+        while (true) {
+            // consume all gas
+        }
+    }
+}

contracts/mock/Foreign721.sol

@@ -31,3 +31,16 @@ contract Foreign721 is ERC721Upgradeable {
         selfdestruct(payable(msg.sender));
     }
 }
+
+/*
+ * @title Foreign721 that consumes all gas when transfer is called
+ *
+ * @notice Mock ERC-(721) for Unit Testing
+ */
+contract Foreign721GasTheft is Foreign721 {
+    function safeTransferFrom(address, address, uint256, bytes memory) public virtual override {
+        while (true) {
+            // consume all gas
+        }
+    }
+}

contracts/protocol/facets/ExchangeHandlerFacet.sol

@@ -701,29 +701,36 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
 
             address sender = msgSender();
 
+            uint256 twinCount = twinIds.length;
+
+            // Fetch twin: up to 20,000 gas
+            // Handle individual outcome: up to 120,000 gas
+            // Handle overall outcome: up to 200,000 gas
+            // Next line would overflow if twinCount > (type(uint256).max - MINIMAL_RESIDUAL_GAS)/SINGLE_TWIN_RESERVED_GAS
+            // Oveflow happens for twinCount ~ 9.6x10^71, which is impossible to achieve
+            uint256 reservedGas = twinCount * SINGLE_TWIN_RESERVED_GAS + MINIMAL_RESIDUAL_GAS;
+
             // Visit the twins
-            for (uint256 i = 0; i < twinIds.length; i++) {
+            for (uint256 i = 0; i < twinCount; i++) {
                 // Get the twin
                 (, Twin storage twin) = fetchTwin(twinIds[i]);
 
-                // Transfer the token from the seller's assistant to the buyer
-                // N.B. Using call here so as to normalize the revert reason
-                bytes memory result;
                 bool success;
                 uint256 tokenId = twin.tokenId;
-                TokenType tokenType = twin.tokenType;
-
-                // Shouldn't decrement supply if twin supply is unlimited
-                if (twin.supplyAvailable != type(uint256).max) {
-                    // Decrement by 1 if token type is NonFungible otherwise decrement amount (i.e, tokenType is MultiToken or FungibleToken)
-                    twin.supplyAvailable = twin.tokenType == TokenType.NonFungibleToken
-                        ? twin.supplyAvailable - 1
-                        : twin.supplyAvailable - twin.amount;
-                }
 
-                // Calldata to transfer the twin
                 {
-                    bytes memory data;
+                    TokenType tokenType = twin.tokenType;
+
+                    // Shouldn't decrement supply if twin supply is unlimited
+                    if (twin.supplyAvailable != type(uint256).max) {
+                        // Decrement by 1 if token type is NonFungible otherwise decrement amount (i.e, tokenType is MultiToken or FungibleToken)
+                        twin.supplyAvailable = tokenType == TokenType.NonFungibleToken
+                            ? twin.supplyAvailable - 1
+                            : twin.supplyAvailable - twin.amount;
+                    }
+
+                    // Transfer the token from the seller's assistant to the buyer
+                    bytes memory data; // Calldata to transfer the twin
 
                     if (tokenType == TokenType.FungibleToken) {
                         // ERC-20 style transfer
@@ -745,7 +752,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
                             tokenId,
                             ""
                         );
-                    } else if (twin.tokenType == TokenType.MultiToken) {
+                    } else if (tokenType == TokenType.MultiToken) {
                         // ERC-1155 style transfer
                         data = abi.encodeWithSignature(
                             "safeTransferFrom(address,address,uint256,uint256,bytes)",
@@ -757,14 +764,21 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
                         );
                     }
 
-                    // Make call only if code at address exists
-                    if (twin.tokenAddress.isContract()) {
-                        (success, result) = twin.tokenAddress.call(data);
+                    // Make call only if there is enough gas and code at address exists.
+                    // If not, skip the call and mark the transfer as failed
+                    if (gasleft() > reservedGas && twin.tokenAddress.isContract()) {
+                        bytes memory result;
+                        (success, result) = twin.tokenAddress.call{ gas: gasleft() - reservedGas }(data);
+
+                        success = success && (result.length == 0 || abi.decode(result, (bool)));
                     }
                 }
 
+                // Reduce minimum gas required for succesful execution
+                reservedGas -= SINGLE_TWIN_RESERVED_GAS;
+
                 // If token transfer failed
-                if (!success || (result.length > 0 && !abi.decode(result, (bool)))) {
+                if (!success) {
                     raiseDisputeInternal(_exchange, _voucher, seller.id);
 
                     emit TwinTransferFailed(twin.id, twin.tokenAddress, _exchange.id, tokenId, twin.amount, sender);