-
Notifications
You must be signed in to change notification settings - Fork 170
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
ibc: vanilla IBC module #176
Conversation
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.
only small comments:)
Just a question, you mention that next task is developing a forked version of IBC-Go that has hooks notifying new CZ headers
, is this still necessary only for oracle service ?
app/app.go
Outdated
@@ -155,6 +170,8 @@ var ( | |||
stakingtypes.BondedPoolName: {authtypes.Burner, authtypes.Staking}, | |||
stakingtypes.NotBondedPoolName: {authtypes.Burner, authtypes.Staking}, | |||
govtypes.ModuleName: {authtypes.Burner}, | |||
// TODO: decide ZonConcierge's permissiones here |
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.
small nit: permissions
"github.com/spf13/cobra" | ||
|
||
"github.com/cosmos/cosmos-sdk/client" | ||
// "github.com/cosmos/cosmos-sdk/client/flags" |
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.
do we need this commented out thingy ?
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.
This is automatically generated by Ignite CLI. Looks like this package is responsible for parsing flags in CLI. Maybe leave it for now and it might be useful for CLIs in this module.
} | ||
|
||
// OnRecvPacket implements the IBCModule interface | ||
func (im IBCModule) OnRecvPacket( |
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.
just to confirm, this will be main handler in which we will handle data send to babylon ?
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.
Yep. More specifically,
OnRecvPacket
will handle all IBC packets sent from CZs to Babylon- We are looking for a way to get notifications of new headers sent from CZs to Babylon. At the moment this is hidden in IBC's light client and the application layer cannot get any notification. The above future work on forking IBC-Go is one option. Another possible option is to wrap the client module just like what epoching does to staking. Still investigating other less intrusive option and will keep the team updated.
@KonradStaniec After several offline discussions with @fishermanymc , we found that forking IBC-Go is the only viable approach. The only approach I can find other than forking IBC-GO is to intercept headers in a new AnteHandler and inject our logic there. This approach is not desirable since
Thus, the only option left is to fork IBC-Go, use hooks to intercept headers (pre and post verification), and add fork choice rules. |
In addition, headers at the antehandler hasn't passed tendermint consensus yet. So there is no guarantee that all Babylon validators will have the same availability and order of the headers.
… On Oct 27, 2022, at 16:07, Runchao Han ***@***.***> wrote:
@KonradStaniec After several offline discussions with @fishermanymc , we found that forking IBC-Go is the only viable approach. The only approach I can find other than forking IBC-GO is to intercept headers in a new AnteHandler and inject our logic there. This approach is not desirable since
It allows us to receive headers, but does not avoid light clients to freeze upon forks with a valid quorum. The latter is desired for the timestamping service.
It requires AnteHanlder to do the stateful check, i.e., verifying the header's correctness against the current light client ledger. However, AnteHandler is not designed for stateful checks.
Thus, the only option left is to fork IBC-Go, use hooks to intercept headers (pre and post verification), and add fork choice rules.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Fixes BM-247
This PR adds a vanilla IBC-enabled module
zoneconcierge
for Babylon. This module will be used for timestamping CZ headers.The name "Zone concierge" is the idea of @fishermanymc
Steps of finishing this PR
I did the following steps to finish this PR:
ignite scaffold module zoneconcierge --ibc
make proto-gen
and making some minor modificationsignite
and follows the approaches recommended by the Cosmos SDK maintainers.zoneconcierge
module into BabylonApp, by following the official tutorial, the example SimApp, and Cosmos Hub.Functionalities of this module
The new
coincierge
module will be responsible forFuture works
After this PR, we will do the following to implement the canonical chain oracle:
zoneconcierge
to subscribe hooks of new CZ headers