make fix, add getOwner (#111)

Author: anihamde

auction-server/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "auction-server"
-version = "0.9.4"
+version = "0.9.5"
 edition = "2021"
 license-file = "license.txt"
 

auction-server/Cargo.toml

@@ -10,6 +10,8 @@ struct MulticallData {
     address targetContract;
     bytes targetCalldata;
     uint256 bidAmount;
+    uint256 gasLimit;
+    bool revertOnFailure;
 }
 
 function multicall(
@@ -28,8 +30,10 @@ When the relayer calls the `multicall` function it specifies a `permissionKey` a
 The contract will call the `targetContract` using the specified `targetCalldata` with `value:0`
 and expects the balance of the contract to be increased by `bidAmount` if the call was successful.
 Otherwise it will revert the the call and undo all the changes made by that call.
-The revert here is only for a specific call and not the whole transaction.
+Note the revert here is only for a specific call and not the whole transaction.
 A single transaction can contain multiple `MulticallData` where some of them fail and revert.
 
+`gasLimit` specifies the maximum amount of gas that will be forwarded for the external call. `revertOnFailure` is a boolean that determines whether to revert the entire `ExpressRelay` transaction if the external call fails. This is only intended to be set to `true` for simulation purposes; for actual production on-chain submissions, this is intended to be set to `false` so that no individual external call's failure causes failure of the entire `Express Relay` transaction.
+
 `isPermissioned` will return true for the specified `permissionKey`
 if and only if it is an inner call initiated by the `multicall` with the same `permissionKey`.

contracts/ExpressRelay.md

@@ -5,7 +5,6 @@ An opportunity refers to any arbitrary contract call with pre-defined expectatio
 The opportunity adapter handles routing arbitrary calldata to an external contract along with assertions around the
 quantity of ETH and different tokens exchanged as a result of the contract call.
 The main function `executeOpportunity` accepts an `ExecutionParams` type.
-The `executor` should allow this contract to spend their tokens on their behalf using the ERC20 `approve` method.
 
 ```solidity
 struct TokenAmount {
@@ -14,7 +13,11 @@ struct TokenAmount {
 }
 
 struct ExecutionParams {
-  TokenAmount[] sellTokens;
+  ISignatureTransfer.PermitBatchTransferFrom permit;
+  ExecutionWitness witness;
+}
+
+struct ExecutionWitness {
   TokenAmount[] buyTokens;
   address executor;
   address targetContract;
@@ -26,25 +29,27 @@ struct ExecutionParams {
 
 ```
 
-by calling the `executeOpportunity` the contract will:
+The `ISignatureTransfer.PermitBatchTransferFrom` struct can be found [here](https://github.com/Uniswap/permit2/blob/cc56ad0f3439c502c246fc5cfcc3db92bb8b7219/src/interfaces/ISignatureTransfer.sol#L51-L58). This struct will contain the token(s) the user intends to sell and a nonce and deadline for the signature's validity.
+
+by calling the `executeOpportunity` function the contract will:
 
 1. Verify the parameters are valid:
    1. Verify it is being called from the `expressRelay` contract
-   2. Verify that the executor actually signed this payload
-   3. Verify this signature has not been used before
-   4. Verify the block.timestamp is less than or equal to the `validUntil` parameter.
-   5. Verifies there are no duplicate token addresses in `sellTokens` or `buyTokens`
-2. Transfers the sellTokens to the contract itself and approves the `targetContract` to use them
-3. Transfers `targetCallValue` Wrapped ETH from executor and converts them to ETH to be used as the value of the call
+   2. Verify that the executor is the owner of the contract
+   3. Verify the `targetContract` is not itself or the permit2 contract
+   4. Verifies there are no duplicate token addresses in `permit.permitted` or `witness.buyTokens`
+2. Transfers the permitted "sell tokens" to itself via permit2 (which handles validation of the provided signature) and approves the `targetContract` to use them
+3. Converts the necessary amount of Wrapped ETH received from the executor to ETH to be used as the value of the call
 4. Calls the `targetContract` with `targetCalldata` and `targetCallValue`
-5. Checks that the contract has received the tokens specified in `buyTokens`
-6. Transfers the `buyTokens` back to the `executor`
+5. Revokes the allowances of the `targetContract` over the permitted sell tokens
+6. Checks that the contract has received the tokens specified in `buyTokens`
 7. Similar to 3, transfers `bidAmount` Wrapped ETH from executor and sends it to the express relay contract as the bid.
+8. Transfers the `buyTokens` back to the `executor`
 
 If any of the mentioned steps fail the whole call will revert.
 
-⚠️ Calling any ERC20 tokens with custom/malicious behaviour should be handled by the callee
-and is out of the scope for this contract.
+⚠️ Calling any ERC20 tokens with custom/malicious behaviour should be handled by the callee and is out of the scope for this contract.
+
+⚠️ In cases where less than the specified amount of tokens is used by the target call, the remaining tokens will remain in the contract. Similarly, if less than the specified amount of (Wrapped) ETH is used by the target call, the remainder will remain in the contract as WETH. The owner of this contract can withdraw those assets via the `withdrawToken` function.
 
-⚠️ In cases where less than the specified amount of tokens is used by the target call,
-the remaining tokens will remain in the contract and still approved to be used by the `targetContract`
+⚠️ If any ETH is transferred to the contract, the owner can withdraw the ETH via the `withdrawEth`

contracts/OpportunityAdapter.md

@@ -43,6 +43,13 @@ contract OpportunityAdapter is ReentrancyGuard, OpportunityAdapterHasher {
         _;
     }
 
+    /**
+     * @notice getOwner function - returns the address of the owner of the contract
+     */
+    function getOwner() public view returns (address) {
+        return _owner;
+    }
+
     /**
      * @notice getExpressRelay function - returns the address of the express relay authenticated for calling this contract
      */
@@ -250,12 +257,12 @@ contract OpportunityAdapter is ReentrancyGuard, OpportunityAdapterHasher {
             params.witness.targetCallValue
         );
         _revokeAllowances(params.permit, params.witness.targetContract);
+        _settleBid(params.witness.bidAmount);
         _validateAndTransferBuyTokens(
             params.witness.buyTokens,
             params.witness.executor,
             buyTokensBalancesBeforeCall
         );
-        _settleBid(params.witness.bidAmount);
         (
             uint256 ethBalanceAfterCall,
             uint256 wethBalanceAfterCall

contracts/src/opportunity-adapter/OpportunityAdapter.sol

@@ -15,10 +15,28 @@ import "permit2/interfaces/ISignatureTransfer.sol";
 import {PermitSignature, EIP712Domain} from "./PermitSignature.sol";
 
 contract MockTarget {
+    address payable _weth;
+
+    constructor(address weth) {
+        _weth = payable(weth);
+    }
+
     error BadCall();
 
     function doNothing() public payable {}
 
+    function exchangeWethForWeth(
+        uint256 amountIn,
+        uint256 amountOut
+    ) public payable {
+        WETH9(_weth).transferFrom(msg.sender, address(this), amountIn);
+        uint256 balanceWeth = WETH9(_weth).balanceOf(address(this));
+        if (balanceWeth < amountOut) {
+            WETH9(_weth).deposit{value: amountOut - balanceWeth}();
+        }
+        WETH9(_weth).transfer(msg.sender, amountOut);
+    }
+
     function transferTokenToSender(
         address token,
         uint256 amount
@@ -51,6 +69,7 @@ contract OpportunityAdapterIntegrationTest is
     WETH9 weth;
     MyToken buyToken;
     MyToken sellToken;
+    address _expressRelay;
 
     function setUpTokens() internal {
         buyToken = new MyToken("BuyToken", "BT");
@@ -59,8 +78,9 @@ contract OpportunityAdapterIntegrationTest is
     }
 
     function setUpOpportunityAdapter() internal {
+        _expressRelay = makeAddr("expressRelay");
         adapterFactory = new OpportunityAdapterFactory(
-            address(this),
+            _expressRelay,
             address(weth),
             PermitSignature.PERMIT2
         );
@@ -70,7 +90,7 @@ contract OpportunityAdapterIntegrationTest is
         setUpTokens();
         setUpPermit2();
         setUpOpportunityAdapter();
-        mockTarget = new MockTarget();
+        mockTarget = new MockTarget(address(weth));
     }
 
     // successful bids will be received by this contract
@@ -252,7 +272,7 @@ contract OpportunityAdapterIntegrationTest is
         vm.prank(adapterFactory.getExpressRelay());
         vm.expectCall(address(mockTarget), callValue, targetCalldata);
         // We expect the adapter to transfer the bid to the express relay
-        vm.expectCall(address(this), bid, bytes(""));
+        vm.expectCall(_expressRelay, bid, bytes(""));
         vm.expectCall(
             address(weth),
             abi.encodeWithSelector(WETH9.withdraw.selector, callValue)
@@ -302,6 +322,87 @@ contract OpportunityAdapterIntegrationTest is
         adapterFactory.executeOpportunity(executionParams, signature);
     }
 
+    function testExecutionWithWethBuySellTokens(
+        uint256 wethSellTokenAmount,
+        uint256 wethBuyTokenAmount,
+        uint256 bidAmount,
+        uint256 targetCallValue
+    ) public {
+        vm.assume(bidAmount < type(uint256).max - targetCallValue);
+        vm.assume(
+            wethSellTokenAmount <
+                type(uint256).max - bidAmount - targetCallValue
+        );
+        vm.assume(
+            wethBuyTokenAmount <
+                type(uint256).max -
+                    wethSellTokenAmount -
+                    bidAmount -
+                    targetCallValue
+        );
+
+        TokenAmount[] memory sellTokens = new TokenAmount[](1);
+        uint256 sellTokenAmount = wethSellTokenAmount +
+            bidAmount +
+            targetCallValue;
+        sellTokens[0] = TokenAmount(address(weth), sellTokenAmount);
+
+        TokenAmount[] memory buyTokens;
+        if (wethBuyTokenAmount == 0) {
+            buyTokens = new TokenAmount[](0);
+        } else {
+            buyTokens = new TokenAmount[](1);
+            buyTokens[0] = TokenAmount(address(weth), wethBuyTokenAmount);
+        }
+        vm.deal(address(mockTarget), wethBuyTokenAmount);
+
+        bytes memory targetCalldata = abi.encodeWithSelector(
+            mockTarget.exchangeWethForWeth.selector,
+            wethSellTokenAmount,
+            wethBuyTokenAmount
+        );
+        (
+            ExecutionParams memory executionParams,
+            bytes memory signature
+        ) = createExecutionParamsAndSignature(
+                sellTokens,
+                buyTokens,
+                targetCalldata,
+                targetCallValue,
+                bidAmount,
+                block.timestamp + 1000
+            );
+
+        vm.deal(executionParams.witness.executor, sellTokenAmount);
+        vm.startPrank(executionParams.witness.executor);
+        weth.deposit{value: sellTokenAmount}();
+        weth.approve(PERMIT2, sellTokenAmount);
+        vm.stopPrank();
+
+        uint256 balanceWethPlusEthMockTargetPre = address(mockTarget).balance +
+            WETH9(weth).balanceOf(address(mockTarget));
+
+        vm.prank(adapterFactory.getExpressRelay());
+        vm.expectCall(address(mockTarget), targetCalldata);
+        adapterFactory.executeOpportunity(executionParams, signature);
+
+        uint256 balanceWethPlusEthMockTargetPost = address(mockTarget).balance +
+            WETH9(weth).balanceOf(address(mockTarget));
+
+        assertEq(
+            WETH9(weth).balanceOf(executionParams.witness.executor),
+            wethBuyTokenAmount
+        );
+        assertEq(adapterFactory.getExpressRelay().balance, bidAmount);
+        assertEq(
+            balanceWethPlusEthMockTargetPost,
+            balanceWethPlusEthMockTargetPre -
+                wethBuyTokenAmount +
+                targetCallValue +
+                wethSellTokenAmount
+        );
+    }
+
     function testRevertWhenEthBalanceDecrease() public {
         TokenAmount[] memory noTokens = new TokenAmount[](0);
         bytes memory targetCalldata = abi.encodeWithSelector(

contracts/test/OpportunityAdapterIntegration.t.sol

@@ -138,7 +138,7 @@ async def main():
     parser.add_argument(
         "--bid",
         type=int,
-        default=int(2e16),  # To make sure it covers the gas cost
+        default=int(5e17),  # To make sure it covers the gas cost
         help="Default amount of bid for liquidation opportunities",
     )
     parser.add_argument(