-
Notifications
You must be signed in to change notification settings - Fork 131
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
Update Substrate Dependency #566
Conversation
For the build issues you are still seeing, you might want to have a look at Streamline frame_system weight parametrization especially the changes to |
The CI actually gets further than I do - so I haven't gotten around to actually updating our code to match the changes in Substrate yet 😅 |
I think we need to wait until paritytech/substrate#7696 is merged before this compiles. |
@HCastano paritytech/substrate#7696 is merged now. If you can, please pin to libp2p |
This allows us to check limits for extrinsics from other parts of the codebase without pulling in the entire chain runtime.
Since this compiles I think it's ready to be reviewed. However, it's worth noting that to get this to the latest version of Substrate (or at least to one which uses the new Some crates which require updates include:
I think we should try and bump Substrate again in a week or two once these dependencies are updated. |
@@ -155,6 +160,44 @@ impl sp_runtime::traits::Convert<sp_core::H256, AccountId> for AccountIdConverte | |||
hash.to_fixed_bytes().into() | |||
} | |||
} | |||
|
|||
/// Get a struct which defines the weight limits and values used during extrinsic execution. | |||
pub fn runtime_block_weights() -> frame_system::limits::BlockWeights { |
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 moved the builder call (and the length builder below) here because I didn't want other parts of the codebase to be pulling in the runtime code as a dependency, but this isn't super nice. Two questions here:
- Is it okay for me to be pulling stuff in from
frame_system
here? - Is there any way for me to get a shared instance of this that I can use in the runtime but also in the relay and message lane crates?
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.
@tomusdrw Hey can you take a look at this? I feel like you have opinions about this 😛
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 looks good.
- Yes, it actually has less dependencies than
frame-support
anyway. - Long term relay should rely on the metadata instead of the primitives directly.
primitives/millau/src/lib.rs
Outdated
/// Maximum weight of single Millau block. | ||
/// | ||
/// This represents two seconds of compute assuming a target block time of six seconds. | ||
pub const MAXIMUM_BLOCK_WEIGHT: Weight = 2 * WEIGHT_PER_SECOND; |
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.
The value is now the same as in Rialto. I'd prefer to keep different weight/size limits. The idea was to:
- change
Millau
to chain that operates differently fromRialto
(i.e. it has different crypto/hash algorithms, block times, consensus algorithms). This is WIP, but every time I introduce/change some constants, they're a bit different fromMillau
. The direction is to makeMillau
lighter+faster thanRialto
; - that isn't something I work on (and probably there are better ways to do that), but these constant in different chains may be used to compare messages bandwidth under different limits.
So unless you have any arguments against keeping previous constants (or at least having different values in different chains), let's change that.
@@ -155,6 +160,44 @@ impl sp_runtime::traits::Convert<sp_core::H256, AccountId> for AccountIdConverte | |||
hash.to_fixed_bytes().into() | |||
} | |||
} | |||
|
|||
/// Get a struct which defines the weight limits and values used during extrinsic execution. | |||
pub fn runtime_block_weights() -> frame_system::limits::BlockWeights { |
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 looks good.
- Yes, it actually has less dependencies than
frame-support
anyway. - Long term relay should rely on the metadata instead of the primitives directly.
* Remove Filter and use Contains instead * Fixes * Remove patch * Bump Polkadot * bump
* Update `sp-io` dependency * Rename Trait to Config * RustFmt * Bump `sp-io` again * Use new frame_system weight types in Rialto and Millau runtimes * Update test Runtimes to use new weight types * Bump `sp-io` again * Update to not-the latest first. * Update benchmarks. * Another Trai. * Move new weight types into runtime primitive crates This allows us to check limits for extrinsics from other parts of the codebase without pulling in the entire chain runtime. * Remove leftover comments * Move new functions to a better location * Small formatting fixes * Add actual documentation to new weight config types * Decrease maximum block weight of Millau chain * Decreease maximum block length of Millau chain Co-authored-by: Tomasz Drwięga <[email protected]>
* Update `sp-io` dependency * Rename Trait to Config * RustFmt * Bump `sp-io` again * Use new frame_system weight types in Rialto and Millau runtimes * Update test Runtimes to use new weight types * Bump `sp-io` again * Update to not-the latest first. * Update benchmarks. * Another Trai. * Move new weight types into runtime primitive crates This allows us to check limits for extrinsics from other parts of the codebase without pulling in the entire chain runtime. * Remove leftover comments * Move new functions to a better location * Small formatting fixes * Add actual documentation to new weight config types * Decrease maximum block weight of Millau chain * Decreease maximum block length of Millau chain Co-authored-by: Tomasz Drwięga <[email protected]>
This does not update to the latest Substrate just yet, but it is a newest version than what we've been using. This is because there are issues updating to
libp2p
v0.32.2
(see my comment below) which stem from libp2p/rust-libp2p#1873.For reviewers, there are two main changes here. First is the
Trait
toConfig
rename, and the second is the changes to weight types (see comment by @adoerr). The second changes are mostly introduced in da2aee6, so taking a look at just that commit will reduce a bit of noise introduced by the first set of changes.