Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
1 task
d10r opened this issue Mar 19, 2024 · 0 comments · Fixed by #1979
Closed
1 task

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

d10r opened this issue Mar 19, 2024 · 0 comments · Fixed by #1979
Labels
Project: PROTOCOL-EVMv1 Superfluid protocol EVM v1 implementation in Solidity Tag: Quality Related to the quality topic Team: Protocol Protocol Core, Sentinel, Peripherals, Protocol Infrastructure Tools & DevOps

Comments

@d10r
Copy link
Collaborator

d10r commented Mar 19, 2024

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

@d10r d10r added Project: PROTOCOL-EVMv1 Superfluid protocol EVM v1 implementation in Solidity Tag: Quality Related to the quality topic Team: Protocol Protocol Core, Sentinel, Peripherals, Protocol Infrastructure Tools & DevOps labels Mar 19, 2024
@hellwolf hellwolf linked a pull request Jul 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Project: PROTOCOL-EVMv1 Superfluid protocol EVM v1 implementation in Solidity Tag: Quality Related to the quality topic Team: Protocol Protocol Core, Sentinel, Peripherals, Protocol Infrastructure Tools & DevOps
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant