From b7fbb394cfed0f6f9cd43dd15723f50df6bd8b6b Mon Sep 17 00:00:00 2001 From: Charly Date: Mon, 18 Dec 2023 19:15:43 +0100 Subject: [PATCH 01/18] add logic for setting next seq recv, update tests --- modules/core/04-channel/keeper/upgrade.go | 5 +++++ .../core/04-channel/keeper/upgrade_test.go | 22 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 975cdc83bec..53ed8d23137 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -539,6 +539,10 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } + if channel.Ordering == types.ORDERED && upgrade.Fields.Ordering == types.UNORDERED { + k.SetNextSequenceAck(ctx, portID, channelID, 0) + } + // Switch channel fields to upgrade fields and set channel state to OPEN previousState := channel.State channel.Ordering = upgrade.Fields.Ordering @@ -546,6 +550,7 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin channel.ConnectionHops = upgrade.Fields.ConnectionHops channel.State = types.OPEN + k.SetNextSequenceRecv(ctx, portID, channelID, upgrade.LatestSequenceSend+1) k.SetChannel(ctx, portID, channelID, channel) // delete state associated with upgrade which is no longer required. diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 177629d568f..b3299fa3020 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1140,6 +1140,13 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) + // send second packet + sequence0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packet0) + suite.Require().NoError(err) + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED @@ -1150,11 +1157,19 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) - // Ack packet to delete packet commitment before calling WriteUpgradeOpenChannel + // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) + err = path.EndpointA.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) + ctx := suite.chainA.GetContext() + // assert that sequence (set in ChanUpgradeTry) is still 1, because channel was UNORDERED/not updated + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + tc.malleate() if tc.expPanic { @@ -1170,6 +1185,11 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.Require().Equal(mock.UpgradeVersion, channel.Version) suite.Require().Equal(types.ORDERED, channel.Ordering) + // assert that NextSeqRecv is now 3, because seq updated in WriteUpgradeOpenChannel to latest sequence (two packets sent) + 1 + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(3), seq) + // Assert that state stored for upgrade has been deleted upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().Equal(types.Upgrade{}, upgrade) From a3a10ab1704a84dd329006bff28e96344c74c1b7 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 10:41:34 +0100 Subject: [PATCH 02/18] update next seq recv and ack logic to spec --- modules/core/04-channel/keeper/upgrade.go | 19 ++- .../core/04-channel/keeper/upgrade_test.go | 154 +++++++++++++++++- 2 files changed, 162 insertions(+), 11 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 53ed8d23137..50a8c025485 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -539,8 +539,24 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } + counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) + if !found { + panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) + } + + // next seq recv and ack is used for ordered channels to verify the packet has been received/acked in the correct order + // this is no longer necessary if the channel is UNORDERED and should be reset to 1 if channel.Ordering == types.ORDERED && upgrade.Fields.Ordering == types.UNORDERED { - k.SetNextSequenceAck(ctx, portID, channelID, 0) + k.SetNextSequenceRecv(ctx, portID, channelID, 1) + k.SetNextSequenceAck(ctx, portID, channelID, 1) + } + + // next seq recv and ack should updated when moving from UNORDERED to ORDERED using the latest packet sequence sent before the upgrade + // we can be sure that the next packet we are set to receive will be the first packet the counterparty sends after reopening. + // we can be sure that our next acknowledgement will be our first packet sent after upgrade, as the counterparty processed all sent packets after flushing completes. + if channel.Ordering == types.UNORDERED && upgrade.Fields.Ordering == types.ORDERED { + k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.LatestSequenceSend+1) + k.SetNextSequenceAck(ctx, portID, channelID, upgrade.LatestSequenceSend+1) } // Switch channel fields to upgrade fields and set channel state to OPEN @@ -550,7 +566,6 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin channel.ConnectionHops = upgrade.Fields.ConnectionHops channel.State = types.OPEN - k.SetNextSequenceRecv(ctx, portID, channelID, upgrade.LatestSequenceSend+1) k.SetChannel(ctx, portID, channelID, channel) // delete state associated with upgrade which is no longer required. diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index b3299fa3020..baca9bef7cf 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1096,7 +1096,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpen() { } } -func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { +func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { var path *ibctesting.Path testCases := []struct { @@ -1140,11 +1140,11 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - // send second packet - sequence0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + // send second packet from B to A + sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointB.RecvPacket(packet0) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointA.RecvPacket(packet0) suite.Require().NoError(err) path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion @@ -1161,15 +1161,21 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - err = path.EndpointA.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) suite.Require().NoError(err) ctx := suite.chainA.GetContext() - // assert that sequence (set in ChanUpgradeTry) is still 1, because channel was UNORDERED/not updated + + // assert that sequence (reset in ChanUpgradeTry) is 1 because channel is UNORDERED seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) + // assert that NextSeqAck is 1 because channel is UNORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + tc.malleate() if tc.expPanic { @@ -1185,10 +1191,140 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.Require().Equal(mock.UpgradeVersion, channel.Version) suite.Require().Equal(types.ORDERED, channel.Ordering) - // assert that NextSeqRecv is now 3, because seq updated in WriteUpgradeOpenChannel to latest sequence (two packets sent) + 1 + // assert that NextSeqRecv is now 2, because seq updated in WriteUpgradeOpenChannel to latest sequence (two packets sent) + 1 seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) - suite.Require().Equal(uint64(3), seq) + suite.Require().Equal(uint64(2), seq) + + // assert that NextSeqAck is incremented to 2 because channel is now ORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + + // Assert that state stored for upgrade has been deleted + upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(types.Upgrade{}, upgrade) + suite.Require().False(found) + + counterpartyUpgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(types.Upgrade{}, counterpartyUpgrade) + suite.Require().False(found) + + // events := ctx.EventManager().Events().ToABCIEvents() + // expEvents := ibctesting.EventsMap{ + // types.EventTypeChannelUpgradeOpen: { + // types.AttributeKeyPortID: path.EndpointA.ChannelConfig.PortID, + // types.AttributeKeyChannelID: path.EndpointA.ChannelID, + // types.AttributeCounterpartyPortID: path.EndpointB.ChannelConfig.PortID, + // types.AttributeCounterpartyChannelID: path.EndpointB.ChannelID, + // types.AttributeKeyChannelState: types.OPEN.String(), + // types.AttributeKeyUpgradeConnectionHops: channel.ConnectionHops[0], + // types.AttributeKeyUpgradeVersion: channel.Version, + // types.AttributeKeyUpgradeOrdering: channel.Ordering.String(), + // types.AttributeKeyUpgradeSequence: fmt.Sprintf("%d", channel.UpgradeSequence), + // }, + // sdk.EventTypeMessage: { + // sdk.AttributeKeyModule: types.AttributeValueCategory, + // }, + // } + // ibctesting.AssertEventsLegacy(&suite.Suite, expEvents, events) + } + }) + } +} + +func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { + var path *ibctesting.Path + + testCases := []struct { + name string + malleate func() + expPanic bool + }{ + { + name: "success", + malleate: func() {}, + expPanic: false, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + + path = ibctesting.NewPath(suite.chainA, suite.chainB) + path.EndpointA.ChannelConfig.Order = types.ORDERED + path.EndpointB.ChannelConfig.Order = types.ORDERED + suite.coordinator.Setup(path) + + // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packet) + suite.Require().NoError(err) + + // send second packet from B to A + sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointA.RecvPacket(packet0) + suite.Require().NoError(err) + + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED + + suite.Require().NoError(path.EndpointA.ChanUpgradeInit()) + suite.Require().NoError(path.EndpointB.ChanUpgradeTry()) + suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) + suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) + + // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel + err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) + + err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) + + ctx := suite.chainA.GetContext() + + // assert that NextSeqAck is incremented to 2 because channel is still ordered + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + + // assert that sequence (set in ChanUpgradeTry) is incremented to 2 because channel is still ordered + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + + tc.malleate() + + if tc.expPanic { + suite.Require().Panics(func() { + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + }) + } else { + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + channel := path.EndpointA.GetChannel() + + // Assert that channel state has been updated + suite.Require().Equal(types.OPEN, channel.State) + suite.Require().Equal(mock.UpgradeVersion, channel.Version) + suite.Require().Equal(types.UNORDERED, channel.Ordering) + + // assert that NextSeqRecv is now 1, because channel is now UNORDERED + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + + // assert that NextSeqAck is now 1, because channel is now UNORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) // Assert that state stored for upgrade has been deleted upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) From 129cc62a96959a05cc41b7eb8c213e16d64e89cb Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 10:44:07 +0100 Subject: [PATCH 03/18] typos --- modules/core/04-channel/keeper/upgrade.go | 2 +- modules/core/04-channel/keeper/upgrade_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 50a8c025485..5f7d0534f59 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -541,7 +541,7 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) if !found { - panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) + panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } // next seq recv and ack is used for ordered channels to verify the packet has been received/acked in the correct order diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index baca9bef7cf..10be6c6d522 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1191,8 +1191,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { suite.Require().Equal(mock.UpgradeVersion, channel.Version) suite.Require().Equal(types.ORDERED, channel.Ordering) - // assert that NextSeqRecv is now 2, because seq updated in WriteUpgradeOpenChannel to latest sequence (two packets sent) + 1 - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED + // seq updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 suite.Require().True(found) suite.Require().Equal(uint64(2), seq) From c95192b04db37c6c572afd88147374048a3b4771 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 10:54:28 +0100 Subject: [PATCH 04/18] update test --- modules/core/04-channel/keeper/upgrade.go | 10 +++++----- modules/core/04-channel/keeper/upgrade_test.go | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 5f7d0534f59..d39763e3509 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -539,11 +539,6 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } - counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) - if !found { - panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) - } - // next seq recv and ack is used for ordered channels to verify the packet has been received/acked in the correct order // this is no longer necessary if the channel is UNORDERED and should be reset to 1 if channel.Ordering == types.ORDERED && upgrade.Fields.Ordering == types.UNORDERED { @@ -555,6 +550,11 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin // we can be sure that the next packet we are set to receive will be the first packet the counterparty sends after reopening. // we can be sure that our next acknowledgement will be our first packet sent after upgrade, as the counterparty processed all sent packets after flushing completes. if channel.Ordering == types.UNORDERED && upgrade.Fields.Ordering == types.ORDERED { + counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) + if !found { + panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) + } + k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.LatestSequenceSend+1) k.SetNextSequenceAck(ctx, portID, channelID, upgrade.LatestSequenceSend+1) } diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 10be6c6d522..4fae5b3e2db 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1193,6 +1193,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED // seq updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) From 972faa6ae927ae5649c63a3fd2cbe18d263ee3d6 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 12:13:11 +0100 Subject: [PATCH 05/18] wip changes for spec --- modules/core/04-channel/keeper/packet_test.go | 24 ------ modules/core/04-channel/keeper/upgrade.go | 11 +-- .../core/04-channel/keeper/upgrade_test.go | 6 +- modules/core/04-channel/types/upgrade.go | 8 +- modules/core/04-channel/types/upgrade.pb.go | 78 +++++++++---------- proto/ibc/core/channel/v1/upgrade.proto | 12 +-- testing/endpoint.go | 4 +- 7 files changed, 58 insertions(+), 85 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index b2af9329765..fd5895771f3 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -351,30 +351,6 @@ func (suite *KeeperTestSuite) TestRecvPacket() { channel := path.EndpointB.GetChannel() channel.State = types.FLUSHING path.EndpointB.SetChannel(channel) - - // set last packet sent sequence to sequence + 1 - counterpartyUpgrade := types.Upgrade{LatestSequenceSend: sequence + 1} - path.EndpointB.SetChannelCounterpartyUpgrade(counterpartyUpgrade) - }, - nil, - }, - { - "success with an counterparty latest sequence send set to 0", - func() { - suite.coordinator.Setup(path) - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) - suite.Require().NoError(err) - - packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) - - channel := path.EndpointB.GetChannel() - channel.State = types.FLUSHING - path.EndpointB.SetChannel(channel) - - // set last packet sent sequence to zero. - counterpartyUpgrade := types.Upgrade{LatestSequenceSend: 0} - path.EndpointB.SetChannelCounterpartyUpgrade(counterpartyUpgrade) }, nil, }, diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index d39763e3509..19ba9e79930 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -432,12 +432,9 @@ func (k Keeper) WriteUpgradeConfirmChannel(ctx sdk.Context, portID, channelID st previousState := channel.State channel.State = types.FLUSHCOMPLETE k.SetChannel(ctx, portID, channelID, channel) + k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", channel.State) - } else { - // the counterparty upgrade is only required if the channel is still in the FLUSHING state. - // this gets read when timing out and acknowledging packets. - k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) } return channel } @@ -555,8 +552,8 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } - k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.LatestSequenceSend+1) - k.SetNextSequenceAck(ctx, portID, channelID, upgrade.LatestSequenceSend+1) + k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.NextPacketSend) + k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextPacketSend) } // Switch channel fields to upgrade fields and set channel state to OPEN @@ -794,7 +791,7 @@ func (k Keeper) startFlushing(ctx sdk.Context, portID, channelID string, upgrade return errorsmod.Wrapf(types.ErrSequenceSendNotFound, "port ID (%s) channel ID (%s)", portID, channelID) } - upgrade.LatestSequenceSend = nextSequenceSend - 1 + upgrade.NextPacketSend = nextSequenceSend upgrade.Timeout = k.getAbsoluteUpgradeTimeout(ctx) k.SetUpgrade(ctx, portID, channelID, *upgrade) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 4fae5b3e2db..c976417d3f9 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -303,7 +303,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() { nextSequenceSend, found := path.EndpointB.Chain.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceSend(path.EndpointB.Chain.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) suite.Require().True(found) - suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend) + suite.Require().Equal(nextSequenceSend, upgrade.NextPacketSend) } else { suite.assertUpgradeError(err, tc.expError) } @@ -489,7 +489,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() { "fails due to proof verification failure, counterparty update has unexpected sequence", func() { // Decrementing LatestSequenceSend is sufficient to cause the proof to fail. - counterpartyUpgrade.LatestSequenceSend-- + counterpartyUpgrade.NextPacketSend-- }, commitmenttypes.ErrInvalidProof, }, @@ -2039,7 +2039,7 @@ func (suite *KeeperTestSuite) TestStartFlush() { suite.Require().True(ok) suite.Require().Equal(types.FLUSHING, channel.State) - suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend) + suite.Require().Equal(nextSequenceSend, upgrade.NextPacketSend) expectedTimeoutTimestamp := types.DefaultTimeout.Timestamp + uint64(suite.chainB.GetContext().BlockTime().UnixNano()) suite.Require().Equal(expectedTimeoutTimestamp, upgrade.Timeout.Timestamp) diff --git a/modules/core/04-channel/types/upgrade.go b/modules/core/04-channel/types/upgrade.go index e60fc3f624b..40eb943ee80 100644 --- a/modules/core/04-channel/types/upgrade.go +++ b/modules/core/04-channel/types/upgrade.go @@ -12,11 +12,11 @@ import ( ) // NewUpgrade creates a new Upgrade instance. -func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, latestPacketSent uint64) Upgrade { +func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, nextPacketSend uint64) Upgrade { return Upgrade{ - Fields: upgradeFields, - Timeout: timeout, - LatestSequenceSend: latestPacketSent, + Fields: upgradeFields, + Timeout: timeout, + NextPacketSend: nextPacketSend, } } diff --git a/modules/core/04-channel/types/upgrade.pb.go b/modules/core/04-channel/types/upgrade.pb.go index 7764b163ff7..16a110c3579 100644 --- a/modules/core/04-channel/types/upgrade.pb.go +++ b/modules/core/04-channel/types/upgrade.pb.go @@ -25,13 +25,13 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // Upgrade is a verifiable type which contains the relevant information // for an attempted upgrade. It provides the proposed changes to the channel -// end, the timeout for this upgrade attempt and the latest packet sequence sent +// end, the timeout for this upgrade attempt and the next packet sequence // which allows the counterparty to efficiently know the highest sequence it has received. -// The latest sequence send is used for pruning and upgrading from unordered to ordered channels. +// The next packet send is used for pruning and upgrading from unordered to ordered channels. type Upgrade struct { - Fields UpgradeFields `protobuf:"bytes,1,opt,name=fields,proto3" json:"fields"` - Timeout Timeout `protobuf:"bytes,2,opt,name=timeout,proto3" json:"timeout"` - LatestSequenceSend uint64 `protobuf:"varint,3,opt,name=latest_sequence_send,json=latestSequenceSend,proto3" json:"latest_sequence_send,omitempty"` + Fields UpgradeFields `protobuf:"bytes,1,opt,name=fields,proto3" json:"fields"` + Timeout Timeout `protobuf:"bytes,2,opt,name=timeout,proto3" json:"timeout"` + NextPacketSend uint64 `protobuf:"varint,3,opt,name=next_packet_send,json=nextPacketSend,proto3" json:"next_packet_send,omitempty"` } func (m *Upgrade) Reset() { *m = Upgrade{} } @@ -160,33 +160,33 @@ func init() { func init() { proto.RegisterFile("ibc/core/channel/v1/upgrade.proto", fileDescriptor_fb1cef68588848b2) } var fileDescriptor_fb1cef68588848b2 = []byte{ - // 407 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x92, 0xc1, 0x6e, 0xd4, 0x30, - 0x10, 0x86, 0xe3, 0x76, 0xd5, 0xed, 0x1a, 0x28, 0x92, 0xe9, 0x21, 0x5a, 0xa1, 0x74, 0xd9, 0x0b, - 0x7b, 0x69, 0xdc, 0x16, 0x84, 0x00, 0x71, 0x40, 0x95, 0x40, 0x88, 0x0b, 0x92, 0x17, 0x2e, 0x5c, - 0x56, 0x1b, 0x67, 0xc8, 0x5a, 0x4a, 0x3c, 0xc1, 0x76, 0x22, 0xf1, 0x06, 0x1c, 0xfb, 0x08, 0xbc, - 0x0a, 0xb7, 0x1e, 0x7b, 0xe4, 0x84, 0xd0, 0xee, 0x8b, 0xa0, 0x38, 0x49, 0x11, 0x52, 0x6e, 0x99, - 0xcc, 0xf7, 0xff, 0xfe, 0xc7, 0x63, 0xfa, 0x48, 0x25, 0x92, 0x4b, 0x34, 0xc0, 0xe5, 0x66, 0xad, - 0x35, 0xe4, 0xbc, 0x3e, 0xe7, 0x55, 0x99, 0x99, 0x75, 0x0a, 0x71, 0x69, 0xd0, 0x21, 0x7b, 0xa0, - 0x12, 0x19, 0x37, 0x48, 0xdc, 0x21, 0x71, 0x7d, 0x3e, 0x3d, 0xce, 0x30, 0x43, 0xdf, 0xe7, 0xcd, - 0x57, 0x8b, 0x4e, 0x07, 0xdd, 0x7a, 0x95, 0x47, 0xe6, 0x3f, 0x09, 0x1d, 0x7f, 0x6a, 0xfd, 0xd9, - 0x6b, 0x7a, 0xf0, 0x45, 0x41, 0x9e, 0xda, 0x90, 0xcc, 0xc8, 0xe2, 0xce, 0xc5, 0x3c, 0x1e, 0x38, - 0x2a, 0xee, 0xe8, 0xb7, 0x9e, 0xbc, 0x1c, 0x5d, 0xff, 0x3e, 0x09, 0x44, 0xa7, 0x63, 0xaf, 0xe8, - 0xd8, 0xa9, 0x02, 0xb0, 0x72, 0xe1, 0x9e, 0xb7, 0x78, 0x38, 0x68, 0xf1, 0xb1, 0x65, 0x3a, 0x71, - 0x2f, 0x61, 0x67, 0xf4, 0x38, 0x5f, 0x3b, 0xb0, 0x6e, 0x65, 0xe1, 0x6b, 0x05, 0x5a, 0xc2, 0xca, - 0x82, 0x4e, 0xc3, 0xfd, 0x19, 0x59, 0x8c, 0x04, 0x6b, 0x7b, 0xcb, 0xae, 0xb5, 0x04, 0x9d, 0xbe, - 0x1c, 0x7d, 0xff, 0x71, 0x12, 0xcc, 0xaf, 0x08, 0xbd, 0xf7, 0x5f, 0x2a, 0xf6, 0x8c, 0x1e, 0xa2, - 0x49, 0xc1, 0x28, 0x9d, 0xf9, 0x59, 0x8e, 0x2e, 0xa6, 0x83, 0x41, 0x3e, 0x34, 0x90, 0xb8, 0x65, - 0xd9, 0x63, 0x7a, 0x5f, 0xa2, 0xd6, 0x20, 0x9d, 0x42, 0xbd, 0xda, 0x60, 0x69, 0xc3, 0xbd, 0xd9, - 0xfe, 0x62, 0x22, 0x8e, 0xfe, 0xfd, 0x7e, 0x87, 0xa5, 0x65, 0x21, 0x1d, 0xd7, 0x60, 0xac, 0x42, - 0xed, 0xd3, 0x4d, 0x44, 0x5f, 0x76, 0x91, 0xde, 0xd3, 0xbb, 0x6f, 0x8c, 0x41, 0x23, 0x40, 0x82, - 0x2a, 0x1d, 0x9b, 0xd2, 0xc3, 0x7e, 0x26, 0x1f, 0x68, 0x24, 0x6e, 0xeb, 0xc6, 0xab, 0x00, 0x6b, - 0xd7, 0x19, 0xf8, 0x4b, 0x9b, 0x88, 0xbe, 0x6c, 0xbd, 0x2e, 0x97, 0xd7, 0xdb, 0x88, 0xdc, 0x6c, - 0x23, 0xf2, 0x67, 0x1b, 0x91, 0xab, 0x5d, 0x14, 0xdc, 0xec, 0xa2, 0xe0, 0xd7, 0x2e, 0x0a, 0x3e, - 0xbf, 0xc8, 0x94, 0xdb, 0x54, 0x49, 0x2c, 0xb1, 0xe0, 0x12, 0x6d, 0x81, 0x96, 0xab, 0x44, 0x9e, - 0x66, 0xc8, 0xeb, 0xe7, 0xbc, 0xc0, 0xb4, 0xca, 0xc1, 0xb6, 0xfb, 0x3f, 0x7b, 0x7a, 0xda, 0x3f, - 0x01, 0xf7, 0xad, 0x04, 0x9b, 0x1c, 0xf8, 0xf5, 0x3f, 0xf9, 0x1b, 0x00, 0x00, 0xff, 0xff, 0xc0, - 0xc7, 0xee, 0x2b, 0x71, 0x02, 0x00, 0x00, + // 408 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x41, 0x8e, 0xd3, 0x30, + 0x14, 0x86, 0xe3, 0x99, 0x6a, 0x3a, 0x35, 0x50, 0x90, 0x61, 0x11, 0x55, 0x28, 0x53, 0xba, 0xa1, + 0x9b, 0x89, 0x99, 0x01, 0x21, 0x40, 0x2c, 0xd0, 0x48, 0x20, 0xc4, 0x06, 0xe4, 0x81, 0x0d, 0x9b, + 0xaa, 0x71, 0x1e, 0xa9, 0x45, 0xe3, 0x17, 0x6c, 0x27, 0x82, 0x1b, 0xb0, 0x9c, 0x23, 0x70, 0x0d, + 0x6e, 0x30, 0xcb, 0x59, 0xb2, 0x42, 0xa8, 0xbd, 0x08, 0x8a, 0x93, 0x14, 0x21, 0x65, 0x97, 0xf7, + 0xf2, 0xfd, 0xbf, 0x7f, 0xfb, 0xa7, 0xf7, 0x54, 0x22, 0xb9, 0x44, 0x03, 0x5c, 0xae, 0x96, 0x5a, + 0xc3, 0x9a, 0x57, 0x27, 0xbc, 0x2c, 0x32, 0xb3, 0x4c, 0x21, 0x2e, 0x0c, 0x3a, 0x64, 0xb7, 0x55, + 0x22, 0xe3, 0x1a, 0x89, 0x5b, 0x24, 0xae, 0x4e, 0x26, 0x77, 0x32, 0xcc, 0xd0, 0xff, 0xe7, 0xf5, + 0x57, 0x83, 0x4e, 0x7a, 0xdd, 0x3a, 0x95, 0x47, 0x66, 0x3f, 0x09, 0x1d, 0x7e, 0x68, 0xfc, 0xd9, + 0x0b, 0x7a, 0xf0, 0x49, 0xc1, 0x3a, 0xb5, 0x21, 0x99, 0x92, 0xf9, 0xb5, 0xd3, 0x59, 0xdc, 0x73, + 0x54, 0xdc, 0xd2, 0xaf, 0x3c, 0x79, 0x36, 0xb8, 0xfc, 0x7d, 0x14, 0x88, 0x56, 0xc7, 0x9e, 0xd3, + 0xa1, 0x53, 0x39, 0x60, 0xe9, 0xc2, 0x3d, 0x6f, 0x71, 0xb7, 0xd7, 0xe2, 0x7d, 0xc3, 0xb4, 0xe2, + 0x4e, 0xc2, 0xe6, 0xf4, 0x96, 0x86, 0xaf, 0x6e, 0x51, 0x2c, 0xe5, 0x67, 0x70, 0x0b, 0x0b, 0x3a, + 0x0d, 0xf7, 0xa7, 0x64, 0x3e, 0x10, 0xe3, 0x7a, 0xff, 0xce, 0xaf, 0xcf, 0x41, 0xa7, 0xcf, 0x06, + 0xdf, 0x7f, 0x1c, 0x05, 0xb3, 0x0b, 0x42, 0x6f, 0xfc, 0x97, 0x86, 0x3d, 0xa6, 0x87, 0x68, 0x52, + 0x30, 0x4a, 0x67, 0xfe, 0x0e, 0xe3, 0xd3, 0x49, 0x6f, 0x80, 0xb7, 0x35, 0x24, 0x76, 0x2c, 0xbb, + 0x4f, 0x6f, 0x4a, 0xd4, 0x1a, 0xa4, 0x53, 0xa8, 0x17, 0x2b, 0x2c, 0x6c, 0xb8, 0x37, 0xdd, 0x9f, + 0x8f, 0xc4, 0xf8, 0xdf, 0xfa, 0x35, 0x16, 0x96, 0x85, 0x74, 0x58, 0x81, 0xb1, 0x0a, 0xb5, 0x4f, + 0x36, 0x12, 0xdd, 0xd8, 0x46, 0x7a, 0x43, 0xaf, 0xbf, 0x34, 0x06, 0x8d, 0x00, 0x09, 0xaa, 0x70, + 0x6c, 0x42, 0x0f, 0x2d, 0x7c, 0x29, 0x41, 0x4b, 0xf0, 0x81, 0x06, 0x62, 0x37, 0xd7, 0x5e, 0x39, + 0x58, 0xbb, 0xcc, 0xc0, 0x3f, 0xd6, 0x48, 0x74, 0x63, 0xe3, 0x75, 0x76, 0x7e, 0xb9, 0x89, 0xc8, + 0xd5, 0x26, 0x22, 0x7f, 0x36, 0x11, 0xb9, 0xd8, 0x46, 0xc1, 0xd5, 0x36, 0x0a, 0x7e, 0x6d, 0xa3, + 0xe0, 0xe3, 0xd3, 0x4c, 0xb9, 0x55, 0x99, 0xc4, 0x12, 0x73, 0x2e, 0xd1, 0xe6, 0x68, 0xb9, 0x4a, + 0xe4, 0x71, 0x86, 0xbc, 0x7a, 0xc2, 0x73, 0x4c, 0xcb, 0x35, 0xd8, 0xa6, 0xf7, 0x07, 0x8f, 0x8e, + 0xbb, 0xea, 0xdd, 0xb7, 0x02, 0x6c, 0x72, 0xe0, 0x6b, 0x7f, 0xf8, 0x37, 0x00, 0x00, 0xff, 0xff, + 0x71, 0xde, 0xa0, 0x6a, 0x69, 0x02, 0x00, 0x00, } func (m *Upgrade) Marshal() (dAtA []byte, err error) { @@ -209,8 +209,8 @@ func (m *Upgrade) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if m.LatestSequenceSend != 0 { - i = encodeVarintUpgrade(dAtA, i, uint64(m.LatestSequenceSend)) + if m.NextPacketSend != 0 { + i = encodeVarintUpgrade(dAtA, i, uint64(m.NextPacketSend)) i-- dAtA[i] = 0x18 } @@ -337,8 +337,8 @@ func (m *Upgrade) Size() (n int) { n += 1 + l + sovUpgrade(uint64(l)) l = m.Timeout.Size() n += 1 + l + sovUpgrade(uint64(l)) - if m.LatestSequenceSend != 0 { - n += 1 + sovUpgrade(uint64(m.LatestSequenceSend)) + if m.NextPacketSend != 0 { + n += 1 + sovUpgrade(uint64(m.NextPacketSend)) } return n } @@ -484,9 +484,9 @@ func (m *Upgrade) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 3: if wireType != 0 { - return fmt.Errorf("proto: wrong wireType = %d for field LatestSequenceSend", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field NextPacketSend", wireType) } - m.LatestSequenceSend = 0 + m.NextPacketSend = 0 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowUpgrade @@ -496,7 +496,7 @@ func (m *Upgrade) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - m.LatestSequenceSend |= uint64(b&0x7F) << shift + m.NextPacketSend |= uint64(b&0x7F) << shift if b < 0x80 { break } diff --git a/proto/ibc/core/channel/v1/upgrade.proto b/proto/ibc/core/channel/v1/upgrade.proto index 294a8b8c21e..4ba9967a874 100644 --- a/proto/ibc/core/channel/v1/upgrade.proto +++ b/proto/ibc/core/channel/v1/upgrade.proto @@ -9,15 +9,15 @@ import "ibc/core/channel/v1/channel.proto"; // Upgrade is a verifiable type which contains the relevant information // for an attempted upgrade. It provides the proposed changes to the channel -// end, the timeout for this upgrade attempt and the latest packet sequence sent -// which allows the counterparty to efficiently know the highest sequence it has received. -// The latest sequence send is used for pruning and upgrading from unordered to ordered channels. +// end, the timeout for this upgrade attempt and the next packet sequence +// which allows the counterparty to efficiently know the highest sequence it has received. +// The next packet send is used for pruning and upgrading from unordered to ordered channels. message Upgrade { option (gogoproto.goproto_getters) = false; - UpgradeFields fields = 1 [(gogoproto.nullable) = false]; - Timeout timeout = 2 [(gogoproto.nullable) = false]; - uint64 latest_sequence_send = 3; + UpgradeFields fields = 1 [(gogoproto.nullable) = false]; + Timeout timeout = 2 [(gogoproto.nullable) = false]; + uint64 next_packet_send = 3; } // UpgradeFields are the fields in a channel end which may be changed diff --git a/testing/endpoint.go b/testing/endpoint.go index 80e7c4dcda8..143f5928dbc 100644 --- a/testing/endpoint.go +++ b/testing/endpoint.go @@ -868,8 +868,8 @@ func (endpoint *Endpoint) GetProposedUpgrade() channeltypes.Upgrade { ConnectionHops: []string{endpoint.ConnectionID}, Version: endpoint.ChannelConfig.Version, }, - Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0), - LatestSequenceSend: 0, + Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0), + NextPacketSend: 0, } override := endpoint.ChannelConfig.ProposedUpgrade From 3864e91a0a84ea3f4f77cdca648d4600ef7d6853 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 12:24:47 +0100 Subject: [PATCH 06/18] wip set counterparty upgrade --- modules/core/04-channel/keeper/upgrade.go | 4 ++-- modules/core/04-channel/keeper/upgrade_test.go | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 19ba9e79930..eaab6d3c4d0 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -432,10 +432,10 @@ func (k Keeper) WriteUpgradeConfirmChannel(ctx sdk.Context, portID, channelID st previousState := channel.State channel.State = types.FLUSHCOMPLETE k.SetChannel(ctx, portID, channelID, channel) - k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) - k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", channel.State) } + + k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) return channel } diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index c976417d3f9..1465d8c9207 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -936,15 +936,12 @@ func (suite *KeeperTestSuite) TestWriteUpgradeConfirm() { if !tc.hasPacketCommitments { suite.Require().Equal(types.FLUSHCOMPLETE, channel.State) - // Counterparty was set in UPGRADETRY but without timeout, latest sequence send set. - _, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().False(ok, "counterparty upgrade should not be present when there are no in flight packets") } else { suite.Require().Equal(types.FLUSHING, channel.State) - counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(ok, "counterparty upgrade should be present when there are in flight packets") - suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) } + counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(ok, "counterparty upgrade should be present when there are in flight packets") + suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) }) } } From 122c25b4d9c0473b17f43e2e9994a0ff0c24fc10 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 13:26:29 +0100 Subject: [PATCH 07/18] update naming --- modules/core/04-channel/keeper/upgrade.go | 6 +- .../core/04-channel/keeper/upgrade_test.go | 6 +- modules/core/04-channel/types/upgrade.go | 8 +- modules/core/04-channel/types/upgrade.pb.go | 76 +++++++++---------- proto/ibc/core/channel/v1/upgrade.proto | 8 +- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index eaab6d3c4d0..ba221e4fcff 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -552,8 +552,8 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } - k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.NextPacketSend) - k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextPacketSend) + k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.NextSequenceSend) + k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextSequenceSend) } // Switch channel fields to upgrade fields and set channel state to OPEN @@ -791,7 +791,7 @@ func (k Keeper) startFlushing(ctx sdk.Context, portID, channelID string, upgrade return errorsmod.Wrapf(types.ErrSequenceSendNotFound, "port ID (%s) channel ID (%s)", portID, channelID) } - upgrade.NextPacketSend = nextSequenceSend + upgrade.NextSequenceSend = nextSequenceSend upgrade.Timeout = k.getAbsoluteUpgradeTimeout(ctx) k.SetUpgrade(ctx, portID, channelID, *upgrade) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 1465d8c9207..10edcc65f8e 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -303,7 +303,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() { nextSequenceSend, found := path.EndpointB.Chain.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceSend(path.EndpointB.Chain.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) suite.Require().True(found) - suite.Require().Equal(nextSequenceSend, upgrade.NextPacketSend) + suite.Require().Equal(nextSequenceSend, upgrade.NextSequenceSend) } else { suite.assertUpgradeError(err, tc.expError) } @@ -489,7 +489,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() { "fails due to proof verification failure, counterparty update has unexpected sequence", func() { // Decrementing LatestSequenceSend is sufficient to cause the proof to fail. - counterpartyUpgrade.NextPacketSend-- + counterpartyUpgrade.NextSequenceSend-- }, commitmenttypes.ErrInvalidProof, }, @@ -2036,7 +2036,7 @@ func (suite *KeeperTestSuite) TestStartFlush() { suite.Require().True(ok) suite.Require().Equal(types.FLUSHING, channel.State) - suite.Require().Equal(nextSequenceSend, upgrade.NextPacketSend) + suite.Require().Equal(nextSequenceSend, upgrade.NextSequenceSend) expectedTimeoutTimestamp := types.DefaultTimeout.Timestamp + uint64(suite.chainB.GetContext().BlockTime().UnixNano()) suite.Require().Equal(expectedTimeoutTimestamp, upgrade.Timeout.Timestamp) diff --git a/modules/core/04-channel/types/upgrade.go b/modules/core/04-channel/types/upgrade.go index 40eb943ee80..d96e41f5e3c 100644 --- a/modules/core/04-channel/types/upgrade.go +++ b/modules/core/04-channel/types/upgrade.go @@ -12,11 +12,11 @@ import ( ) // NewUpgrade creates a new Upgrade instance. -func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, nextPacketSend uint64) Upgrade { +func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, nextSequenceSend uint64) Upgrade { return Upgrade{ - Fields: upgradeFields, - Timeout: timeout, - NextPacketSend: nextPacketSend, + Fields: upgradeFields, + Timeout: timeout, + NextSequenceSend: nextSequenceSend, } } diff --git a/modules/core/04-channel/types/upgrade.pb.go b/modules/core/04-channel/types/upgrade.pb.go index 16a110c3579..e7931817125 100644 --- a/modules/core/04-channel/types/upgrade.pb.go +++ b/modules/core/04-channel/types/upgrade.pb.go @@ -27,11 +27,11 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // for an attempted upgrade. It provides the proposed changes to the channel // end, the timeout for this upgrade attempt and the next packet sequence // which allows the counterparty to efficiently know the highest sequence it has received. -// The next packet send is used for pruning and upgrading from unordered to ordered channels. +// The next sequence send is used for pruning and upgrading from unordered to ordered channels. type Upgrade struct { - Fields UpgradeFields `protobuf:"bytes,1,opt,name=fields,proto3" json:"fields"` - Timeout Timeout `protobuf:"bytes,2,opt,name=timeout,proto3" json:"timeout"` - NextPacketSend uint64 `protobuf:"varint,3,opt,name=next_packet_send,json=nextPacketSend,proto3" json:"next_packet_send,omitempty"` + Fields UpgradeFields `protobuf:"bytes,1,opt,name=fields,proto3" json:"fields"` + Timeout Timeout `protobuf:"bytes,2,opt,name=timeout,proto3" json:"timeout"` + NextSequenceSend uint64 `protobuf:"varint,3,opt,name=next_sequence_send,json=nextSequenceSend,proto3" json:"next_sequence_send,omitempty"` } func (m *Upgrade) Reset() { *m = Upgrade{} } @@ -160,33 +160,33 @@ func init() { func init() { proto.RegisterFile("ibc/core/channel/v1/upgrade.proto", fileDescriptor_fb1cef68588848b2) } var fileDescriptor_fb1cef68588848b2 = []byte{ - // 408 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x41, 0x8e, 0xd3, 0x30, - 0x14, 0x86, 0xe3, 0x99, 0x6a, 0x3a, 0x35, 0x50, 0x90, 0x61, 0x11, 0x55, 0x28, 0x53, 0xba, 0xa1, - 0x9b, 0x89, 0x99, 0x01, 0x21, 0x40, 0x2c, 0xd0, 0x48, 0x20, 0xc4, 0x06, 0xe4, 0x81, 0x0d, 0x9b, - 0xaa, 0x71, 0x1e, 0xa9, 0x45, 0xe3, 0x17, 0x6c, 0x27, 0x82, 0x1b, 0xb0, 0x9c, 0x23, 0x70, 0x0d, - 0x6e, 0x30, 0xcb, 0x59, 0xb2, 0x42, 0xa8, 0xbd, 0x08, 0x8a, 0x93, 0x14, 0x21, 0x65, 0x97, 0xf7, - 0xf2, 0xfd, 0xbf, 0x7f, 0xfb, 0xa7, 0xf7, 0x54, 0x22, 0xb9, 0x44, 0x03, 0x5c, 0xae, 0x96, 0x5a, - 0xc3, 0x9a, 0x57, 0x27, 0xbc, 0x2c, 0x32, 0xb3, 0x4c, 0x21, 0x2e, 0x0c, 0x3a, 0x64, 0xb7, 0x55, - 0x22, 0xe3, 0x1a, 0x89, 0x5b, 0x24, 0xae, 0x4e, 0x26, 0x77, 0x32, 0xcc, 0xd0, 0xff, 0xe7, 0xf5, - 0x57, 0x83, 0x4e, 0x7a, 0xdd, 0x3a, 0x95, 0x47, 0x66, 0x3f, 0x09, 0x1d, 0x7e, 0x68, 0xfc, 0xd9, - 0x0b, 0x7a, 0xf0, 0x49, 0xc1, 0x3a, 0xb5, 0x21, 0x99, 0x92, 0xf9, 0xb5, 0xd3, 0x59, 0xdc, 0x73, - 0x54, 0xdc, 0xd2, 0xaf, 0x3c, 0x79, 0x36, 0xb8, 0xfc, 0x7d, 0x14, 0x88, 0x56, 0xc7, 0x9e, 0xd3, - 0xa1, 0x53, 0x39, 0x60, 0xe9, 0xc2, 0x3d, 0x6f, 0x71, 0xb7, 0xd7, 0xe2, 0x7d, 0xc3, 0xb4, 0xe2, - 0x4e, 0xc2, 0xe6, 0xf4, 0x96, 0x86, 0xaf, 0x6e, 0x51, 0x2c, 0xe5, 0x67, 0x70, 0x0b, 0x0b, 0x3a, - 0x0d, 0xf7, 0xa7, 0x64, 0x3e, 0x10, 0xe3, 0x7a, 0xff, 0xce, 0xaf, 0xcf, 0x41, 0xa7, 0xcf, 0x06, - 0xdf, 0x7f, 0x1c, 0x05, 0xb3, 0x0b, 0x42, 0x6f, 0xfc, 0x97, 0x86, 0x3d, 0xa6, 0x87, 0x68, 0x52, - 0x30, 0x4a, 0x67, 0xfe, 0x0e, 0xe3, 0xd3, 0x49, 0x6f, 0x80, 0xb7, 0x35, 0x24, 0x76, 0x2c, 0xbb, - 0x4f, 0x6f, 0x4a, 0xd4, 0x1a, 0xa4, 0x53, 0xa8, 0x17, 0x2b, 0x2c, 0x6c, 0xb8, 0x37, 0xdd, 0x9f, - 0x8f, 0xc4, 0xf8, 0xdf, 0xfa, 0x35, 0x16, 0x96, 0x85, 0x74, 0x58, 0x81, 0xb1, 0x0a, 0xb5, 0x4f, - 0x36, 0x12, 0xdd, 0xd8, 0x46, 0x7a, 0x43, 0xaf, 0xbf, 0x34, 0x06, 0x8d, 0x00, 0x09, 0xaa, 0x70, - 0x6c, 0x42, 0x0f, 0x2d, 0x7c, 0x29, 0x41, 0x4b, 0xf0, 0x81, 0x06, 0x62, 0x37, 0xd7, 0x5e, 0x39, - 0x58, 0xbb, 0xcc, 0xc0, 0x3f, 0xd6, 0x48, 0x74, 0x63, 0xe3, 0x75, 0x76, 0x7e, 0xb9, 0x89, 0xc8, - 0xd5, 0x26, 0x22, 0x7f, 0x36, 0x11, 0xb9, 0xd8, 0x46, 0xc1, 0xd5, 0x36, 0x0a, 0x7e, 0x6d, 0xa3, - 0xe0, 0xe3, 0xd3, 0x4c, 0xb9, 0x55, 0x99, 0xc4, 0x12, 0x73, 0x2e, 0xd1, 0xe6, 0x68, 0xb9, 0x4a, - 0xe4, 0x71, 0x86, 0xbc, 0x7a, 0xc2, 0x73, 0x4c, 0xcb, 0x35, 0xd8, 0xa6, 0xf7, 0x07, 0x8f, 0x8e, - 0xbb, 0xea, 0xdd, 0xb7, 0x02, 0x6c, 0x72, 0xe0, 0x6b, 0x7f, 0xf8, 0x37, 0x00, 0x00, 0xff, 0xff, - 0x71, 0xde, 0xa0, 0x6a, 0x69, 0x02, 0x00, 0x00, + // 406 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x41, 0x6f, 0xd3, 0x30, + 0x14, 0xc7, 0xe3, 0xad, 0x5a, 0x57, 0x03, 0x03, 0x19, 0x0e, 0x51, 0x85, 0xb2, 0xd2, 0x0b, 0x3d, + 0xb0, 0x98, 0x0d, 0x84, 0x00, 0x71, 0x40, 0x93, 0x40, 0x88, 0x0b, 0x92, 0x0b, 0x17, 0x2e, 0x55, + 0xe3, 0x3c, 0x52, 0x4b, 0x8d, 0x5f, 0xb0, 0x9d, 0x08, 0xbe, 0x01, 0xc7, 0x7d, 0x04, 0xbe, 0x08, + 0xf7, 0x1d, 0x77, 0xe4, 0x84, 0x50, 0xfb, 0x45, 0x50, 0x9c, 0xa4, 0x68, 0x52, 0x6e, 0x7e, 0x7e, + 0xbf, 0xff, 0xdf, 0xff, 0xe7, 0x47, 0x1f, 0xa8, 0x44, 0x72, 0x89, 0x06, 0xb8, 0x5c, 0x2d, 0xb5, + 0x86, 0x35, 0xaf, 0x4e, 0x79, 0x59, 0x64, 0x66, 0x99, 0x42, 0x5c, 0x18, 0x74, 0xc8, 0xee, 0xaa, + 0x44, 0xc6, 0x35, 0x12, 0xb7, 0x48, 0x5c, 0x9d, 0x8e, 0xef, 0x65, 0x98, 0xa1, 0xef, 0xf3, 0xfa, + 0xd4, 0xa0, 0xe3, 0x5e, 0xb7, 0x4e, 0xe5, 0x91, 0xe9, 0x2f, 0x42, 0x87, 0x9f, 0x1a, 0x7f, 0xf6, + 0x9a, 0x1e, 0x7c, 0x51, 0xb0, 0x4e, 0x6d, 0x48, 0x26, 0x64, 0x76, 0xe3, 0x6c, 0x1a, 0xf7, 0x3c, + 0x15, 0xb7, 0xf4, 0x5b, 0x4f, 0x9e, 0x0f, 0x2e, 0xff, 0x1c, 0x07, 0xa2, 0xd5, 0xb1, 0x57, 0x74, + 0xe8, 0x54, 0x0e, 0x58, 0xba, 0x70, 0xcf, 0x5b, 0xdc, 0xef, 0xb5, 0xf8, 0xd8, 0x30, 0xad, 0xb8, + 0x93, 0xb0, 0x47, 0x94, 0x69, 0xf8, 0xe6, 0x16, 0x16, 0xbe, 0x96, 0xa0, 0x25, 0x2c, 0x2c, 0xe8, + 0x34, 0xdc, 0x9f, 0x90, 0xd9, 0x40, 0xdc, 0xa9, 0x3b, 0xf3, 0xb6, 0x31, 0x07, 0x9d, 0xbe, 0x1c, + 0xfc, 0xf8, 0x79, 0x1c, 0x4c, 0x2f, 0x08, 0xbd, 0x75, 0x2d, 0x11, 0x7b, 0x46, 0x0f, 0xd1, 0xa4, + 0x60, 0x94, 0xce, 0xfc, 0x1c, 0x47, 0x67, 0xe3, 0xde, 0x10, 0x1f, 0x6a, 0x48, 0xec, 0x58, 0xf6, + 0x90, 0xde, 0x96, 0xa8, 0x35, 0x48, 0xa7, 0x50, 0x2f, 0x56, 0x58, 0xd8, 0x70, 0x6f, 0xb2, 0x3f, + 0x1b, 0x89, 0xa3, 0xff, 0xd7, 0xef, 0xb0, 0xb0, 0x2c, 0xa4, 0xc3, 0x0a, 0x8c, 0x55, 0xa8, 0x7d, + 0xb6, 0x91, 0xe8, 0xca, 0x36, 0xd2, 0x7b, 0x7a, 0xf3, 0x8d, 0x31, 0x68, 0x04, 0x48, 0x50, 0x85, + 0x63, 0x63, 0x7a, 0xd8, 0x4d, 0xe4, 0x03, 0x0d, 0xc4, 0xae, 0xae, 0xbd, 0x72, 0xb0, 0x76, 0x99, + 0x81, 0xff, 0xb0, 0x91, 0xe8, 0xca, 0xc6, 0xeb, 0x7c, 0x7e, 0xb9, 0x89, 0xc8, 0xd5, 0x26, 0x22, + 0x7f, 0x37, 0x11, 0xb9, 0xd8, 0x46, 0xc1, 0xd5, 0x36, 0x0a, 0x7e, 0x6f, 0xa3, 0xe0, 0xf3, 0x8b, + 0x4c, 0xb9, 0x55, 0x99, 0xc4, 0x12, 0x73, 0x2e, 0xd1, 0xe6, 0x68, 0xb9, 0x4a, 0xe4, 0x49, 0x86, + 0xbc, 0x7a, 0xce, 0x73, 0x4c, 0xcb, 0x35, 0xd8, 0x66, 0xf7, 0x8f, 0x9f, 0x9e, 0x74, 0xeb, 0x77, + 0xdf, 0x0b, 0xb0, 0xc9, 0x81, 0x5f, 0xfd, 0x93, 0x7f, 0x01, 0x00, 0x00, 0xff, 0xff, 0xcc, 0x31, + 0xc0, 0xc6, 0x6d, 0x02, 0x00, 0x00, } func (m *Upgrade) Marshal() (dAtA []byte, err error) { @@ -209,8 +209,8 @@ func (m *Upgrade) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if m.NextPacketSend != 0 { - i = encodeVarintUpgrade(dAtA, i, uint64(m.NextPacketSend)) + if m.NextSequenceSend != 0 { + i = encodeVarintUpgrade(dAtA, i, uint64(m.NextSequenceSend)) i-- dAtA[i] = 0x18 } @@ -337,8 +337,8 @@ func (m *Upgrade) Size() (n int) { n += 1 + l + sovUpgrade(uint64(l)) l = m.Timeout.Size() n += 1 + l + sovUpgrade(uint64(l)) - if m.NextPacketSend != 0 { - n += 1 + sovUpgrade(uint64(m.NextPacketSend)) + if m.NextSequenceSend != 0 { + n += 1 + sovUpgrade(uint64(m.NextSequenceSend)) } return n } @@ -484,9 +484,9 @@ func (m *Upgrade) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 3: if wireType != 0 { - return fmt.Errorf("proto: wrong wireType = %d for field NextPacketSend", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field NextSequenceSend", wireType) } - m.NextPacketSend = 0 + m.NextSequenceSend = 0 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowUpgrade @@ -496,7 +496,7 @@ func (m *Upgrade) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - m.NextPacketSend |= uint64(b&0x7F) << shift + m.NextSequenceSend |= uint64(b&0x7F) << shift if b < 0x80 { break } diff --git a/proto/ibc/core/channel/v1/upgrade.proto b/proto/ibc/core/channel/v1/upgrade.proto index 4ba9967a874..81530ed2a28 100644 --- a/proto/ibc/core/channel/v1/upgrade.proto +++ b/proto/ibc/core/channel/v1/upgrade.proto @@ -11,13 +11,13 @@ import "ibc/core/channel/v1/channel.proto"; // for an attempted upgrade. It provides the proposed changes to the channel // end, the timeout for this upgrade attempt and the next packet sequence // which allows the counterparty to efficiently know the highest sequence it has received. -// The next packet send is used for pruning and upgrading from unordered to ordered channels. +// The next sequence send is used for pruning and upgrading from unordered to ordered channels. message Upgrade { option (gogoproto.goproto_getters) = false; - UpgradeFields fields = 1 [(gogoproto.nullable) = false]; - Timeout timeout = 2 [(gogoproto.nullable) = false]; - uint64 next_packet_send = 3; + UpgradeFields fields = 1 [(gogoproto.nullable) = false]; + Timeout timeout = 2 [(gogoproto.nullable) = false]; + uint64 next_sequence_send = 3; } // UpgradeFields are the fields in a channel end which may be changed From 81c31eeb5873ee222ed4e95d658e7f66ad53a39b Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 13:33:33 +0100 Subject: [PATCH 08/18] update endpoint --- testing/endpoint.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/endpoint.go b/testing/endpoint.go index 143f5928dbc..fcf5099562f 100644 --- a/testing/endpoint.go +++ b/testing/endpoint.go @@ -868,8 +868,8 @@ func (endpoint *Endpoint) GetProposedUpgrade() channeltypes.Upgrade { ConnectionHops: []string{endpoint.ConnectionID}, Version: endpoint.ChannelConfig.Version, }, - Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0), - NextPacketSend: 0, + Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0), + NextSequenceSend: 0, } override := endpoint.ChannelConfig.ProposedUpgrade From 0ca4bcaf13af4dc7c3aefe466775d7b446a92a16 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 14:09:41 +0100 Subject: [PATCH 09/18] spec changes --- modules/core/04-channel/keeper/upgrade.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index ba221e4fcff..63f35a692e0 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -323,10 +323,6 @@ func (k Keeper) WriteUpgradeAckChannel(ctx sdk.Context, portID, channelID string if !k.HasInflightPackets(ctx, portID, channelID) { channel.State = types.FLUSHCOMPLETE k.SetChannel(ctx, portID, channelID, channel) - } else { - // the counterparty upgrade is only required if the channel is still in the FLUSHING state. - // this gets read when timing out and acknowledging packets. - k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) } upgrade, found := k.GetUpgrade(ctx, portID, channelID) @@ -337,6 +333,7 @@ func (k Keeper) WriteUpgradeAckChannel(ctx sdk.Context, portID, channelID string upgrade.Fields.Version = counterpartyUpgrade.Fields.Version k.SetUpgrade(ctx, portID, channelID, upgrade) + k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade) k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "state", channel.State.String()) return channel, upgrade From a7a0834b43bcd246f5cfde77a268970bdc4f8224 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 14:25:42 +0100 Subject: [PATCH 10/18] test that counterparty upgrade has been set --- modules/core/04-channel/keeper/upgrade_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 10edcc65f8e..726794c5bcc 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -646,13 +646,10 @@ func (suite *KeeperTestSuite) TestWriteChannelUpgradeAck() { if !tc.hasPacketCommitments { suite.Require().Equal(types.FLUSHCOMPLETE, channel.State) - _, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().False(ok) - } else { - counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(ok) - suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) } + counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(ok) + suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) }) } } From 9a6f4c12becd8dd65ce52ac3423385f92787f9ea Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 16:14:19 +0100 Subject: [PATCH 11/18] update err message --- modules/core/04-channel/keeper/upgrade_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 726794c5bcc..0b5aef88e9e 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -937,7 +937,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeConfirm() { suite.Require().Equal(types.FLUSHING, channel.State) } counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(ok, "counterparty upgrade should be present when there are in flight packets") + suite.Require().True(ok, "counterparty upgrade should be present") suite.Require().Equal(proposedUpgrade, counterpartyUpgrade) }) } From ed80a45e7b3ea4663dfe48e9627980d8ca8c13f9 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 19 Dec 2023 16:34:25 +0100 Subject: [PATCH 12/18] pr review --- modules/core/04-channel/keeper/upgrade.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 63f35a692e0..6df38e61bd4 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -533,6 +533,11 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin panic(fmt.Errorf("could not find upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) } + counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) + if !found { + panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) + } + // next seq recv and ack is used for ordered channels to verify the packet has been received/acked in the correct order // this is no longer necessary if the channel is UNORDERED and should be reset to 1 if channel.Ordering == types.ORDERED && upgrade.Fields.Ordering == types.UNORDERED { @@ -544,11 +549,6 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin // we can be sure that the next packet we are set to receive will be the first packet the counterparty sends after reopening. // we can be sure that our next acknowledgement will be our first packet sent after upgrade, as the counterparty processed all sent packets after flushing completes. if channel.Ordering == types.UNORDERED && upgrade.Fields.Ordering == types.ORDERED { - counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, portID, channelID) - if !found { - panic(fmt.Errorf("could not find counterparty upgrade when updating channel state, channelID: %s, portID: %s", channelID, portID)) - } - k.SetNextSequenceRecv(ctx, portID, channelID, counterpartyUpgrade.NextSequenceSend) k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextSequenceSend) } From 7b2b487180ceac56e2d58092cc9efd54810c70fc Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 19 Dec 2023 20:12:02 +0100 Subject: [PATCH 13/18] naming nit --- .../core/04-channel/keeper/upgrade_test.go | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 0b5aef88e9e..006a6e5aa1f 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1128,17 +1128,17 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequence0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointB.RecvPacket(packet) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packet0) suite.Require().NoError(err) // send second packet from B to A - sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequence1, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointA.RecvPacket(packet0) + packet1 := types.NewPacket(ibctesting.MockPacketData, sequence1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointA.RecvPacket(packet1) suite.Require().NoError(err) path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion @@ -1152,10 +1152,10 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel - err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + err = path.EndpointA.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + err = path.EndpointB.AcknowledgePacket(packet1, ibctesting.MockAcknowledgement) suite.Require().NoError(err) ctx := suite.chainA.GetContext() @@ -1254,17 +1254,17 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequenc0, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointB.RecvPacket(packet) + packe0 := types.NewPacket(ibctesting.MockPacketData, sequenc0, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packe0) suite.Require().NoError(err) // send second packet from B to A - sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequence1, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointA.RecvPacket(packet0) + packet1 := types.NewPacket(ibctesting.MockPacketData, sequence1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointA.RecvPacket(packet1) suite.Require().NoError(err) path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion @@ -1278,10 +1278,10 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel - err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + err = path.EndpointA.AcknowledgePacket(packe0, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + err = path.EndpointB.AcknowledgePacket(packet1, ibctesting.MockAcknowledgement) suite.Require().NoError(err) ctx := suite.chainA.GetContext() From 70973c938ef821e0e06cd58294efbeac95a50c50 Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 20 Dec 2023 12:05:32 +0100 Subject: [PATCH 14/18] pr review --- modules/core/04-channel/keeper/upgrade.go | 2 +- .../core/04-channel/keeper/upgrade_test.go | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 6df38e61bd4..97e0c8d7814 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -545,7 +545,7 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin k.SetNextSequenceAck(ctx, portID, channelID, 1) } - // next seq recv and ack should updated when moving from UNORDERED to ORDERED using the latest packet sequence sent before the upgrade + // next seq recv and ack should updated when moving from UNORDERED to ORDERED using the counterparty NextSequenceSend as set just after blocking new packet sends. // we can be sure that the next packet we are set to receive will be the first packet the counterparty sends after reopening. // we can be sure that our next acknowledgement will be our first packet sent after upgrade, as the counterparty processed all sent packets after flushing completes. if channel.Ordering == types.UNORDERED && upgrade.Fields.Ordering == types.ORDERED { diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 0b5aef88e9e..0813fd87c3c 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1128,16 +1128,16 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequenceA, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + packet := types.NewPacket(ibctesting.MockPacketData, sequenceA, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) // send second packet from B to A - sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequenceB, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequenceB, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) err = path.EndpointA.RecvPacket(packet0) suite.Require().NoError(err) @@ -1160,7 +1160,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { ctx := suite.chainA.GetContext() - // assert that sequence (reset in ChanUpgradeTry) is 1 because channel is UNORDERED + // assert that NextSeqRecv is 1 because channel is UNORDERED seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) @@ -1186,7 +1186,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { suite.Require().Equal(types.ORDERED, channel.Ordering) // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED - // seq updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 + // NextSeqRecv updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) @@ -1254,16 +1254,16 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequenceA, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + packet := types.NewPacket(ibctesting.MockPacketData, sequenceA, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) // send second packet from B to A - sequence0, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequenceB, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequence0, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + packet0 := types.NewPacket(ibctesting.MockPacketData, sequenceB, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) err = path.EndpointA.RecvPacket(packet0) suite.Require().NoError(err) @@ -1291,7 +1291,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { suite.Require().True(found) suite.Require().Equal(uint64(2), seq) - // assert that sequence (set in ChanUpgradeTry) is incremented to 2 because channel is still ordered + // assert that NextSeqRecv is incremented to 2 because channel is still ordered seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) From 9e96b136a71432fc3af8e4c906d3844f4b46d6f0 Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 20 Dec 2023 12:28:04 +0100 Subject: [PATCH 15/18] refactor test --- .../core/04-channel/keeper/upgrade_test.go | 237 +++++++++--------- 1 file changed, 122 insertions(+), 115 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 0813fd87c3c..9ec67e1502f 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1090,7 +1090,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpen() { } } -func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { +func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { var path *ibctesting.Path testCases := []struct { @@ -1134,42 +1134,20 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) - // send second packet from B to A - sequenceB, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) - suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequenceB, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointA.RecvPacket(packet0) - suite.Require().NoError(err) - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED suite.Require().NoError(path.EndpointA.ChanUpgradeInit()) suite.Require().NoError(path.EndpointB.ChanUpgradeTry()) suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) - // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel + // Ack packet to delete packet commitments before calling WriteUpgradeOpenChannel err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) - suite.Require().NoError(err) - ctx := suite.chainA.GetContext() - // assert that NextSeqRecv is 1 because channel is UNORDERED - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(1), seq) - - // assert that NextSeqAck is 1 because channel is UNORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(1), seq) - tc.malleate() if tc.expPanic { @@ -1183,18 +1161,6 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { // Assert that channel state has been updated suite.Require().Equal(types.OPEN, channel.State) suite.Require().Equal(mock.UpgradeVersion, channel.Version) - suite.Require().Equal(types.ORDERED, channel.Ordering) - - // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED - // NextSeqRecv updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(2), seq) - - // assert that NextSeqAck is incremented to 2 because channel is now ORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(2), seq) // Assert that state stored for upgrade has been deleted upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -1228,18 +1194,92 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_UnorderedToOrdered() { } } -func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { +func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() { var path *ibctesting.Path testCases := []struct { - name string - malleate func() - expPanic bool + name string + malleate func() + preUpgrade func() + postUpgrade func() }{ { - name: "success", - malleate: func() {}, - expPanic: false, + name: "success: ORDERED -> UNORDERED", + malleate: func() { + path.EndpointA.ChannelConfig.Order = types.ORDERED + path.EndpointB.ChannelConfig.Order = types.ORDERED + + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED + }, + preUpgrade: func() { + // assert that NextSeqAck is incremented to 2 because channel is still ordered + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + + // assert that NextSeqRecv is incremented to 2 because channel is still ordered + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + }, + postUpgrade: func() { + channel := path.EndpointA.GetChannel() + + // Assert that channel state has been updated + suite.Require().Equal(types.OPEN, channel.State) + suite.Require().Equal(types.UNORDERED, channel.Ordering) + + // assert that NextSeqRecv is now 1, because channel is now UNORDERED + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + + // assert that NextSeqAck is now 1, because channel is now UNORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + }, + }, + { + name: "success: UNORDERED -> ORDERED", + malleate: func() { + path.EndpointA.ChannelConfig.Order = types.UNORDERED + path.EndpointB.ChannelConfig.Order = types.UNORDERED + + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED + }, + preUpgrade: func() { + // assert that NextSeqRecv is 1 because channel is UNORDERED + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + + // assert that NextSeqAck is 1 because channel is UNORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(1), seq) + + }, + postUpgrade: func() { + channel := path.EndpointA.GetChannel() + + // Assert that channel state has been updated + suite.Require().Equal(types.OPEN, channel.State) + suite.Require().Equal(types.ORDERED, channel.Ordering) + + // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED + // NextSeqRecv updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + + // assert that NextSeqAck is incremented to 2 because channel is now ORDERED + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), seq) + }, }, } @@ -1249,106 +1289,73 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_OrderedToUnordered() { suite.SetupTest() path = ibctesting.NewPath(suite.chainA, suite.chainB) - path.EndpointA.ChannelConfig.Order = types.ORDERED - path.EndpointB.ChannelConfig.Order = types.ORDERED + + tc.malleate() + suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. sequenceA, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequenceA, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointB.RecvPacket(packet) + packetA := types.NewPacket(ibctesting.MockPacketData, sequenceA, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packetA) suite.Require().NoError(err) // send second packet from B to A sequenceB, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet0 := types.NewPacket(ibctesting.MockPacketData, sequenceB, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) - err = path.EndpointA.RecvPacket(packet0) + packetB := types.NewPacket(ibctesting.MockPacketData, sequenceB, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointA.RecvPacket(packetB) suite.Require().NoError(err) - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED - suite.Require().NoError(path.EndpointA.ChanUpgradeInit()) suite.Require().NoError(path.EndpointB.ChanUpgradeTry()) suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) // Ack packets to delete packet commitments before calling WriteUpgradeOpenChannel - err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + err = path.EndpointA.AcknowledgePacket(packetA, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - err = path.EndpointB.AcknowledgePacket(packet0, ibctesting.MockAcknowledgement) + err = path.EndpointB.AcknowledgePacket(packetB, ibctesting.MockAcknowledgement) suite.Require().NoError(err) - ctx := suite.chainA.GetContext() - - // assert that NextSeqAck is incremented to 2 because channel is still ordered - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(2), seq) - - // assert that NextSeqRecv is incremented to 2 because channel is still ordered - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(2), seq) + // pre upgrade assertions + tc.preUpgrade() tc.malleate() + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - if tc.expPanic { - suite.Require().Panics(func() { - suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - }) - } else { - suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - channel := path.EndpointA.GetChannel() - - // Assert that channel state has been updated - suite.Require().Equal(types.OPEN, channel.State) - suite.Require().Equal(mock.UpgradeVersion, channel.Version) - suite.Require().Equal(types.UNORDERED, channel.Ordering) + // post upgrade assertions + tc.postUpgrade() - // assert that NextSeqRecv is now 1, because channel is now UNORDERED - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(1), seq) + // Assert that state stored for upgrade has been deleted + upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(types.Upgrade{}, upgrade) + suite.Require().False(found) - // assert that NextSeqAck is now 1, because channel is now UNORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found) - suite.Require().Equal(uint64(1), seq) + counterpartyUpgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(types.Upgrade{}, counterpartyUpgrade) + suite.Require().False(found) - // Assert that state stored for upgrade has been deleted - upgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().Equal(types.Upgrade{}, upgrade) - suite.Require().False(found) - - counterpartyUpgrade, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().Equal(types.Upgrade{}, counterpartyUpgrade) - suite.Require().False(found) - - // events := ctx.EventManager().Events().ToABCIEvents() - // expEvents := ibctesting.EventsMap{ - // types.EventTypeChannelUpgradeOpen: { - // types.AttributeKeyPortID: path.EndpointA.ChannelConfig.PortID, - // types.AttributeKeyChannelID: path.EndpointA.ChannelID, - // types.AttributeCounterpartyPortID: path.EndpointB.ChannelConfig.PortID, - // types.AttributeCounterpartyChannelID: path.EndpointB.ChannelID, - // types.AttributeKeyChannelState: types.OPEN.String(), - // types.AttributeKeyUpgradeConnectionHops: channel.ConnectionHops[0], - // types.AttributeKeyUpgradeVersion: channel.Version, - // types.AttributeKeyUpgradeOrdering: channel.Ordering.String(), - // types.AttributeKeyUpgradeSequence: fmt.Sprintf("%d", channel.UpgradeSequence), - // }, - // sdk.EventTypeMessage: { - // sdk.AttributeKeyModule: types.AttributeValueCategory, - // }, - // } - // ibctesting.AssertEventsLegacy(&suite.Suite, expEvents, events) - } + // events := ctx.EventManager().Events().ToABCIEvents() + // expEvents := ibctesting.EventsMap{ + // types.EventTypeChannelUpgradeOpen: { + // types.AttributeKeyPortID: path.EndpointA.ChannelConfig.PortID, + // types.AttributeKeyChannelID: path.EndpointA.ChannelID, + // types.AttributeCounterpartyPortID: path.EndpointB.ChannelConfig.PortID, + // types.AttributeCounterpartyChannelID: path.EndpointB.ChannelID, + // types.AttributeKeyChannelState: types.OPEN.String(), + // types.AttributeKeyUpgradeConnectionHops: channel.ConnectionHops[0], + // types.AttributeKeyUpgradeVersion: channel.Version, + // types.AttributeKeyUpgradeOrdering: channel.Ordering.String(), + // types.AttributeKeyUpgradeSequence: fmt.Sprintf("%d", channel.UpgradeSequence), + // }, + // sdk.EventTypeMessage: { + // sdk.AttributeKeyModule: types.AttributeValueCategory, + // }, + // } + // ibctesting.AssertEventsLegacy(&suite.Suite, expEvents, events) }) } } From 8fdc92792dd2eb728a243e8c6c7fed5c1d8753d1 Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 20 Dec 2023 12:32:27 +0100 Subject: [PATCH 16/18] small test changes --- modules/core/04-channel/keeper/upgrade_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index cb05bcbc775..ebf0277a5fc 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1391,9 +1391,9 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.coordinator.Setup(path) // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. - sequenceA, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - packet := types.NewPacket(ibctesting.MockPacketData, sequenceA, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + packet := types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) @@ -1405,7 +1405,7 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) suite.Require().NoError(path.EndpointB.ChanUpgradeConfirm()) - // Ack packet to delete packet commitments before calling WriteUpgradeOpenChannel + // Ack packet to delete packet commitment before calling WriteUpgradeOpenChannel err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) suite.Require().NoError(err) From 7d7f60a66041d20f52739d395e47c315ab72f4bd Mon Sep 17 00:00:00 2001 From: Charly Date: Wed, 20 Dec 2023 12:40:06 +0100 Subject: [PATCH 17/18] lint --- modules/core/04-channel/keeper/upgrade_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index ebf0277a5fc..ec04df2a5c7 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1523,7 +1523,6 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() { seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) - }, postUpgrade: func() { channel := path.EndpointA.GetChannel() From 608d9e2398208b4a26e0f6bba0d7fde80d58eca3 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 20 Dec 2023 14:39:29 +0200 Subject: [PATCH 18/18] Add key for storing counterparty next sequence send during WriteUpgradeOpen. (#5460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add key for storing counterparty next sequence send. * nit: function order localtion * SetNextSequenceSend -> SetCounterpartyNextSequenceSend * Add assertions to check for correct values set in tests. * Rename keys. * Apply suggestions from code review Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --------- Co-authored-by: Carlos Rodriguez Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/core/04-channel/keeper/keeper.go | 36 ++++++++++-- modules/core/04-channel/keeper/upgrade.go | 10 ++++ .../core/04-channel/keeper/upgrade_test.go | 58 ++++++++++++++++--- modules/core/24-host/packet_keys.go | 25 +++++--- 4 files changed, 108 insertions(+), 21 deletions(-) diff --git a/modules/core/04-channel/keeper/keeper.go b/modules/core/04-channel/keeper/keeper.go index 77d79db8c71..63847fd175d 100644 --- a/modules/core/04-channel/keeper/keeper.go +++ b/modules/core/04-channel/keeper/keeper.go @@ -611,10 +611,17 @@ func (k Keeper) HasInflightPackets(ctx sdk.Context, portID, channelID string) bo return iterator.Valid() } -// GetAcknowledgementPruningSequence gets a channel's acknowledgement pruning sequence from the store. -func (k Keeper) GetAcknowledgementPruningSequence(ctx sdk.Context, portID, channelID string) (uint64, bool) { +// SetPruningSequenceEnd sets the channel's pruning sequence end to the store. +func (k Keeper) SetPruningSequenceEnd(ctx sdk.Context, portID, channelID string, sequence uint64) { store := ctx.KVStore(k.storeKey) - bz := store.Get(host.AckPruningSequenceKey(portID, channelID)) + bz := sdk.Uint64ToBigEndian(sequence) + store.Set(host.PruningSequenceEndKey(portID, channelID), bz) +} + +// GetPruningSequenceEnd gets a channel's pruning sequence end from the store. +func (k Keeper) GetPruningSequenceEnd(ctx sdk.Context, portID, channelID string) (uint64, bool) { + store := ctx.KVStore(k.storeKey) + bz := store.Get(host.PruningSequenceEndKey(portID, channelID)) if len(bz) == 0 { return 0, false } @@ -622,11 +629,28 @@ func (k Keeper) GetAcknowledgementPruningSequence(ctx sdk.Context, portID, chann return sdk.BigEndianToUint64(bz), true } -// SetAcknowledgementPruningSequence sets a channel's acknowledgement pruning sequence to the store. -func (k Keeper) SetAcknowledgementPruningSequence(ctx sdk.Context, portID, channelID string, sequence uint64) { +// SetPruningSequenceStart sets a channel's pruning sequence start to the store. +func (k Keeper) SetPruningSequenceStart(ctx sdk.Context, portID, channelID string, sequence uint64) { store := ctx.KVStore(k.storeKey) bz := sdk.Uint64ToBigEndian(sequence) - store.Set(host.AckPruningSequenceKey(portID, channelID), bz) + store.Set(host.PruningSequenceStartKey(portID, channelID), bz) +} + +// GetPruningSequenceStart gets a channel's pruning sequence start from the store. +func (k Keeper) GetPruningSequenceStart(ctx sdk.Context, portID, channelID string) uint64 { + store := ctx.KVStore(k.storeKey) + bz := store.Get(host.PruningSequenceStartKey(portID, channelID)) + if len(bz) == 0 { + return 0 + } + + return sdk.BigEndianToUint64(bz) +} + +// HasPruningSequenceStart returns true if the pruning sequence start is set for the specified channel. +func (k Keeper) HasPruningSequenceStart(ctx sdk.Context, portID, channelID string) bool { + store := ctx.KVStore(k.storeKey) + return store.Has(host.PruningSequenceStartKey(portID, channelID)) } // PruneAcknowledgements prunes packet acknowledgements from the store that have a sequence number less than or equal to the pruning sequence. diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 6fae6151262..5f451955606 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -582,6 +582,16 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin k.SetNextSequenceAck(ctx, portID, channelID, upgrade.NextSequenceSend) } + // set the counterparty next sequence send as pruning sequence end in order to have upper bound to prune to + k.SetPruningSequenceEnd(ctx, portID, channelID, counterpartyUpgrade.NextSequenceSend) + + // First upgrade for this channel will set the pruning sequence to 1, the starting sequence for pruning. + // Subsequent upgrades will not modify the pruning sequence thereby allowing pruning to continue from the last + // pruned sequence. + if !k.HasPruningSequenceStart(ctx, portID, channelID) { + k.SetPruningSequenceStart(ctx, portID, channelID, 1) + } + // Switch channel fields to upgrade fields and set channel state to OPEN previousState := channel.State channel.Ordering = upgrade.Fields.Ordering diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index ec04df2a5c7..b9a89bd6202 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1476,32 +1476,53 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() { path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.UNORDERED }, preUpgrade: func() { + ctx := suite.chainA.GetContext() + // assert that NextSeqAck is incremented to 2 because channel is still ordered - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) // assert that NextSeqRecv is incremented to 2 because channel is still ordered - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) + + // Assert that pruning sequence start has not been initialized. + suite.Require().False(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + + // Assert that pruning sequence end has not been set + counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().False(found) + suite.Require().Equal(uint64(0), counterpartyNextSequenceSend) }, postUpgrade: func() { channel := path.EndpointA.GetChannel() + ctx := suite.chainA.GetContext() // Assert that channel state has been updated suite.Require().Equal(types.OPEN, channel.State) suite.Require().Equal(types.UNORDERED, channel.Ordering) // assert that NextSeqRecv is now 1, because channel is now UNORDERED - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) // assert that NextSeqAck is now 1, because channel is now UNORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) + + // Assert that pruning sequence start has been initialized (set to 1) + suite.Require().True(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + pruningSeq := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(uint64(1), pruningSeq) + + // Assert that pruning sequence end has been set correctly + counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), counterpartySequenceSend) }, }, { @@ -1514,18 +1535,29 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() { path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED }, preUpgrade: func() { + ctx := suite.chainA.GetContext() + // assert that NextSeqRecv is 1 because channel is UNORDERED - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) // assert that NextSeqAck is 1 because channel is UNORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(1), seq) + + // Assert that pruning sequence start has not been initialized. + suite.Require().False(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + + // Assert that pruning sequence end has not been set + counterpartyNextSequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().False(found) + suite.Require().Equal(uint64(0), counterpartyNextSequenceSend) }, postUpgrade: func() { channel := path.EndpointA.GetChannel() + ctx := suite.chainA.GetContext() // Assert that channel state has been updated suite.Require().Equal(types.OPEN, channel.State) @@ -1533,14 +1565,24 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel_Ordering() { // assert that NextSeqRecv is incremented to 2, because channel is now ORDERED // NextSeqRecv updated in WriteUpgradeOpenChannel to latest sequence (one packet sent) + 1 - seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceRecv(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) // assert that NextSeqAck is incremented to 2 because channel is now ORDERED - seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + seq, found = suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceAck(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found) suite.Require().Equal(uint64(2), seq) + + // Assert that pruning sequence start has been initialized (set to 1) + suite.Require().True(suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.HasPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)) + pruningSeq := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceStart(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().Equal(uint64(1), pruningSeq) + + // Assert that pruning sequence end has been set correctly + counterpartySequenceSend, found := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetPruningSequenceEnd(ctx, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found) + suite.Require().Equal(uint64(2), counterpartySequenceSend) }, }, } diff --git a/modules/core/24-host/packet_keys.go b/modules/core/24-host/packet_keys.go index 147515c237a..75730e57d73 100644 --- a/modules/core/24-host/packet_keys.go +++ b/modules/core/24-host/packet_keys.go @@ -10,7 +10,8 @@ const ( KeyPacketCommitmentPrefix = "commitments" KeyPacketAckPrefix = "acks" KeyPacketReceiptPrefix = "receipts" - KeyAckPruningSequence = "ackPruningSequence" + KeyPruningSequenceStart = "pruningSequenceStart" + KeyPruningSequenceEnd = "pruningSequenceEnd" ) // ICS04 @@ -92,14 +93,24 @@ func PacketReceiptKey(portID, channelID string, sequence uint64) []byte { return []byte(PacketReceiptPath(portID, channelID, sequence)) } -// AckPruningSequencePath defines the path under which the pruning sequence is stored -func AckPruningSequencePath(portID, channelID string) string { - return fmt.Sprintf("%s/%s", KeyAckPruningSequence, channelPath(portID, channelID)) +// PruningSequenceStartPath defines the path under which the pruning sequence starting value is stored +func PruningSequenceStartPath(portID, channelID string) string { + return fmt.Sprintf("%s/%s", KeyPruningSequenceStart, channelPath(portID, channelID)) } -// AckPruningSequenceKey returns the store key for the pruning sequence of a particular channel -func AckPruningSequenceKey(portID, channelID string) []byte { - return []byte(AckPruningSequencePath(portID, channelID)) +// PruningSequenceStartKey returns the store key for the pruning sequence start of a particular channel +func PruningSequenceStartKey(portID, channelID string) []byte { + return []byte(PruningSequenceStartPath(portID, channelID)) +} + +// PruningSequenceEndPath defines the path under which the pruning sequence end is stored +func PruningSequenceEndPath(portID, channelID string) string { + return fmt.Sprintf("%s/%s", KeyPruningSequenceEnd, channelPath(portID, channelID)) +} + +// PruningSequenceEndKey returns the store key for the pruning sequence end of a particular channel +func PruningSequenceEndKey(portID, channelID string) []byte { + return []byte(PruningSequenceEndPath(portID, channelID)) } func sequencePath(sequence uint64) string {