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.

Sign up for GitHub

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

Jump to bottom

feat: ics 29 packet callbacks #357

Merged
merged 43 commits into from
Dec 21, 2021
Merged

feat: ics 29 packet callbacks #357

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
7b51ebd
do handshake logic, create test file
AdityaSripal Aug 3, 2021
150211d
do cap logic and fix build
AdityaSripal Aug 5, 2021
412f3b6
Merge remote-tracking branch 'upstream/aditya/handshake' into feat/ic…
charleenfei Aug 27, 2021
734ead7
initial scaffolding
charleenfei Aug 12, 2021
1e894fe
added ics4 callbacks
charleenfei Aug 27, 2021
f735e57
Merge branch 'ics29-fee-middleware' into feat/ics-29-packetcallbacks
charleenfei Oct 6, 2021
435a2fa
Merge branch 'feat/ics-29-packetcallbacks' of github.com:cosmos/ibc-g…
charleenfei Nov 10, 2021
8d0c194
Merge branch 'ics29-fee-middleware' into feat/ics-29-packetcallbacks
charleenfei Nov 10, 2021
bfa8795
wip
charleenfei Nov 16, 2021
09985ce
update fee proto to include refund acc in storage of IdentifiedPacketFee
charleenfei Nov 16, 2021
ff7db8a
update yaml
charleenfei Nov 16, 2021
8c6d8ec
Merge branch 'update/fee_proto' into feat/ics-29-packetcallbacks
charleenfei Nov 16, 2021
9d9bcae
finish logic in packet callbacks
charleenfei Nov 16, 2021
d8e4429
wip
charleenfei Nov 17, 2021
e514f1a
store refund address in IdentifiedPacketFee
AdityaSripal Nov 17, 2021
f561cc0
Merge branch 'store_refund_address' into feat/ics-29-packetcallbacks
charleenfei Nov 18, 2021
97cc863
wip
charleenfei Nov 24, 2021
0a0022d
Merge branch 'ics29-fee-middleware' into feat/ics-29-packetcallbacks
charleenfei Nov 24, 2021
8e83ba2
proto file
charleenfei Nov 26, 2021
40951eb
incentivized ack proto
charleenfei Nov 26, 2021
e8b2ddc
Merge branch 'feat/incentivised_ack_proto' into feat/ics-29-packetcal…
charleenfei Nov 26, 2021
04763b6
wip
charleenfei Nov 30, 2021
e574499
Merge branch 'ics29-fee-middleware' of github.com:cosmos/ibc-go into …
charleenfei Nov 30, 2021
e7297a8
Merge branch 'ics29-fee-middleware' into feat/ics-29-packetcallbacks
charleenfei Nov 30, 2021
db93ac9
packet callbacks w ack
charleenfei Nov 30, 2021
2870993
testing wip
charleenfei Dec 1, 2021
64f77b9
Merge branch 'ics29-fee-middleware' of github.com:cosmos/ibc-go into …
charleenfei Dec 1, 2021
721a6c7
Merge branch 'ics29-fee-middleware' into feat/ics-29-packetcallbacks
charleenfei Dec 1, 2021
9f00d29
finished testing
charleenfei Dec 6, 2021
0844485
app changes
charleenfei Dec 7, 2021
e374cb2
initial pr review comments
charleenfei Dec 7, 2021
232e906
initial comments
charleenfei Dec 8, 2021
a8c69db
add tests
charleenfei Dec 8, 2021
6c85913
semantics
charleenfei Dec 8, 2021
9d84834
logic changes re: comments
charleenfei Dec 10, 2021
141d281
add cacheCtx
charleenfei Dec 10, 2021
76b7de9
tests
charleenfei Dec 10, 2021
7ced819
comments
charleenfei Dec 10, 2021
dc33d66
diff channel, test
charleenfei Dec 20, 2021
903b9d5
fix escrow bug
charleenfei Dec 20, 2021
8fc3aac
fix test
charleenfei Dec 20, 2021
2a061e4
sdk.AccAddress casting
charleenfei Dec 21, 2021
aae78ce
comments
charleenfei Dec 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions modules/apps/29-fee/ibc_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package fee

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
Expand Down Expand Up @@ -208,8 +206,8 @@ func (im IBCModule) OnAcknowledgementPacket(
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
}

ack := types.IncentivizedAcknowledgement{}
if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
ack := new(types.IncentivizedAcknowledgement)
if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, ack); err != nil {
return sdkerrors.Wrapf(err, "cannot unmarshal ICS-29 incentivized packet acknowledgement: %v", ack)
}

Expand All @@ -224,9 +222,10 @@ func (im IBCModule) OnAcknowledgementPacket(
// cache context before trying to distribute the fee
cacheCtx, writeFn := ctx.CacheContext()

fmt.Println("forward", ack.ForwardRelayerAddress)
forwardRelayer, _ := sdk.AccAddressFromBech32(ack.ForwardRelayerAddress)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably check the error

Copy link
Contributor Author

@charleenfei charleenfei Dec 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

yes...but then should we forego distribution entirely if this error exists? in the case only this address is incorrect or doesnt exist, then it seems extreme

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we handle this here already implicitly -> https://github.com/cosmos/ibc-go/blob/feat/ics-29-packetcallbacks/modules/apps/29-fee/keeper/escrow.go#L56

So no need to return an error here

refundAcc, _ := sdk.AccAddressFromBech32(identifiedPacketFee.RefundAddress)

err := im.keeper.DistributeFee(cacheCtx, sdk.AccAddress(identifiedPacketFee.RefundAddress), sdk.AccAddress(ack.ForwardRelayerAddress), relayer, packetId)
err := im.keeper.DistributeFee(cacheCtx, refundAcc, forwardRelayer, relayer, packetId)

if err == nil {
// write the cache and then call underlying callback
Expand Down Expand Up @@ -261,7 +260,7 @@ func (im IBCModule) OnTimeoutPacket(
// cache context before trying to distribute the fee
cacheCtx, writeFn := ctx.CacheContext()

err := im.keeper.DistributeFeeTimeout(cacheCtx, sdk.AccAddress(identifiedPacketFee.RefundAddress), relayer, channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence))
err := im.keeper.DistributeFeeTimeout(cacheCtx, sdk.AccAddress(identifiedPacketFee.RefundAddress), relayer, packetId)

if err == nil {
// write the cache and then call underlying callback
Expand Down
5 changes: 2 additions & 3 deletions modules/apps/29-fee/ibc_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,10 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
false,
},
{
"error on distribute fee",
"error on distribute fee (blocked address)",
func() {
blockedAddr := suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), types.ModuleName).GetAddress()

fmt.Println("blocked", blockedAddr.String())
ack = types.IncentivizedAcknowledgement{
Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(),
ForwardRelayerAddress: blockedAddr.String(),
Expand Down Expand Up @@ -660,7 +659,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
suite.Require().Equal(
sdk.Coin{
Denom: ibctesting.TestCoin.Denom,
Amount: sdk.NewInt(99999999999600),
Amount: sdk.NewInt(100000000000000),
},
suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom))
} else {
Expand Down