-
Notifications
You must be signed in to change notification settings - Fork 71
feat: add multicall to simulator #402
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
Conversation
|
🤖 Bytecode changes detected! EIP-712 domain versions have been automatically updated for: Simulator |
src/Simulator.sol
Outdated
| /// If parsing fails (gasUsed == 0), this function stores the orchestrator's error in memory | ||
| /// so the caller can bubble it up using bubbleUpMulticall3Error. | ||
| /// @param multicall3 The multicall3 contract address | ||
| /// @param preCalls Array of Call3 structs representing calls to execute before the orchestrator call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: preCalls can be a bit confusing, could we just add a smol note that this is different than intent.precalls
| IMulticall3.Result memory lastResult = results[results.length - 1]; | ||
| lastReturnData = lastResult.returnData; | ||
|
|
||
| // Parse gasUsed from the orchestrator's return data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only returning the gasUsed by the intent right? would be interested in the overal gasUsed of the whole multicall including the intent and precalls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually both. used by intent (so we can cap its execution) and by intent+precalls so we can price it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, i think the current gasUsed is the value to cap, we need another totalGas or something with the preCall
74a27d3 to
161bd4c
Compare
|
🤖 Bytecode changes detected! EIP-712 domain versions have been automatically updated for: Simulator |
No description provided.