Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

WASM spec tests wasms and cpps #7784

Closed
wants to merge 5 commits into from
Closed

Conversation

jeffreyssmith2nd
Copy link
Contributor

Change Description

Adds the WASM spec tests WASM and C++ files.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@b1bart
Copy link
Contributor

b1bart commented Aug 22, 2019

It seems like each one of these unit tests redundantly tests a laundry list of things in order to look similar to the other "chain" unit tests we have. By this, I mean that each test:

  1. instantiates 1 tester (or more in the case of a validating_tester).
  2. Pre-activates protocol upgrades
  3. Activates all known protocol features
  4. installs eosio.bios
  5. creates accounts/keys/etc
  6. creates and signs a transaction
  7. creates and signs a block
  8. etc etc

Is there an achievable test rigging that only instantiates minimal parts of the webassembly execution core and validates that the provided actions either throw as expected or complete successfully without the extra framing of the blockchain?

I'm going to assume no but, I'd like to accumulate the reasons why that is the case somewhere and start plotting a course to de-coupling the systems that are preventing it.

For now, I'm at least aware that there is a tight coupling of

  • the intrinsic interface to an apply_context
  • the apply_context to a controller

@larryk85
Copy link
Contributor

It seems like each one of these unit tests redundantly tests a laundry list of things in order to look similar to the other "chain" unit tests we have. By this, I mean that each test:

  1. instantiates 1 tester (or more in the case of a validating_tester).
  2. Pre-activates protocol upgrades
  3. Activates all known protocol features
  4. installs eosio.bios
  5. creates accounts/keys/etc
  6. creates and signs a transaction
  7. creates and signs a block
  8. etc etc

Is there an achievable test rigging that only instantiates minimal parts of the webassembly execution core and validates that the provided actions either throw as expected or complete successfully without the extra framing of the blockchain?

I'm going to assume no but, I'd like to accumulate the reasons why that is the case somewhere and start plotting a course to de-coupling the systems that are preventing it.

For now, I'm at least aware that there is a tight coupling of

  • the intrinsic interface to an apply_context
  • the apply_context to a controller

That is exactly the reason, we use eosio_assert for checking assert return conditions. One solution to the problem is that we could define a new eosio_assert that has an unreachable instruction on the false branch. This would allow us to call into the backends without having to go through all of the chain/controller setup. This would mean that these asserts would all through an unreachable exception and we would be blind to the exact failure, but they would at least act as a canary that something has went awry and needs further investigation.

@b1bart
Copy link
Contributor

b1bart commented Aug 22, 2019

That is exactly the reason, we use eosio_assert for checking assert return conditions. One solution to the problem is that we could define a new eosio_assert that has an unreachable instruction on the false branch.

The direction I'm going in my head is to decouple (at compile time hopefully) the available imports to the linking phase of wasm from the controller so that tests can instantiate a wasm context with a mock intrinsic layer (as we do in the native tester in the CDT). I'm sure there are other touch points during the validation and injection phases that would need to be mockable too so that tests have very fine control over what is offered to the backends and what the tests' expectations are.

Is this in the realm of feasible or am I talking moon-talk?

@spoonincode
Copy link
Contributor

It wouldn't take much effort to drive these tests just from wasm_interface (unfortunately wasm_interface today wants to know about controller because it wants to look in the DB for the code_objects, this shouldn't be too hard to refactor around). Then all you have left is the need for a dummy apply_context. I don't think you need to go so far down the rabbit hole as mocking out the intrinsics if I'm understanding your goal correctly

@b1bart
Copy link
Contributor

b1bart commented Aug 23, 2019

For this instance, mocking intrinsics is probably not a goal in itself. Separating wasm_interface from controller seems worthy if we can find a zero-runtinme cost way of doing it. Just as we have a lot of controller plumbing in these spec tests to test the expectations of the WebAssembly execution, we have lots of contracts in our unit tests specifically designed to drive scenarios on controller that could be made thinner if we could simply replace the call to web-assembly with a set of expected intrinsic calls and their parameters. Save the end-to-end work for the tests that are actually testing some end-to-end expectations.

from @larryk85 's comment it does sound like we are getting some level of feedback from eosio_assert in the test cases that may make it useful to have at least some of the intrinsic plumbing in place to extract information from the tests?

@b1bart
Copy link
Contributor

b1bart commented Aug 23, 2019

Also note, this is outside of a review on this PR because any effort to de-couple this and make our testing thinner and yet more effective would be an additional effort in the future.

@jeffreyssmith2nd jeffreyssmith2nd mentioned this pull request Sep 20, 2019
3 tasks
@jeffreyssmith2nd
Copy link
Contributor Author

Closing in favor of #7955

@jeffreyssmith2nd jeffreyssmith2nd deleted the wasm-spec-tests-wasms branch September 23, 2019 13:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants