Skip to content

feat: Unit tests for Packet Forward Middleware #8313

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

Open
wants to merge 4 commits into
base: tamjid/pfm-homecoming
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ require (
github.com/hdevalence/ed25519consensus v0.2.0 // indirect
github.com/holiman/uint256 v1.3.2 // indirect
github.com/huandu/skiplist v1.2.0 // indirect
github.com/iancoleman/orderedmap v0.3.0 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
github.com/icza/dyno v0.0.0-20230330125955-09f820a8d9c0 // indirect
github.com/improbable-eng/grpc-web v0.15.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,8 @@ github.com/huandu/go-assert v1.1.5/go.mod h1:yOLvuqZwmcHIC5rIzrBhT7D3Q9c3GFnd0Jr
github.com/huandu/skiplist v1.2.0 h1:gox56QD77HzSC0w+Ws3MH3iie755GBJU1OER3h5VsYw=
github.com/huandu/skiplist v1.2.0/go.mod h1:7v3iFjLcSAzO4fN5B8dvebvo/qsfumiLiDXMrPiHF9w=
github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg=
github.com/iancoleman/orderedmap v0.3.0 h1:5cbR2grmZR/DiVt+VJopEhtVs9YGInGIxAoMJn+Ichc=
github.com/iancoleman/orderedmap v0.3.0/go.mod h1:XuLcCUkdL5owUCQeF2Ue9uuw1EptkJDkXXS7VoV7XGE=
github.com/iancoleman/strcase v0.3.0 h1:nTXanmYxhfFAMjZL34Ov6gkzEsSJZ5DbhxWjvSASxEI=
github.com/iancoleman/strcase v0.3.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/packet-forward-middleware/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ Generally without `memo` to handle, all handling by this module is delegated to
5. `B` Handle denom. If denom prefix is from `B`, remove it. If denom prefix is other chain - add `B` prefix.
6. `B` Take fee, create new ICS-004 packet with timeout from forward for next step, and remaining inner `memo`.
7. `B` Send transfer to `C` with parameters obtained from `memo`. Tokens burnt or escrowed here.
8. `B` Store tracking `in flight packet` under next `(channel, port, ICS-20 transfer sequence)`, do not `ACK` packet yet.
9. `C` Handle ICS-020 packet as usual.
8. `B` Store tracking `in flight packet` under next `(channel, port, ICS-20 transfer sequence)`, do not `ACK` packet yet.
9. `C` Handle ICS-020 packet as usual.
10. `B` On ICS-020 ACK from `C` find `in flight packet`, delete it and write `ACK` for original packet from `A`.
11. `A` Handle ICS-020 `ACK` as usual

Expand Down
12 changes: 8 additions & 4 deletions modules/apps/packet-forward-middleware/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,16 @@ func (im IBCMiddleware) OnRecvPacket(

logger.Debug("packetForwardMiddleware OnRecvPacket",
"sequence", packet.Sequence,
"src-channel", packet.SourceChannel, "src-port", packet.SourcePort,
"dst-channel", packet.DestinationChannel, "dst-port", packet.DestinationPort,
"amount", data.Amount, "denom", data.Denom, "memo", data.Memo,
"src-channel", packet.SourceChannel,
"src-port", packet.SourcePort,
"dst-channel", packet.DestinationChannel,
"dst-port", packet.DestinationPort,
"amount", data.Amount,
"denom", data.Denom,
"memo", data.Memo,
)

d := make(map[string]interface{})
d := make(map[string]any)
err := json.Unmarshal([]byte(data.Memo), &d)
if err != nil || d["forward"] == nil {
// not a packet that should be forwarded
Expand Down
254 changes: 254 additions & 0 deletions modules/apps/packet-forward-middleware/ibc_middleware_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
package packetforward_test

import (
"encoding/json"
"log"
"testing"

"github.com/stretchr/testify/suite"

packetforward "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware"
packetforwardkeeper "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/keeper"
packetforwardtypes "github.com/cosmos/ibc-go/v10/modules/apps/packet-forward-middleware/types"
transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v10/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v10/testing"
)

// CallbacksTestSuite defines the needed instances and methods to test callbacks
type PFMTestSuite struct {
suite.Suite

coordinator *ibctesting.Coordinator

chainA *ibctesting.TestChain
chainB *ibctesting.TestChain
chainC *ibctesting.TestChain

pathAB *ibctesting.Path
pathBC *ibctesting.Path
}

func TestPFMTestSuite(t *testing.T) {
suite.Run(t, new(PFMTestSuite))
}

// setupChains sets up a coordinator with 2 test chains.
func (s *PFMTestSuite) setupChains() {
s.coordinator = ibctesting.NewCoordinator(s.T(), 3)
s.chainA = s.coordinator.GetChain(ibctesting.GetChainID(1))
s.chainB = s.coordinator.GetChain(ibctesting.GetChainID(2))
s.chainC = s.coordinator.GetChain(ibctesting.GetChainID(3))

s.pathAB = ibctesting.NewTransferPath(s.chainA, s.chainB)
s.pathAB.Setup()

s.pathBC = ibctesting.NewTransferPath(s.chainB, s.chainC)
s.pathBC.Setup()
}

func (s *PFMTestSuite) TestOnRecvPacket() {
Copy link
Member

Choose a reason for hiding this comment

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

Make a single TestOnRecvPacket with table test cases. See the ibc-go test cases for an example

s.setupChains()

ctx := s.chainA.GetContext()
version := s.pathAB.EndpointA.GetChannel().Version
relayerAddr := s.chainA.SenderAccount.GetAddress()

// PacketForwardMiddleware
pfm := s.pktForwardMiddleware(s.chainA, transfertypes.ModuleName)
ack := pfm.OnRecvPacket(ctx, version, channeltypes.Packet{}, relayerAddr)
s.Require().False(ack.Success())

expectedAck := &channeltypes.Acknowledgement{}
err := s.chainA.Codec.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
s.Require().NoError(err)

s.Require().Equal("ABCI code: 12: error handling packet: see events for details", expectedAck.GetError())
}

func (s *PFMTestSuite) TestOnRecvPacket_Nomemo() {
s.setupChains()

ctx := s.chainA.GetContext()
version := s.pathAB.EndpointA.GetChannel().Version
relayerAddr := s.chainA.SenderAccount.GetAddress()
receiverAddr := s.chainB.SenderAccount.GetAddress()

packet := s.transferPacket(relayerAddr.String(), receiverAddr.String(), s.pathAB, 0, "{}")

// PacketForwardMiddleware
pfm := s.pktForwardMiddleware(s.chainA, transfertypes.ModuleName)
ack := pfm.OnRecvPacket(ctx, version, packet, relayerAddr)
s.Require().True(ack.Success())

expectedAck := &channeltypes.Acknowledgement{}
err := s.chainA.Codec.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
s.Require().NoError(err)

s.Require().Equal("", expectedAck.GetError())
s.Require().ElementsMatch([]byte{1}, expectedAck.GetResult())
}

func (s *PFMTestSuite) TestOnRecvPacket_InvalidReceiver() {
s.setupChains()

ctx := s.chainA.GetContext()
version := s.pathAB.EndpointA.GetChannel().Version
relayerAddr := s.chainA.SenderAccount.GetAddress()

packet := s.transferPacket(relayerAddr.String(), "", s.pathAB, 0, nil)

// PacketForwardMiddleware
pfm := s.pktForwardMiddleware(s.chainA, transfertypes.ModuleName)
ack := pfm.OnRecvPacket(ctx, version, packet, relayerAddr)
s.Require().False(ack.Success())

expectedAck := &channeltypes.Acknowledgement{}
err := s.chainA.Codec.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
s.Require().NoError(err)

s.Require().Equal("ABCI code: 5: error handling packet: see events for details", expectedAck.GetError())
s.Require().Empty(expectedAck.GetResult())
}

func (s *PFMTestSuite) TestOnRecvPacket_NoForward() {
s.setupChains()

ctx := s.chainA.GetContext()
version := s.pathAB.EndpointA.GetChannel().Version

senderAddr := s.chainA.SenderAccount.GetAddress()
receiverAddr := s.chainB.SenderAccount.GetAddress()

packet := s.transferPacket(senderAddr.String(), receiverAddr.String(), s.pathAB, 0, nil)

// PacketForwardMiddleware
pfm := s.pktForwardMiddleware(s.chainA, transfertypes.ModuleName)
ack := pfm.OnRecvPacket(ctx, version, packet, senderAddr)
s.Require().True(ack.Success())

expectedAck := &channeltypes.Acknowledgement{}
err := s.chainA.Codec.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
s.Require().NoError(err)
s.Require().Equal("", expectedAck.GetError())

s.Require().Equal([]byte{1}, expectedAck.GetResult())
}

func (s *PFMTestSuite) TestOnRecvPacket_RecvPacketFailed() {
s.setupChains()

transferKeeper := s.chainA.GetSimApp().TransferKeeper
ctx := s.chainA.GetContext()
// Also can be done if send amount is 0
transferKeeper.SetParams(ctx, transfertypes.Params{ReceiveEnabled: false})

version := s.pathAB.EndpointA.GetChannel().Version

senderAddr := s.chainA.SenderAccount.GetAddress()
receiverAddr := s.chainB.SenderAccount.GetAddress()
metadata := &packetforwardtypes.PacketMetadata{
Forward: &packetforwardtypes.ForwardMetadata{
Receiver: receiverAddr.String(),
Port: s.pathAB.EndpointA.ChannelConfig.PortID,
Channel: s.pathAB.EndpointA.ChannelID,
},
}
packet := s.transferPacket(senderAddr.String(), receiverAddr.String(), s.pathAB, 0, metadata)

// PacketForwardMiddleware
pfm := s.pktForwardMiddleware(s.chainA, transfertypes.ModuleName)
ack := pfm.OnRecvPacket(ctx, version, packet, senderAddr)
s.Require().False(ack.Success())

expectedAck := &channeltypes.Acknowledgement{}

err := s.chainA.Codec.UnmarshalJSON(ack.Acknowledgement(), expectedAck)
s.Require().NoError(err)
log.Printf("Error: %s", expectedAck.GetError())
s.Require().Equal("packet-forward-middleware error: error receiving packet: ack error: {\"error\":\"ABCI code: 8: error handling packet: see events for details\"}", expectedAck.GetError())

s.Require().Equal([]byte(nil), expectedAck.GetResult())
}

func (s *PFMTestSuite) TestOnRecvPacket_ForwardNoFee() {
s.setupChains()

senderAddr := s.chainA.SenderAccount.GetAddress()
receiverAddr := s.chainC.SenderAccount.GetAddress()
metadata := &packetforwardtypes.PacketMetadata{
Forward: &packetforwardtypes.ForwardMetadata{
Receiver: receiverAddr.String(),
Port: s.pathBC.EndpointA.ChannelConfig.PortID,
Channel: s.pathBC.EndpointA.ChannelID,
},
}
packet := s.transferPacket(senderAddr.String(), receiverAddr.String(), s.pathAB, 0, metadata)
version := s.pathAB.EndpointA.GetChannel().Version
ctxB := s.chainB.GetContext()

pfmB := s.pktForwardMiddleware(s.chainB, transfertypes.ModuleName)
ack := pfmB.OnRecvPacket(ctxB, version, packet, senderAddr)
s.Require().Nil(ack)

// Check that chain C has received the packet
//
ctxC := s.chainC.GetContext()
packet = s.transferPacket(senderAddr.String(), receiverAddr.String(), s.pathBC, 0, nil)
version = s.pathBC.EndpointA.GetChannel().Version

pfmC := s.pktForwardMiddleware(s.chainC, transfertypes.ModuleName)
ack = pfmC.OnRecvPacket(ctxC, version, packet, senderAddr)
s.Require().NotNil(ack)

// Ack on chainC
packet = s.transferPacket(senderAddr.String(), receiverAddr.String(), s.pathBC, 1, nil)
err := pfmC.OnAcknowledgementPacket(ctxC, version, packet, ack.Acknowledgement(), senderAddr)
s.Require().NoError(err)

// Ack on ChainB
err = pfmB.OnAcknowledgementPacket(ctxB, version, packet, ack.Acknowledgement(), senderAddr)
s.Require().NoError(err)
}

func (s *PFMTestSuite) pktForwardMiddleware(chain *ibctesting.TestChain, module string) packetforward.IBCMiddleware {

Check failure on line 214 in modules/apps/packet-forward-middleware/ibc_middleware_test.go

View workflow job for this annotation

GitHub Actions / lint

(*PFMTestSuite).pktForwardMiddleware - module always receives transfertypes.ModuleName ("transfer") (unparam)

Check failure on line 214 in modules/apps/packet-forward-middleware/ibc_middleware_test.go

View workflow job for this annotation

GitHub Actions / lint

(*PFMTestSuite).pktForwardMiddleware - module always receives transfertypes.ModuleName ("transfer") (unparam)
Copy link
Member

Choose a reason for hiding this comment

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

As linter says, PFM is always wrapping transfer so we don't need to do this

pfmKeeper := chain.GetSimApp().PFMKeeper

ibcModule, ok := chain.App.GetIBCKeeper().PortKeeper.Route(module)
s.Require().True(ok)

ibcMiddleware := packetforward.NewIBCMiddleware(ibcModule, &pfmKeeper, 0, packetforwardkeeper.DefaultForwardTransferPacketTimeoutTimestamp)
return ibcMiddleware
}

func (s *PFMTestSuite) transferPacket(sender string, receiver string, path *ibctesting.Path, seq uint64, metadata any) channeltypes.Packet {
s.T().Helper()
tokenPacket := transfertypes.FungibleTokenPacketData{
Denom: "uatom",
Amount: "100",
Sender: sender,
Receiver: receiver,
}

if metadata != nil {
if mStr, ok := metadata.(string); ok {
tokenPacket.Memo = mStr
} else {
memo, err := json.Marshal(metadata)
s.Require().NoError(err)
tokenPacket.Memo = string(memo)
}
}

tokenData, err := transfertypes.ModuleCdc.MarshalJSON(&tokenPacket)
s.Require().NoError(err)

return channeltypes.Packet{
SourcePort: path.EndpointA.ChannelConfig.PortID,
SourceChannel: path.EndpointA.ChannelID,
DestinationPort: path.EndpointB.ChannelConfig.PortID,
DestinationChannel: path.EndpointB.ChannelID,
Data: tokenData,
Sequence: seq,
}
}
9 changes: 6 additions & 3 deletions modules/apps/packet-forward-middleware/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,12 @@ func (k *Keeper) ForwardTransferPacket(
)

k.Logger(ctx).Debug("packetForwardMiddleware ForwardTransferPacket",
"port", metadata.Port, "channel", metadata.Channel,
"sender", receiver, "receiver", metadata.Receiver,
"amount", token.Amount.String(), "denom", token.Denom,
"port", metadata.Port,
"channel", metadata.Channel,
"sender", receiver,
"receiver", metadata.Receiver,
"amount", token.Amount.String(),
"denom", token.Denom,
)

// send tokens to destination
Expand Down
1 change: 1 addition & 0 deletions modules/light-clients/08-wasm/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ require (
github.com/herumi/bls-eth-go-binary v1.31.0 // indirect
github.com/holiman/uint256 v1.3.2 // indirect
github.com/huandu/skiplist v1.2.0 // indirect
github.com/iancoleman/orderedmap v0.3.0 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
github.com/improbable-eng/grpc-web v0.15.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions modules/light-clients/08-wasm/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,7 @@ github.com/huandu/go-assert v1.1.5/go.mod h1:yOLvuqZwmcHIC5rIzrBhT7D3Q9c3GFnd0Jr
github.com/huandu/skiplist v1.2.0 h1:gox56QD77HzSC0w+Ws3MH3iie755GBJU1OER3h5VsYw=
github.com/huandu/skiplist v1.2.0/go.mod h1:7v3iFjLcSAzO4fN5B8dvebvo/qsfumiLiDXMrPiHF9w=
github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg=
github.com/iancoleman/orderedmap v0.3.0 h1:5cbR2grmZR/DiVt+VJopEhtVs9YGInGIxAoMJn+Ichc=
github.com/iancoleman/strcase v0.3.0 h1:nTXanmYxhfFAMjZL34Ov6gkzEsSJZ5DbhxWjvSASxEI=
github.com/iancoleman/strcase v0.3.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down
1 change: 1 addition & 0 deletions simapp/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ require (
github.com/hdevalence/ed25519consensus v0.2.0 // indirect
github.com/holiman/uint256 v1.3.2 // indirect
github.com/huandu/skiplist v1.2.0 // indirect
github.com/iancoleman/orderedmap v0.3.0 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
github.com/improbable-eng/grpc-web v0.15.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions simapp/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,7 @@ github.com/huandu/go-assert v1.1.5/go.mod h1:yOLvuqZwmcHIC5rIzrBhT7D3Q9c3GFnd0Jr
github.com/huandu/skiplist v1.2.0 h1:gox56QD77HzSC0w+Ws3MH3iie755GBJU1OER3h5VsYw=
github.com/huandu/skiplist v1.2.0/go.mod h1:7v3iFjLcSAzO4fN5B8dvebvo/qsfumiLiDXMrPiHF9w=
github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg=
github.com/iancoleman/orderedmap v0.3.0 h1:5cbR2grmZR/DiVt+VJopEhtVs9YGInGIxAoMJn+Ichc=
github.com/iancoleman/strcase v0.3.0 h1:nTXanmYxhfFAMjZL34Ov6gkzEsSJZ5DbhxWjvSASxEI=
github.com/iancoleman/strcase v0.3.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down
Loading
Loading