superfluid-finance / protocol-monorepo

Superfluid Protocol Monorepo: the specification, implementations, peripherals and development kits.
https://www.superfluid.finance
Other
875 stars 240 forks source link

[ETHEREUM-CONTRACTS] Investigate 64/64 rule failure with try/catch #1898

Closed d10r closed 4 months ago

d10r commented 8 months ago

What & Why

When working on #1879, 2 mitigations other than removing the try/catch were attempted

  1. Change of order of instructions
    uint256 gasLeftBefore = gasleft();

    originally was followed by a call

    address constantOutflowNFTAddress = _canCallNFTHook(flowVars.token);

    and by an if statement.

So the assumption was that taking the snapshot as last instruction before the try would solve the problem. But this turned out to not be the case, the test still failed.

  1. try/catch may add overhead making it not suited for use with the 63/64 rule

Thus I locally tried to replace it with low-level calls, following the implementation done for SuperApp callbacks (see here). To my surprise, it still failed. However I was focused on getting the task done (vs understanding what exactly was happening), which in case of the FlowNFTs could be achieved by simply not allowing reverts. So what I was seeing could have been a red herring.

However we want to keep track of this and eventually figure out what exactly was the issue here, so we can avoid it causing troubles in other occasions.

AC