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

chore: bump subxt and subxt-signer versions #1722

Merged
merged 9 commits into from
Aug 26, 2024
Merged

Conversation

AlexD10S
Copy link
Contributor

@AlexD10S AlexD10S commented Aug 7, 2024

This PR upgrades the subxt and subxt-signer library from version 0.35.3 to 0.37.0.
This upgrade is necessary because version 0.37 of subxt includes support for the signed extension CheckMetadataHash.

This solves an issue when uploading a smart contract on certain chains.
How to Replicate the Issue:

// Generate a contracts parachain
pop new parachain contracts-parachain --template contracts
cd contracts-parachain
// Run it
pop up parachain -f ./network.toml
// Generate a new smart contract
cd .. & pop new contract
// Try to upload it
cargo contract upload --suri //Alice --url ws://127.0.0.1:57869 -x

This will throw the error:

ERROR: Extrinsic params error: The chain expects a signed extension with the name CheckMetadataHash, but we did not provide one

This issue is resolved with this upgrade.

Note
In addition to bumping the subxt version, some other changes were made:

  • Remove the file metadata_v11.scale and its test. The test was failing here. Based on what I read in an issue opened in the subxt repo link, it seems the only way to fix it is to regenerated the metadata files used in the tests after the upgrade.
    However, using the latest subxt-cli to regenerate the file for V11 was showing an error:
The node can only return version 14 metadata using the legacy API, but you've requested a different version.

The reason for this error is that subxt doesn't support anything before V14. Since cargo-contract relies heavily on subxt, the best approach seems to be supporting only from V14 onwards.

  • The CI was failing during the clippy checks, so I had to add some code to fix that. To make it easier for the reviewer, all changes related to this issue can be seen in this commit: 37cfd93

Copy link

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +32 to +33
//! internal storage structure, we parse the file and overwrite those paths relative to
//! the host machine.

Choose a reason for hiding this comment

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

This seams incorrect format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CI was failing because of this issue. You can run it locally with:

cargo +nightly  clippy --message-format=json --profile debug-ci --all-features --all-targets -- -D warnings

Copy link
Contributor

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

contract-build seems to be depending on a yanked version of zip crate.
See: https://crates.io/crates/zip/versions

Maybe we could bump it here as well

Copy link

@peterwht peterwht left a comment

Choose a reason for hiding this comment

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

LGTM. I think we just need a small change to the changelog. See below.

CHANGELOG.md Show resolved Hide resolved
@AlexD10S AlexD10S requested a review from peterwht August 23, 2024 15:10
@smiasojed
Copy link
Collaborator

Maybe we should add some information to the README that c-c is compatible starting from version 14. Previously, we supported very old versions—I tested it starting from version 8.

@Daanvdplas Daanvdplas merged commit 29af9ee into master Aug 26, 2024
18 checks passed
@Daanvdplas Daanvdplas deleted the alex/bump-subxt-version branch August 26, 2024 08:27
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