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

func (vm *VM) IBCPacketReceive consistency #398

Closed
webmaster128 opened this issue Jan 26, 2023 · 4 comments · Fixed by #465
Closed

func (vm *VM) IBCPacketReceive consistency #398

webmaster128 opened this issue Jan 26, 2023 · 4 comments · Fixed by #465
Milestone

Comments

@webmaster128
Copy link
Member

Analogue to to the other IBC functions in lib.go, IBCPacketReceive should return an IBCReceiveResponse instead of IBCReceiveResult as the first return value. This basically reverts c300106.

Currently wasmd requires special error handling to handle both error types: https://github.com/CosmWasm/wasmd/blob/c7670fae4e6ca24d7fd6bfa6768f166b36976432/x/wasm/keeper/relay.go#L135-L137

A more straight forward way to do this adding this back to wasmvm:

	if resp.Err != "" {
		return nil, gasUsed, fmt.Errorf("%s", resp.Err)
	}
	return resp.Ok, gasUsed, nil

Now in order to allow wasmd to get full access to the error returned by the contract, we can use a dedicated error type for that case.

@webmaster128 webmaster128 added this to the 2.0.0 milestone Jan 26, 2023
@webmaster128
Copy link
Member Author

The other (maybe simpler) option is to always return the inner result to wasmd

@alpe
Copy link
Contributor

alpe commented Mar 27, 2023

The error handling in wasmd is going to be modified in CosmWasm/wasmd#1220 to handle resp.Err and errors returned to the function call differently. Therefore the current api in wasmvm makes sense so that we can separate the different cases and provide a simple way for contract developers to create error ACKs.

@webmaster128
Copy link
Member Author

Therefore the current api in wasmvm makes sense so that we can separate the different cases and provide a simple way for contract developers to create error ACKs.

Right. Then we should always return the result for the other APIs at the same level for consistency. wasmd can then handle the cases separately or just turn the error result in an error.

@alpe
Copy link
Contributor

alpe commented Mar 27, 2023

For all other IBC methods we do not have 2 different error scenarios with different db transaction behaviour. IMHO we should accept that IBCPacketReceive is different and not make everything equal.
Is there a simple way to communicate this in code and doc better?

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 a pull request may close this issue.

2 participants