wasm-forge / ic-wasi-polyfill

The polyfill implementation for WASI functions in the IC environment
MIT License
17 stars 4 forks source link

init broken when canister init/post_upgrade have params #4

Closed lastmjs closed 1 year ago

lastmjs commented 1 year ago

I have a pretty unique situation so I'm not exactly sure if this issue stems from what I'm describing. In Kybra (Python CDK), we are using a post_install script with a deployer canister to install over-sized Wasm binaries. In init and post_upgrade I make sure to call ic_wasi_polyfill::init(0); as the first line. But, when init or post_upgrade have params, I keep getting [Canister rrkah-fqaaa-aaaaa-aaaaq-cai] Panicked at 'calledOption::unwrap()on aNonevalue', /home/lastmjs/.config/kybra/1.68.2/git/checkouts/ic-wasi-polyfill-1dfc1df43c8aa084/40f3ab7/src/lib.rs:673:32.

I'm guessing this is because the Candid serialization/deserialization is happening before the call in init/post_upgrade, and some of that triggers some Wasi polyfill code that expects the RNG to exist.

I'm not sure the best path forward, but an easy fix would be to ensure the RNG starts off with a seed of 0 if necessary, so that at least it won't break. Right now I can't even install/post_upgrade, so in my fork I'll have to do my fix.

wasm-forge commented 1 year ago

Please note, in order to support the 32-byte seed I've changed the interface a bit:

#[ic_cdk::init]
fn init() {
    ic_wasi_polyfill::init(&[0u8;32]);
}
lastmjs commented 1 year ago

Of course, I'm behind a bit because of some dependency issues with Candid

lastmjs commented 1 year ago

I'm on the latest and am using the new interface, this problem still persists for me. It seems to be a pattern that it presents on tests with post_upgrade parameters

lastmjs commented 1 year ago

I would guess that __ic_custom_random_get is being called before the canister has a chance to call init when there are params?

lastmjs commented 1 year ago

Perhaps something like this could fix the issue, though I'm not sure it's the best solution:

static RNG : RefCell<Option<rand::rngs::StdRng>> = RefCell::new(Some(rand::rngs::StdRng::from_seed([0u8; 32])));
lastmjs commented 1 year ago

This is still a problem for me. I'm now integrating ic-wasi-polyfill into a new project, Azle, and this line is throwing even though I call ic_wasi_polyfill::init as the very first thing in my init/post_upgrade methods: https://github.com/wasm-forge/ic-wasi-polyfill/blob/main/src/lib.rs#L882

It seems that __ic_custom_random_get is called before I get a chance to call ic_wasi_polyfill::init, thus there is a panic. That seed should always have a value or something else should be done, I don't know how to get around this right now but to fork the repo and provide this fix myself.

wasm-forge commented 1 year ago

I've changed the implementation to avoid uninitialized usage of the seed. The first call would use the default seed instead of throwing exception.

wasm-forge commented 1 year ago

Fixed in this pull request: https://github.com/wasm-forge/ic-wasi-polyfill/pull/10