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

Out of gas handling #82

Merged
merged 9 commits into from May 18, 2020
Merged

Out of gas handling #82

merged 9 commits into from May 18, 2020

Conversation

reuvenpo
Copy link
Contributor

@reuvenpo reuvenpo commented May 13, 2020

This PR aims to fix #29 and fix #79.
It depends on CosmWasm/cosmwasm#331

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beautiful

src/api.rs Show resolved Hide resolved
src/api.rs Outdated Show resolved Hide resolved
src/iterator.rs Outdated Show resolved Hide resolved
src/iterator.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@webmaster128
Copy link
Member

webmaster128 commented May 15, 2020

Doesn't this resolve #73 as well? Or at least a big part of it?

@reuvenpo
Copy link
Contributor Author

Doesn't this resolve #73 as well? Or at least a big part of it?

To a large extent, yeah, but not fully, yet. I can mark it as related since to get all the information across from the Go callback to the Go caller, it needs to basically be fixed as well.

src/api.rs Outdated Show resolved Hide resolved
GoResult::Panic => Err(make_ffi_foreign_panic()),
GoResult::BadArgument => Err(make_ffi_bad_argument()),
GoResult::OutOfGas => Err(make_ffi_out_of_gas()),
GoResult::Other => Err(make_ffi_other("Unspecified error in Go code")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, so this is the reason we need error mutability: we only get the i32 from Go.

Copy link
Contributor Author

@reuvenpo reuvenpo May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no and yes.
we get it from go, but we also create the FfiError in go-cosmwasm, so we can have a method on GoResult that converts the GoResult to an FfiError, but also takes an Into<String> which is only used if it's Other

@webmaster128 webmaster128 merged commit d2847d0 into master May 18, 2020
@webmaster128 webmaster128 deleted the out-of-gas-handling branch May 18, 2020 15:07
faddat pushed a commit to faddat/go-cosmwasm that referenced this pull request Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return gas used also on errors Gas consumed
2 participants