-
Notifications
You must be signed in to change notification settings - Fork 375
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
Remote & Cross-VM Ethereum transaction support #967
Conversation
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.
Overall looks good to me except for few comments,
- Since we are using global nonce for every tx from every origins (xcm & xvm), is there any implications or limitation due to this design? If there is, we can add that to our builder docs.
Also would it be better to use separate nonce for XCM and XVM? - We should not allow for arbitary size inputs via XCM since we can receive very large inputs via XCM which we have no control over.
- This design allows CREATE operation (deploy evm contract) via XCM context that can inflate proof size since we are allowing unbounded input, and even if we make input bounded, it will be useless for contracts larger than allowed size.
If possible we should not allow this, IMO cons far outweigh pros - How will it impact evm tracing? I'm not sure if tx coming from XCM/XVM will have tracing data associated with them.
Once PR #970 is merged, it would be good to have some e2e tests for this.
I don't see any limitations so far. Using separate nonce for XCM and XVM will make it possible to have tx hash collision.
Yes, I was also planning to set the
Good point. Will update.
I'm not sure about EVM tracing TBH. If it didn't work, we could research the solution, which I think is not in the scope of this PR.
E2e tests will be added after Shibuya integration. |
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.
Looks good!
Just some minor comments.
Minimum allowed line rate is |
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
Pull Request Summary
pallet-ethereum-checked
to handle Remote & Cross-VM Ethereum transaction.Check list