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

feat: adding OnChanUpgradeInit implementation to transfer #4126

Merged
merged 12 commits into from
Jul 24, 2023
Merged
12 changes: 10 additions & 2 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,16 @@ func (im IBCModule) OnTimeoutPacket(
}

// OnChanUpgradeInit implements the IBCModule interface
func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, sequence uint64, version, previousVersion string) (string, error) {
return version, nil
func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, upgradeSequence uint64, upgradeVersion, previousVersion string) (string, error) {
if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
return "", err
}

if upgradeVersion != types.Version {
return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, upgradeVersion)
}

return upgradeVersion, nil
}

// OnChanUpgradeTry implements the IBCModule interface
Expand Down
73 changes: 73 additions & 0 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/cosmos/ibc-go/v7/modules/apps/transfer"
"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
Expand Down Expand Up @@ -239,3 +240,75 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
})
}
}

func (suite *TransferTestSuite) TestOnChanUpgradeInit() {
var path *ibctesting.Path

testCases := []struct {
name string
malleate func()
expError error
}{
{
"success",
func() {}, // successful happy path for a standalone transfer app is swapping out the underlying connection
Copy link
Contributor

Choose a reason for hiding this comment

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

i like this convention of explaining the success case inline

nil,
},
{
"invalid upgrade connection",
Copy link
Contributor

Choose a reason for hiding this comment

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

In theory we don't need this test here, right? Because the test will fail here before reaching the transfer callback, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, this is failing well before the application is ever called, I'd be fine with removing it as there is already tests using the mock app in core for this. I just put it here to make things whole (testing version, ordering and connection)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave this for now, we can remove later if needs be

func() {
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.ConnectionHops = []string{"connection-100"}
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.ConnectionHops = []string{"connection-100"}
},
connectiontypes.ErrConnectionNotFound,
},
{
"invalid upgrade ordering",
func() {
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = channeltypes.ORDERED
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Ordering = channeltypes.ORDERED
},
channeltypes.ErrInvalidChannelOrdering,
},
{
"invalid upgrade version",
func() {
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = "invalid-version"
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = "invalid-version"
},
types.ErrInvalidVersion,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()

path = NewTransferPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)

// configure the channel upgrade to modify the underlying connection
upgradePath := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(upgradePath)

path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.ConnectionHops = []string{upgradePath.EndpointA.ConnectionID}
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.ConnectionHops = []string{upgradePath.EndpointB.ConnectionID}

tc.malleate()

err := path.EndpointA.ChanUpgradeInit()

expPass := tc.expError == nil
if expPass {
suite.Require().NoError(err)

upgrade := path.EndpointA.GetChannelUpgrade()
suite.Require().Equal(upgradePath.EndpointA.ConnectionID, upgrade.Fields.ConnectionHops[0])
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expError)
}
})
}
}