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

Add [ink::e2e-test] with a new E2E testing crate à la ink_waterfall #1234

Closed
cmichi opened this issue Apr 25, 2022 · 8 comments
Closed

Add [ink::e2e-test] with a new E2E testing crate à la ink_waterfall #1234

cmichi opened this issue Apr 25, 2022 · 8 comments
Assignees
Labels
A-examples [examples] Work item A-ink_env [ink_env] work item B-design Designing a new component, interface or functionality. B-enhancement New feature or request C-discussion An issue for discussion for a given topic.

Comments

@cmichi
Copy link
Collaborator

cmichi commented Apr 25, 2022

Persisting the results from our discussion on Saturday:

  • We want to remove the existing off-chain testing environment (engine) and replace it with an E2E testing framework.
  • This E2E testing framework will be a new crate based on ink_waterfall. So we want to make the waterfall a product as part of ink!.
  • We will retain the [ink::test] syntax, but it will then instead spawn a substrate-contracts-node (if one is not already spawned) and execute real transactions via cargo-contract instantiate/call/….

The reason behind this is that we found that we need a proper E2E testing framework to keep up with competing products. In the past users found it very irritating that they can't test e.g. the call builder in our off-chain testing environment. As we move more towards interoperability (XCM & Co.) the need for E2E testing will need to be prioritized. By making the waterfall part of ink! in this way we can in the future also use this setup to test contracts that interact with parachains.

It will also remove the need of duplicating the API and logic of pallet-contracts in ink!.

@cmichi cmichi added B-enhancement New feature or request B-design Designing a new component, interface or functionality. A-examples [examples] Work item A-ink_env [ink_env] work item labels Apr 25, 2022
@cmichi cmichi self-assigned this Apr 25, 2022
@cmichi cmichi added the C-discussion An issue for discussion for a given topic. label Apr 25, 2022
@ascjones
Copy link
Collaborator

I would add that there are potentially benefits to removing some abstractions required for the offchain env in the ink! codebase. It would certainly remove a lot of code, and allow more flexibility for more radical reductions in code size.

@h4x3rotab
Copy link

E2E testing framework is good. However there are many things to consider. For example:

  1. Performance. Usually running the unit tests takes less than one second. If we keep only e2e tests, afaik, we need at least minutes to bring up the node, submit the transactions, and setup the browser. It slows down the development. One solution I can come up with is to build a customized node with instant seal engine (just like Ganache), and run polkadot.js based test scripts without involving the browser. Although I haven't tried RedSpot, I guess it go with this way. Generally I would be happy if the test can be done in 30s.

  2. Customization. A lot of parachains will build their own ChainExtension. Any E2E test framework should allow the the developer to use their own configuration in pallet-contracts. Taking Phala Fat Contract as a special example, it has a heavily tweaked runtime environment with its own RPC API outside the substrate node. It would be nice if the E2E test framework should be flexible enough to support our use case.

  3. Interpretability of the debugging process. The best part of the current unit test framework is that it allows us to access all the internal states, functions and get a understandable stack backtrace output. Once we compile it as WASM and run in a VM (especially WASMI), a lot of information for debugging will be lost. It will also be hard to access the contract internals at the rust level. I didn't come up with ideas to mitigate this problem.

@cmichi cmichi changed the title Remove ink_engine and replace [ink::test] with a new E2E testing crate à la ink_waterfall Add [ink::e2e-test] with a new E2E testing crate à la ink_waterfall Jul 28, 2022
@cmichi cmichi mentioned this issue Jul 28, 2022
@athei athei moved this to Backlog 🗒 in Smart Contracts Aug 10, 2022
@kayabaNerve
Copy link

I would like to re-iterate customization.

What if instead of natively running the Rust, as it does now, and instead of spawning a node, as proposed, an e2e-test setup and ran a full instance of wasmi? This may be best done via modularizing the contracts pallet to support such instances (if not already sufficiently modular). That could then be wrapped by a test, directly feeding TXs, or the pallet, feeding in all TXs from a block.

@athei
Copy link
Contributor

athei commented Oct 19, 2022

Usually running the unit tests takes less than one second. If we keep only e2e tests, afaik, we need at least minutes to bring up the node, submit the transactions, and setup the browser.

This is not true. Substrate contract node startups in a second and uses instant seal. Why you would need to start a browser I don't understand. No wait time here.

Any E2E test framework should allow the the developer to use their own configuration in pallet-contracts. Taking Phala Fat Contract as a special example, it has a heavily tweaked runtime environment with its own RPC API outside the substrate node. It would be nice if the E2E test framework should be flexible enough to support our use case.

I don't see any problem in doing that. In the end it just submits extrinsics.

3. Interpretability of the debugging process. The best part of the current unit test framework is that it allows us to access all the internal states, functions and get a understandable stack backtrace output. Once we compile it as WASM and run in a VM (especially WASMI), a lot of information for debugging will be lost. It will also be hard to access the contract internals at the rust level. I didn't come up with ideas to mitigate this problem.

This is true. However, as of right now this is barely usable anyways as the off chain environment is kind of bogus. The plan is to implement trace based debugging in wasmi. So wasmi emits a trace of the execution on dry-run which can then be downloaded and used to step through the execution.

What if instead of natively running the Rust, as it does now, and instead of spawning a node, as proposed, an e2e-test setup and ran a full instance of wasmi? This may be best done via modularizing the contracts pallet to support such instances (if not already sufficiently modular). That could then be wrapped by a test, directly feeding TXs, or the pallet, feeding in all TXs from a block.

This is just what we have now with extra steps. You would still need to emulate the rest of the chain. pallet-contracts doesn't exist in a vacuum. What about chain extensions etc.

@h4x3rotab
Copy link

h4x3rotab commented Oct 19, 2022

I don't see any problem in doing that. In the end it just submits extrinsics.

Phat Contract runs in an offchain daemon. So instead of spawning up a new substrate node, we will need to spawn a full stack. In addition, the query is served by the daemon instead of substrate rpc, and the contract call extrinsic also has a very different format (e.g. fully e2e encrypted).

Recently there are a bunch of nice work in progress worth taking a look as well:

  • Swanky: the ink version alternative of Truffle / Hardhat. It's written in Typescript, and thus much easier to make the contract implementation swappable to support Phat Contract
  • Supercolony's advanced unit test framework. It tries to fill the missing functionalities in the ink_env offchain runtime. I've been testing it recently and it works amazingly well in cross-contract call secenario.

@athei
Copy link
Contributor

athei commented Oct 19, 2022

Phat Contract runs in an offchain daemon. So instead of spawning up a new substrate node, we will need to spawn a full stack. In addition, the query is served by the daemon instead of substrate rpc, and the contract call extrinsic also has a very different format (e.g. fully e2e encrypted).

Okay but in the end you just have an endpoint where the tests submit their extrinsics against? So you just point the tests to this endpoint instead of letting the tests spawn it.

Recently there are a bunch of nice work in progress worth taking a look as well:

So even less incentive for us to maintain our own off-chain testing implementation?

@h4x3rotab
Copy link

h4x3rotab commented Oct 19, 2022

Phat Contract runs in an offchain daemon. So instead of spawning up a new substrate node, we will need to spawn a full stack. In addition, the query is served by the daemon instead of substrate rpc, and the contract call extrinsic also has a very different format (e.g. fully e2e encrypted).

Okay but in the end you just have an endpoint where the tests submit their extrinsics against? So you just point the tests to this endpoint instead of letting the tests spawn it.

Well, things will get complicated in our specific case (though we understand we are a very special case, and it's impossible to support every cases). Let me give you the example.

On our side, to submit a contract call, the contract client (like ContractPromise in polkadot.js or subxt) need to negotiate with the daemon to make a key agreement. Then it encrypts the call data, wraps the call by an extrinsic targeting to our pallet (not pallet-contracts), and submit to our Substrate node. To make a query, it does the similar thing but it sends the encrypted RPC to the daemon.

Ideally if there's a common interface like below, and we just provide an alternative implementation, it should work:

trait InkClient {
  type QueryOption;
  type TxOption;
  fn query(&self, address: AccountId, data: Data, options: QueryOption);
  fn tx(&self, address: AccountId, data: Data, options: TxOption);
}

This is actually what we wanted to push the Polkadot.js upstream to do and we didn't make progress. But in the reality, it also brings the maintenance burden to your side.

Recently there are a bunch of nice work in progress worth taking a look as well:

So even less incentive for us to maintain our own off-chain testing implementation?

E2E is of course very useful, but I just want to point out the directions the ecosystem projects have chosen are different. Personally if the unit test framework works well (like with Supercolony's ink_env fork mentioned above), I tend to invest heavily in unit test (faster and interperterable) and write less E2E test. I will still write an E2E test, but maybe in Typescript for flexibility.

@HCastano
Copy link
Contributor

We now have E2E testing available as part of the ink! 4.0 release

@github-project-automation github-project-automation bot moved this from In Progress 🛠 to Done ✅ in Smart Contracts Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-examples [examples] Work item A-ink_env [ink_env] work item B-design Designing a new component, interface or functionality. B-enhancement New feature or request C-discussion An issue for discussion for a given topic.
Projects
None yet
Development

No branches or pull requests

6 participants