near / near-sdk-rs

Rust library for writing NEAR smart contracts
https://near-sdk.io
Apache License 2.0
453 stars 241 forks source link

wasm32-unknown-unknown unit-tests #467

Open matklad opened 3 years ago

matklad commented 3 years ago

Crazy idea to follow up on sandbox discussion. TL;DR: cargo test --target wasm32-unknown-unknown could just work.

Today, near-sdk-rs contracts support unit-tests (#<span class="error">[test]</span>), but in a very round-about way. To run unit-tests, you need to compile the contract for host. That means that contract code can't assume that it runs under wasm, which is a rather big assumption.

It's simpler to require that contract always compiles to wasm (there's an [upcoming cargo feature](https://github.com/rust-lang/cargo/issues/9406) for this), and just make unit-test working with cargo test --target wasm32-unknown-unknown. Amusingly, that [almost works](https://internals.rust-lang.org/t/running-tests-on-wasm32-unknown-unknown/15010?u=matklad) today.

The main trick here are [custom cargo runners](https://doc.rust-lang.org/cargo/reference/config.html#targettriplerunner) – you can tell cargo "please, run .wasm file using this runner (eg, wasmtime, or standalone-vm-runner)". On the Rust side, cargo test --no-run produces a perfectly normal .wasm file – Rust's test harness is compatible with wasm.

In practiece, I think we should do the following:

matklad commented 3 years ago

Thinking more about it, I think there's a better practical solution here -- use wasm32-wasi as a target tripple for testing. The wasi subset actually used by the tests is pretty small, so supporting that in the sandbox shoulnd't be a problem (and I bet that's already avaialble in wasmer):

  (import "wasi_snapshot_preview1" "args_get" (func $wasi_snapshot_preview1.args_get (type $t10)))
  (import "wasi_snapshot_preview1" "args_sizes_get" (func $wasi_snapshot_preview1.args_sizes_get (type $t10)))
  (import "wasi_snapshot_preview1" "clock_time_get" (func $wasi_snapshot_preview1.clock_time_get (type $t12)))
  (import "wasi_snapshot_preview1" "fd_close" (func $wasi_snapshot_preview1.fd_close (type $t9)))
  (import "wasi_snapshot_preview1" "fd_read" (func $wasi_snapshot_preview1.fd_read (type $t13)))
  (import "wasi_snapshot_preview1" "fd_write" (func $wasi_snapshot_preview1.fd_write (type $t13)))
  (import "wasi_snapshot_preview1" "path_filestat_get" (func $wasi_snapshot_preview1.path_filestat_get (type $t14)))
  (import "wasi_snapshot_preview1" "path_open" (func $wasi_snapshot_preview1.path_open (type $t15)))
  (import "wasi_snapshot_preview1" "sched_yield" (func $wasi_snapshot_preview1.sched_yield (type $t16)))
  (import "wasi_snapshot_preview1" "random_get" (func $wasi_snapshot_preview1.random_get (type $t10)))
  (import "wasi_snapshot_preview1" "fd_prestat_get" (func $wasi_snapshot_preview1.fd_prestat_get (type $t10)))
  (import "wasi_snapshot_preview1" "fd_prestat_dir_name" (func $wasi_snapshot_preview1.fd_prestat_dir_name (type $t11)))
  (import "wasi_snapshot_preview1" "proc_exit" (func $wasi_snapshot_preview1.proc_exit (type $t1)))
  (import "wasi_snapshot_preview1" "environ_sizes_get" (func $wasi_snapshot_preview1.environ_sizes_get (type $t10)))
  (import "wasi_snapshot_preview1" "environ_get" (func $wasi_snapshot_preview1.environ_get (type $t10)))
ailisp commented 3 years ago

provide a runner that can run .wasm files with near contract ABI. Seems like the sandbox binary can get the ability to run .wasm directly, without running the chain/network/etc.

One thing to note, this sandbox is at a different level of the near-sandbox, which acts on the RPC level - contracts are deployed, then call both by submit an rpc to a running near-sandbox process. This "sandbox" is close to near-vm-standalone, which, used as a cargo runner you provide contract, function call args via command line arguments. And, it's unit test, cross contract calls are mocked and tested in single step.

Should we just make from near-vm-runner-standalone? Or wrap vm-runner-standalone as a new, "vm only" mode for near-sandbox. The second looks unnecessary to me, but we can do it if make sense on developer experience or maintainability

matklad commented 3 years ago

Yeah, it's true that what we need here is something a-la standalone runner. Logistically, it's simpler to ship a single binary to the users, which can both run the wasm contract directly, as well as serve as a full-fledged RPC node.

matklad commented 3 years ago

Thinking more about it,

Thinking even more, using wasi just to get the standard test harness working is solving the wrong problem. We don't need standard harness, better if sandbox runs the contract directly. We need to use a custom test macro for that, but overall UX seems much better that way. See https://github.com/matklad/webassembly-test for a proof of concept.

austinabell commented 3 years ago

Just to confirm, the intention of this is to allow unit tests to be run as compiled to wasm32-unknown-unknown, or do you mean by extension running those tests on the sandbox by deploying this modified contract which contains the test methods and calling those methods through RPC?

If the former, is the benefit just so that there are no wasm arch-specific logic bugs?

Potentially what we could do also, to not complicate the sandbox binary, is have a test annotation, say #[near::test] which would wrap the test so that it initializes the runtime (so that the syscalls are setup for wasm arch) and runs the test through this? I'm not quite sure how possible/easy this would be, but would be cool to have built in to the rust test framework instead of requiring a binary to run the tests

matklad commented 3 years ago

Just to confirm, the intention of this is to allow unit tests to be run as compiled to wasm32-unknown-unknown,

Yes, the intention is to just run unit-tests compiled to wasm32-unknown-unknown. The benefits here are:

but would be cool to have built in to the rust test framework instead of requiring a binary to run the tests

I am not sure how that would work -- in the end, you have a .wasm blob, and you need to run it somehow.

MaksymZavershynskyi commented 3 years ago

@mikedotexe , @matklad . To clarify, this seems to be a great idea, but it does not seem to be higher in priority than other items that we planned for Q3 in Contract Runtime and Developer Platform. It also does not supersede sandbox-based testing framework.

matklad commented 3 years ago

Yeah, agree that there isn't anything immediately actionable here, sorry for not making this clear in the original description.

What would be useful though it to create some sort of a meta issue which describes the short/medium term roadmap for near contract testing.

I still don't have even a remote understanding of where near-sdk-rs unit-tests, sim tests, and near-sdk-as unit-tests should be half a year from now. That would be a useful input to issues like: https://github.com/near/nearcore/issues/4379.

petersalomonsen commented 2 years ago

I think this has become even more important if we want to combine Rust and Javascript contracts with QuickJS which is written in C. In this case I have provided static libraries compiled for wasm target, but that does not go very well together with near-sdk-rs unit tests which are compiled for the host.

I'm working on being able to create unit/integration tests here: https://github.com/petersalomonsen/quickjs-rust-near

austinabell commented 2 years ago

related https://github.com/near/nearcore/issues/7220.

The only issues I have with this proposal are that:

The reason for the request in the issue above is to have a more minimal wrapper that has all of the core execution logic that more closely matches the actual execution on-chain without the large overhead of sandbox. This way, Rust tests could just compile the library, and for JS/other languages, a binary could be created, exposing the interface to be able to execute these.

This definitely feels like it's worth having a high-level discussion created for the high-level and long-term goals of testing cross-language.

wdyt @ChaoticTempest @ailisp

petersalomonsen commented 2 years ago

I understand there might be some challenges here and issues with the testing tools of near-sdk-rs (which does not support the wasm target), but still I think it's worth to explore how far we can get with testing in wasm. At least it makes it very much easier when you deal with libraries linked from C (like QuickJS). First I tried with the host platform target, but on my Mac M1 this is even more fuzz since QuickJS compiles for ARM by default. I will then have to tweak libraries I link to make sure they compile for x86. So if we can get everything working on one common target (wasm), that would be the easiest from the "user" perspective.

Here's my initial attempt on testing with the wasm target (using normal #[cfg(test)]:

image
petersalomonsen commented 2 years ago

and here's the first test run in github actions: https://github.com/petersalomonsen/quickjs-rust-near/runs/7886061254?check_suite_focus=true