Relax trade verification checks for pre-interactions (#3081)

# Description
Currently the trade verification does a few checks and actions on behalf
of the user that requested the quote to make the verification possible.
For example if a user has ETH and no WETH but requests a quote for WETH
we'd try wrap the necessary WETH before doing the fake settlement in the
simulation.
We also do a check before the settlement to verify that the user has the
required sell token balance. That way we can return a more helpful error
message than when the settlement contract reverts due to the missing
funds.

Both of these things could fail if the user provides pre-hooks that will
only set up the necessary trade pre-conditions for the `from` address.

# Changes
Adjusted the simulation code such that we can control whether some
reasonable pre-conditions will be set up as part of the simulation or
not.
If a user provides pre-conditions we now assume that they are necessary
to set things up and we don't do our set up to not interfere with that.

## How to test
Added an e2e test that transfers the needed sell tokens from a safe to
the trader address in a pre-hook. This test fails without the change
introduced in this PR.

Edit: The test works locally but appears to cause issues in CI for
whatever reason. :/
Edit2: It's actually an existing test that fails exactly because of the
reason described 👇

While working on the test I learned about a niche edge case that
currently can't be verified: If you try to get a quote for a `from`
address that is a contract that will only be deployed in the
pre-interaction.
The reason is that before we do the simulation we check whether the
`from` address is a contract. If it is a contract we use some state
override magic to make our fake user during the simulation behave like
the contract that was deployed at that address. But we can't know that
the `from` address will contain contract code if the contract will only
be deployed in the pre-interaction.

crates/contracts/solidity/Solver.sol

@@ -35,6 +35,10 @@ contract Solver {
     /// @param nativeToken - ERC20 version of the chain's token
     /// @param receiver - address receiving the bought tokens
     /// @param settlementCall - the calldata of the `settle()` call
+    /// @param mockPreconditions - controls whether things like ETH wrapping
+    ///            or setting allowance should be done on behalf of the
+    ///            user to support quote verification even if the user didn't
+    ///            wrap their ETH or set the necessary allowances yet.
     ///
     /// @return gasUsed - gas used for the `settle()` call
     /// @return queriedBalances - list of balances stored during the simulation
@@ -46,19 +50,35 @@ contract Solver {
         address buyToken,
         address nativeToken,
         address payable receiver,
-        bytes calldata settlementCall
+        bytes calldata settlementCall,
+        bool mockPreconditions
     ) external returns (
         uint256 gasUsed,
         uint256[] memory queriedBalances
     ) {
         require(msg.sender == address(this), "only simulation logic is allowed to call 'swap' function");
+
         // Prepare the trade in the context of the trader so we are allowed
         // to set approvals and things like that.
-        Trader(trader).prepareSwap(settlementContract, sellToken, sellAmount, nativeToken, receiver);
+        if (mockPreconditions) {
+            Trader(trader)
+                .prepareSwap(
+                    settlementContract,
+                    sellToken,
+                    sellAmount,
+                    nativeToken
+                );
+        }
+
+        // Warm the storage for sending ETH to smart contract addresses.
+        // We allow this call to revert becaues it was either unnecessary in the first place
+        // or failing to send `ETH` to the `receiver` will cause a revert in the settlement
+        // contract.
+        receiver.call{value: 0}("");
+
         this.storeBalance(sellToken, address(settlementContract), false);
         this.storeBalance(buyToken, address(settlementContract), false);
         uint256 gasStart = gasleft();
-        // TODO can we assume the overhead of this function call to be negligible due to inlining?
         address(settlementContract).doCall(settlementCall);
         gasUsed = gasStart - gasleft() - _simulationOverhead;
         this.storeBalance(sellToken, address(settlementContract), false);
@@ -69,7 +89,7 @@ contract Solver {
     /// @dev Helper function that reads the `owner`s balance for a given `token` and
     /// stores it. These stored balances will be returned as part of the simulation
     /// `Summary`.
-    /// @param token - which token's we read the balance from
+    /// @param token - which token we read the balance from
     /// @param owner - whos balance we are reading
     /// @param countGas - controls whether this gas cost should be discounted from the settlement gas.
     function storeBalance(address token, address owner, bool countGas) external {

crates/contracts/solidity/Trader.sol

@@ -61,21 +61,18 @@ contract Trader {
     // settlement contract anyway.
     receive() external payable {}
 
-    /// @dev Prepares everything needed by the trader for successfully executing the swap.
-    /// This includes giving the required approval, wrapping the required ETH (if needed)
-    /// and warming the needed storage for sending native ETH to smart contracts.
+    /// @dev Executes needed actions on behalf of the trader to make the trade possible.
+    ///      (e.g. wrapping ETH and setting approvals)
     /// @param settlementContract - pass in settlement contract because it does not have
     /// a stable address in tests.
     /// @param sellToken - token being sold by the trade
     /// @param sellAmount - expected amount to be sold according to the quote
     /// @param nativeToken - ERC20 version of the chain's native token
-    /// @param receiver - address that will receive the bought tokens
     function prepareSwap(
         ISettlement settlementContract,
         address sellToken,
         uint256 sellAmount,
-        address nativeToken,
-        address payable receiver
+        address nativeToken
     ) external {
         require(!alreadyCalled(), "prepareSwap can only be called once");
 
@@ -105,11 +102,6 @@ contract Trader {
 
         uint256 availableSellToken = IERC20(sellToken).balanceOf(address(this));
         require(availableSellToken >= sellAmount, "trader does not have enough sell_token");
-        // Warm the storage for sending ETH to smart contract addresses.
-        // We allow this call to revert becaues it was either unnecessary in the first place
-        // or failing to send `ETH` to the `receiver` will cause a revert in the settlement
-        // contract.
-        receiver.call{value: 0}("");
     }
 
     /// @dev Validate all signature requests. This makes "signing" CoW protocol

crates/e2e/tests/e2e/hooks.rs

@@ -9,8 +9,10 @@ use {
     ethcontract::{Bytes, H160, U256},
     model::{
         order::{OrderCreation, OrderCreationAppData, OrderKind},
+        quote::{OrderQuoteRequest, OrderQuoteSide, SellAmount},
         signature::{hashed_eip712_message, EcdsaSigningScheme, Signature},
     },
+    number::nonzero::U256 as NonZeroU256,
     reqwest::StatusCode,
     secp256k1::SecretKey,
     serde_json::json,
@@ -42,6 +44,12 @@ async fn local_node_gas_limit() {
     run_test(gas_limit).await;
 }
 
+#[tokio::test]
+#[ignore]
+async fn local_node_quote_verification() {
+    run_test(quote_verification).await;
+}
+
 async fn gas_limit(web3: Web3) {
     let mut onchain = OnchainComponents::deploy(web3).await;
 
@@ -304,8 +312,14 @@ async fn signature(web3: Web3) {
     // Place Orders
     let mut order = OrderCreation {
         from: Some(safe.address()),
+        // Quotes for trades where the pre-interactions deploy a contract
+        // at the `from` address currently can't be verified.
+        // To not throw an error because we can't get a verifiable quote
+        // we make the order partially fillable and sell slightly more than
+        // `from` currently has.
+        sell_amount: to_wei(6),
+        partially_fillable: true,
         sell_token: token.address(),
-        sell_amount: to_wei(5),
         buy_token: onchain.contracts().weth.address(),
         buy_amount: to_wei(3),
         valid_to: model::time::now_in_epoch_seconds() + 300,
@@ -469,3 +483,108 @@ async fn partial_fills(web3: Web3) {
         2.into()
     );
 }
+
+/// Checks that quotes can be verified which need the pre-hooks
+/// to run before the requested trade could be executed.
+async fn quote_verification(web3: Web3) {
+    let mut onchain = OnchainComponents::deploy(web3.clone()).await;
+
+    let chain_id = web3.eth().chain_id().await.unwrap();
+
+    let [trader] = onchain.make_accounts(to_wei(1)).await;
+    let [solver] = onchain.make_solvers(to_wei(1)).await;
+
+    let safe_infra = safe::Infrastructure::new(&web3).await;
+
+    // Prepare the Safe creation transaction, but don't execute it! This will
+    // be executed as a pre-hook.
+    let safe_creation_builder = safe_infra.factory.create_proxy(
+        safe_infra.singleton.address(),
+        ethcontract::Bytes(
+            safe_infra
+                .singleton
+                .setup(
+                    vec![trader.address()], // owners
+                    1.into(),               // threshold
+                    H160::default(),        // delegate call
+                    Bytes::default(),       // delegate call bytes
+                    safe_infra.fallback.address(),
+                    H160::default(), // relayer payment token
+                    0.into(),        // relayer payment amount
+                    H160::default(), // relayer address
+                )
+                .tx
+                .data
+                .unwrap()
+                .0,
+        ),
+    );
+    let safe_address = safe_creation_builder.clone().view().call().await.unwrap();
+    safe_creation_builder.clone().send().await.unwrap();
+
+    let safe = Safe::deployed(
+        chain_id,
+        GnosisSafe::at(&web3, safe_address),
+        trader.clone(),
+    );
+
+    let [token] = onchain
+        .deploy_tokens_with_weth_uni_v2_pools(to_wei(100_000), to_wei(100_000))
+        .await;
+    token.mint(safe.address(), to_wei(5)).await;
+    tx!(
+        trader.account(),
+        token.approve(onchain.contracts().allowance, to_wei(5))
+    );
+
+    // Sign transaction transfering 5 token from the safe to the trader
+    // to fund the trade in a pre-hook.
+    let transfer_builder = safe.sign_transaction(
+        token.address(),
+        token
+            .transfer(trader.address(), to_wei(5))
+            .tx
+            .data
+            .unwrap()
+            .0,
+        0.into(),
+    );
+    let transfer = Hook {
+        target: transfer_builder.tx.to.unwrap(),
+        call_data: transfer_builder.tx.data.unwrap().0,
+        gas_limit: 100_000,
+    };
+
+    tracing::info!("Starting services.");
+    let services = Services::new(&onchain).await;
+    services.start_protocol(solver).await;
+
+    let quote = services
+        .submit_quote(&OrderQuoteRequest {
+            from: trader.address(),
+            sell_token: token.address(),
+            buy_token: onchain.contracts().weth.address(),
+            side: OrderQuoteSide::Sell {
+                sell_amount: SellAmount::BeforeFee {
+                    value: NonZeroU256::try_from(to_wei(5)).unwrap(),
+                },
+            },
+            app_data: OrderCreationAppData::Full {
+                full: json!({
+                    "metadata": {
+                        "hooks": {
+                            "pre": [transfer],
+                        },
+                    },
+                })
+                .to_string(),
+            },
+            ..Default::default()
+        })
+        .await
+        .unwrap();
+
+    // quote can be verified although the trader only get the necessary
+    // sell tokens with a pre-hook
+    assert!(quote.verified);
+}

crates/shared/src/price_estimation/trade_verifier.rs

@@ -131,6 +131,11 @@ impl TradeVerifier {
                 self.native_token,
                 verification.receiver,
                 Bytes(settlement.data.unwrap().0),
+                // only if the user did not provide pre-interactions is it safe
+                // to set up the trade's pre-conditions on behalf of the user.
+                // if the user provided pre-interactions it's reasonable to assume
+                // that they will set up all the necessary details for the trade.
+                verification.pre_interactions.is_empty(),
             )
             .tx;