-
Notifications
You must be signed in to change notification settings - Fork 105
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
Use hashed description #884
Conversation
2c4d357
to
a2f205c
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #884 +/- ##
==========================================
- Coverage 69.80% 69.76% -0.04%
==========================================
Files 47 47
Lines 1679 1677 -2
Branches 58 58
==========================================
- Hits 1172 1170 -2
Misses 489 489
Partials 18 18
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
2, | ||
( | ||
GlobalConsensus(Ethereum { chain_id }), | ||
AccountKey20 { network: None, key: *origin.as_fixed_bytes() }, |
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.
When would we set a network id in an account key?
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.
Guess already set in GlobalConsensus(network)
above so can just ignore here?
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.
An example of where we "could" use network id is if we are specifying an ERC20 token contract on an EVM based parachain and potentially transferring it over the bridge and we would want a way to specify its Network as Kusama
so we could handle it specially in some way. But I say "could" here because it's hypothetical.
If you look at some places in the source code, we often ignore network id. Such as here.
There are also some places where it makes sense to validate it if present like here.
In this case yrong is correct in that it is set in GlobalConsensus so we can set it as None on the key itself.
Co-authored-by: David Dunn <26876072+doubledup@users.noreply.github.com>
c3fcb2e
to
41a49de
Compare
This reverts commit d26678d.
https://linear.app/snowfork/issue/SNO-529
Cumulus companion: Snowfork/cumulus#44