-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix(builder): handling of errors when deploying contracts (create2) #839
base: main
Are you sure you want to change the base?
Conversation
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit d46835f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
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.
LGTM! A thing to note is that when executing the transaction after failure without using CREATE2 to be available to parse the error, there's a slight chance that the execution doesn't fail on this case, depending the users' implementation. And the user would end up deploying a contract without using CREATE2, and not sure what would happen with the build.
There's a non chance of this, but could happen.
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.
do we have a test verifying that this decoding functionality works?
is it possible that, rather than utilizing low level error decoding capabilities, we detect any error that cannon has awareness of, even in other contracts, so far?
ideally we use the CannonTraceError
, which prints out error and stack trace using data from all contracts in the project
The problem right now is that the "error" may be unknown if the constructor is executed and we havent imported the contract ABI yet.
…on into fix/handle-create2-revert
Yes, I have tested it, and it worked as expected. The main issue is that we can't decode the error if the transaction reverts because the Arachnid deployer doesn't return the underlying error. The Arachnid deployer returns |
while it is true that arachnid does not return the ultimate error, if we print out the txn trace, the error should be visible in the second layer of the trace, right? |
Based on some tests I conducted, there is no second layer of the trace. It's traced until the '0x'. That's why I implemented this workaround to get the underlying revert reason. Do you have another idea in mind? Here is an example of how they look: Without this code:
With this code:
|
This PR aims to show the error when a contract reverts during deployment. It will handle deployments with CREATE2.
Closes https://linear.app/usecannon/issue/CAN-204/verify-contract-create-transaction-before-sending-to-network