Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify startFlushUpgradeHandshake to be used as a write function #4360

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b0d51f0
WIP: adding initial implementation of changes
damiannolan Aug 7, 2023
ba8fb6c
proto format
damiannolan Aug 7, 2023
f4e5f9c
commenting out more failing tests from timeouts
damiannolan Aug 8, 2023
0bae135
Merge branch '04-channel-upgrades' into damian/4237-modify-chan-upgra…
damiannolan Aug 8, 2023
166e4a9
Merge branch '04-channel-upgrades' into damian/4237-modify-chan-upgra…
damiannolan Aug 8, 2023
a140964
Merge branch '04-channel-upgrades' into damian/4237-modify-chan-upgra…
damiannolan Aug 14, 2023
60940de
fix compiler error
damiannolan Aug 14, 2023
de2173f
commenting out failing testcases due to timeout logic
damiannolan Aug 14, 2023
56ef614
Merge branch '04-channel-upgrades' into damian/4237-modify-chan-upgra…
damiannolan Aug 14, 2023
b38c6f9
fix: reorder proto msg fields correctly
damiannolan Aug 14, 2023
74eb09e
refactor: move increment upgrade sequence to write fn, rename current…
damiannolan Aug 14, 2023
287e13a
refactor: rename msg server vars for consistency
damiannolan Aug 14, 2023
101fce0
update FirstChannelID to FirstConnectionID in msg validate basic tests
damiannolan Aug 15, 2023
bbdfe6b
rename test var and use mock.UpgradeVersion
damiannolan Aug 15, 2023
20ecdba
Merge branch '04-channel-upgrades' into damian/4237-modify-chan-upgra…
damiannolan Aug 15, 2023
af1b009
comment out failing tests
damiannolan Aug 15, 2023
aa4ace7
Merge branch '04-channel-upgrades' into damian/4247-modify-init-state
damiannolan Aug 16, 2023
250cc99
refactor upgrade init state to open. refactor crossing hellos and try…
damiannolan Aug 16, 2023
9cebf01
updating godoc and error return in chanUpgradeAck2
damiannolan Aug 16, 2023
f434996
refactor: rm unnecessary args in application upgrade init callback
damiannolan Aug 16, 2023
60ca446
Merge branch '04-channel-upgrades' into damian/cian/refactor-app-cb-args
damiannolan Aug 16, 2023
bb89523
chore: implement startFlushing function and refactor tests
chatton Aug 16, 2023
6457764
Merge branch '04-channel-upgrades' into cian/issue#4256-modify-startf…
chatton Aug 16, 2023
991c055
chore: clarify docstrings
chatton Aug 16, 2023
ce7531b
Merge branch '04-channel-upgrades' into cian/issue#4256-modify-startf…
chatton Aug 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions modules/core/04-channel/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,12 @@ package keeper
import (
sdk "github.com/cosmos/cosmos-sdk/types"

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
)

// StartFlushUpgradeHandshake is a wrapper around startFlushUpgradeHandshake to allow the function to be directly called in tests.
func (k Keeper) StartFlushUpgradeHandshake(
ctx sdk.Context,
portID,
channelID string,
proposedUpgradeFields types.UpgradeFields,
counterpartyChannel types.Channel,
counterpartyUpgrade types.Upgrade,
proofCounterpartyChannel,
proofCounterpartyUpgrade []byte,
proofHeight clienttypes.Height,
) error {
return k.startFlushUpgradeHandshake(ctx, portID, channelID, proposedUpgradeFields, counterpartyChannel, counterpartyUpgrade, proofCounterpartyChannel, proofCounterpartyUpgrade, proofHeight)
// StartFlushing is a wrapper around startFlushing to allow the function to be directly called in tests.
func (k Keeper) StartFlushing(ctx sdk.Context, portID, channelID string) error {
return k.startFlushing(ctx, portID, channelID)
}

// ValidateSelfUpgradeFields is a wrapper around validateSelfUpgradeFields to allow the function to be directly called in tests.
Expand Down
53 changes: 27 additions & 26 deletions modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,7 @@ func (k Keeper) ChanUpgradeTry(
return types.Upgrade{}, errorsmod.Wrap(err, "failed to verify counterparty upgrade")
}

if err := k.startFlushUpgradeHandshake(
ctx,
portID, channelID,
proposedUpgradeFields,
counterpartyChannel,
types.NewUpgrade(counterpartyUpgradeFields, types.Timeout{}, 0),
proofCounterpartyChannel, proofCounterpartyUpgrade,
proofHeight,
); err != nil {
if err := k.startFlushing(ctx, portID, channelID); err != nil {
return types.Upgrade{}, err
}

Expand Down Expand Up @@ -300,8 +292,7 @@ func (k Keeper) ChanUpgradeAck(
return types.NewUpgradeError(channel.UpgradeSequence, errorsmod.Wrap(err, "counterparty upgrade timeout has passed"))
}

if err := k.startFlushUpgradeHandshake(ctx, portID, channelID, upgrade.Fields, counterpartyChannel, counterpartyUpgrade,
proofChannel, proofUpgrade, proofHeight); err != nil {
if err := k.startFlushing(ctx, portID, channelID); err != nil {
return err
}

Expand Down Expand Up @@ -718,21 +709,9 @@ func (k Keeper) WriteUpgradeTimeoutChannel(
emitChannelUpgradeTimeoutEvent(ctx, portID, channelID, channel, upgrade)
}

// startFlushUpgradeHandshake will verify the counterparty proposed upgrade and the current channel state.
// Once the counterparty information has been verified, it will be validated against the self proposed upgrade.
// If any of the proposed upgrade fields are incompatible, an upgrade error will be returned resulting in an
// aborted upgrade.
func (k Keeper) startFlushUpgradeHandshake(
ctx sdk.Context,
portID,
channelID string,
proposedUpgradeFields types.UpgradeFields,
counterpartyChannel types.Channel,
counterpartyUpgrade types.Upgrade,
proofCounterpartyChannel,
proofCounterpartyUpgrade []byte,
proofHeight clienttypes.Height,
) error {
// startFlushing will set the upgrade last packet send and continue blocking the upgrade from continuing until all
// in-flight packets have been flushed.
func (k Keeper) startFlushing(ctx sdk.Context, portID, channelID string) error {
channel, found := k.GetChannel(ctx, portID, channelID)
if !found {
return errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
Expand All @@ -747,9 +726,31 @@ func (k Keeper) startFlushUpgradeHandshake(
return errorsmod.Wrapf(connectiontypes.ErrInvalidConnectionState, "connection state is not OPEN (got %s)", connectiontypes.State(connection.GetState()).String())
}

channel.State = types.STATE_FLUSHING
k.SetChannel(ctx, portID, channelID, channel)

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

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 = getUpgradeTimeout()
k.SetUpgrade(ctx, portID, channelID, upgrade)

return nil
}

// TODO: use a hard coded value for now. Will be resolved in https://github.com/cosmos/ibc-go/issues/4313
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4313 (for linking purposes)

func getUpgradeTimeout() types.Timeout {
return types.NewTimeout(clienttypes.NewHeight(1, 1000), 0)
}

// syncUpgradeSequence ensures current upgrade handshake only continues if both channels are using the same upgrade sequence,
// otherwise an upgrade error is returned so that an error receipt will be written so that the upgrade handshake may be attempted again with synchronized sequences.
func (k Keeper) syncUpgradeSequence(ctx sdk.Context, portID, channelID string, channel types.Channel, counterpartyUpgradeSequence uint64) error {
Expand Down
52 changes: 21 additions & 31 deletions modules/core/04-channel/keeper/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
commitmenttypes.ErrInvalidProof,
},
{
"startFlushUpgradeHandshake fails due to mismatch in upgrade ordering",
"fails due to mismatch in upgrade ordering",
func() {
upgrade := path.EndpointA.GetChannelUpgrade()
upgrade.Fields.Ordering = types.NONE
Expand Down Expand Up @@ -1376,16 +1376,8 @@ func (suite *KeeperTestSuite) TestWriteUpgradeCancelChannel() {
// }
// }

// TestStartFlushUpgradeHandshake tests the startFlushUpgradeHandshake.
// UpgradeInit will be run on chainA and startFlushUpgradeHandshake
// will be called on chainB
func (suite *KeeperTestSuite) TestStartFlushUpgradeHandshake() {
var (
path *ibctesting.Path
upgrade types.Upgrade
counterpartyChannel types.Channel
counterpartyUpgrade types.Upgrade
)
func (suite *KeeperTestSuite) TestStartFlush() {
var path *ibctesting.Path

testCases := []struct {
name string
Expand Down Expand Up @@ -1434,38 +1426,36 @@ func (suite *KeeperTestSuite) TestStartFlushUpgradeHandshake() {
suite.coordinator.Setup(path)

path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion

err := path.EndpointA.ChanUpgradeInit()
suite.Require().NoError(err)

// ensure proof verification succeeds
err = path.EndpointB.UpdateClient()
// crossing hellos so that the upgrade is created on chain B.
// the ChanUpgradeInit sub protocol is also called when it is not a crossing hello situation.
err = path.EndpointB.ChanUpgradeInit()
suite.Require().NoError(err)

proofChannel, proofUpgrade, proofHeight := path.EndpointB.QueryChannelUpgradeProof()
counterpartyChannel = path.EndpointA.GetChannel()

var found bool
counterpartyUpgrade, found = path.EndpointA.Chain.App.GetIBCKeeper().ChannelKeeper.GetUpgrade(path.EndpointA.Chain.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().True(found)

// ensure that the channel has a valid upgrade sequence
channel := path.EndpointB.GetChannel()
channel.UpgradeSequence = 1
path.EndpointB.SetChannel(channel)

path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion
upgrade = path.EndpointB.GetProposedUpgrade()

tc.malleate()

err = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.StartFlushUpgradeHandshake(
suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, upgrade.Fields,
counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight,
err = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.StartFlushing(
suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID,
)

if tc.expError != nil {
suite.assertUpgradeError(err, tc.expError)
} else {
channel := path.EndpointB.GetChannel()
upgrade := path.EndpointB.GetChannelUpgrade()

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.STATE_FLUSHING, channel.State)
suite.Require().Equal(nextSequenceSend-1, upgrade.LatestSequenceSend)

// TODO: fix in https://github.com/cosmos/ibc-go/issues/4313
suite.Require().Equal(types.NewTimeout(clienttypes.NewHeight(1, 1000), 0), upgrade.Timeout)
suite.Require().NoError(err)
}
})
Expand Down
Loading