Add relayer address to module callbacks #206
Conversation
ethanfrey
requested review from
AdityaSripal,
colin-axner and
fedekunze
as code owners
ethanfrey
force-pushed
the
expose-packet-signer
branch
from
ed583f4 to
0ad5eec
Compare
|
Thanks for the quick review. I updated the CHANGELOG, but I am not sure how to maintain the migration file, and would be happy if you could handle that |
Codecov Report
@@ Coverage Diff @@
## main #206 +/- ##
==========================================
- Coverage 80.26% 80.17% -0.09%
==========================================
Files 109 109
Lines 6485 6497 +12
==========================================
+ Hits 5205 5209 +4
- Misses 925 929 +4
- Partials 355 359 +4
|
| @@ -527,6 +538,11 @@ func (k Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*c | |||
| func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeoutOnClose) (*channeltypes.MsgTimeoutOnCloseResponse, error) { | |||
| ctx := sdk.UnwrapSDKContext(goCtx) | |||
|
|
|||
| relayer, err := sdk.AccAddressFromBech32(msg.Signer) | |||
There was a problem hiding this comment.
@AdityaSripal this was already implemented.
Timeout and TimeoutOnClose messages both call the same module callback, so I had to update them to get the code to compile
|
This pr is outdated based on recent discussion, correct? A generalized fee wouldn't pass the relayer address into the application module callbacks? |
|
This is not outdated. The implementation or general fee payment may use application logic (with decorators for example). Or may be baked in a lower level. I personally think we will likely use the module interface with some decorator. A few people requested that it be possible for applications to incentivize directly (like the DEX paying for incoming orders, or some altruistic pool sponsoring one app/channel). This is absolutely needed for that use-case. |
|
cosmos/ibc#579 is merged. Are there any other changes requested on this before merge? |
I just wanted to add a little note in the migration docs. I didn't have the permissions to push directly to this branch |
update migration docs
@colin-axner I merged your PR. Thanks for the notes. I still can't find out why all my branches on forks do not allow pushing from maintainers. (This goes for other repos too) I have looked before but not found the proper switch |
Very odd. Well, opening a pr to your fork works for now 👍 |
|
@ethanfrey could you update to the latest commit of main? Then it should auto merge |
Description
Implements cosmos/ibc#579
Adds the
relayer sdk.AccAddresstoOnRecvPacket,OnAcknowledgementPacket,OnTimeoutPacketcallbacks forIBCModule.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes