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

A0-2730: bump subxt to 0.28 #1335

Merged

Conversation

ggawryal
Copy link
Contributor

@ggawryal ggawryal commented Aug 8, 2023

Description

Bump subxt from 0.25 to 0.28.0 (Changelog).
Subxt is slowly removing the dependency on substrate (making it optional) in order to reduce wasm build size, so there are some breaking changes. Most importantly, a suxbt::utils::AccountId32 type is introduced, which is basically a stripped sp_runtime::AccountId32.

[EDIT 1] We've decided to continue using substrate's AccountId, so in subxt codegen, we had to substitute subxt::utils::AccountId into that type. Unfortunately, this means that we have to wrap it into subxt::utils::Static<>. Higher level code, outside aleph_zero.rs can, and is returning plain unwrapped AccountId. Subxt also introduced suxbt::utils::MultiAddress in place of sp_runtime::MultiAddress, but currently switching to the former for us is not causing any compilation errors, so we should be safe to use it.

Version 0.28 is the highest compatible with our substrate's version (next one uses newer wasmtime).

I've run only unit and e2e tests in aleph-node - please let me know if I need to test something else (ui, manual etc).

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have created new documentation
  • I have bumped aleph-client version if relevant

@ghost ghost added the pending label Aug 8, 2023
@ggawryal ggawryal changed the title A0 2730 bump subxt to 0.28 A0-2730: bump subxt to 0.28 Aug 8, 2023
aleph-client/src/lib.rs Outdated Show resolved Hide resolved
e2e-tests/src/ban.rs Outdated Show resolved Hide resolved
@ghost ghost removed the pending label Aug 9, 2023
@ggawryal ggawryal marked this pull request as ready for review August 11, 2023 14:45
@pmikolajczyk41 pmikolajczyk41 self-requested a review August 14, 2023 10:08
@kostekIV kostekIV self-requested a review August 14, 2023 10:12
Copy link
Contributor

@kostekIV kostekIV left a comment

Choose a reason for hiding this comment

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

There are some problems related to the change of AccountId from substrate one. Pls do:

  1. substitute type for substrate one (you may need to use Static
  2. update readme for codegen cmd to include this
  3. In public API return AccountId not Static

aleph-client/Cargo.toml Show resolved Hide resolved
aleph-client/src/pallets/staking.rs Outdated Show resolved Hide resolved
aleph-client/src/pallets/staking.rs Outdated Show resolved Hide resolved
e2e-tests/src/ban.rs Outdated Show resolved Hide resolved
@ggawryal ggawryal marked this pull request as draft August 16, 2023 12:41
@ggawryal
Copy link
Contributor Author

ggawryal commented Aug 17, 2023

Switched to substrate's AccountId as suggested. There is --subsititute-type flag in codegen which should be used for that purpose, but unfortunately it seems that it cannot override default substitutions (particularly sp_core::crypto::AccountId -> subxt::utils::AccountId32), so I had to use sed. The problem is resolved in subxt 0.29, but to use it we would have to bump substrate by one version.

@ggawryal ggawryal marked this pull request as ready for review August 17, 2023 09:17
aleph-client/src/lib.rs Show resolved Hide resolved
@ggawryal ggawryal added this pull request to the merge queue Aug 17, 2023
@ggawryal ggawryal removed this pull request from the merge queue due to a manual request Aug 17, 2023
@ggawryal ggawryal added this pull request to the merge queue Aug 17, 2023
Merged via the queue into Cardinal-Cryptography:main with commit 9001200 Aug 17, 2023
50 checks passed
@ggawryal ggawryal deleted the a0-2730-bump-subxt-to-0.28 branch August 17, 2023 13:03
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