Investigate even more erc20 validity checks using batch calls

[vkgnosis]

When I made https://github.com/gnosis/gp-v2-services/pull/469 I thought I was only able to simulate (call) a single transaction which limits what can be checked. For example I thought I wouldn't be able to simulate the user first approving the token and then the allowance manager transfering. Felix suggested that using batch calls this might be possible so I'm using this issue as a reminder to myself to investigate.

[vkgnosis]

Tried this but didn't work:

    #[tokio::test]
    async fn foo() {
        let node = "https://staging-openethereum.mainnet.gnosisdev.com";
        crate::tracing::initialize("debug");
        let transport = LoggingTransport::new(web3::transports::Http::new(node).unwrap());
        let web3 = web3::Web3::new(transport.clone());
        let token = contracts::ERC20::at(
            &web3,
            "0x6b175474e89094c44da98b954eedeac495271d0f"
                .parse()
                .unwrap(),
        );
        let acc1: H160 = "0x13aec50f5d3c011cd3fed44e2a30c515bd8a5a06"
            .parse()
            .unwrap();
        let acc2: H160 = "0x7D3f034b9B5A1F3f4208aadAA609a5c86879B1a1"
            .parse()
            .unwrap();
        let amount = primitive_types::U256::from(10u128.pow(18));
        let mut batch = ethcontract::batch::CallBatch::new(transport);
        let fut1 = token
            .transfer(acc2, amount)
            .view()
            .from(acc1)
            .batch_call(&mut batch);
        let fut2 = token.balance_of(acc2).from(acc2).batch_call(&mut batch);
        let fut3 = token
            .transfer(acc1, amount)
            .view()
            .from(acc2)
            .batch_call(&mut batch);
        batch.execute_all(100).await;
        dbg!(fut1.await.unwrap());
        dbg!(fut2.await.unwrap());
        dbg!(fut3.await.unwrap());

Outputs

[shared/src/current_block.rs:207] fut1.await = Ok(
    true,
)
[shared/src/current_block.rs:208] fut2.await = Ok(
    0,
)
[shared/src/current_block.rs:209] fut3.await = Err(
    MethodError {
        signature: "transfer(address,uint256):(bool)",
        inner: Revert(
            Some(
                "Dai/insufficient-balance",
            ),
        ),
    },
)

So the first transactions succeeds but we see the balance didn't get updated. Also tried adding block(BlockId::Number(BlockNumber::Pending)) to every transaction but didn't help. Am I doing something wrong?

[fleupold]

Right, this is not possible because the first transaction doesn't actually update any state (it's a read). It might work if it's an actual transaction that is being sent (but in that case the batch might not return until the tx is mined and this is not supported by ethcontracts).

One other option would be to use something like https://github.com/makerdao/multicall which might allow simulating a transaction followed by a static call (but I'm not sure if this fits our use case, as the msg.sender would be the Multicall contract).

[nlordell]

To summarize what we talked about in the weekly sync:

The idea would be to add a method to simulate a transfer with a balance check to make sure the ERC20 behaves in the "standard" way (i.e. non-deflationary and does not take fees from the transferred amount). This method could be added to a storage accessible reader contract as to not be part of the actual settlement contract. Here is the existing reader contract: https://github.com/gnosis/gp-v2-contracts/blob/main/src/contracts/reader/SettlementStorageReader.sol

Basically, something like (in pseudo-code):

function checkTokenTransfer(interactions, token, expectedTransfer) {
  address randomAddress = address(uint160(uint256(block.hash)));
  uint256 startingBalance = token.balanceOf(randomAddress);
  foreach (interaction of interactions) {
    // NOTE: allow interactions to the allowance manager
    interaction.execute()
  }
  token.transfer(randomAddress, expectedTransfer);
  uint256 finalBalance = token.balanceOf(randomAddress);
  require(finalBalance - startingBalance == expectedTransfer);
}

We use interactions so that we can have different ways of transferring the tokens:

• If the order is selling the token we want to test, then we transfer from the user to the settlement contract with the allowance manager
• If the order is a sell order, then we can use whatever is easiest (Uniswap pair for example) to get some of the token into the settlement contract and try transferring it.

[vkgnosis]

Link from Nick: https://openethereum.github.io/JSONRPC-trace-module#trace_callmany

[fleupold]

One more advantage about the checkTokenTransfer would be that it could potentially give us an exact gas estimate of the baseline trade and thus the appropriate minFee (although that would include the overhead of our routing - at the moment min fee is the cost the user would have if they traded with Uniswap directly).

[nlordell]

Closing as we now have a trace_callMany-based bad token detection.