spacemeshos / svm

SVM - Spacemesh Virtual Machine
https://spacemesh.io
MIT License
85 stars 14 forks source link

Initialize each Wasm Instance with the Transaction's binary data #436

Closed YaronWittenstein closed 2 years ago

YaronWittenstein commented 2 years ago

Depends on: #446

  1. Allocate memory to copy the transaction later.

One way to implement that would be by adding a new self.call_alloc right after this line:

 let out_tx = self.call_alloc(instance, env, tx.len())?;

Right after https://github.com/spacemeshos/svm/blob/6edb73de199fafce0953f82af061ca1090ff911c/crates/runtime/src/runtime/runtime.rs#L162

That implies that call_with_alloc expect tx: &[u8] as a parameter.

And later add

set_tx(env, tx, out_tx.returns;);

right after https://github.com/spacemeshos/svm/blob/6edb73de199fafce0953f82af061ca1090ff911c/crates/runtime/src/runtime/runtime.rs#L169

Another way is to calling self.call_alloc once asking for allocating the size of calldata.len() + tx.len().

  1. Copy the binary transaction to memory: The implementation of set_tx should share the same code as the current set_calldata. I'd suggest extracting the common code of the current set_calldata to a newset_data helper.

The implementation of set_calldata will delegate to set_data and keep the current: env.borrow_mut().set_calldata(calldata_offset, len);

Similary, the implementation of the new set_tx will also call set_data and then have: env.borrow_mut().set_data(offset, len);

Of course, the FuncEnv should be extended to support set_tx and hold tx: Option<(usize, size)> in the same way as done to the calldata here: https://github.com/spacemeshos/svm/blob/6edb73de199fafce0953f82af061ca1090ff911c/crates/runtime/src/func_env.rs#L118

  1. Remove self.run calling self.wasmer_call internally The current code has some optimization for cases the calldata would be empty:

https://github.com/spacemeshos/svm/blob/6edb73de199fafce0953f82af061ca1090ff911c/crates/runtime/src/runtime/runtime.rs#L125-L129

Since we'd always want to copy the binary transaction into Memory it'd make sense to remove this optimization and just call self.call_with_alloc. Further, it's better to keep the code simpler and cleaner. This optimization doesn't seem to be worth it in the first place.

If you remove this optimization please delete this assertion: https://github.com/spacemeshos/svm/blob/master/crates/runtime/src/runtime/runtime.rs#L160

YaronWittenstein commented 2 years ago

I'm closing this issue. Then, I'll create a new one relevant for the Simple Coin Iteration #1, which will be implemented differently than this issue.

@neysofu FYI