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

[test] use subxt as alternative integration test for substrate #965

Closed
extraymond opened this issue Aug 15, 2022 · 9 comments
Closed

[test] use subxt as alternative integration test for substrate #965

extraymond opened this issue Aug 15, 2022 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@extraymond
Copy link
Contributor

extraymond commented Aug 15, 2022

Info

motivation:
using subxt allows us to reuse code from palelt-contract, alongside the rpc's. It might be useful to catch bugs related to pallet-contracts on the runtime interface layer.

todos:

  • create a test lib with subxt, with metadata.scale obtained from the latest substrate-contracts-node
  • try deploy/call solang compiled contracts with pallet-contract related rpc and extrinsics
  • (optional) try preparing selector and decoding return_value and eventrs with cargo-transcoder if the metadata format are unified

related projects
paritytech/subxt
paritytech/substrate-contracts-node

@xermicus
Copy link
Contributor

xermicus commented Aug 16, 2022

Thinking about this: Why not integrate subxt directly into Solang? This would allow us to build a neat CLI user interface (upload, instatiate and RPC call contracts), similar to what cargo contract offers. Consequentially, this can then be used for integration tests. The CLI can be made to be target agnostic, so we could at one point implement it for Solana as well.

What do you think @seanyoung @LucasSte ?

@seanyoung
Copy link
Contributor

seanyoung commented Aug 16, 2022

I think it's a good idea. My only concern is that this is a question of where this (upload, instantiate, call) functionality should ideally live.

  • Add some functions to subxt
  • Add functionality to cargo contract
  • Add functionality to solang

I don't know what the answer to this is. I do think that if it can live in another repo, then that saves us the effort of looking after it 😆

@xermicus
Copy link
Contributor

xermicus commented Aug 16, 2022

This ideally just adds functionality to solang, possibly implemented in one or more sub-command. I had a quick look at the subxt examples and at first glance this seems easy to do for Substrate.

If we want to keep the code added to solang minimal, we could just implement the Frontend (as subcommands) and possibly define some trait abstractions for any supported --target backends. So that the concrete implementations could live in their own crate if necessary.

@xermicus xermicus added enhancement New feature or request question Further information is requested labels Aug 16, 2022
@seanyoung
Copy link
Contributor

Just to be clear, I don't mind this functionality living the solang repo, it just feels like most it should be shared with other repos.

@extraymond
Copy link
Contributor Author

extraymond commented Aug 16, 2022

I think it depends on whether cargo-contract want to loosen their extrinsic parts to be ink-independant. Because it's already using subxt to do the endeavors, and they provide most of the common usages of contract related operations.

If these part of cargo-contract can decoupled from expecting a ink project already exists, we might be able to reuse most of the parts there. https://github.com/paritytech/cargo-contract/tree/master/src/cmd/extrinsics

Currently it's assuming a ink manifest exists in the first place, after that I think it's mostly pallet-contract realted, which should apply to solang as well. https://github.com/paritytech/cargo-contract/blob/a248a4d704b4ea3fb44fa556c52abe418af20c6b/src/cmd/extrinsics/instantiate.rs#L137

@extraymond
Copy link
Contributor Author

Some progress here.
I've managed to add a symmetric tests using subxt against the existing integration/substrate suite.

Current working branch:
https://github.com/extraymond/solang/tree/subxt-tests

Currently without the same ABI format, we'll need to manually construct the selector, I've added the ContractTranscode crate just once we have the ABI format compatible with ink_metadata.

Next step for me would be port most of the test logic from the existing test to this new test package.

Currently can be tried with after staring the dev node in background:
substrate-contracts-node --dev

And test it with:
cargo test -p subxt-tests

@extraymond
Copy link
Contributor Author

extraymond commented Aug 26, 2022

Four more tests added, currently half way there.
It seems event parsing would be different between solang and ink.

For solang, which follows ethereum standards, should be parse 1 byte for topic_id and rest for body.
But for ink, at least contract-transcode uses 1 extra byte to determine the length of the raw body. This might prevents solang contracts events to be decoded using contract-transcode.

https://github.com/paritytech/cargo-contract/blob/f903c0349ba97b0219e14efcd81f67f4c48733eb/transcode/src/lib.rs#L253
Might block use-ink/cargo-contract#659

@extraymond
Copy link
Contributor Author

Currently all ported. But currently only works against v3 metadata, need to wait for v4 to properly merge it.
This also requires polkadot.js apps to follow v4.

@xermicus
Copy link
Contributor

Closed by #990 Thanks @extraymond!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants