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

Unable to build sp-runtime after cargo update #688

Open
Serial-ATA opened this issue Jan 29, 2025 · 15 comments · Fixed by #689
Open

Unable to build sp-runtime after cargo update #688

Serial-ATA opened this issue Jan 29, 2025 · 15 comments · Fixed by #689

Comments

@Serial-ATA
Copy link

sp-runtime depends on parity-scale-codec 3.6.12 (https://crates.io/crates/sp-runtime/40.1.0/dependencies), and pinning it to that version makes it possible to build again.

Running cargo update bumps the version to 3.7.2, which causes the following:

  error[E0277]: the trait bound `generic::header::Header<Number, Hash>: Encode` is not satisfied
      --> <path>/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sp-runtime-39.0.5/src/generic/header.rs:75:39
       |
  75   | impl<Number, Hash> traits::Header for Header<Number, Hash>
       |                                       ^^^^^^^^^^^^^^^^^^^^ the trait `Encode` is not implemented for `generic::header::Header<Number, Hash>`
       |
       = note: required for `generic::header::Header<Number, Hash>` to implement `Codec`
  note: required by a bound in `traits::Header`
      --> <path>/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sp-runtime-39.0.5/src/traits.rs:1199:24
       |
  1198 | pub trait Header:
       |           ------ required by a bound in this trait
  1199 |     Clone + Send + Sync + Codec + Eq + MaybeSerialize + Debug + TypeInfo + 'static
       |                           ^^^^^ required by this bound in `Header`
  help: consider extending the `where` clause, but there might be an alternative better way to express this requirement
       |
  78   |     Hash: HashT, generic::header::Header<Number, Hash>: Encode
       |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  error[E0277]: the trait bound `generic::header::Header<Number, Hash>: Encode` is not satisfied
     --> <path>/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sp-runtime-39.0.5/src/generic/header.rs:148:17
      |
  148 |         Hash::hash_of(self)
      |         ------------- ^^^^ the trait `Encode` is not implemented for `generic::header::Header<Number, Hash>`
      |         |
      |         required by a bound introduced by this call
      |
  note: required by a bound in `traits::Hash::hash_of`
     --> <path>/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/sp-runtime-39.0.5/src/traits.rs:973:16
      |
  973 |     fn hash_of<S: Encode>(s: &S) -> Self::Output {
      |                   ^^^^^^ required by this bound in `Hash::hash_of`
  help: consider extending the `where` clause, but there might be an alternative better way to express this requirement
      |
  143 |     Hash: HashT, generic::header::Header<Number, Hash>: Encode
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  For more information about this error, try `rustc --explain E0277`.
  error: could not compile `sp-runtime` (lib) due to 2 previous errors
@bkchr
Copy link
Member

bkchr commented Jan 29, 2025

@Serial-ATA do you have a link to the repo? So, we can reproduce this.

@Serial-ATA
Copy link
Author

@bkchr https://github.com/tangle-network/gadget

Specifically, the package in blueprints/incredible-squaring. The workspace Cargo.lock still has parity-scale-codec 3.6.12, so you should be able to build before cargo update.

@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

 1031  git clone https://github.com/tangle-network/gadget
 1032  cd gadget/
 1033  cd blueprints/incredible-squaring
 1036  cargo update -p parity-scale-codec
 1037  cargo check

This returns success to me

EDIT: but running cargo update and not cargo update -p parity-scale-codec, now I can reproduce.

@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

My guess is that one PR has added where Hash: Encode bound in the derive macro, or at least some unwanted bound

EDIT: my new guess is that it is actually the compact bound that is wrong.

@xlc
Copy link

xlc commented Jan 30, 2025

we having the same issue here https://github.com/open-web3-stack/open-runtime-module-library/actions/runs/13042465350/job/36387082114
force to use 3.6.12 workarounds it

@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

I think this is the fix: #689

@serban300
Copy link
Contributor

Thank you for the report ! Taking a look.

@serban300
Copy link
Contributor

Yes, #689 should fix this. Yanked 3.7.2 and will publish 3.7.3 after the fix is merged

@serban300 serban300 self-assigned this Jan 30, 2025
@Shr1ftyy
Copy link

Image

@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

Image

what do you mean, failing at compilation because of a bounds issue is not a CVE.

@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

3.7.3 published

@serban300
Copy link
Contributor

@Serial-ATA @xlc please can you check if 3.7.3 fixed the issue ?

@serban300 serban300 removed their assignment Jan 30, 2025
@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

Sorry there might still be some issue in regards to bounds: #691

@xlc
Copy link

xlc commented Jan 30, 2025

open-web3-stack/open-runtime-module-library#1020 CI still failing

@gui1117 gui1117 reopened this Jan 30, 2025
@gui1117
Copy link
Contributor

gui1117 commented Jan 30, 2025

The revert for 508 and 512 wasn't complete. Then it compiled due to the change in 616:

So the PR 508 make use of compact type for max encoded len,
Then PR 512 also improve on the code.
Then PR 616 adds some bound for compact type.
Then the revert of 508 and 512 doesn't actually completely revert, and still make use of the compact type for max encoded len.

So now staking doesn't compile because Compact<Balance>: MaxEncodedLen is not bound.

solution:

  • revert 508 completly:
diff --git a/derive/src/max_encoded_len.rs b/derive/src/max_encoded_len.rs
index 8a592e1..d773096 100644
--- a/derive/src/max_encoded_len.rs
+++ b/derive/src/max_encoded_len.rs
@@ -85,14 +85,8 @@ fn fields_length_expr(fields: &Fields, crate_path: &syn::Path) -> proc_macro2::T
        // caused the issue.
        let expansion = fields_iter.map(|field| {
                let ty = &field.ty;
-               if utils::is_compact(field) {
-                       quote_spanned! {
-                               ty.span() => .saturating_add(<#crate_path::Compact::<#ty> as #crate_path::MaxEncodedLen>::max_encoded_len())
-                       }
-               } else {
-                       quote_spanned! {
-                               ty.span() => .saturating_add(<#ty as #crate_path::MaxEncodedLen>::max_encoded_len())
-                       }
+               quote_spanned! {
+                       ty.span() => .saturating_add(<#ty as #crate_path::MaxEncodedLen>::max_encoded_len())
                }
        });
        quote! {
  • try to do some fix, but they might introduce some breaking change:
diff --git a/derive/src/max_encoded_len.rs b/derive/src/max_encoded_len.rs
index 8a592e1..f09f6c8 100644
--- a/derive/src/max_encoded_len.rs
+++ b/derive/src/max_encoded_len.rs
@@ -44,7 +44,7 @@ pub fn derive_max_encoded_len(input: proc_macro::TokenStream) -> proc_macro::Tok
                None,
                has_dumb_trait_bound(&input.attrs),
                &crate_path,
-               false,
+               true,
        ) {
                return e.to_compile_error().into();
        }
@@ -87,7 +87,9 @@ fn fields_length_expr(fields: &Fields, crate_path: &syn::Path) -> proc_macro2::T
                let ty = &field.ty;
                if utils::is_compact(field) {
                        quote_spanned! {
-                               ty.span() => .saturating_add(<#crate_path::Compact::<#ty> as #crate_path::MaxEncodedLen>::max_encoded_len())
+                               ty.span() => .saturating_add(
+                                       <<#ty as #crate_path::HasCompact>::Type as #crate_path::MaxEncodedLen>::max_encoded_len()
+                               )
                        }
                } else {
                        quote_spanned! {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants