diff --git a/.github/workflows/actions/use-nodes/action.yml b/.github/workflows/actions/use-nodes/action.yml index b577f8cf7a..06fc74a554 100644 --- a/.github/workflows/actions/use-nodes/action.yml +++ b/.github/workflows/actions/use-nodes/action.yml @@ -9,14 +9,14 @@ runs: - name: Download substrate-node binary id: download-substrate-binary - uses: dawidd6/action-download-artifact@07ab29fd4a977ae4d2b275087cf67563dfdf0295 # v9 + uses: dawidd6/action-download-artifact@4c1e823582f43b179e2cbb49c3eade4e41f992e2 # v10 with: workflow: build-nodes.yml name: nightly-substrate-binary - name: Download polkadot binary id: download-polkadot-binary - uses: dawidd6/action-download-artifact@07ab29fd4a977ae4d2b275087cf67563dfdf0295 # v9 + uses: dawidd6/action-download-artifact@4c1e823582f43b179e2cbb49c3eade4e41f992e2 # v10 with: workflow: build-nodes.yml name: nightly-polkadot-binary diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index bc793af5dd..e1d34e0f36 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -6,11 +6,16 @@ on: # master or release-like branches: branches: - master + # If we want to backport changes to an old release, push a branch + # eg v0.40.x and CI will run on it. PRs merging to such branches + # will also trigger CI. + - v0.[0-9]+.x pull_request: # Run jobs for any external PR that wants # to merge to master, too: branches: - master + - v0.[0-9]+.x concurrency: group: ${{ github.ref }}-${{ github.workflow }} diff --git a/Cargo.lock b/Cargo.lock index 9be4a2b1e4..0548c11918 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5602,6 +5602,7 @@ dependencies = [ "pretty_assertions", "quote", "scale-info", + "scale-typegen", "scale-typegen-description", "scale-value", "serde", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 6b91fea425..47e8ddccee 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -27,6 +27,7 @@ chain-spec-pruning = ["smoldot"] [dependencies] subxt-codegen = { workspace = true } +scale-typegen = { workspace = true } subxt-utils-fetchmetadata = { workspace = true, features = ["url"] } subxt-utils-stripmetadata = { workspace = true } subxt-metadata = { workspace = true } diff --git a/cli/src/commands/codegen.rs b/cli/src/commands/codegen.rs index e7b8f070c5..10744aed76 100644 --- a/cli/src/commands/codegen.rs +++ b/cli/src/commands/codegen.rs @@ -210,8 +210,18 @@ fn codegen( codegen.no_docs() } - let metadata = subxt_metadata::Metadata::decode(&mut &*metadata_bytes) - .map_err(|e| eyre!("Cannot decode the provided metadata: {e}"))?; + let metadata = { + let mut metadata = subxt_metadata::Metadata::decode(&mut &*metadata_bytes) + .map_err(|e| eyre!("Cannot decode the provided metadata: {e}"))?; + + // Run this first to ensure type paths are unique (which may result in 1,2,3 suffixes being added + // to type paths), so that when we validate derives/substitutions below, they are allowed for such + // types. See . + scale_typegen::utils::ensure_unique_type_paths(metadata.types_mut()) + .expect("ensure_unique_type_paths should not fail; please report an issue."); + + metadata + }; // Configure derives: let global_derives = raw_derives diff --git a/codegen/src/api/storage.rs b/codegen/src/api/storage.rs index 88ad1cedfe..922aca7d7a 100644 --- a/codegen/src/api/storage.rs +++ b/codegen/src/api/storage.rs @@ -110,47 +110,49 @@ fn generate_storage_entry_fns( StorageEntryType::Map { key_ty, hashers, .. } => { - match &type_gen - .resolve_type(*key_ty) - .expect("key type should be present") - .type_def - { - // An N-map; return each of the keys separately. - TypeDef::Tuple(tuple) => { - let key_count = tuple.fields.len(); - let hasher_count = hashers.len(); - if hasher_count != 1 && hasher_count != key_count { - return Err(CodegenError::InvalidStorageHasherCount { - storage_entry_name: storage_entry.name().to_owned(), - key_count, - hasher_count, - }); - } - - let mut map_entry_keys: Vec = vec![]; - for (idx, field) in tuple.fields.iter().enumerate() { - // Note: these are in bounds because of the checks above, qed; - let hasher = if idx >= hasher_count { - hashers[0] - } else { - hashers[idx] - }; - map_entry_keys.push(map_entry_key(idx, field.id, hasher)); - } - map_entry_keys - } - // A map with a single key; return the single key. - _ => { - let Some(hasher) = hashers.first() else { + if hashers.len() == 1 { + // If there's exactly 1 hasher, then we have a plain StorageMap. We can't + // break the key down (even if it's a tuple) because the hasher applies to + // the whole key. + vec![map_entry_key(0, *key_ty, hashers[0])] + } else { + // If there are multiple hashers, then we have a StorageDoubleMap or StorageNMap. + // We expect the key type to be tuple, and we will return a MapEntryKey for each + // key in the tuple. + let hasher_count = hashers.len(); + let tuple = match &type_gen + .resolve_type(*key_ty) + .expect("key type should be present") + .type_def + { + TypeDef::Tuple(tuple) => tuple, + _ => { return Err(CodegenError::InvalidStorageHasherCount { storage_entry_name: storage_entry.name().to_owned(), key_count: 1, - hasher_count: 0, + hasher_count, }); - }; + } + }; - vec![map_entry_key(0, *key_ty, *hasher)] + // We should have the same number of hashers and keys. + let key_count = tuple.fields.len(); + if hasher_count != key_count { + return Err(CodegenError::InvalidStorageHasherCount { + storage_entry_name: storage_entry.name().to_owned(), + key_count, + hasher_count, + }); } + + // Collect them together. + tuple + .fields + .iter() + .zip(hashers) + .enumerate() + .map(|(idx, (field, hasher))| map_entry_key(idx, field.id, *hasher)) + .collect() } } }; diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index a9bfc6e004..33161ae850 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -312,9 +312,8 @@ fn subxt_type_gen_settings( crate_path: &syn::Path, should_gen_docs: bool, ) -> TypeGeneratorSettings { - // If we're using codec::Encode or codec::Decode derives, then we want to - // output #[codec(index = N)] and #[codec(compact)] attrs, else we don't. - let insert_codec_attributes = derives.default_derives().derives().iter().any(|path| { + // Are we using codec::Encode or codec::Decode derives? + let are_codec_derives_used = derives.default_derives().derives().iter().any(|path| { let mut segments_backwards = path.segments.iter().rev(); let ident = segments_backwards.next(); let module = segments_backwards.next(); @@ -325,12 +324,9 @@ fn subxt_type_gen_settings( is_ident_match && is_module_match }); - // If we're inserting the codec attributes, we also should use `CompactAs` where necessary. - let compact_as_type_path = if insert_codec_attributes { - Some(parse_quote!(#crate_path::ext::codec::CompactAs)) - } else { - None - }; + // If we're inserting the codec derives, we also should use `CompactAs` where necessary. + let compact_as_type_path = + are_codec_derives_used.then(|| parse_quote!(#crate_path::ext::codec::CompactAs)); TypeGeneratorSettings { types_mod_ident: parse_quote!(runtime_types), @@ -340,8 +336,10 @@ fn subxt_type_gen_settings( decoded_bits_type_path: Some(parse_quote!(#crate_path::utils::bits::DecodedBits)), compact_as_type_path, compact_type_path: Some(parse_quote!(#crate_path::ext::codec::Compact)), - insert_codec_attributes, alloc_crate_path: AllocCratePath::Custom(parse_quote!(#crate_path::alloc)), + // Note: even when we don't use codec::Encode and codec::Decode, we need to keep #[codec(...)] + // attributes because `#[codec(skip)]` is still used/important with `EncodeAsType` and `DecodeAsType`. + insert_codec_attributes: true, } } diff --git a/core/src/storage/storage_key.rs b/core/src/storage/storage_key.rs index 73084e748f..bac737c66f 100644 --- a/core/src/storage/storage_key.rs +++ b/core/src/storage/storage_key.rs @@ -32,34 +32,45 @@ impl StorageHashers { .resolve(*key_ty) .ok_or(MetadataError::TypeNotFound(*key_ty))?; - if let TypeDef::Tuple(tuple) = &ty.type_def { - if hashers.len() == 1 { - // use the same hasher for all fields, if only 1 hasher present: - let hasher = hashers[0]; - for f in tuple.fields.iter() { - hashers_and_ty_ids.push((hasher, f.id)); - } - } else if hashers.len() < tuple.fields.len() { - return Err(StorageAddressError::WrongNumberOfHashers { - hashers: hashers.len(), - fields: tuple.fields.len(), - } - .into()); - } else { - for (i, f) in tuple.fields.iter().enumerate() { - hashers_and_ty_ids.push((hashers[i], f.id)); - } - } + if hashers.len() == 1 { + // If there's exactly 1 hasher, then we have a plain StorageMap. We can't + // break the key down (even if it's a tuple) because the hasher applies to + // the whole key. + hashers_and_ty_ids = vec![(hashers[0], *key_ty)]; } else { - if hashers.len() != 1 { + // If there are multiple hashers, then we have a StorageDoubleMap or StorageNMap. + // We expect the key type to be tuple, and we will return a MapEntryKey for each + // key in the tuple. + let hasher_count = hashers.len(); + let tuple = match &ty.type_def { + TypeDef::Tuple(tuple) => tuple, + _ => { + return Err(StorageAddressError::WrongNumberOfHashers { + hashers: hasher_count, + fields: 1, + } + .into()); + } + }; + + // We should have the same number of hashers and keys. + let key_count = tuple.fields.len(); + if hasher_count != key_count { return Err(StorageAddressError::WrongNumberOfHashers { - hashers: hashers.len(), - fields: 1, + hashers: hasher_count, + fields: key_count, } .into()); } - hashers_and_ty_ids.push((hashers[0], *key_ty)); - }; + + // Collect them together. + hashers_and_ty_ids = tuple + .fields + .iter() + .zip(hashers) + .map(|(field, hasher)| (*hasher, field.id)) + .collect(); + } } Ok(Self { hashers_and_ty_ids }) diff --git a/macro/src/lib.rs b/macro/src/lib.rs index 355372f672..c3e00da1b9 100644 --- a/macro/src/lib.rs +++ b/macro/src/lib.rs @@ -102,7 +102,17 @@ fn subxt_inner(args: TokenStream, item_mod: syn::ItemMod) -> Result. + scale_typegen::utils::ensure_unique_type_paths(metadata.types_mut()) + .expect("ensure_unique_type_paths should not fail; please report an issue."); + + metadata + }; let mut codegen = CodegenBuilder::new(); @@ -135,6 +145,7 @@ fn subxt_inner(args: TokenStream, item_mod: syn::ItemMod) -> Result Result<(), subxt::Error> // This is what the generated code hashes a `session().key_owner(..)` key into: let actual_key = node_runtime::storage() .session() - .key_owner(KeyTypeId([1, 2, 3, 4]), vec![5, 6, 7, 8]); + .key_owner((KeyTypeId([1, 2, 3, 4]), vec![5, 6, 7, 8])); let actual_key_bytes = api.storage().address_bytes(&actual_key)?; // Let's manually hash to what we assume it should be and compare: @@ -80,13 +80,12 @@ async fn storage_n_mapish_key_is_properly_created() -> Result<(), subxt::Error> // Hash the prefix to the storage entry: let mut bytes = sp_core::twox_128("Session".as_bytes()).to_vec(); bytes.extend(&sp_core::twox_128("KeyOwner".as_bytes())[..]); - // Both keys, use twox64_concat hashers: - let key1 = [1u8, 2, 3, 4].encode(); - let key2 = vec![5u8, 6, 7, 8].encode(); - bytes.extend(sp_core::twox_64(&key1)); - bytes.extend(&key1); - bytes.extend(sp_core::twox_64(&key2)); - bytes.extend(&key2); + // Key is a tuple of 2 args, so encode each arg and then hash the concatenation: + let mut key_bytes = vec![]; + [1u8, 2, 3, 4].encode_to(&mut key_bytes); + vec![5u8, 6, 7, 8].encode_to(&mut key_bytes); + bytes.extend(sp_core::twox_64(&key_bytes)); + bytes.extend(&key_bytes); bytes }; diff --git a/testing/substrate-runner/src/lib.rs b/testing/substrate-runner/src/lib.rs index cce1131ca1..cb305fc14a 100644 --- a/testing/substrate-runner/src/lib.rs +++ b/testing/substrate-runner/src/lib.rs @@ -123,7 +123,7 @@ impl SubstrateNodeBuilder { ) -> Result { let mut cmd = Command::new(binary_path); - cmd.env("RUST_LOG", "info,libp2p_tcp=debug") + cmd.env("RUST_LOG", "info,libp2p_tcp=debug,litep2p::tcp=debug") .stdout(process::Stdio::piped()) .stderr(process::Stdio::piped()) .arg("--dev") @@ -295,6 +295,14 @@ fn try_find_substrate_port_from_output(r: impl Read + Send + 'static) -> Substra .rsplit_once("New listen address: /ip4/127.0.0.1/tcp/") // slightly newer message: .or_else(|| line.rsplit_once("New listen address address=/ip4/127.0.0.1/tcp/")) + // Newest message using the litep2p backend: + .or_else(|| { + // The line looks like: + // `start tcp transport listen_addresses=["/ip6/::/tcp/30333", "/ip4/0.0.0.0/tcp/30333"]` + // we'll split once to find the line itself and then again to find the ipv4 port. + line.rsplit_once("start tcp transport listen_addresses=") + .and_then(|(_, after)| after.split_once("/ip4/0.0.0.0/tcp/")) + }) .map(|(_, address_str)| address_str); if let Some(line_port) = p2p_port_line { diff --git a/testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr b/testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr index 333a888b37..a0e35c3950 100644 --- a/testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr +++ b/testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr @@ -11,9 +11,7 @@ error: Type `Event` does not exist at path `sp_runtime::multiaddress::Event` pallet_grandpa::pallet::Event pallet_treasury::pallet::Event pallet_conviction_voting::pallet::Event - pallet_referenda::pallet::Event pallet_ranked_collective::pallet::Event - pallet_referenda::pallet::Event pallet_whitelist::pallet::Event polkadot_runtime_common::claims::pallet::Event pallet_utility::pallet::Event