search

KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
102 stars 94 forks source link

[bug][wasm] defi framework support beyond web_sys::window context #1948

Open ca333 opened 1 year ago

ca333 commented 1 year ago

test-environment: Chrome 116.0.5845.96 (Official Build) (x86_64) and 116.0.5845.110 (Official Build) (arm64)

issue: https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/mm2src/mm2_db/src/indexed_db/drivers/builder.rs#L76 and https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/mm2src/mm2_net/src/wasm_http.rs#L148 assume we are executing within a browser window context. However, the code can also be executed in a "background" context, such as a (service) worker or potentially other wasm runtimes ("server-side wasm"). Thus if triggered from such a background context, we face the following panics:

common:474] panicked at '!window', mm2s rc/mm2 db/src/indexed db/drivers/builder. rs: 76: 40 and common: 474] panicked at '!window', mm2src/mm2 net/src/wasm http. rs: 148:40

on a side-note: faced/facing a similar issue with wasm-timer in our deps tree, which will be resolved with https://github.com/KomodoPlatform/komodo-defi-framework/pull/1878 (thanks @ozkanonur - confirmed/tested with https://github.com/KomodoPlatform/komodo-defi-framework/tree/ca_dev) and by removing the wasm-timer dependency - was/is a similar issue (lack of worker context support) - err: common: 474] panicked at 'not in a browser', Lgithub/home/cargo/registry/src/github.com-1ecc6299db9ec823/wasm-timer-0.2.5/src/wasm.rs:58:19.


p.s. pushed a "experimental fix-attempt" in https://github.com/KomodoPlatform/komodo-defi-framework/commit/ae4a809e226f2447ab0478eff8e1e0ec3dd2e357 / https://github.com/KomodoPlatform/komodo-defi-framework/commit/f7b7f1555aaeba069a7403c9ec3287b7f80f7f03 into https://github.com/KomodoPlatform/komodo-defi-framework/commits/ca_dev_wasm (for inspiration)

onur-ozkan commented 1 year ago

Thank you for the comprehensive report and the ref work. If there isn't anyone available in the team to take on this task at the moment, I can allocate time to work on it the week after next week.

cc @KomodoPlatform/mm2

onur-ozkan commented 1 year ago

In addition, this makes me think why we don't have E2E tests for gui platforms including WASM with +80% coverage.

onur-ozkan commented 1 year ago

In addition, this makes me think why we don't have E2E tests for gui platforms including WASM with +80% coverage.

These tests could be seamlessly integrated into cross-test pipelines with frameworks such as https://zuul-ci.org. By this approach, we could ensure that pull requests causing problems to our official mm2 clients, which are challenging to identify through manual means, are never merged.