zoneconcierge: parameterise timeout for IBC packets #386
Conversation
|
|
||
| // ibc_packet_timeout_minutes is the time period after which an unrelayed | ||
| // IBC packet becomes timeout, measured in minutes | ||
| uint32 ibc_packet_timeout_minutes = 1 |
There was a problem hiding this comment.
Maybe make this seconds? Minutes seems counter-intuitive (and limiting) even though we plan to use a value in the hours range.
There was a problem hiding this comment.
I would also say that seconds are better choice. They are in general more ubiquitous interval time format used.
| // NewGenesis creates a new GenesisState instance | ||
| func NewGenesis(params Params) *GenesisState { | ||
| return &GenesisState{ | ||
| PortId: PortID, |
There was a problem hiding this comment.
Is it possible to move the port ID to params? Or this doesn't ever change?
There was a problem hiding this comment.
From my understanding the PortID should not change for a given application. An example is the interchain transfer app where port ID is also outside the params
|
|
||
| // ibc_packet_timeout_minutes is the time period after which an unrelayed | ||
| // IBC packet becomes timeout, measured in minutes | ||
| uint32 ibc_packet_timeout_minutes = 1 |
There was a problem hiding this comment.
I would also say that seconds are better choice. They are in general more ubiquitous interval time format used.
|
|
||
| // GetParams get all parameters as types.Params | ||
| // SetParams sets the x/zoneconcierge module parameters. | ||
| func (k Keeper) SetParams(ctx sdk.Context, p types.Params) error { |
There was a problem hiding this comment.
Just fyi, I params should be upgradebale by gov proposal, you need to have separate msg MsgUpdateParams which will be handled by zoneconcierge and be send by gov module. The best example how to wire it up is in btccheckpoint or epoching modules.
There was a problem hiding this comment.
Good point! Fixed
x/zoneconcierge/types/params.go
Outdated
|
|
||
| // Validate validates the set of params | ||
| func (p Params) Validate() error { | ||
| if p.IbcPacketTimeoutMinutes == 0 { |
There was a problem hiding this comment.
maybe it makes sense to also intruduce some kinda upper limit, to avoid acciendly setting timeout till the end of universe ? maybe 1 year would be good ?
|
Thanks @KonradStaniec @vitsalis for the helpful comments! I have revised the PR, including:
|
This PR introduces timeout parameter for IBC packets. It means the time taken for an unrelayed IBC packet to become timeout. Given that ZoneConcierge enforces ordered IBC channel, any timeout packet will make the IBC channel to be closed. This is necessary for preventing people from spamming Babylon with many IBC channels without using them.
The default timeout is set 24 hours. It can be upgraded by gov props. In the future maybe we can further narrow it down.