Skip to content
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

Merged
merged 19 commits into from
Dec 16, 2020
Merged

Update Substrate Dependency #566

merged 19 commits into from
Dec 16, 2020

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Dec 8, 2020

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 to Config 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.

@adoerr
Copy link
Contributor

adoerr commented Dec 9, 2020

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 bin/node-template/runtime/src/lib.rs

@HCastano
Copy link
Contributor Author

HCastano commented Dec 9, 2020

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 bin/node-template/runtime/src/lib.rs

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 😅

@HCastano
Copy link
Contributor Author

HCastano commented Dec 9, 2020

I think we need to wait until paritytech/substrate#7696 is merged before this compiles.

@mxinden
Copy link

mxinden commented Dec 9, 2020

@HCastano paritytech/substrate#7696 is merged now. If you can, please pin to libp2p v0.32.1 as that pins to async-tls v0.10.2.

@HCastano HCastano added A-chores Something that has to be done, as part of regular maintenance B-Substrate <> Substrate dependencies Pull requests that update a dependency file labels Dec 9, 2020
@HCastano
Copy link
Contributor Author

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 libp2p release) requires updates to some upstream crates. In particular we need updates for crates which use v0.17 of the secp256k1 crate since it requires that the cc crate is <= v1.0.41 - making it incompatible with version used in the dependency tree of the new libp2p release.

Some crates which require updates include:

  • ethereum-tx-sign
  • parity-crypto

I think we should try and bump Substrate again in a week or two once these dependencies are updated.

@HCastano HCastano marked this pull request as ready for review December 11, 2020 17:29
@@ -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 {
Copy link
Contributor Author

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:

  1. Is it okay for me to be pulling stuff in from frame_system here?
  2. 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?

Copy link
Contributor Author

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 😛

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

  1. Yes, it actually has less dependencies than frame-support anyway.
  2. Long term relay should rely on the metadata instead of the primitives directly.

/// 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;
Copy link
Contributor

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:

  1. change Millau to chain that operates differently from Rialto (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 from Millau. The direction is to make Millau lighter+faster than Rialto;
  2. 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

  1. Yes, it actually has less dependencies than frame-support anyway.
  2. Long term relay should rely on the metadata instead of the primitives directly.

@tomusdrw tomusdrw merged commit 64dab9c into master Dec 16, 2020
@tomusdrw tomusdrw deleted the hc-update-substrate branch December 16, 2020 09:52
@svyatonik svyatonik mentioned this pull request Dec 16, 2020
svyatonik pushed a commit that referenced this pull request Jul 17, 2023
* Remove Filter and use Contains instead

* Fixes

* Remove patch

* Bump Polkadot

* bump
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Mar 27, 2024
* 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]>
serban300 pushed a commit to serban300/parity-bridges-common that referenced this pull request Apr 8, 2024
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chores Something that has to be done, as part of regular maintenance dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants