-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix(engine): return engine error from instruction invocation #655
fix(engine): return engine error from instruction invocation #655
Conversation
b95435f
to
973276b
Compare
973276b
to
3c8e589
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, much simpler than what I was trying to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, the new error message Fee transaction rejected: Shard was rejected: Validators decided to abort: Execution failure: Runtime error: Resource did not contain sufficient balance: Bucket contained insufficient revealed funds. Required: 1500, Available: 100
this will be super useful when testing
* development: fix!: peer sync (tari-project#657) fix(vn/ui): committee bucket information (tari-project#656) fix(engine): return engine error from instruction invocation (tari-project#655) fix(walletd): fix race condition causing transaction to be marked as invalid (tari-project#654) ci: double test timeout fix(vn-webui): fix incorrect jsonrpc address (tari-project#645) fix: add feeclaim_xxx address parsing (tari-project#648)
Description
fix(engine): return engine error from instruction invocation
fix: only unlock on abort if inputs were originally locked
fix(wallet): propagate result error from vn to wallet cli
fix(wallet/cli): enable amounts bigger than u32::MAX to be input as an arg
Motivation and Context
Hook up errors returned by the runtime to the final transaction result. This takes precedence over a panic (which MUST always occur after an engine error).
Hook up the result properly so that it is returned even when the transaction is aborted. This allows the client to get the actual error that occurred during execution.
How Has This Been Tested?
New engine unit test that performs an invalid engine call, the WASM panics and we check that the result includes the underlying engine error rather than the null ptr panic error.
Manually, send more funds than available.
What process can a PR reviewer use to test or verify this change?
Create a wallet and send more funds than available, the error is now output rather than a unknown error or WASM panic text
Breaking Changes