Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Wasmer to 0.17.0 and upgrade downcasting of errors #2055

Closed
ilblackdragon opened this issue Feb 1, 2020 · 13 comments · Fixed by #2668
Closed

Upgrade Wasmer to 0.17.0 and upgrade downcasting of errors #2055

ilblackdragon opened this issue Feb 1, 2020 · 13 comments · Fixed by #2668
Assignees
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) P-high Priority: High

Comments

@ilblackdragon
Copy link
Member

ilblackdragon commented Feb 1, 2020

We need to upgrade to Wasmer 0.17.0 and upgrade downcasting of errors such that we can diffirentiate between host and guest panics, see: wasmerio/wasmer#1328.
0.17.0 also includes fixes for the following critical bugs:
wasmerio/wasmer#1436
wasmerio/wasmer#1412

And it includes NaN canonicalization: wasmerio/wasmer#1303

For more context see #2316 (comment) . Currently we are printing "unknown" error all the time. See the report below.


Receiving output in nearcore:

Bad error case! Output is non-deterministic TypeId { t: 7549865886324542212 } "unknown error"

Steps to repro:

Output from near:

Error:  { Error: The used access key requires exactly one FunctionCall action
    at Object.parseRpcError (/usr/local/lib/node_modules/near-shell/node_modules/nearlib/lib/utils/rpc_errors.js:25:19)
    at Account.signAndSendTransaction (/usr/local/lib/node_modules/near-shell/node_modules/nearlib/lib/account.js:87:36)
    at process._tickCallback (internal/process/next_tick.js:68:7) type: 'ActionError', index: 0 }
@ilblackdragon
Copy link
Member Author

Ok, isolated it to having prints in my contracts.
We should probably either block contracts with prints or make this errors more eligible.

The "used access key requires" is different errors, both in nearlib and somewhere else. Investigating.

@ilblackdragon
Copy link
Member Author

Filed the error as separate issue in nearlib - near/near-api-js#210

@evgenykuzyakov
Copy link
Collaborator

The first part:

Bad error case! Output is non-deterministic TypeId { t: 7549865886324542212 } "unknown error"

Might be related to Wasm Traps. It should be addressed by near/near-sdk-rs#73

@SkidanovAlex SkidanovAlex added this to the MainNet milestone Mar 4, 2020
@SkidanovAlex SkidanovAlex added the A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) label Mar 4, 2020
@ilblackdragon ilblackdragon removed this from the MainNet milestone Mar 20, 2020
@ilblackdragon
Copy link
Member Author

@evgenykuzyakov can you test and closed if it's fixed?

@MaksymZavershynskyi
Copy link
Contributor

MaksymZavershynskyi commented Mar 22, 2020

I think it is still happening. I asked Wasmer to clarify why is it happening.

This is blocked by Wasmer response and therefore I assigned pessimistic ETA in case modifications in Wasmer are needed.

@evgenykuzyakov
Copy link
Collaborator

I'm not working on this

@MaksymZavershynskyi
Copy link
Contributor

Blocked by: wasmerio/wasmer#1328

@MaksymZavershynskyi
Copy link
Contributor

Should be resolved by #2316

@lexfrl
Copy link
Contributor

lexfrl commented Apr 6, 2020

Should be resolved by #2316

Unfortunately, Wasmer doesn't return ExceptionCode's today (for single pass backend). Let's wait for wasmerio/wasmer#1328

@MaksymZavershynskyi
Copy link
Contributor

More context:
#2316 (comment)

@MaksymZavershynskyi
Copy link
Contributor

Blocked by wasmerio/wasmer#1338

@MaksymZavershynskyi MaksymZavershynskyi changed the title Calling function prints unexpected log in nearcore binary Upgrade downcasting of Wasmer errors. Attempt 2 May 5, 2020
@MaksymZavershynskyi MaksymZavershynskyi added the P-high Priority: High label May 11, 2020
@MaksymZavershynskyi MaksymZavershynskyi changed the title Upgrade downcasting of Wasmer errors. Attempt 2 Upgrade Wasmer to 0.17.0 and upgrade downcasting of errors May 11, 2020
@olonho
Copy link
Contributor

olonho commented May 18, 2020

Seems due to rust-lang/rust#68905 we have to update the Rust toolchain.

@olonho
Copy link
Contributor

olonho commented May 18, 2020

nightly-2020-05-16 works fine.

olonho added a commit that referenced this issue May 18, 2020
Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 18, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 18, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 18, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
olonho added a commit that referenced this issue May 21, 2020
Used https://crates.io/crates/rust-latest to find latest suitable nightly.

Fixes #2055

Test plan
---------
cargo test --all
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-transaction-runtime Area: transaction runtime (transaction and receipts processing, state transition, etc) P-high Priority: High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants