Skip to content

Conversation

@gui1117
Copy link
Contributor

@gui1117 gui1117 commented Jan 30, 2025

let's release @serban300

@gui1117
Copy link
Contributor Author

gui1117 commented Jan 30, 2025

Ok I will release

@gui1117 gui1117 merged commit ef8afe3 into master Jan 30, 2025
17 checks passed
@gui1117 gui1117 deleted the gui-new-release branch January 30, 2025 08:54
@gui1117
Copy link
Contributor Author

gui1117 commented Jan 30, 2025

published

@cmichi
Copy link

cmichi commented Jan 30, 2025

Thanks for the fix. But with 3.7.3 some Substrate deps now fail:

error[E0277]: the trait bound `parity_scale_codec::Compact<Balance>: MaxEncodedLen` is not satisfied
   --> substrate/primitives/staking/src/lib.rs:451:2
    |
451 |     MaxEncodedLen,
    |     ^^^^^^^^^^^^^ the trait `MaxEncodedLen` is not implemented for `parity_scale_codec::Compact<Balance>`

@gui1117
Copy link
Contributor Author

gui1117 commented Jan 30, 2025

Yes I guess we can fix it with:

diff --git a/derive/src/max_encoded_len.rs b/derive/src/max_encoded_len.rs
index 8a592e1..2b60963 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();
        }

@cmichi
Copy link

cmichi commented Jan 30, 2025

@gui1117 Okay, are you yanking + doing a follow-up release?

@serban300
Copy link
Contributor

@cmichi yes, I yanked 3.7.3 for the moment. Will fix the issue and do a follow-up release after

@cmichi
Copy link

cmichi commented Jan 30, 2025

Thank you!

To me, this signals room for improvement in the testing setup before a release:

image

For ink!, this always creates unnecessary work. Users get compilation errors for their smart contracts and start reporting them in our repos/chats.

IIRC all these issues would have been caught if polkadot-sdk would have been compiled with the new crates before a release. This could even be added as a canary check to the CI in this repo, by cloning polkadot-sdk and appending a [patch] to its workspace Cargo.toml.

@serban300
Copy link
Contributor

Yes, agree. Sorry about that. I actually did compile polkadot-sdk with the new crate and it always worked. I guess I did something wrong. Probably it was because of SKIP_WASM_BUILD=1. Now I did unset SKIP_WASM_BUILD and I can reproduce the issue

@gui1117
Copy link
Contributor Author

gui1117 commented Jan 30, 2025

Yes, agree. Sorry about that. I actually did compile polkadot-sdk with the new crate and it always worked. I guess I did something wrong. Probably it was because of SKIP_WASM_BUILD=1. Now I did unset SKIP_WASM_BUILD and I can reproduce the issue

My guess is that only parity-scale-codec was updated, but still using parity-scale-codec-derive version 3.6.x

It happened to me.

Also I think we can make parity-scale-codec depends on parity-scale-codec-derive of the same version maybe? Not sure.

@cmichi
Copy link

cmichi commented Jan 30, 2025

My guess is that only parity-scale-codec was updated, but still using parity-scale-codec-derive version 3.6.x

It happened to me.

I noticed the same thing yesterday. @serban300 I also believe this is why your compilation still worked. The parity-scale-codec-derive shows up only in the Cargo.lock, so you'd need to make sure that you also pull in the updated version of that crate when doing a test run.

@ggwpez
Copy link
Member

ggwpez commented Jan 30, 2025

We could try to move this repos release responsibility formally to the Release team once it is staffed up. We should for sure have some CI that checks the compilation against the SDK before we publish it.

@serban300
Copy link
Contributor

@cmichi all the backwards incompatibility issues should have been fixed and we're ready to release v3.7.4. I compiled the repos mentioned here with the upcoming release (you can find details in comment: #701 (comment))

Also it polkadot-sdk was compiled with it: paritytech/polkadot-sdk#7417

Do you have any other repo we could try to compile with it ?

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 this pull request may close these issues.

5 participants