Skip to content

Commit

Permalink
Revert "imp: remove LatestSequenceSend (#5108)"
Browse files Browse the repository at this point in the history
This reverts commit 34cbe05.
  • Loading branch information
charleenfei committed Dec 18, 2023
1 parent 914a78d commit 0c25c47
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 35 deletions.
1 change: 1 addition & 0 deletions e2e/tests/core/02-client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func (s *ClientTestSuite) TestScheduleIBCUpgrade_Succeeds() {
})

t.Run("ensure legacy proposal does not succeed", func(t *testing.T) {

authority, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA)
s.Require().NoError(err)
s.Require().NotNil(authority)
Expand Down
18 changes: 18 additions & 0 deletions modules/core/04-channel/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,24 @@ func (k Keeper) RecvPacket(
return errorsmod.Wrapf(types.ErrInvalidChannelState, "expected channel state to be one of [%s, %s], but got %s", types.OPEN, types.FLUSHING, channel.State)
}

// in the case of the channel being in FLUSHING we need to ensure that the the counterparty last sequence send
// is less than or equal to the packet sequence.
if channel.State == types.FLUSHING {
counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, packet.GetDestPort(), packet.GetDestChannel())
if !found {
return errorsmod.Wrapf(types.ErrUpgradeNotFound, "counterparty upgrade not found for channel: %s", packet.GetDestChannel())
}

// only error if the counterparty latest sequence send is set (> 0)
counterpartyLatestSequenceSend := counterpartyUpgrade.LatestSequenceSend
if counterpartyLatestSequenceSend != 0 && packet.GetSequence() > counterpartyLatestSequenceSend {
return errorsmod.Wrapf(
types.ErrInvalidPacket,
"failed to receive packet, cannot flush packet at sequence greater than counterparty last sequence send (%d) > (%d)", packet.GetSequence(), counterpartyLatestSequenceSend,
)
}
}

// Authenticate capability to ensure caller has authority to receive packet on this channel
capName := host.ChannelCapabilityPath(packet.GetDestPort(), packet.GetDestChannel())
if !k.scopedKeeper.AuthenticateCapability(ctx, chanCap, capName) {
Expand Down
61 changes: 61 additions & 0 deletions modules/core/04-channel/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,70 @@ 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,
},
{
"failure while upgrading channel, counterparty upgrade not found",
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)
},
types.ErrUpgradeNotFound,
},
{
"failure while upgrading channel, packet sequence > counterparty last send sequence",
func() {
suite.coordinator.Setup(path)
// send 2 packets so that when LatestSequenceSend is set to sequence - 1, it is not 0.
_, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
suite.Require().NoError(err)
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 sequence - 1
counterpartyUpgrade := types.Upgrade{LatestSequenceSend: sequence - 1}
path.EndpointB.SetChannelCounterpartyUpgrade(counterpartyUpgrade)
},
types.ErrInvalidPacket,
},
{
"failure while upgrading channel, channel in flush complete state",
func() {
Expand Down
6 changes: 6 additions & 0 deletions modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
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
18 changes: 17 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 @@ -1857,7 +1868,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
5 changes: 3 additions & 2 deletions modules/core/04-channel/types/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import (
// NewUpgrade creates a new Upgrade instance.
func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout) Upgrade {
return Upgrade{
Fields: upgradeFields,
Timeout: timeout,
Fields: upgradeFields,
Timeout: timeout,
LatestSequenceSend: latestPacketSent,

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / lint

undefined: latestPacketSent) (typecheck)

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / build (arm64)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / build (amd64)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (00)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (01)

undefined: latestPacketSent

Check failure on line 19 in modules/core/04-channel/types/upgrade.go

View workflow job for this annotation

GitHub Actions / tests (01)

undefined: latestPacketSent
}
}

Expand Down
87 changes: 59 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.

8 changes: 5 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,14 @@ 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
// to allow the counterparty to block sends after the upgrade has started.
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 0c25c47

Please sign in to comment.