-
Notifications
You must be signed in to change notification settings - Fork 258
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
Subxt Metadata: #[no_std]
compatibility
#1401
Changes from 12 commits
a262766
8aa4bec
55cfb23
1b4e371
d679e95
7954815
fbee16d
7aa6989
addc4c8
87200b4
57743c3
408f81e
f41f090
ce17d82
5d6e0d5
cb0d35d
d447f7c
9c8fee0
1aebd91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -419,3 +419,32 @@ jobs: | |
|
||
- if: "failure()" | ||
uses: "andymckay/cancel-action@b9280e3f8986d7a8e91c7462efc0fa318010c8b1" # v0.3 | ||
|
||
no-std-tests: | ||
name: "Test (no_std)" | ||
runs-on: ubuntu-latest | ||
needs: [machete, docs] | ||
timeout-minutes: 30 | ||
steps: | ||
- name: Checkout sources | ||
uses: actions/checkout@v4 | ||
|
||
# Note: needs nighly toolchain because otherwise we cannot define custom lang-items. | ||
- name: Install Rust nightly toolchain | ||
uses: actions-rs/toolchain@v1 | ||
with: | ||
profile: minimal | ||
toolchain: nightly | ||
override: true | ||
|
||
- name: Rust Cache | ||
uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2.7.3 | ||
|
||
# Note: in `no_std` no real tests are possible, we just run a binary with some tests in it which panic upon failure. | ||
- name: Run no_std tests | ||
run: | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, we can use a single line syntax |
||
cargo run | ||
working-directory: testing/no-std-tests | ||
|
||
- if: "failure()" | ||
uses: "andymckay/cancel-action@b9280e3f8986d7a8e91c7462efc0fa318010c8b1" # v0.3 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ members = [ | |
# We exclude any crates that would depend on non mutually | ||
# exclusive feature flags and thus can't compile with the | ||
# workspace: | ||
exclude = ["testing/wasm-rpc-tests", "testing/wasm-lightclient-tests", "signer/wasm-tests", "examples/wasm-example", "examples/parachain-example"] | ||
exclude = ["testing/no-std-tests", "testing/wasm-rpc-tests", "testing/wasm-lightclient-tests", "signer/wasm-tests", "examples/wasm-example", "examples/parachain-example"] | ||
resolver = "2" | ||
|
||
[workspace.package] | ||
|
@@ -57,16 +57,19 @@ base58 = { version = "0.2.0" } | |
bitvec = { version = "1", default-features = false } | ||
blake2 = { version = "0.10.6", default-features = false } | ||
clap = { version = "4.4.18", features = ["derive", "cargo"] } | ||
cfg-if = "1.0.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we using this? |
||
criterion = "0.4" | ||
codec = { package = "parity-scale-codec", version = "3.4.0", default-features = false } | ||
codec = { package = "parity-scale-codec", version = "3.6.9", default-features = false } | ||
color-eyre = "0.6.1" | ||
console_error_panic_hook = "0.1.7" | ||
darling = "0.20.3" | ||
derivative = "2.2.0" | ||
derive_more = "0.99.17" | ||
either = "1.9.0" | ||
frame-metadata = { version = "16.0.0", default-features = false, features = ["current", "std"] } | ||
frame-metadata = { version = "16.0.0", default-features = false } | ||
futures = { version = "0.3.30", default-features = false, features = ["std"] } | ||
getrandom = { version = "0.2", default-features = false } | ||
hashbrown = "0.14.3" | ||
hex = "0.4.3" | ||
heck = "0.4.1" | ||
impl-serde = { version = "0.4.0" } | ||
|
@@ -78,7 +81,7 @@ proc-macro-error = "1.0.4" | |
proc-macro2 = "1.0.78" | ||
quote = "1.0.35" | ||
regex = "1.10.3" | ||
scale-info = "2.10.0" | ||
scale-info = { version = "2.10.0", default-features = false } | ||
scale-value = "0.13.0" | ||
scale-bits = "0.4.0" | ||
scale-decode = "0.10.0" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,31 +2,37 @@ | |
// This file is dual-licensed as Apache-2.0 or GPL-3.0. | ||
// see LICENSE for license details. | ||
|
||
use alloc::string::String; | ||
use derive_more::Display; | ||
|
||
mod v14; | ||
mod v15; | ||
|
||
/// An error emitted if something goes wrong converting [`frame_metadata`] | ||
/// types into [`crate::Metadata`]. | ||
#[derive(Debug, thiserror::Error, PartialEq, Eq)] | ||
#[derive(Debug, Display, PartialEq, Eq)] | ||
#[non_exhaustive] | ||
pub enum TryFromError { | ||
/// Type missing from type registry | ||
#[error("Type id {0} is expected but not found in the type registry")] | ||
#[display(fmt = "Type id {_0} is expected but not found in the type registry")] | ||
TypeNotFound(u32), | ||
/// Type was not a variant/enum type | ||
#[error("Type {0} was not a variant/enum type, but is expected to be one")] | ||
#[display(fmt = "Type {_0} was not a variant/enum type, but is expected to be one")] | ||
VariantExpected(u32), | ||
/// An unsupported metadata version was provided. | ||
#[error("Cannot convert v{0} metadata into Metadata type")] | ||
#[display(fmt = "Cannot convert v{_0} metadata into Metadata type")] | ||
UnsupportedMetadataVersion(u32), | ||
/// Type name missing from type registry | ||
#[error("Type name {0} is expected but not found in the type registry")] | ||
#[display(fmt = "Type name {_0} is expected but not found in the type registry")] | ||
TypeNameNotFound(String), | ||
/// Invalid type path. | ||
#[error("Type has an invalid path {0}")] | ||
#[display(fmt = "Type has an invalid path {_0}")] | ||
InvalidTypePath(String), | ||
} | ||
|
||
#[cfg(feature = "std")] | ||
impl std::error::Error for TryFromError {} | ||
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Annoyingly, this is, I think, the only reason that you actually need an "std" feature flag at all for this crate (same is true for some of the scale-* libraries too). Can't wait for Error trait in no-std; then a bunch of crates can just be no-std by default without any feature flag! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yupp, the new error trait looked quite weird in Rust but indeed would be awesome with core::Error :) |
||
|
||
impl From<crate::Metadata> for frame_metadata::RuntimeMetadataPrefixed { | ||
fn from(value: crate::Metadata) -> Self { | ||
let m: frame_metadata::v15::RuntimeMetadataV15 = value.into(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,15 @@ | |
// This file is dual-licensed as Apache-2.0 or GPL-3.0. | ||
// see LICENSE for license details. | ||
|
||
use std::collections::HashMap; | ||
|
||
use super::TryFromError; | ||
use crate::Metadata; | ||
use alloc::borrow::ToOwned; | ||
use alloc::string::String; | ||
use alloc::vec; | ||
use alloc::vec::Vec; | ||
use core::fmt::Write; | ||
use frame_metadata::{v14, v15}; | ||
use hashbrown::HashMap; | ||
use scale_info::TypeDef; | ||
|
||
impl TryFrom<v14::RuntimeMetadataV14> for Metadata { | ||
|
@@ -31,27 +35,27 @@ fn v15_to_v14(mut metadata: v15::RuntimeMetadataV15) -> v14::RuntimeMetadataV14 | |
let extrinsic_type = scale_info::Type { | ||
path: scale_info::Path { | ||
segments: vec![ | ||
"primitives".to_string(), | ||
"runtime".to_string(), | ||
"generic".to_string(), | ||
"UncheckedExtrinsic".to_string(), | ||
"primitives".to_owned(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an |
||
"runtime".to_owned(), | ||
"generic".to_owned(), | ||
"UncheckedExtrinsic".to_owned(), | ||
], | ||
}, | ||
type_params: vec![ | ||
scale_info::TypeParameter::<scale_info::form::PortableForm> { | ||
name: "Address".to_string(), | ||
name: "Address".to_owned(), | ||
ty: Some(metadata.extrinsic.address_ty), | ||
}, | ||
scale_info::TypeParameter::<scale_info::form::PortableForm> { | ||
name: "Call".to_string(), | ||
name: "Call".to_owned(), | ||
ty: Some(metadata.extrinsic.call_ty), | ||
}, | ||
scale_info::TypeParameter::<scale_info::form::PortableForm> { | ||
name: "Signature".to_string(), | ||
name: "Signature".to_owned(), | ||
ty: Some(metadata.extrinsic.signature_ty), | ||
}, | ||
scale_info::TypeParameter::<scale_info::form::PortableForm> { | ||
name: "Extra".to_string(), | ||
name: "Extra".to_owned(), | ||
ty: Some(metadata.extrinsic.extra_ty), | ||
}, | ||
], | ||
|
@@ -342,7 +346,7 @@ fn generate_outer_enums( | |
let Some(last) = call_path.last_mut() else { | ||
return Err(TryFromError::InvalidTypePath("RuntimeCall".into())); | ||
}; | ||
*last = "RuntimeError".to_string(); | ||
*last = "RuntimeError".to_owned(); | ||
generate_outer_error_enum_type(metadata, call_path) | ||
}; | ||
|
||
|
@@ -368,7 +372,10 @@ fn generate_outer_error_enum_type( | |
return None; | ||
}; | ||
|
||
let path = format!("{}Error", pallet.name); | ||
// Note: using the `alloc::format!` macro like in `let path = format!("{}Error", pallet.name);` | ||
// leads to linker errors about extern function `_Unwind_Resume` not being defined. | ||
let mut path = String::new(); | ||
write!(path, "{}Error", pallet.name).expect("Cannot panic, qed;"); | ||
let ty = error.ty.id.into(); | ||
|
||
Some(scale_info::Variant { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd incline to start this test after the following
needs: [clippy, wasm_clippy, check, wasm_check, docs]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't agree, this no-std check/test should be cheap and runs faster than for example
clippy
it's not really a test. It just checks that a binary builds without std enabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, we'd need to adjust the
needs: [clippy, wasm_clippy, check, wasm_check, docs]
to includeno-std-tests
as well :DThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, sounds good :)