Skip to content

Commit

Permalink
imp: remove LatestSequenceSend (#5108)
Browse files Browse the repository at this point in the history 
* del: latestsequencesend field

* fix: comment

* gofumpt

* del: tests

* del leftover tests part

* adressing comments

* addressing comment

---------

Co-authored-by: sangier <stefano@Stefanos-MacBook-Pro.local>
Co-authored-by: sangier <stefano@interchain.io>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
  • Loading branch information
@sangier
4 people authored Nov 14, 2023
1 parent 658b00f commit 34cbe05
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 181 deletions.
1 change: 0 additions & 1 deletion e2e/tests/core/02-client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ 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: 9 additions & 9 deletions e2e/testsuite/grpc_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ func (s *E2ETestSuite) InitGRPCClients(chain *cosmos.CosmosChain) {
FeeQueryClient: feetypes.NewQueryClient(grpcConn),
ICAControllerQueryClient: controllertypes.NewQueryClient(grpcConn),
ICAHostQueryClient: hosttypes.NewQueryClient(grpcConn),
BankQueryClient: banktypes.NewQueryClient(grpcConn),
GovQueryClient: govtypesv1beta1.NewQueryClient(grpcConn),
GovQueryClientV1: govtypesv1.NewQueryClient(grpcConn),
GroupsQueryClient: grouptypes.NewQueryClient(grpcConn),
ParamsQueryClient: paramsproposaltypes.NewQueryClient(grpcConn),
AuthQueryClient: authtypes.NewQueryClient(grpcConn),
AuthZQueryClient: authz.NewQueryClient(grpcConn),
ConsensusServiceClient: cmtservice.NewServiceClient(grpcConn),
UpgradeQueryClient: upgradetypes.NewQueryClient(grpcConn),
BankQueryClient: banktypes.NewQueryClient(grpcConn),
GovQueryClient: govtypesv1beta1.NewQueryClient(grpcConn),
GovQueryClientV1: govtypesv1.NewQueryClient(grpcConn),
GroupsQueryClient: grouptypes.NewQueryClient(grpcConn),
ParamsQueryClient: paramsproposaltypes.NewQueryClient(grpcConn),
AuthQueryClient: authtypes.NewQueryClient(grpcConn),
AuthZQueryClient: authz.NewQueryClient(grpcConn),
ConsensusServiceClient: cmtservice.NewServiceClient(grpcConn),
UpgradeQueryClient: upgradetypes.NewQueryClient(grpcConn),
}
}

Expand Down
18 changes: 0 additions & 18 deletions modules/core/04-channel/keeper/packet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,24 +132,6 @@ 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: 0 additions & 61 deletions modules/core/04-channel/keeper/packet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,70 +347,9 @@ 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: 0 additions & 6 deletions modules/core/04-channel/keeper/upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,12 +763,6 @@ 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
17 changes: 0 additions & 17 deletions modules/core/04-channel/keeper/upgrade_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,6 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() {
suite.Require().NoError(err)
suite.Require().NotEmpty(upgrade)
suite.Require().Equal(proposedUpgrade.Fields, upgrade.Fields)

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 @@ -489,14 +485,6 @@ 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 @@ -1757,12 +1745,7 @@ 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: 2 additions & 3 deletions modules/core/04-channel/types/upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import (
// NewUpgrade creates a new Upgrade instance.
func NewUpgrade(upgradeFields UpgradeFields, timeout Timeout, latestPacketSent uint64) Upgrade {
return Upgrade{
Fields: upgradeFields,
Timeout: timeout,
LatestSequenceSend: latestPacketSent,
Fields: upgradeFields,
Timeout: timeout,
}
}

Expand Down
87 changes: 28 additions & 59 deletions modules/core/04-channel/types/upgrade.pb.go
View file

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

8 changes: 3 additions & 5 deletions proto/ibc/core/channel/v1/upgrade.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ 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
// to allow the counterparty to block sends after the upgrade has started.
// end and the timeout for this upgrade attempt.
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];
}

// UpgradeFields are the fields in a channel end which may be changed
Expand Down
3 changes: 1 addition & 2 deletions testing/endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,8 +873,7 @@ 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),
}

override := endpoint.ChannelConfig.ProposedUpgrade
Expand Down

0 comments on commit 34cbe05

Please sign in to comment.