Make cargo contract work with AccountId20 and EthereumSignature

[JoshOrndorff]

This PR is my latest attempt to make cargo contract work with the AccountId20 and EthereumSignature types. Currently cargo contract assumes that the chain will use the AccountId32 type, but actually Substrate is generic over this type.

This PR replaces #1183, and is a much more systematic approach. I've understood the main parts of this repo's design as well as subxt's. However, this code is still not quite compiling. I would would really appreciate some guidance from someone who understands this stuff. (Maybe @ascjones or @athei or @jsdw ?

Motivation:

This is for the Polkadot Blockchain Academy's contract pow chain. This chain will host both evm contracts and pallet-contracts. We are planning to make it use AccountId20 and EthereumSignature just like Moonbeam does so that users only have a single notion of account. I've got this working in Polkadot-Blockchain-Academy/Academy-PoW#26 and even Polkadot js is supporting it :) The last concern is that cargo contracts cannot interact with the chain. And we would really like to include cargo contract as a central part of the lesson.

[JoshOrndorff]

To add a little more detail, these are the kinds of compile errors I'm down to.

error[E0277]: the trait bound `&contract_transcode::AccountId20: Borrow<account::AccountId20>` is not satisfied
   --> crates/cargo-contract/src/cmd/info.rs:103:57
    |
103 |             api::storage().contracts().contract_info_of(&self.contract);
    |                                        ---------------- ^^^^^^^^^^^^^^ the trait `Borrow<account::AccountId20>` is not implemented for `&contract_transcode::AccountId20`
    |                                        |
    |                                        required by a bound introduced by this call
    |
note: required by a bound in `contracts::storage::StorageApi::contract_info_of`
   --> crates/cargo-contract/src/cmd/runtime_api/mod.rs:19:1
    |
19  | / #[subxt::subxt(
20  | |     runtime_metadata_path = "src/cmd/runtime_api/academy_pow.scale",
21  | |     substitute_type(
22  | |         path = "sp_weights::weight_v2::Weight",
...   |
28  | |     )
29  | | )]
    | |__^ required by this bound in `StorageApi::contract_info_of`
    = note: this error originates in the attribute macro `subxt::subxt` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `account::AccountId20: From<contract_transcode::AccountId20>` is not satisfied
   --> crates/cargo-contract/src/cmd/extrinsics/call.rs:220:35
    |
220 |             self.contract.clone().into(),
    |                                   ^^^^ the trait `From<contract_transcode::AccountId20>` is not implemented for `account::AccountId20`
    |
    = note: required for `contract_transcode::AccountId20` to implement `Into<account::AccountId20>`

As far as I can tell, contract_transcode::AccountId20 is just a re-export of account::AccountId20. So I'm not really understanding the error.

[jsdw]

error[E0277]: the trait bound &contract_transcode::AccountId20: Borrow<account::AccountId20> is not satisfied

As far as I can tell, contract_transcode::AccountId20 is just a re-export of account::AccountId20. So I'm not really understanding the error.

Without having had a chance to dig further, when this sort of error crops up, my first thought is "are there dupe versions of crates in the tree which make it that contract_transcode::AccountId20 isnt actually equal to account::AccountId20"?

[xermicus]

AccountId should probably just be generic and configurable? Same for Balance and Hash types, how does it work for these?

[SkymanOne]

What specifically do you need these types for? I assume that's for submitting transactions and queries. Then yes, we can just introduce generic notion of accounts and make them configurable either via env vars or additional command args

[JoshOrndorff]

AccountId should probably just be generic and configurable? Same for Balance and Hash types, how does it work for these?

@xermicus Yes that is my exact goal. As a first step, I'm trying to just make it work with AccountId20 and EthereumSignature only. I figured that would be a more concrete starting place, on the way to proper genericity.

It seems like you think this should be reasonably achievable. Is that right? This PR is my best attempt and it doesn't compile. I would be very grateful if you could try to build it and take a look.

What specifically do you need these types for? I assume that's for submitting transactions and queries. Then yes, we can just introduce generic notion of accounts and make them configurable either via env vars or additional command args

@SkymanOne This is for signing and submitting transactions. So that means for instantiating contracts, and calling them. It sounds like you have some confidence about how to introduce this generic account type. Could you please either take a look at my attempt or give it a try? I would be very grateful. I've tried my best and feel pretty close, but I'm not sure how to make this compile.

[athei]

This should be configured within the contract using the Environment trait. Since the environment config is written to metadata it can be read there by cargo contract.

[JoshOrndorff]

@jsdw I also thought that maybe the dependencies were crossed and I was using 2 different types. Here's what I've done so far.

Ask cargo tree about what depends on my account crate. This looks correct, and while that is necessary, it is not sufficient.

$ cargo tree -i account

account v0.1.1 (/home/joshy/ProgrammingProjects/cargo-contract/crates/account)
├── cargo-contract v3.0.1 (/home/joshy/ProgrammingProjects/cargo-contract/crates/cargo-contract)
└── contract-transcode v3.0.1 (/home/joshy/ProgrammingProjects/cargo-contract/crates/transcode)
    └── cargo-contract v3.0.1 (/home/joshy/ProgrammingProjects/cargo-contract/crates/cargo-contract)

Write a test in cantract-contract to make sure they are the same type.

// Sanity check to make sure that I'm not accidentally using two different AccountId20 types.
// Until this crate actually compiles, you have to comment out pretty much all of this file to make the test build.
#[test]
fn make_sure_that_account_id_20_types_are_the_same_type() {
    let a = contract_transcode::AccountId20::default();
    let b = account::AccountId20::default();

    assert_eq!(a, b, "If this compiles at all, they are the same type. (I guess it should pass too.)")
}

Indeed the test compiles and passes. These results combined with the fact that I double and triple checked all the imports yesterday make me pretty sure they are the same type. So I just don't get what the compiler is trying to tell me.

[ascjones]

Until now we have only targeted a single node configuration, the recent substrate-contracts-node, so both the Environment types (AccountId, Balance etc.) are fixed and the generated types and call API (from subxt metadata) are fixed against this target node.

What we want is to be able to target different node configurations. I see a couple of different approaches:

1. Make the Environment types generic everywhere they are used, maintain multiple versions of the subxt metadata for each type of node. At runtime detect which type of node we are targeting and invoke the command with the concrete Environment types configuration
   1a. Alternatively replace the subxt metadata with handcrafted calls with generic parameters for Environment types
2. Introduce a plugin model whereby we can support any number of configurations/versions of chains, you just need to install a plugin which conforms to the given API for the supported commands.

[SkymanOne]

Until now we have only targeted a single node configuration, the recent substrate-contracts-node, so both the Environment types (AccountId, Balance etc.) are fixed and the generated types and call API (from subxt metadata) are fixed against this target node.

What we want is to be able to target different node configurations. I see a couple of different approaches:

1. Make the Environment types generic everywhere they are used, maintain multiple versions of the subxt metadata for each type of node. At runtime detect which type of node we are targeting and invoke the command with the concrete Environment types configuration
   1a. Alternatively replace the subxt metadata with handcrafted calls with generic parameters for Environment types
2. Introduce a plugin model whereby we can support any number of configurations/versions of chains, you just need to install a plugin which conforms to the given API for the supported commands.

I am in favour of the latter. Since it is more agile and easier for developers to add support for their chains to cargo-contract

[ascjones]

Related issue #1168

[JoshOrndorff]

I understand that a proper generic design might take more than a week or two. So I would be happy to help with that in whatever time frame makes sense.

For immediate use at PBA, I hope to get this branch working an use it as a fork. Could any of you take a look at my work here, and help me understand the compile issue I'm facing. And whether my work is close to complete?

[ascjones]

Closing this, since this PR itself does not attempt to implement the generic solution of being able to target different configurations/versions of pallet-contract. We should implement support for different AccountId and Signature types as part of the overarching #1168