Skip to content

Commit

Permalink
fix:ics29: WriteAck update + adding success bool to IncentivizedAck (#…
Browse files Browse the repository at this point in the history
…952)

* fix: updating WriteAck & adding Success boolean to IncentivizedAcknowledgement

* feat: adding check of is fee enabled

* nit: change successful to underlying_application_success

* test: adding seperate test for fee disabled write async

* Update modules/apps/29-fee/ibc_module_test.go

Co-authored-by: Aditya <[email protected]>

* test: adding check to compare hash of acks

* fix: var name

Co-authored-by: Aditya <[email protected]>
  • Loading branch information
seantking and AdityaSripal authored Feb 21, 2022
1 parent c14d2b4 commit 99db143
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 36 deletions.
1 change: 1 addition & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ It contains the raw acknowledgement bytes, as well as the forward relayer addres
| ----- | ---- | ----- | ----------- |
| `result` | [bytes](#bytes) | | |
| `forward_relayer_address` | [string](#string) | | |
| `underlying_app_success` | [bool](#bool) | | |



Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (im IBCModule) OnRecvPacket(
return nil
}

return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement())
return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement(), ack.Success())
}

// OnAcknowledgementPacket implements the IBCModule interface
Expand Down
35 changes: 24 additions & 11 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
)
Expand Down Expand Up @@ -461,11 +462,18 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
true,
},
{
"source relayer is empty string",
"async write acknowledgement: ack is nil",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
// setup mock callback
suite.chainB.GetSimApp().FeeMockModule.IBCApp.OnRecvPacket = func(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {
return nil
}
},
false,
true,
true,
},
{
Expand All @@ -476,6 +484,14 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
true,
false,
},
{
"forward address is not found",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
},
false,
true,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -509,19 +525,16 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
case !tc.feeEnabled:
suite.Require().Equal(ibcmock.MockAcknowledgement, result)

case tc.forwardRelayer:
ack := types.IncentivizedAcknowledgement{
Result: ibcmock.MockAcknowledgement.Acknowledgement(),
ForwardRelayerAddress: suite.chainB.SenderAccount.GetAddress().String(),
}
suite.Require().Equal(ack, result)
case tc.forwardRelayer && result == nil:
suite.Require().Equal(nil, result)

case !tc.forwardRelayer:
ack := types.IncentivizedAcknowledgement{
expectedAck := types.IncentivizedAcknowledgement{
Result: ibcmock.MockAcknowledgement.Acknowledgement(),
ForwardRelayerAddress: "",
UnderlyingAppSuccess: true,
}
suite.Require().Equal(ack, result)
suite.Require().Equal(expectedAck, result)
}
})
}
Expand Down
9 changes: 7 additions & 2 deletions modules/apps/29-fee/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@ func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability,

// WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
// ICS29 WriteAcknowledgement is used for asynchronous acknowledgements
func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement []byte) error {
func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error {
if !k.IsFeeEnabled(ctx, packet.GetDestPort(), packet.GetDestChannel()) {
// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, acknowledgement)
}

// retrieve the forward relayer that was stored in `onRecvPacket`
packetId := channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence())
relayer, _ := k.GetForwardRelayerAddress(ctx, packetId)

k.DeleteForwardRelayerAddress(ctx, packetId)

ack := types.NewIncentivizedAcknowledgement(relayer, acknowledgement)
ack := types.NewIncentivizedAcknowledgement(relayer, acknowledgement.Acknowledgement(), acknowledgement.Success())

// ics4Wrapper may be core IBC or higher-level middleware
return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)
Expand Down
30 changes: 29 additions & 1 deletion modules/apps/29-fee/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
timeoutTimestamp,
)

ack := []byte("ack")
ack := channeltypes.NewResultAcknowledgement([]byte("success"))
chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)

// malleate test case
Expand All @@ -64,3 +64,31 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
})
}
}

func (suite *KeeperTestSuite) TestWriteAcknowledgementAsyncFeeDisabled() {
// open incentivized channel
suite.coordinator.Setup(suite.path)
suite.chainB.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, "channel-0")

// build packet
timeoutTimestamp := ^uint64(0)
packet := channeltypes.NewPacket(
[]byte("packetData"),
1,
suite.path.EndpointA.ChannelConfig.PortID,
suite.path.EndpointA.ChannelID,
suite.path.EndpointB.ChannelConfig.PortID,
suite.path.EndpointB.ChannelID,
clienttypes.ZeroHeight(),
timeoutTimestamp,
)

ack := channeltypes.NewResultAcknowledgement([]byte("success"))
chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)

err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)
suite.Require().NoError(err)

packetAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
suite.Require().Equal(packetAck, channeltypes.CommitAcknowledgement(ack.Acknowledgement()))
}
5 changes: 3 additions & 2 deletions modules/apps/29-fee/types/ack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@ import (
)

// NewIncentivizedAcknowledgement creates a new instance of IncentivizedAcknowledgement
func NewIncentivizedAcknowledgement(relayer string, ack []byte) IncentivizedAcknowledgement {
func NewIncentivizedAcknowledgement(relayer string, ack []byte, success bool) IncentivizedAcknowledgement {
return IncentivizedAcknowledgement{
Result: ack,
ForwardRelayerAddress: relayer,
UnderlyingAppSuccess: success,
}
}

// Success implements the Acknowledgement interface. The acknowledgement is
// considered successful if the forward relayer address is empty. Otherwise it is
// considered a failed acknowledgement.
func (ack IncentivizedAcknowledgement) Success() bool {
return ack.ForwardRelayerAddress != ""
return ack.UnderlyingAppSuccess
}

// Acknowledgement implements the Acknowledgement interface. It returns the
Expand Down
81 changes: 62 additions & 19 deletions modules/apps/29-fee/types/ack.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions proto/ibc/applications/fee/v1/ack.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ import "gogoproto/gogo.proto";
message IncentivizedAcknowledgement {
bytes result = 1;
string forward_relayer_address = 2 [(gogoproto.moretags) = "yaml:\"forward_relayer_address\""];
bool underlying_app_success = 3 [(gogoproto.moretags) = "yaml:\"underlying_app_successl\""];
}

0 comments on commit 99db143

Please sign in to comment.