pwasm-token-example
pwasm-token-example copied to clipboard
openethereum
idea for better rust wasm contract development/testing experience
not sure if this is the right repo for this issue. if someone points me to a better repo i'll move the issue there.
i know rust wasm contract development is super early days. even so the contract developer/tester experience could already be improved a lot.
room for improvement imho:
- mutable global state (
EXTERNS) - boilerplate
- unused
selfhttps://github.com/paritytech/pwasm-token-example/blob/b23d486120a2007a3727864deb7b67f79dc5bf50/src/token.rs#L79
it would be great if we could write rust contracts a bit like this (wishful thinking):
#[contract]
mod token_contract {
static TOTAL_SUPPLY_KEY: H256 = H256([2,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]);
static OWNER_KEY: H256 = H256([3,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]);
fn constructor<C: Context>(&mut context: C, total_supply: U256) {
let sender = context.sender();
// Set up the total supply for the token
context.storage().write(&TOTAL_SUPPLY_KEY, &total_supply.into()).unwrap();
// Give all tokens to the contract owner
context.storage().write(&balance_key(&sender), &total_supply.into()).unwrap();
// Set the contract owner
context.storage().write(&OWNER_KEY, &H256::from(sender).into()).unwrap();
}
fn totalSupply<C: Context>(&context: C) -> U256 {
context.storage().read(&TOTAL_SUPPLY_KEY).unwrap_or([0u8; 32]).into()
}
// ...
}
this looks much cleaner to me.
it requires no knowledge about global mutable state to understand.
testing would be more natural, require no macros and no setting of global mutable state:
#[test]
fn should_succeed_in_creating_max_possible_amount_of_tokens() {
let mut context = SomeContextBuilder::new().build();
// set total supply to maximum value of an unsigned 256 bit integer
let total_supply = U256::from_dec_str("115792089237316195423570985008687907853269984665640564039457584007913129639935").unwrap();
assert_eq!(total_supply, U256::max_value());
constructor(&mut context, total_supply);
assert_eq!(totalSupply(&context), total_supply);
}
what do you think?
for actually using contracts (not testing) there would be a default Context implementation which would delegate to extern "C" fn
to clarify:
i'm proposing an attribute or macro which makes a smart contract and it's ABI from a module like definition.
functions within that definition take a C: Context as the first argument.
this replaces the pwasm_std::externs and pwasm_std::storage. that makes things more flexible and makes testing straightforward.
less boilerplate because there no longer is a need to specify both trait and impl for a contract.
no weird unused self references in trait impl of the contract.
I like your ideas. The way we are mocking externs right now seems not very nice, to be honest.
pwasm-std could provide the real context with external calls and we could pass it in _call.
Maybe context.storage().write instead of storage_write (and context.storage().read) is a bit overkill.. @NikVolf and @pepyakin . Guys what do you think?
@fckt
pwasm-stdcould provide the real context with external calls and we could pass it in_call.
👍
such approach has a lot of downsides
- you can't implement multiple contracts with known interface (trait) this way
- you can't split interface (trait) and it's implementation
- you can't generate abi if you know only interface (trait)
@NikVolf great that we start a discussion on this
the points you raise focus on the removal of the trait in the example.
do you see any downsides to passing in a C: Context instead of
using pwasm_std::storage and pwasm_std::externs?
i see that as independent from removing the trait.
one could remove the trait without switching to context or
switch to context without removing the trait.
you can't implement multiple contracts with known interface (trait) this way
good point. but i'm sure one could model the problem in a way where one could use traits if that is needed and not use traits if they are not needed and would just be boilerplate.
you can't split interface (trait) and it's implementation
other than 1) what is gained by splitting trait and implementation?
you can't generate abi if you know only interface (trait)
can you elaborate?
seems like you currently generate the abi using only the trait: https://github.com/paritytech/pwasm-token-example/blob/45b3d2864ae51bd297c2a753340870261b9a4716/src/token.rs#L50
i'm sure one can derive the abi from the trait, a module like thing or anything else that contains enough information.
Better to keep contract Endpoint and Client generation using trait, but I like idea of context..
@snd yeah, downside with Context is pretty simple - you should always prefer implementation complexity over public api complexity. Currently pwasm-std api is pretty simple and consist only of functions, introducing Context there and making contracts interface generic over this Context increases public api complexity, which should (and can) be avoided.
other than 1) what is gained by splitting trait and implementation?
it's easy, i can declare trait in one crate and other crate will just import this trait and implement it for it's own structures, with abi already defined as exactly the same across all implementations.
So user, to guarantee that his contract is compliant to the specific abi, just implements the trait and that's all.
it's easy, i can declare trait in one crate and other crate will just import this trait and implement it for it's own structures, with abi already defined as exactly the same across all implementations.
So user, to guarantee that his contract is compliant to the specific abi, just implements the trait and that's all.
Like Solidity interfaces http://solidity.readthedocs.io/en/develop/contracts.html#interfaces
the trait stuff is clear to me. just thought 1) and 2) were essentially the same point and wanted to know whether there's more to it.
On the second thought, idea with context might be useful, if traits will not be generic over it.
@fckt might come with proof-of-concept in this repo (it can be achieved without touching pwasm-std at all at this point)
Like so https://play.rust-lang.org/?gist=6570adb28cbd3bb1c139e08389a6a12b&version=stable
Like so https://play.rust-lang.org/?gist=6570adb28cbd3bb1c139e08389a6a12b&version=stable
i already like this much better than the current way 👍
i don't like that the following nontrivial boilerplate is now required for every contract:
struct Token<'a, T: 'a + Context> {
context: &'a mut T
}
impl<'a, T: 'a + Context> Token<'a, T> {
fn new (context: &'a mut T) -> Token<'a, T> {
Token{ context: context }
}
}
one could write a macro to automate that. but macros obfuscate things and add complexity. not saying one should never use them but a solution without them is often simpler.
the generics with lifetime params are not the easiest and might result in difficult error messages. would be nice if we could find a solution without that.
let's keep iterating!
why ctor if i may ask? unless one comes from C# it will cause confusion. confusion leads to bugs.
why not constructor? it's unmistakeable!
Just wanted to note about gist is that better always require &mut self in the Context. Rust &mut is not only about mutability, but also about uniqueness, and Context should always be unique.
Not to mention that tests might want to write some logging data when storage_read is invoked, for example.
Can we just have:
-
impl Context for &mut TestContext -
impl Context for RealContext, where RealContext is juststruct RealContext
?
https://play.rust-lang.org/?gist=99209e78f4bbd2de7d8df909f9d4a2e4&version=stable
@pepyakin yeap, the context gives us this flexibility. In the _call user could use RealContext with externs. In tests he could mock it.
why ctor if i may ask? unless one comes from C# it will cause confusion. confusion leads to bugs. why not constructor? it's unmistakeable!
Name ctor comes from how Solidity encodes call to the constructor. https://github.com/paritytech/pwasm-abi/blob/master/derive/src/lib.rs#L95
Name ctor comes from how Solidity encodes call to the constructor. https://github.com/paritytech/pwasm-abi/blob/master/derive/src/lib.rs#L95
isn't that an implementation detail that shouldn't affect the public API?
ctor
it has nothing to do with solidity actually, it's just me; i will rename it in abi reimplementation i'm working on now
just looked at https://play.rust-lang.org/?gist=6570adb28cbd3bb1c139e08389a6a12b&version=stable again
in the test at the bottom: having to use explicit blocks and recreating the contract many times because the contract exclusively mutably borrows the context is not the best developer experience.
that's just a minor thing though
@pepyakin really like how your modification removed the need for lifetime params for the real (non testing) case
polkadot uses the concept of an Externalities trait (similar to Context idea) as well:
https://github.com/paritytech/polkadot/blob/2ae67514569af4c1c55f19692bf2dd84307af78c/state_machine/src/lib.rs#L164
maybe we can get some useful ideas from their code
i like the word Externalities
@snd, it has little to do with context and tests, it's how runtime works
parity has the same "externalities" (and with much more functions) for both evm and wasm https://github.com/paritytech/parity/blob/master/ethcore/src/externalities.rs
it has quite a bit to do with context in the sense that the context idea and externalities both model abilities of an execution environment (and those environments are even similar) through a trait. so design decisions in one could be useful in the other
We have a current limitation with a "context" approach: currently we cant set that context on Client, cause in Client we directly use ext::call https://github.com/paritytech/pwasm-token-example/blob/master/src/token.rs#L49
