Skip to content

Conversation

@AlexD10S
Copy link
Member

@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

@AlexD10S AlexD10S force-pushed the alex/bump-subxt-version branch from 76ae46d to 643bb61 Compare August 16, 2024 08:23
@Daanvdplas Daanvdplas requested a review from pgherveou August 22, 2024 10:20
Copy link
Contributor

@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.
Copy link
Contributor

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
Member 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
Member

@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
Contributor

@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.

@AlexD10S AlexD10S requested a review from peterwht August 23, 2024 15:10
@smiasojed
Copy link
Contributor

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
@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.

6 participants