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

Bump Substrate and subxt dependencies #1549

Merged
merged 10 commits into from
Jan 27, 2023
Merged

Bump Substrate and subxt dependencies #1549

merged 10 commits into from
Jan 27, 2023

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Dec 16, 2022

Bumps subxt to v0.26.0. Also updates the Substrate dependencies to the matching
versions used by subxt.

We also need to bump `subxt` to a version that uses the same versions of
our Substrate deps.
Not sure why we're going through this hidden internal module
@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2023

Codecov Report

Merging #1549 (48d7fed) into master (33b04bc) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 48d7fed differs from pull request most recent head 9e44a14. Consider uploading reports for the commit 9e44a14 to get more accurate results

@@            Coverage Diff             @@
##           master    #1549      +/-   ##
==========================================
+ Coverage   70.43%   70.44%   +0.01%     
==========================================
  Files         207      207              
  Lines        6395     6395              
==========================================
+ Hits         4504     4505       +1     
+ Misses       1891     1890       -1     
Impacted Files Coverage Δ
crates/e2e/src/client.rs 36.36% <ø> (ø)
crates/e2e/src/lib.rs 25.00% <ø> (ø)
crates/e2e/src/xts.rs 0.00% <ø> (ø)
crates/metadata/src/layout/mod.rs 80.83% <0.00%> (+0.83%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ascjones
Copy link
Collaborator

subxt new version released: https://crates.io/crates/subxt/0.26.0

@HCastano HCastano marked this pull request as ready for review January 27, 2023 02:11
<C::ExtrinsicParams as ExtrinsicParams<C::Index, C::Hash>>::OtherParams: Default,

C::AccountId: serde::de::DeserializeOwned,
C::AccountId: scale::Codec,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why they removed the Parameter (which is Codec + Clone etc. constraint from type AccountId

Copy link
Contributor Author

@HCastano HCastano Jan 30, 2023

Choose a reason for hiding this comment

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

@jsdw I meant to ping you about this last week, but yeah this shouldn't have been removed imo. If it was it should've also been mentioned in the release notes - I only saw it while looking through the PR diff

Copy link
Collaborator

@ascjones ascjones Jan 30, 2023

Choose a reason for hiding this comment

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

I spoke with @jsdw about this and the outcome was that all bounds which are not required within subxt itself are removed. And that if we want global bounds we should use our own local trait.

paritytech/subxt#804

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM!

struct RpcInstantiateRequest<C: subxt::Config, E: Environment> {
struct RpcInstantiateRequest<C: subxt::Config, E: Environment>
where
C::AccountId: scale::Codec,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I reckon this bound on the struct is not necessary

@ascjones ascjones merged commit 7a04cf7 into master Jan 27, 2023
@ascjones ascjones deleted the hc-bump-substrate-deps branch January 27, 2023 10:13
@ascjones ascjones mentioned this pull request Feb 1, 2023
@ascjones ascjones mentioned this pull request Feb 15, 2023
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.

3 participants