From 27d234b15362981299a96b7189e8e01ad0a6ac3e Mon Sep 17 00:00:00 2001 From: Gjermund Garaba Date: Fri, 26 Jul 2024 11:45:21 +0200 Subject: [PATCH] test: add NewForwardErrorAcknowledgement to multihop forwarding test (#6907) * test: add NewForwardErrorAcknowledgement to multihop forwarding test * add forward packet deletion assertion + fix wrong packet used in ack fetch --- .../transfer/keeper/relay_forwarding_test.go | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/modules/apps/transfer/keeper/relay_forwarding_test.go b/modules/apps/transfer/keeper/relay_forwarding_test.go index 542af7a4f59..8e54eac8f6e 100644 --- a/modules/apps/transfer/keeper/relay_forwarding_test.go +++ b/modules/apps/transfer/keeper/relay_forwarding_test.go @@ -1145,6 +1145,9 @@ func (suite *ForwardingTestSuite) TestForwardingWithMoreThanOneHop() { suite.Require().NoError(err) } +// TestMultihopForwardingErrorAcknowledgement tests the scenario in which a packet goes from +// A to D, using B and C as forwarding hops. The packet fails on D where we set the ReceiveEnabled +// param to false. We verify that funds are properly returned to A. func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() { // Setup A->B->C->D coinOnA := ibctesting.TestCoin @@ -1226,7 +1229,7 @@ func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() { err = pathCtoD.EndpointB.UpdateClient() suite.Require().NoError(err) - // force an error acknowledgement by disabling the receive param on chain D. + // force an error acknowledgement by disabling the ReceiveEnabled param on chain D. ctx := pathCtoD.EndpointB.Chain.GetContext() pathCtoD.EndpointB.Chain.GetSimApp().TransferKeeper.SetParams(ctx, types.NewParams(true, false)) @@ -1234,6 +1237,10 @@ func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() { suite.Require().NoError(err) suite.Require().NotNil(result) + // forward packet exists on C before ack + _, found := suite.chainC.GetSimApp().TransferKeeper.GetForwardedPacket(suite.chainC.GetContext(), pathCtoD.EndpointA.ChannelConfig.PortID, pathCtoD.EndpointA.ChannelID, packetFromBtoC.GetSequence()) + suite.Require().True(found) + // propagate the acknowledgement from chain D to chain A. ack, err := ibctesting.ParseAckFromEvents(result.Events) suite.Require().NoError(err) @@ -1242,15 +1249,46 @@ func (suite *ForwardingTestSuite) TestMultihopForwardingErrorAcknowledgement() { result, err = pathCtoD.EndpointA.AcknowledgePacketWithResult(packetFromCtoD, ack) suite.Require().NoError(err) + // forward packet is deleted after ack + _, found = suite.chainC.GetSimApp().TransferKeeper.GetForwardedPacket(suite.chainC.GetContext(), pathCtoD.EndpointA.ChannelConfig.PortID, pathCtoD.EndpointA.ChannelID, packetFromBtoC.GetSequence()) + suite.Require().False(found) + + // Ensure that chainC has an ack. + storedAck, found := suite.chainC.App.GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainC.GetContext(), pathBtoC.EndpointB.ChannelConfig.PortID, pathBtoC.EndpointB.ChannelID, packetFromBtoC.GetSequence()) + suite.Require().True(found, "chainC does not have an ack") + + // And that this ack is of the type we expect (Error due to ReceiveEnabled param being false) + initialErrorAck := channeltypes.NewErrorAcknowledgement(types.ErrReceiveDisabled) + forwardErrorAck := internaltypes.NewForwardErrorAcknowledgement(packetFromCtoD, initialErrorAck) + ackbytes := channeltypes.CommitAcknowledgement(forwardErrorAck.Acknowledgement()) + suite.Require().Equal(ackbytes, storedAck) + ack, err = ibctesting.ParseAckFromEvents(result.Events) suite.Require().NoError(err) err = pathBtoC.EndpointA.UpdateClient() suite.Require().NoError(err) + // forward packet exists on B before ack + _, found = suite.chainB.GetSimApp().TransferKeeper.GetForwardedPacket(suite.chainB.GetContext(), pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, packetFromAtoB.GetSequence()) + suite.Require().True(found) + result, err = pathBtoC.EndpointA.AcknowledgePacketWithResult(packetFromBtoC, ack) suite.Require().NoError(err) + // forward packet is deleted after ack + _, found = suite.chainB.GetSimApp().TransferKeeper.GetForwardedPacket(suite.chainB.GetContext(), pathBtoC.EndpointA.ChannelConfig.PortID, pathBtoC.EndpointA.ChannelID, packetFromAtoB.GetSequence()) + suite.Require().False(found) + + // Ensure that chainB has an ack. + storedAck, found = suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), pathAtoB.EndpointB.ChannelConfig.PortID, pathAtoB.EndpointB.ChannelID, packetFromAtoB.GetSequence()) + suite.Require().True(found, "chainB does not have an ack") + + // And that this ack is of the type we expect (Error due to ReceiveEnabled param being false) + forwardErrorAck = internaltypes.NewForwardErrorAcknowledgement(packetFromBtoC, forwardErrorAck) + ackbytes = channeltypes.CommitAcknowledgement(forwardErrorAck.Acknowledgement()) + suite.Require().Equal(ackbytes, storedAck) + ack, err = ibctesting.ParseAckFromEvents(result.Events) suite.Require().NoError(err)