Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Snowbridge: Support bridging native ETH #6855
Snowbridge: Support bridging native ETH #6855
Changes from 14 commits
41bed52
dd51d23
e5a6eb7
0d6fbd4
f15c012
d4d25d2
7c838e5
a98d3e7
f174719
e1a6f18
5b50f11
e272391
005e62c
d499776
8799c23
d7b33dc
17d98b7
c34c94b
60f65a9
124e313
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 could be a const fn.
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.
Add a comment here explaining that a token address of
0x00...
is equivalent to native ether.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.
Address in 17d98b7.
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.
DQ: Don't we need to check here that
network == Ethereum(chain_id: 1)
?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.
I saw that too and checked if it needs validation, it doesn't:
convert_token_address()
is internal fn called byconvert_register_token()
orconvert_send_token()
which are converting Ethereum commands into XCM programs, somewhere above on the callstack the source was already validated as being Ethereum and the only thing that varies here is the Ethereumchain_id
(testnet or prod).Maybe for clarity and hardening this helper function should take only
chain_id
and hardcodeEthereum
as network: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.
Hey thanks for setting the label. So the chain_id comes directly from the message encoded on the solidity side, where it is taken from the on-chain variable
block.chainid
polkadot-sdk/bridges/snowbridge/primitives/router/src/inbound/mod.rs
Line 304 in 1059be7
polkadot-sdk/bridges/snowbridge/primitives/router/src/inbound/mod.rs
Line 194 in 1059be7
polkadot-sdk/bridges/snowbridge/pallets/inbound-queue/src/lib.rs
Lines 279 to 280 in 1059be7
So it does not need to be validated explicitly. It will be validated implicitly by message verification through the light client.