Skip to content
This repository has been archived by the owner on Feb 14, 2021. It is now read-only.

accessing/modifying external once set & question about external in general #13

Closed
snd opened this issue Nov 13, 2017 · 7 comments
Closed
Assignees

Comments

@snd
Copy link

snd commented Nov 13, 2017

i'm in the process of writing tests for https://github.com/paritytech/pwasm-token-example
(working on branch https://github.com/paritytech/pwasm-token-example/tree/tests) and thinking about how to improve the testing experience for rust pwasm contracts.

now i'm stuck writing a test that involves:

  1. setting some testing external a (pwasm_test::set_external)
  2. calling a contract function that uses some externals
  3. accessing testing external a again either to assert something on it or in order to modify it
    3.1 often one will want to have sender return different things for different contract calls in tests while reusing the rest of the external state
  4. calling a contract function that uses some externals

3. seems hard to do. pwasm_test::set_external moves the external and now it's hard to access it.
pointers towards implementing pwasm_test::get_external would be greatly appreciated!

test_with_external! to me seems useful only for trivial tests currently as most real world tests will involve testing interaction with external by accessing external again after the test body has run.

maybe i'm missing something obvious!

a related question:

can someone enlighten me why we are using the whole EXTERNAL thread local global mutable state (https://github.com/paritytech/pwasm-test/blob/master/src/externs.rs#L12)?

if the contract functions took an impl of External as an argument then we would not need global mutable state.
from the contract developers/testers perspective that would make things much more simple, easier to reason about, harder to screw up.

@NikVolf
Copy link
Collaborator

NikVolf commented Nov 13, 2017

This global External is created for tests purposes only, regular contract code has nothing to do with it, it works with just external (extern "C" fn) functions.

i think get_external should solve most of these test problems, @fckt

@snd
Copy link
Author

snd commented Nov 13, 2017

@fckt it would be great if you could implement get_external (generic and with a downcast to the concrete type) as that would indeed solve my current test problems

@snd
Copy link
Author

snd commented Nov 13, 2017

@NikVolf do i understand this correctly?:
functions in pwasm_std::storage and pwasm_std::extern call c externs.
those are implemented in the wasm env where the contracts are run.
for testing those externs are implemented in pwasm_test::externs and those implementations forward to thread local EXTERNAL which can be switched out

@lexfrl
Copy link
Contributor

lexfrl commented Nov 13, 2017

functions in pwasm_std::storage and pwasm_std::extern call c externs.
those are implemented in the wasm env where the contracts are run.
for testing those externs are implemented in pwasm_test::externs and those implementations forward to thread local EXTERNAL which can be switched out

that's correct

@snd
Copy link
Author

snd commented Nov 13, 2017

i tried a few approaches in implementing get_external. no luck yet.

the most promising way goes through Box.downcast https://doc.rust-lang.org/std/boxed/struct.Box.html#method.downcast, which requires a conversion from the trait object
to an Any object which would require modification of the External trait to include an as_any(&self) -> &Any which all External implementations would have to implement.
the Any type would then contain the necessary metadata required for the downcast.

one could also do some nasty unsafe transmute stuff but we shouldn't consider that.

there doesn't seem to be an easy way. all approaches feel somewhat hacky!

@snd
Copy link
Author

snd commented Nov 14, 2017

@fckt i figured out a way to implement get_external: https://github.com/paritytech/pwasm-token-example/blob/7baef94ba65a4af16f4cb14874d396413b16c7a6/src/token.rs#L220

it also requires the following change: 2fcb56d

a downside is that any testing implementations of External now need to implement as_any:
https://github.com/paritytech/pwasm-token-example/blob/7baef94ba65a4af16f4cb14874d396413b16c7a6/src/token.rs#L213
this is required because of the way rusts Any faux dynamic typing works and adds meta information that can later be used for downcasting.

i hope our discussion in openethereum/pwasm-token-example#10 will lead to an overall better solution that will remove the need for set_external and get_external.

at least now i can continue writing tests for https://github.com/paritytech/pwasm-token-example

@lexfrl
Copy link
Contributor

lexfrl commented Feb 6, 2018

close due to inactivity

@lexfrl lexfrl closed this as completed Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants