Skip to content

Commit

Permalink
revert "imp: remove LatestSequenceSend (#5108)" (#5432)
Browse files Browse the repository at this point in the history
  • Loading branch information
charleenfei authored Dec 18, 2023
1 parent 4d2ce43 commit 9c346c5
Show file tree
Hide file tree
Showing 12 changed files with 135 additions and 38 deletions.
1 change: 1 addition & 0 deletions modules/core/03-connection/keeper/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,7 @@ func (suite *KeeperTestSuite) TestVerifyUpgrade() {
upgrade = channeltypes.NewUpgrade(
channeltypes.NewUpgradeFields(channeltypes.UNORDERED, []string{path.EndpointA.ConnectionID}, "v1.0.0"),
channeltypes.NewTimeout(clienttypes.ZeroHeight(), 100000),
0,
)

suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgrade)
Expand Down
1 change: 1 addition & 0 deletions modules/core/04-channel/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,7 @@ func (suite *KeeperTestSuite) TestQueryUpgrade() {
expectedUpgrade = types.NewUpgrade(
types.NewUpgradeFields(types.UNORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewTimeout(clienttypes.ZeroHeight(), 1000000),
0,
)

suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, expectedUpgrade)
Expand Down
1 change: 0 additions & 1 deletion modules/core/04-channel/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ func (k Keeper) RecvPacket(
// incrementing nextSequenceRecv and storing under this chain's channelEnd identifiers
// Since this is the receiving chain, our channelEnd is packet's destination port and channel
k.SetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel(), nextSequenceRecv)

}

// log that a packet has been received & executed
Expand Down
24 changes: 24 additions & 0 deletions modules/core/04-channel/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,30 @@ 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,
},
Expand Down
8 changes: 7 additions & 1 deletion modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (k Keeper) ChanUpgradeTry(
proofHeight, proofCounterpartyUpgrade,
channel.Counterparty.PortId,
channel.Counterparty.ChannelId,
types.NewUpgrade(counterpartyUpgradeFields, types.Timeout{}),
types.NewUpgrade(counterpartyUpgradeFields, types.Timeout{}, 0),
); err != nil {
return types.Channel{}, types.Upgrade{}, errorsmod.Wrap(err, "failed to verify counterparty upgrade")
}
Expand Down Expand Up @@ -769,6 +769,12 @@ func (k Keeper) startFlushing(ctx sdk.Context, portID, channelID string, upgrade
channel.State = types.FLUSHING
k.SetChannel(ctx, portID, channelID, channel)

nextSequenceSend, found := k.GetNextSequenceSend(ctx, portID, channelID)
if !found {
return errorsmod.Wrapf(types.ErrSequenceSendNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
}

upgrade.LatestSequenceSend = nextSequenceSend - 1
upgrade.Timeout = k.getAbsoluteUpgradeTimeout(ctx)
k.SetUpgrade(ctx, portID, channelID, *upgrade)

Expand Down
27 changes: 26 additions & 1 deletion modules/core/04-channel/keeper/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,10 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() {
suite.Require().NoError(err)
suite.Require().NotEmpty(upgrade)
suite.Require().Equal(proposedUpgrade.Fields, upgrade.Fields)
suite.Require().Equal(path.EndpointA.GetChannel().UpgradeSequence, path.EndpointB.GetChannel().UpgradeSequence)

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)
} else {
suite.assertUpgradeError(err, tc.expError)
}
Expand Down Expand Up @@ -482,6 +485,14 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
},
commitmenttypes.ErrInvalidProof,
},
{
"fails due to proof verification failure, counterparty update has unexpected sequence",
func() {
// Decrementing LatestSequenceSend is sufficient to cause the proof to fail.
counterpartyUpgrade.LatestSequenceSend--
},
commitmenttypes.ErrInvalidProof,
},
{
"fails due to mismatch in upgrade ordering",
func() {
Expand Down Expand Up @@ -1824,6 +1835,15 @@ func (suite *KeeperTestSuite) TestStartFlush() {
},
connectiontypes.ErrInvalidConnectionState,
},
{
"next sequence send not found",
func() {
// Delete next sequence send key from store
store := suite.chainB.GetContext().KVStore(suite.chainB.GetSimApp().GetKey(exported.StoreKey))
store.Delete(host.NextSequenceSendKey(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))
},
types.ErrSequenceSendNotFound,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1857,7 +1877,12 @@ func (suite *KeeperTestSuite) TestStartFlush() {
suite.assertUpgradeError(err, tc.expError)
} else {
channel := path.EndpointB.GetChannel()

nextSequenceSend, ok := suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.GetNextSequenceSend(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().True(ok)

suite.Require().Equal(types.FLUSHING, channel.State)
suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend)

expectedTimeoutTimestamp := types.DefaultTimeout.Timestamp + uint64(suite.chainB.GetContext().BlockTime().UnixNano())
suite.Require().Equal(expectedTimeoutTimestamp, upgrade.Timeout.Timestamp)
Expand Down
3 changes: 3 additions & 0 deletions modules/core/04-channel/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ func (suite *TypesTestSuite) TestMsgChannelUpgradeAckValidateBasic() {
upgrade := types.NewUpgrade(
types.NewUpgradeFields(types.ORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewTimeout(clienttypes.NewHeight(1, 100), 0),
0,
)

msg = types.NewMsgChannelUpgradeAck(
Expand All @@ -834,6 +835,7 @@ func (suite *TypesTestSuite) TestMsgChannelUpgradeAckGetSigners() {
upgrade := types.NewUpgrade(
types.NewUpgradeFields(types.ORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewTimeout(clienttypes.NewHeight(1, 100), 0),
0,
)

msg := types.NewMsgChannelUpgradeAck(
Expand Down Expand Up @@ -919,6 +921,7 @@ func (suite *TypesTestSuite) TestMsgChannelUpgradeConfirmValidateBasic() {
counterpartyUpgrade := types.NewUpgrade(
types.NewUpgradeFields(types.UNORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewTimeout(clienttypes.NewHeight(0, 10000), timeoutTimestamp),
0,
)

msg = types.NewMsgChannelUpgradeConfirm(
Expand Down
7 changes: 4 additions & 3 deletions modules/core/04-channel/types/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (
)

// NewUpgrade creates a new Upgrade instance.
func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout) Upgrade {
func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, latestPacketSent uint64) Upgrade {
return Upgrade{
Fields: upgradeFields,
Timeout: timeout,
Fields: upgradeFields,
Timeout: timeout,
LatestSequenceSend: latestPacketSent,
}
}

Expand Down
88 changes: 60 additions & 28 deletions modules/core/04-channel/types/upgrade.pb.go

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

1 change: 1 addition & 0 deletions modules/core/04-channel/types/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func (suite *TypesTestSuite) TestUpgradeValidateBasic() {
upgrade = types.NewUpgrade(
types.NewUpgradeFields(types.ORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewTimeout(clienttypes.NewHeight(0, 100), 0),
0,
)

tc.malleate()
Expand Down
9 changes: 6 additions & 3 deletions proto/ibc/core/channel/v1/upgrade.proto
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +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 and the timeout for this upgrade attempt.
// 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.
message Upgrade {
option (gogoproto.goproto_getters) = false;

UpgradeFields fields = 1 [(gogoproto.nullable) = false];
Timeout timeout = 2 [(gogoproto.nullable) = false];
UpgradeFields fields = 1 [(gogoproto.nullable) = false];
Timeout timeout = 2 [(gogoproto.nullable) = false];
uint64 latest_sequence_send = 3;
}

// UpgradeFields are the fields in a channel end which may be changed
Expand Down
3 changes: 2 additions & 1 deletion testing/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,8 @@ func (endpoint *Endpoint) GetProposedUpgrade() channeltypes.Upgrade {
ConnectionHops: []string{endpoint.ConnectionID},
Version: endpoint.ChannelConfig.Version,
},
Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0),
Timeout: channeltypes.NewTimeout(endpoint.Counterparty.Chain.GetTimeoutHeight(), 0),
LatestSequenceSend: 0,
}

override := endpoint.ChannelConfig.ProposedUpgrade
Expand Down

0 comments on commit 9c346c5

Please sign in to comment.