dfinity / interface-spec

IC Interface Specification
https://khsfq-wqaaa-aaaak-qckvq-cai.icp0.io/docs
37 stars 20 forks source link

enable calling stable memory system API in WASM start function #140

Closed mraszyk closed 1 year ago

mraszyk commented 1 year ago

The mainnet's WASM execution engine is going to convert calls to stable memory system API into native memory operations which makes it challenging to prevent calls to stable memory system API in the WASM start function. Since we do not see any important reason to prevent such calls in the WASM start function, we adjust the specification to enable them.

netlify[bot] commented 1 year ago

Deploy Preview for ic-interface-spec ready!

Name Link
Latest commit 921ad20a30f3cfd4790ddfdc498c766398b85315
Latest deploy log https://app.netlify.com/sites/ic-interface-spec/deploys/6452b7cfe722ed000808e7a0
Deploy Preview https://deploy-preview-140--ic-interface-spec.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

nomeata commented 1 year ago

There were some good Wasmy reasons from @rossberg why we usually don't allow API access from the start function. @crusso, do you remember?

Maybe stable memory API is fine, since its supposed to be a stop gap for direct memory instructions.

Why is canister_init not good enough, though?

mraszyk commented 1 year ago

Maybe stable memory API is fine, since its supposed to be a stop gap for direct memory instructions.

I didn't get this comment.

Why is canister_init not good enough, though?

With an upcoming feature, it'd be more difficult to prevent such calls to stable memory API in the start function so let's just allow them.

nomeata commented 1 year ago

Sorry for being vage. I think the argument was something like that the Wasm standard makes no guarantee that imported functions are available and fully working before the start function is run, and I think if you have multiple wasm modules that mutually import each other, then some start function has to be run first.

It's mostly theoretical, I guess, in our environment we know that the system API works already in start.

Maybe don't worry then :-)

adambratschikaye commented 1 year ago

I looked into this a little bit - it does seem to be true that the spec leaves it to the embedder to decide how imports are resolved, so it would be possible for a given embedder to allow mutual imports between two modules. But on the other hand, the spec does specify that imports can be called from the start function (see these tests). It also looks like Wasmtime itself wasn't set up to allow mutual imports - when instantiating a module, it's expected that all it's imports are provided (see here or here). I think you could still hack it together by instantiating separate Wasmtime Instances and then providing implementations that call each other, but I don't really think that's something we would want to do.