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

Runtime call fails with IncompatibleCodegen error code when filtering pallets in generated code #1659

Closed
pgherveou opened this issue Jun 26, 2024 · 1 comment

Comments

@pgherveou
Copy link
Contributor

Here is the repro walk through:

The example should work if you generate the metadata with
subxt metadata --url ws://localhost:9944 -o metadata.scale

It will fail with Error: Metadata(IncompatibleCodegen)
when the code is generated with subxt metadata --pallets "Balances,Timestamp,Contracts,ContractsEvm,System" --url ws://localhost:9944 -o metadata.scale

@pgherveou pgherveou changed the title Receive an IncompatibleCodegen error code when filtering pallets in generated code Runtime call fails with IncompatibleCodegen error code when filtering pallets in generated code Jun 26, 2024
@jsdw
Copy link
Collaborator

jsdw commented Jul 24, 2024

I dug into this, and the type that has an incompatible hash is the thing returned from the runtime API call (pallet_contracts::primitives::ContractResult); type ID 172 in the "full" metadata and 165 in the "small" metadata. The reason that this type hashes to different values is because it returns an events field containing the outer RuntimeEvent enum, which is stripped down in the "smalL" metadata and thus isn't equal.

In the validation logic we have this OuterEnumHashes type which is made use of when generating the overall metadata hash. The point of this struct is to return the hash for only the relevant parts of the outer enums, so eg if we strip pallets, we will only compare the part of the enums that hasn't been stripped away and ignore the rest. In much of the rest of the type generation code, we just pass OuterEnumHashes::empty(), which means we don't do anything smart.

One possible fix for this issue is to extend the validation logic so that we can always pass an optional list of the pallets we actually want to compare, and then pass this through to places. However, I'm not sure if this is a valid fix or not. It's ok if the user passes some types (eg arguments to a runtime call) that are stripped down, because they will still be compatible with the full types. But what do we do if we get handed back a full type from the node, and only have a stripped down type to put it into (like what would be the case here with the ContractResult type).

Possibly, we need to change the metadata stripping to:

  • Only strip variants from the overarching Call/Event/Error enums if they are not used as a return type for any storage/constant/runtime API that is being kept around.
  • This would ofc lead to metadata not being made much smaller in these cases, and then we should probably emit a warning and ideally point to the place(s) that prevented this stripping.
  • Perhaps tooling could exist to show the size of each call/storage/etc so its clearer which ones would cause this bloat.
  • Perhaps stripping could be more granular eg to keep only specific calls etc around. This owuld be a pain for users though, and more complexity, but would allow heavy calls to be stripped away if not needed.

An alternative to all of this is to acknowledge that trying to strip metadata adds a ton of complexity, and just to remove the ability to strip metadata in the first place, avoiding all of these issues and simplifying the codebase a bunch.

For reference, the metadata SCALE files I used:

metadata-small.scale.txt
metadata.scale.txt

I modified the repro to this to highlight the issue (assuming subxt is checked out in an adjacent folder and checked out to c3267ed; current head on master):

use subxt_metadata::Metadata;
use parity_scale_codec::Decode;

fn main() -> Result<(), Box<dyn std::error::Error>> {

    let files = [
        (172, "full", "./metadata.scale"),
        (165, "small", "./metadata-small.scale"),
    ];

    for (type_id, name, file) in files {
        println!("###################################");
        println!("Metadata: {name}");
        println!("###################################\n");

        let md = std::fs::read(file).expect("cannot read metadata");
        let md = Metadata::decode(&mut &*md).expect("cannot decode metadata");

        let outer_enum_hashes = subxt_metadata::OuterEnumHashes::empty();
        let hash = subxt_metadata::get_type_hash(md.types(), type_id, &outer_enum_hashes);

        println!("\n{}\n", hex::encode(hash));
    }

    Ok(())
}
[package]
name = "subxt-repro"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
hex = "0.4.3"
parity-scale-codec = "3.6.12"
subxt-metadata = "0.37.0"

[patch.crates-io]
subxt-metadata = { path = "../subxt/metadata" }

pkhry added a commit that referenced this issue Oct 14, 2024
@pkhry pkhry closed this as completed in ae0fce8 Oct 21, 2024
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

No branches or pull requests

2 participants