Skip to content

Commit

Permalink
Adding additional comments and changing version variable names (#6345)
Browse files Browse the repository at this point in the history
  • Loading branch information
chatton authored May 21, 2024
1 parent f39d173 commit 9b39944
Show file tree
Hide file tree
Showing 14 changed files with 53 additions and 51 deletions.
8 changes: 4 additions & 4 deletions e2e/tests/transfer/localhost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() {

t.Run("channel open init localhost", func(t *testing.T) {
msgChanOpenInit := channeltypes.NewMsgChannelOpenInit(
transfertypes.PortID, transfertypes.Version,
transfertypes.PortID, transfertypes.V2,
channeltypes.UNORDERED, []string{exported.LocalhostConnectionID},
transfertypes.PortID, rlyWallet.FormattedAddress(),
)
Expand All @@ -83,10 +83,10 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() {

t.Run("channel open try localhost", func(t *testing.T) {
msgChanOpenTry := channeltypes.NewMsgChannelOpenTry(
transfertypes.PortID, transfertypes.Version,
transfertypes.PortID, transfertypes.V2,
channeltypes.UNORDERED, []string{exported.LocalhostConnectionID},
transfertypes.PortID, msgChanOpenInitRes.ChannelId,
transfertypes.Version, localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(),
transfertypes.V2, localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(),
)

txResp := s.BroadcastMessages(ctx, chainA, rlyWallet, msgChanOpenTry)
Expand All @@ -98,7 +98,7 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() {
t.Run("channel open ack localhost", func(t *testing.T) {
msgChanOpenAck := channeltypes.NewMsgChannelOpenAck(
transfertypes.PortID, msgChanOpenInitRes.ChannelId,
msgChanOpenTryRes.ChannelId, transfertypes.Version,
msgChanOpenTryRes.ChannelId, transfertypes.V2,
localhost.SentinelProof, clienttypes.ZeroHeight(), rlyWallet.FormattedAddress(),
)

Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/transfer/upgrades_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_
s.Require().NoError(err)

s.Require().Equal(channeltypes.OPEN, channel.State, "the channel state is not OPEN")
s.Require().Equal(transfertypes.Version, channel.Version, "the channel version is not ics20-1")
s.Require().Equal(transfertypes.V2, channel.Version, "the channel version is not ics20-2")

errorReceipt, err := s.QueryUpgradeError(ctx, chainA, channelA.PortID, channelA.ChannelID)
s.Require().NoError(err)
Expand All @@ -374,7 +374,7 @@ func (s *TransferChannelUpgradesTestSuite) TestChannelUpgrade_WithFeeMiddleware_
s.Require().NoError(err)

s.Require().Equal(channeltypes.OPEN, channel.State, "the channel state is not OPEN")
s.Require().Equal(transfertypes.Version, channel.Version, "the channel version is not ics20-1")
s.Require().Equal(transfertypes.V2, channel.Version, "the channel version is not ics20-2")

errorReceipt, err := s.QueryUpgradeError(ctx, chainB, channelB.PortID, channelB.ChannelID)
s.Require().NoError(err)
Expand Down
6 changes: 3 additions & 3 deletions e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s *E2ETestSuite) ConfigureRelayer(ctx context.Context, chainA, chainB ibc.
channelOptions := ibc.DefaultChannelOpts()
// TODO: better way to do this.
// For now, set the version to the latest transfer module version
channelOptions.Version = transfertypes.Version
channelOptions.Version = transfertypes.V2

if channelOpts != nil {
channelOpts(&channelOptions)
Expand Down Expand Up @@ -401,7 +401,7 @@ func (s *E2ETestSuite) GetRelayerExecReporter() *testreporter.RelayerExecReporte
// TransferChannelOptions configures both of the chains to have non-incentivized transfer channels.
func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOptions) {
return func(opts *ibc.CreateChannelOptions) {
opts.Version = transfertypes.Version
opts.Version = transfertypes.V2
opts.SourcePortName = transfertypes.PortID
opts.DestPortName = transfertypes.PortID
}
Expand All @@ -411,7 +411,7 @@ func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOpt
func (s *E2ETestSuite) FeeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) {
versionMetadata := feetypes.Metadata{
FeeVersion: feetypes.Version,
AppVersion: transfertypes.Version,
AppVersion: transfertypes.V2,
}
versionBytes, err := feetypes.ModuleCdc.MarshalJSON(&versionMetadata)
s.Require().NoError(err)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/keeper/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (suite *KeeperTestSuite) TestIncentivizePacketEvent() {
func (suite *KeeperTestSuite) TestDistributeFeeEvent() {
// create an incentivized transfer path
path := ibctesting.NewPath(suite.chainA, suite.chainB)
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2}))
path.EndpointA.ChannelConfig.Version = feeTransferVersion
path.EndpointB.ChannelConfig.Version = feeTransferVersion
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
Expand Down
8 changes: 4 additions & 4 deletions modules/apps/29-fee/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
// Integration test to ensure ics29 works with ics20
func (suite *FeeTestSuite) TestFeeTransfer() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2}))
path.EndpointA.ChannelConfig.Version = feeTransferVersion
path.EndpointB.ChannelConfig.Version = feeTransferVersion
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
Expand Down Expand Up @@ -94,13 +94,13 @@ func (suite *FeeTestSuite) TestTransferFeeUpgrade() {
// configure the initial path to create a regular transfer channel
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
path.EndpointB.ChannelConfig.PortID = transfertypes.PortID
path.EndpointA.ChannelConfig.Version = transfertypes.Version
path.EndpointB.ChannelConfig.Version = transfertypes.Version
path.EndpointA.ChannelConfig.Version = transfertypes.V2
path.EndpointB.ChannelConfig.Version = transfertypes.V2

path.Setup()

// configure the channel upgrade to upgrade to an incentivized fee enabled transfer channel
upgradeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
upgradeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.V2}))
path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = upgradeVersion
path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = upgradeVersion

Expand Down
6 changes: 3 additions & 3 deletions modules/apps/callbacks/callbacks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (s *CallbacksTestSuite) SetupTransferTest() {

s.path.EndpointA.ChannelConfig.PortID = ibctesting.TransferPort
s.path.EndpointB.ChannelConfig.PortID = ibctesting.TransferPort
s.path.EndpointA.ChannelConfig.Version = transfertypes.Version
s.path.EndpointB.ChannelConfig.Version = transfertypes.Version
s.path.EndpointA.ChannelConfig.Version = transfertypes.V2
s.path.EndpointB.ChannelConfig.Version = transfertypes.V2

s.path.Setup()
}
Expand All @@ -88,7 +88,7 @@ func (s *CallbacksTestSuite) SetupTransferTest() {
func (s *CallbacksTestSuite) SetupFeeTransferTest() {
s.setupChains()

feeTransferVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: transfertypes.Version}))
feeTransferVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feetypes.Metadata{FeeVersion: feetypes.Version, AppVersion: transfertypes.V2}))
s.path.EndpointA.ChannelConfig.Version = feeTransferVersion
s.path.EndpointB.ChannelConfig.Version = feeTransferVersion
s.path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
Expand Down
6 changes: 3 additions & 3 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (im IBCModule) OnChanOpenInit(

// default to latest supported version
if strings.TrimSpace(version) == "" {
version = types.Version
version = types.V2
}

if !slices.Contains(types.SupportedVersions, version) {
Expand Down Expand Up @@ -120,7 +120,7 @@ func (im IBCModule) OnChanOpenTry(
}

if !slices.Contains(types.SupportedVersions, counterpartyVersion) {
return types.Version, nil
return types.V2, nil
}

// OpenTry must claim the channelCapability that IBC passes into the callback
Expand Down Expand Up @@ -375,7 +375,7 @@ func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
}

if !slices.Contains(types.SupportedVersions, counterpartyVersion) {
return types.Version, nil
return types.V2, nil
}

return counterpartyVersion, nil
Expand Down
28 changes: 14 additions & 14 deletions modules/apps/transfer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,24 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
expVersion string
}{
{
"success", func() {}, true, types.Version,
"success", func() {}, true, types.V2,
},
{
// connection hops is not used in the transfer application callback,
// it is already validated in the core OnChanUpgradeInit.
"success: invalid connection hops", func() {
path.EndpointA.ConnectionID = "invalid-connection-id"
}, true, types.Version,
}, true, types.V2,
},
{
"success: empty version string", func() {
channel.Version = ""
}, true, types.Version,
}, true, types.V2,
},
{
"success: ics20-1 legacy", func() {
channel.Version = types.Version1
}, true, types.Version1,
channel.Version = types.V1
}, true, types.V1,
},
{
"max channels reached", func() {
Expand Down Expand Up @@ -93,7 +93,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
Ordering: channeltypes.UNORDERED,
Counterparty: counterparty,
ConnectionHops: []string{path.EndpointA.ConnectionID},
Version: types.Version,
Version: types.V2,
}

var err error
Expand Down Expand Up @@ -134,17 +134,17 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
expVersion string
}{
{
"success", func() {}, true, types.Version,
"success", func() {}, true, types.V2,
},
{
"success: counterparty version is legacy ics20-1", func() {
counterpartyVersion = types.Version1
}, true, types.Version1,
counterpartyVersion = types.V1
}, true, types.V1,
},
{
"success: invalid counterparty version, we use our proposed version", func() {
counterpartyVersion = "version"
}, true, types.Version,
}, true, types.V2,
},
{
"max channels reached", func() {
Expand Down Expand Up @@ -185,9 +185,9 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
Ordering: channeltypes.UNORDERED,
Counterparty: counterparty,
ConnectionHops: []string{path.EndpointA.ConnectionID},
Version: types.Version,
Version: types.V2,
}
counterpartyVersion = types.Version
counterpartyVersion = types.V2

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)
Expand Down Expand Up @@ -242,7 +242,7 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
path := ibctesting.NewTransferPath(suite.chainA, suite.chainB)
path.SetupConnections()
path.EndpointA.ChannelID = ibctesting.FirstChannelID
counterpartyVersion = types.Version
counterpartyVersion = types.V2

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
suite.Require().NoError(err)
Expand Down Expand Up @@ -405,7 +405,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() {
expPass := tc.expError == nil
if expPass {
suite.Require().NoError(err)
suite.Require().Equal(types.Version, version)
suite.Require().Equal(types.V2, version)
} else {
suite.Require().Error(err)
suite.Require().Contains(err.Error(), tc.expError.Error())
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/mbt_relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func AddressFromTla(addr []string) string {
s = addr[2]
} else if len(addr[2]) == 0 {
// escrow address: ics20-1\x00port/channel
s = fmt.Sprintf("%s\x00%s/%s", types.Version1, addr[0], addr[1])
s = fmt.Sprintf("%s\x00%s/%s", types.V1, addr[0], addr[1])
} else {
panic(errors.New("failed to convert from TLA+ address: neither simple nor escrow address"))
}
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
}

// ics20-1 only supports a single token, so if that is the current version, we must only process a single token.
if appVersion == types.Version1 && len(tokens) > 1 {
if appVersion == types.V1 && len(tokens) > 1 {
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot transfer multiple tokens with ics20-1")
}

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {

// explicitly set to ics20-1 which does not support multi-denom
path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) {
channel.Version = types.Version1
channel.Version = types.V1
})
},
ibcerrors.ErrInvalidRequest,
Expand Down
16 changes: 9 additions & 7 deletions modules/apps/transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,17 @@ const (
)

const (
// Version defines the current version the IBC transfer
// module supports
Version = "ics20-2"
// V1 defines first version of the IBC transfer module
V1 = "ics20-1"

// Version1 defines first version of the IBC transfer module
Version1 = "ics20-1"
// V2 defines the current version the IBC transfer
// module supports
V2 = "ics20-2"

// escrowAddressVersion should remain as ics20-1 to avoid the address changing.
escrowAddressVersion = Version1
// this address has been reasoned about to avoid collisions with other addresses
// https://github.com/cosmos/cosmos-sdk/issues/7737#issuecomment-735671951
escrowAddressVersion = V1
)

var (
Expand All @@ -53,7 +55,7 @@ var (
DenomTraceKey = []byte{0x02}

// SupportedVersions defines all versions that are supported by the module
SupportedVersions = []string{Version, Version1}
SupportedVersions = []string{V2, V1}
)

// GetEscrowAddress returns the escrow address for the specified channel.
Expand Down
4 changes: 2 additions & 2 deletions testing/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ func NewTransferPath(chainA, chainB *TestChain) *Path {
path := NewPath(chainA, chainB)
path.EndpointA.ChannelConfig.PortID = TransferPort
path.EndpointB.ChannelConfig.PortID = TransferPort
path.EndpointA.ChannelConfig.Version = transfertypes.Version
path.EndpointB.ChannelConfig.Version = transfertypes.Version
path.EndpointA.ChannelConfig.Version = transfertypes.V2
path.EndpointB.ChannelConfig.Version = transfertypes.V2

return path
}
Expand Down
10 changes: 5 additions & 5 deletions testing/solomachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (solo *Solomachine) ConnOpenAck(chain *TestChain, clientID, connectionID st
func (solo *Solomachine) ChanOpenInit(chain *TestChain, connectionID string) string {
msgChanOpenInit := channeltypes.NewMsgChannelOpenInit(
transfertypes.PortID,
transfertypes.Version,
transfertypes.V2,
channeltypes.UNORDERED,
[]string{connectionID},
transfertypes.PortID,
Expand All @@ -332,12 +332,12 @@ func (solo *Solomachine) ChanOpenInit(chain *TestChain, connectionID string) str
// ChanOpenAck performs the channel open ack handshake step on the tendermint chain for the associated
// solo machine client.
func (solo *Solomachine) ChanOpenAck(chain *TestChain, channelID string) {
tryProof := solo.GenerateChanOpenTryProof(transfertypes.PortID, transfertypes.Version, channelID)
tryProof := solo.GenerateChanOpenTryProof(transfertypes.PortID, transfertypes.V2, channelID)
msgChanOpenAck := channeltypes.NewMsgChannelOpenAck(
transfertypes.PortID,
channelID,
channelIDSolomachine,
transfertypes.Version,
transfertypes.V2,
tryProof,
clienttypes.ZeroHeight(),
chain.SenderAccount.GetAddress().String(),
Expand All @@ -351,7 +351,7 @@ func (solo *Solomachine) ChanOpenAck(chain *TestChain, channelID string) {
// ChanCloseConfirm performs the channel close confirm handshake step on the tendermint chain for the associated
// solo machine client.
func (solo *Solomachine) ChanCloseConfirm(chain *TestChain, portID, channelID string) {
initProof := solo.GenerateChanClosedProof(portID, transfertypes.Version, channelID)
initProof := solo.GenerateChanClosedProof(portID, transfertypes.V2, channelID)
msgChanCloseConfirm := channeltypes.NewMsgChannelCloseConfirm(
portID,
channelID,
Expand Down Expand Up @@ -442,7 +442,7 @@ func (solo *Solomachine) TimeoutPacket(chain *TestChain, packet channeltypes.Pac

// TimeoutPacketOnClose creates a channel closed and unreceived packet proof and broadcasts a MsgTimeoutOnClose.
func (solo *Solomachine) TimeoutPacketOnClose(chain *TestChain, packet channeltypes.Packet, channelID string) {
closedProof := solo.GenerateChanClosedProof(transfertypes.PortID, transfertypes.Version, channelID)
closedProof := solo.GenerateChanClosedProof(transfertypes.PortID, transfertypes.V2, channelID)
unreceivedProof := solo.GenerateReceiptAbsenceProof(packet)
msgTimeout := channeltypes.NewMsgTimeoutOnClose(
packet,
Expand Down

0 comments on commit 9b39944

Please sign in to comment.