-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[Native bridge move] tests for msg encoding and some changes #16216
[Native bridge move] tests for msg encoding and some changes #16216
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
a7588b8
to
95696e3
Compare
0a2e950
to
f592dbb
Compare
95696e3
to
d08934e
Compare
f592dbb
to
ef8a214
Compare
d08934e
to
369f43a
Compare
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.
it looks good to me, can we look at the one comment and reply?
thanks so much
@@ -191,6 +204,10 @@ module bridge::message { | |||
token_type: u8, | |||
amount: u64 | |||
): BridgeMessage { |
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.
Shouldn't we also validate that the route is valid when creating these msgs?
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 just a constructor function for the BridgeMessage struct so not doing any checks here, the message won't be able to get any signature from the validators if the route is invalid anyway, so won't do any harm to the bridge
ef8a214
to
316f23b
Compare
369f43a
to
479bd95
Compare
74e0904
to
c33b058
Compare
c33b058
to
c17b4c8
Compare
## Description Describe the changes or additions included in this PR. ## Test Plan How did you test the new or updated feature? --- If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --------- Co-authored-by: patrick <[email protected]>
c17b4c8
to
4d71b18
Compare
Description
assert_valid_chain_id
and call it in all places where the value could come from unverified user inputTest Plan
How did you test the new or updated feature?
If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section.
Type of Change (Check all that apply)
Release notes