Skip to content

Commit

Permalink
Change address type to string (cosmos#6009)
Browse files Browse the repository at this point in the history
Closes: cosmos#5977
  • Loading branch information
cwgoes authored Apr 16, 2020
1 parent 06b85f9 commit 3cc4063
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 46 deletions.
6 changes: 1 addition & 5 deletions x/ibc/20-transfer/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,14 @@ func GetTransferTxCmd(cdc *codec.Codec) *cobra.Command {
if err != nil {
return err
}
receiver, err := sdk.AccAddressFromBech32(args[3])
if err != nil {
return err
}

// parse coin trying to be sent
coins, err := sdk.ParseCoins(args[4])
if err != nil {
return err
}

msg := types.NewMsgTransfer(srcPort, srcChannel, uint64(destHeight), coins, sender, receiver)
msg := types.NewMsgTransfer(srcPort, srcChannel, uint64(destHeight), coins, sender, args[3])
if err := msg.ValidateBasic(); err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions x/ibc/20-transfer/client/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func RegisterRoutes(cliCtx context.CLIContext, r *mux.Router) {

// TransferTxReq defines the properties of a transfer tx request's body.
type TransferTxReq struct {
BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"`
DestHeight uint64 `json:"dest_height" yaml:"dest_height"`
Amount sdk.Coins `json:"amount" yaml:"amount"`
Receiver sdk.AccAddress `json:"receiver" yaml:"receiver"`
BaseReq rest.BaseReq `json:"base_req" yaml:"base_req"`
DestHeight uint64 `json:"dest_height" yaml:"dest_height"`
Amount sdk.Coins `json:"amount" yaml:"amount"`
Receiver string `json:"receiver" yaml:"receiver"`
}
2 changes: 1 addition & 1 deletion x/ibc/20-transfer/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func handleMsgTransfer(ctx sdk.Context, k Keeper, msg MsgTransfer) (*sdk.Result,
sdk.EventTypeMessage,
sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory),
sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender.String()),
sdk.NewAttribute(AttributeKeyReceiver, msg.Receiver.String()),
sdk.NewAttribute(AttributeKeyReceiver, msg.Receiver),
),
)

Expand Down
6 changes: 3 additions & 3 deletions x/ibc/20-transfer/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (suite *HandlerTestSuite) TestHandleMsgTransfer() {
suite.Require().Nil(err, "transfer module could not claim capability")

ctx := suite.chainA.GetContext()
msg := transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins2, testAddr1, testAddr2)
msg := transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins2, testAddr1, testAddr2.String())
res, err := handler(ctx, msg)
suite.Require().Error(err)
suite.Require().Nil(res, "%+v", res) // channel does not exist
Expand All @@ -103,14 +103,14 @@ func (suite *HandlerTestSuite) TestHandleMsgTransfer() {
suite.Require().NotNil(res, "%+v", res) // successfully executed

// test when the source is false
msg = transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins2, testAddr1, testAddr2)
msg = transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins2, testAddr1, testAddr2.String())
_ = suite.chainA.App.BankKeeper.SetBalances(ctx, testAddr1, testPrefixedCoins2)

res, err = handler(ctx, msg)
suite.Require().Error(err)
suite.Require().Nil(res, "%+v", res) // incorrect denom prefix

msg = transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins1, testAddr1, testAddr2)
msg = transfer.NewMsgTransfer(testPort1, testChannel1, 10, testPrefixedCoins1, testAddr1, testAddr2.String())
suite.chainA.App.SupplyKeeper.SetSupply(ctx, supply.NewSupply(testPrefixedCoins1))
_ = suite.chainA.App.BankKeeper.SetBalances(ctx, testAddr1, testPrefixedCoins1)

Expand Down
4 changes: 2 additions & 2 deletions x/ibc/20-transfer/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ const (

// define variables used for testing
var (
testAddr1 = sdk.AccAddress([]byte("testaddr1"))
testAddr2 = sdk.AccAddress([]byte("testaddr2"))
testAddr1, _ = sdk.AccAddressFromBech32("cosmos1scqhwpgsmr6vmztaa7suurfl52my6nd2kmrudl")
testAddr2, _ = sdk.AccAddressFromBech32("cosmos1scqhwpgsmr6vmztaa7suurfl52my6nd2kmrujl")

testCoins, _ = sdk.ParseCoins("100atom")
prefixCoins = sdk.NewCoins(sdk.NewCoin("bank/firstchannel/atom", sdk.NewInt(100)))
Expand Down
30 changes: 22 additions & 8 deletions x/ibc/20-transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ func (k Keeper) SendTransfer(
sourceChannel string,
destHeight uint64,
amount sdk.Coins,
sender,
receiver sdk.AccAddress,
sender sdk.AccAddress,
receiver string,
) error {
sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, sourcePort, sourceChannel)
if !found {
Expand All @@ -54,7 +54,8 @@ func (k Keeper) createOutgoingPacket(
destinationPort, destinationChannel string,
destHeight uint64,
amount sdk.Coins,
sender, receiver sdk.AccAddress,
sender sdk.AccAddress,
receiver string,
) error {
channelCap, ok := k.scopedKeeper.GetCapability(ctx, ibctypes.ChannelCapabilityPath(sourcePort, sourceChannel))
if !ok {
Expand Down Expand Up @@ -123,7 +124,7 @@ func (k Keeper) createOutgoingPacket(
}

packetData := types.NewFungibleTokenPacketData(
amount, sender, receiver,
amount, sender.String(), receiver,
)

packet := channel.NewPacket(
Expand All @@ -149,7 +150,14 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channel.Packet, data types.
prefix := types.GetDenomPrefix(packet.GetDestPort(), packet.GetDestChannel())
source := strings.HasPrefix(data.Amount[0].Denom, prefix)

// decode the receiver address
receiver, err := sdk.AccAddressFromBech32(data.Receiver)
if err != nil {
return err
}

if source {

// mint new tokens if the source of the transfer is the same chain
if err := k.supplyKeeper.MintCoins(
ctx, types.GetModuleAccountName(), data.Amount,
Expand All @@ -159,7 +167,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channel.Packet, data types.

// send to receiver
return k.supplyKeeper.SendCoinsFromModuleToAccount(
ctx, types.GetModuleAccountName(), data.Receiver, data.Amount,
ctx, types.GetModuleAccountName(), receiver, data.Amount,
)
}

Expand All @@ -178,7 +186,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channel.Packet, data types.

// unescrow tokens
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
return k.bankKeeper.SendCoins(ctx, escrowAddress, data.Receiver, coins)
return k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, coins)
}

func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channel.Packet, data types.FungibleTokenPacketData, ack types.FungibleTokenPacketAcknowledgement) error {
Expand All @@ -203,6 +211,12 @@ func (k Keeper) refundPacketAmount(ctx sdk.Context, packet channel.Packet, data
prefix := types.GetDenomPrefix(packet.GetSourcePort(), packet.GetSourceChannel())
source := strings.HasPrefix(data.Amount[0].Denom, prefix)

// decode the sender address
sender, err := sdk.AccAddressFromBech32(data.Sender)
if err != nil {
return err
}

if source {
coins := make(sdk.Coins, len(data.Amount))
for i, coin := range data.Amount {
Expand All @@ -215,7 +229,7 @@ func (k Keeper) refundPacketAmount(ctx sdk.Context, packet channel.Packet, data

// unescrow tokens back to sender
escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
return k.bankKeeper.SendCoins(ctx, escrowAddress, data.Sender, coins)
return k.bankKeeper.SendCoins(ctx, escrowAddress, sender, coins)
}

// mint vouchers back to sender
Expand All @@ -225,5 +239,5 @@ func (k Keeper) refundPacketAmount(ctx sdk.Context, packet channel.Packet, data
return err
}

return k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.GetModuleAccountName(), data.Sender, data.Amount)
return k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.GetModuleAccountName(), sender, data.Amount)
}
8 changes: 4 additions & 4 deletions x/ibc/20-transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
tc.malleate()

err = suite.chainA.App.TransferKeeper.SendTransfer(
suite.chainA.GetContext(), testPort1, testChannel1, 100, tc.amount, testAddr1, testAddr2,
suite.chainA.GetContext(), testPort1, testChannel1, 100, tc.amount, testAddr1, testAddr2.String(),
)

if tc.expPass {
Expand All @@ -107,7 +107,7 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
}

func (suite *KeeperTestSuite) TestOnRecvPacket() {
data := types.NewFungibleTokenPacketData(prefixCoins2, testAddr1, testAddr2)
data := types.NewFungibleTokenPacketData(prefixCoins2, testAddr1.String(), testAddr2.String())

testCases := []struct {
msg string
Expand Down Expand Up @@ -164,7 +164,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
// TestOnAcknowledgementPacket tests that successful acknowledgement is a no-op
// and failure acknowledment leads to refund
func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {
data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1, testAddr2)
data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1.String(), testAddr2.String())
testCoins2 := sdk.NewCoins(sdk.NewCoin("testportid/secondchannel/atom", sdk.NewInt(100)))

successAck := types.FungibleTokenPacketAcknowledgement{
Expand Down Expand Up @@ -233,7 +233,7 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {

// TestOnTimeoutPacket test private refundPacket function since it is a simple wrapper over it
func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1, testAddr2)
data := types.NewFungibleTokenPacketData(prefixCoins, testAddr1.String(), testAddr2.String())
testCoins2 := sdk.NewCoins(sdk.NewCoin("testportid/secondchannel/atom", sdk.NewInt(100)))

testCases := []struct {
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/20-transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func (am AppModule) OnRecvPacket(
sdk.NewEvent(
EventTypePacket,
sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory),
sdk.NewAttribute(AttributeKeyReceiver, data.Receiver.String()),
sdk.NewAttribute(AttributeKeyReceiver, data.Receiver),
sdk.NewAttribute(AttributeKeyValue, data.Amount.String()),
),
)
Expand Down Expand Up @@ -291,7 +291,7 @@ func (am AppModule) OnAcknowledgementPacket(
sdk.NewEvent(
EventTypePacket,
sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory),
sdk.NewAttribute(AttributeKeyReceiver, data.Receiver.String()),
sdk.NewAttribute(AttributeKeyReceiver, data.Receiver),
sdk.NewAttribute(AttributeKeyValue, data.Amount.String()),
sdk.NewAttribute(AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success)),
),
Expand Down Expand Up @@ -327,7 +327,7 @@ func (am AppModule) OnTimeoutPacket(
ctx.EventManager().EmitEvent(
sdk.NewEvent(
EventTypeTimeout,
sdk.NewAttribute(AttributeKeyRefundReceiver, data.Sender.String()),
sdk.NewAttribute(AttributeKeyRefundReceiver, data.Sender),
sdk.NewAttribute(AttributeKeyRefundValue, data.Amount.String()),
sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory),
),
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/20-transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ type MsgTransfer struct {
DestHeight uint64 `json:"dest_height" yaml:"dest_height"` // the current height of the destination chain
Amount sdk.Coins `json:"amount" yaml:"amount"` // the tokens to be transferred
Sender sdk.AccAddress `json:"sender" yaml:"sender"` // the sender address
Receiver sdk.AccAddress `json:"receiver" yaml:"receiver"` // the recipient address on the destination chain
Receiver string `json:"receiver" yaml:"receiver"` // the recipient address on the destination chain
}

// NewMsgTransfer creates a new MsgTransfer instance
func NewMsgTransfer(
sourcePort, sourceChannel string, destHeight uint64, amount sdk.Coins, sender, receiver sdk.AccAddress,
sourcePort, sourceChannel string, destHeight uint64, amount sdk.Coins, sender sdk.AccAddress, receiver string,
) MsgTransfer {
return MsgTransfer{
SourcePort: sourcePort,
Expand Down Expand Up @@ -58,7 +58,7 @@ func (msg MsgTransfer) ValidateBasic() error {
if msg.Sender.Empty() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing sender address")
}
if msg.Receiver.Empty() {
if msg.Receiver == "" {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing recipient address")
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions x/ibc/20-transfer/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const (

var (
addr1 = sdk.AccAddress("testaddr1")
addr2 = sdk.AccAddress("testaddr2")
addr2 = sdk.AccAddress("testaddr2").String()
emptyAddr sdk.AccAddress

coins, _ = sdk.ParseCoins("100atom")
Expand Down Expand Up @@ -59,7 +59,7 @@ func TestMsgTransferValidation(t *testing.T) {
NewMsgTransfer(validPort, validChannel, 10, invalidDenomCoins, addr1, addr2), // invalid amount
NewMsgTransfer(validPort, validChannel, 10, negativeCoins, addr1, addr2), // amount contains negative coin
NewMsgTransfer(validPort, validChannel, 10, coins, emptyAddr, addr2), // missing sender address
NewMsgTransfer(validPort, validChannel, 10, coins, addr1, emptyAddr), // missing recipient address
NewMsgTransfer(validPort, validChannel, 10, coins, addr1, ""), // missing recipient address
NewMsgTransfer(validPort, validChannel, 10, sdk.Coins{}, addr1, addr2), // not possitive coin
}

Expand Down
12 changes: 6 additions & 6 deletions x/ibc/20-transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import (
// FungibleTokenPacketData defines a struct for the packet payload
// See FungibleTokenPacketData spec: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#data-structures
type FungibleTokenPacketData struct {
Amount sdk.Coins `json:"amount" yaml:"amount"` // the tokens to be transferred
Sender sdk.AccAddress `json:"sender" yaml:"sender"` // the sender address
Receiver sdk.AccAddress `json:"receiver" yaml:"receiver"` // the recipient address on the destination chain
Amount sdk.Coins `json:"amount" yaml:"amount"` // the tokens to be transferred
Sender string `json:"sender" yaml:"sender"` // the sender address
Receiver string `json:"receiver" yaml:"receiver"` // the recipient address on the destination chain
}

// NewFungibleTokenPacketData contructs a new FungibleTokenPacketData instance
func NewFungibleTokenPacketData(
amount sdk.Coins, sender, receiver sdk.AccAddress) FungibleTokenPacketData {
amount sdk.Coins, sender, receiver string) FungibleTokenPacketData {
return FungibleTokenPacketData{
Amount: amount,
Sender: sender,
Expand Down Expand Up @@ -45,10 +45,10 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error {
if !ftpd.Amount.IsValid() {
return sdkerrors.ErrInvalidCoins
}
if ftpd.Sender.Empty() {
if ftpd.Sender == "" {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing sender address")
}
if ftpd.Receiver.Empty() {
if ftpd.Receiver == "" {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing receiver address")
}
return nil
Expand Down
10 changes: 5 additions & 5 deletions x/ibc/20-transfer/types/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import (
// TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData
func TestFungibleTokenPacketDataValidateBasic(t *testing.T) {
testPacketDataTransfer := []FungibleTokenPacketData{
NewFungibleTokenPacketData(coins, addr1, addr2), // valid msg
NewFungibleTokenPacketData(invalidDenomCoins, addr1, addr2), // invalid amount
NewFungibleTokenPacketData(negativeCoins, addr1, addr2), // amount contains negative coin
NewFungibleTokenPacketData(coins, emptyAddr, addr2), // missing sender address
NewFungibleTokenPacketData(coins, addr1, emptyAddr), // missing recipient address
NewFungibleTokenPacketData(coins, addr1.String(), addr2), // valid msg
NewFungibleTokenPacketData(invalidDenomCoins, addr1.String(), addr2), // invalid amount
NewFungibleTokenPacketData(negativeCoins, addr1.String(), addr2), // amount contains negative coin
NewFungibleTokenPacketData(coins, emptyAddr.String(), addr2), // missing sender address
NewFungibleTokenPacketData(coins, addr1.String(), emptyAddr.String()), // missing recipient address
}

testCases := []struct {
Expand Down

0 comments on commit 3cc4063

Please sign in to comment.