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

Unable to use ink tests with custom environment #1935

Open
Kofituo opened this issue Oct 6, 2023 · 6 comments
Open

Unable to use ink tests with custom environment #1935

Kofituo opened this issue Oct 6, 2023 · 6 comments

Comments

@Kofituo
Copy link

Kofituo commented Oct 6, 2023

I'm trying to write a smart contract for a custom blockchain with AccoutId of type [u8; 20]. Here's my environment

pub type AccountId = [u8; 20];

impl Environment for CustomEnvironment {
    const MAX_EVENT_TOPICS: usize = <DefaultEnvironment as Environment>::MAX_EVENT_TOPICS;
    type AccountId = AccountId;
    type Balance = <DefaultEnvironment as Environment>::Balance;
    type Hash = <DefaultEnvironment as Environment>::Hash;
    type Timestamp = <DefaultEnvironment as Environment>::Timestamp;
    type BlockNumber = <DefaultEnvironment as Environment>::BlockNumber;
    type ChainExtension = ink::env::NoChainExtension;
}

#[ink::contract(env = crate:: CustomEnvironment)]
mod custom_contract {
    // --Snip---
}

I'm unable to use this within ink tests since From<[u8; 32]> is hardcoded into functions such as set_callee and set_contract.

Is there a workaround for it or ink! just doesn't support other account formats yet?

I'm also unable to instantiate a contract (instantiate) and ultimately sign transactions since that's also hardcoded to use Keypair signer which is a concrete type wrapper over [u8; 32] as well.

Here's how I'm writing the e2e tests by the way: link

@SkymanOne
Copy link
Contributor

It is a good spot we should make AccountId more generic. The fix in tests should easy. However, the reason why you might not be able to instantiate the contract is because pallet-contracts is configured to use the default AccountId.

If you are using some custom node configuration, can you please provide the link so I can reproduce the issue?

@Kofituo
Copy link
Author

Kofituo commented Oct 25, 2023

The custom node is frontier. So basically that node with pallet-contracts setup (similar to this).
However, you mentioned that pallet-contracts uses AccountId. Do you mean pallet-contracts defaults to AccountId32? From what I just checked, it uses AccountId provided by frame_system pallet

@SkymanOne
Copy link
Contributor

Do you mean pallet-contracts defaults to AccountId32

Essentially, yes, substrate-contracts-node using AccountId32 which resolves to [u8; 32].

Off-chain testing environment makes an assumption about this in order to generate a set of default accounts. I need to think about the proper way to make AccountId configurable, coz this breaks some assumptions in our testing frameworks

@Kofituo
Copy link
Author

Kofituo commented Oct 25, 2023

Yeah. Probably AccountId should be configurable but also have a default value if none is specified

@SkymanOne
Copy link
Contributor

SkymanOne commented Nov 4, 2023

After some investigation, it would be difficult to support non-standard generic account types in both unit and e2e testing. This is due to the fact that the testing environment assumes AccountId: From<[u8; 32]> allowing the convert keypairs from subxt into public addresses and so on, it is also used when generating default test accounts (Alice, Bob, Dave, ...) assuming the From<[u8; 32]> implementation.

Making this generic can be done with:

  1. Introducing the const generic N for From<[u8; N]> allowing the developer to specify the size of the AccountId manually
  2. Completely removing the notion of From<> and introducing the trait DefaultAccounts and other traits allowing the developer to specify default test accounts himself

The second approach is less feasible because the E2E framework heavily relies on sr25519 for signing transactions, and uses default polakdot config.

Making this customisable would probably require a lot of configuration on the developer side, something that we want to avoid.

If you want to test your contracts with non-32-byte AccountId, I suggest you do this on the frontend side using JavaScript.

@Kofituo
Copy link
Author

Kofituo commented Nov 5, 2023

How you suggest I do that? @SkymanOne

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