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

[Do not merge] ADR-8 vertical spike #1368

Closed
wants to merge 3 commits into from
Closed

[Do not merge] ADR-8 vertical spike #1368

wants to merge 3 commits into from

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Apr 27, 2023

Spike to discuss a concrete example.
This includes a some of hacks to make it work without modifying wasmvm.

A good start would be to look at TestIBCAdr8WithTransfer for the test scenario. Ics29 fees or edge cases are not covered
The middleware integration is done in app.go

For the callback to the contracts, 2 new methods are added to the wasmvm interface and mocked for proof of concept

  • OnIBCPacketAcked
  • OnIBCPacketTimedOut

For the callback registration a custom message was added that the reflect contract emits in the test:

type RegisterPacketProcessedCallback struct {
	// Sequence is the ibc packet sequence number
	Sequence            uint64 `json:"sequence"`
	ChannelID           string `json:"channel_id"`
	PortID              string `json:"port_id"`
	MaxCallbackGasLimit uint64 `json:"max_callback_gas_limit"`
}

The message is handled in the XMessageHandler and contains some ugly hack for the reflect contract.

Authorization

  • on callback registration, it is checked that the actor is a contract
  • on ack/timeout handling, the packet sender is only accepted for a registered hook

New Packages

  • xwasmvm are extensions to WasmVm for the spike only. If they make sense, then they should be moved upstream
  • xadr8 are common parts that are not CosmWasm specific. If they make sense, they could be living in ibc-go for example

x/xadr8/packet_decoder.go Fixed Show fixed Hide fixed
@alpe alpe added the spike Demo to showcase an idea. label Apr 27, 2023
var _ porttypes.Middleware = &IBCMiddleware{}

// IBCMiddlewareAdapter is an adapter to build a Middleware for IBC modules
type IBCMiddlewareAdapter struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unused

package xadr8

import (
"reflect"

Check notice

Code scanning / CodeQL

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism
type UnRegisterPacketProcessedCallback struct {
Sequence uint64 `json:"sequence"`
ChannelID string `json:"channel_id"`
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

requires PortID as well

@alpe
Copy link
Contributor Author

alpe commented May 19, 2023

In case somebody reads the article by the IBC team which goes in some different direction. This spike was built for them to demo the integration and callback registration. There were also a call and long discussions on internal slack.
This is an important feature for the community. We need to discuss internally how to proceed, now.

@webmaster128
Copy link
Member

webmaster128 commented Jul 10, 2024

Closing in favour of ADR-8 implementation deployed as part of 0.52

@webmaster128 webmaster128 deleted the adr8_spike branch July 10, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spike Demo to showcase an idea.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants