Skip to content

Commit

Permalink
fix: enforce checksum format for asset address in ZRC20 (#3278)
Browse files Browse the repository at this point in the history
* add checksum check

* changelog

* add migration script

* add logs

* Update pkg/crypto/evm_address.go

Co-authored-by: Dmitry S <[email protected]>

* remove duplicated logs

---------

Co-authored-by: Dmitry S <[email protected]>
  • Loading branch information
lumtis and swift1337 authored Dec 12, 2024
1 parent be8783b commit 6bbe9ea
Show file tree
Hide file tree
Showing 12 changed files with 371 additions and 55 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* [3225](https://github.com/zeta-chain/node/pull/3225) - use separate database file names for btc signet and testnet4
* [3242](https://github.com/zeta-chain/node/pull/3242) - set the `Receiver` of `MsgVoteInbound` to the address pulled from solana memo
* [3253](https://github.com/zeta-chain/node/pull/3253) - fix solana inbound version 0 queries and move tss keysign prior to relayer key checking
* [3278](https://github.com/zeta-chain/node/pull/3278) - enforce checksum format for asset address in ZRC20

## v23.0.0

Expand Down
12 changes: 0 additions & 12 deletions pkg/crypto/address.go

This file was deleted.

37 changes: 0 additions & 37 deletions pkg/crypto/address_test.go

This file was deleted.

31 changes: 31 additions & 0 deletions pkg/crypto/evm_address.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package crypto

import (
"strings"

"github.com/ethereum/go-ethereum/common"

"github.com/zeta-chain/node/pkg/constant"
)

// IsEmptyAddress returns true if the address is empty
func IsEmptyAddress(address common.Address) bool {
return address == (common.Address{}) || address.Hex() == constant.EVMZeroAddress
}

// IsEVMAddress returns true if the string is an EVM address
// independently of the checksum format
func IsEVMAddress(address string) bool {
return len(address) == 42 && strings.HasPrefix(address, "0x") && common.IsHexAddress(address)
}

// IsChecksumAddress returns true if the EVM address string is a valid checksum address
// See https://eips.ethereum.org/EIPS/eip-55
func IsChecksumAddress(address string) bool {
return address == common.HexToAddress(address).Hex()
}

// ToChecksumAddress returns the checksum address of the given EVM address
func ToChecksumAddress(address string) string {
return common.HexToAddress(address).Hex()
}
156 changes: 156 additions & 0 deletions pkg/crypto/evm_address_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package crypto

import (
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/require"
"github.com/zeta-chain/node/pkg/constant"
"testing"
)

func TestIsEmptyAddress(t *testing.T) {
tests := []struct {
name string
address common.Address
want bool
}{
{
name: "empty address",
address: common.Address{},
want: true,
},
{
name: "zero address",
address: common.HexToAddress(constant.EVMZeroAddress),
want: true,
},
{
name: "non empty address",
address: common.HexToAddress("0x1"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.EqualValues(t, tt.want, IsEmptyAddress(tt.address))
})
}
}

func TestIsEVMAddress(t *testing.T) {
tests := []struct {
name string
address string
want bool
}{
{
name: "EVM address",
address: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
want: true,
},
{
name: "EVM address with invalid checksum",
address: "0x5a4f260a7D716c859A2736151CB38b9c58C32c64",
want: true,
},
{
name: "EVM address all lowercase",
address: "0x5a4f260a7d716c859a2736151cb38b9c58c32c64",
want: true,
},
{
name: "EVM address all uppercase",
address: "0x5A4F260A7D716C859A2736151CB38B9C58C32C64",
want: true,
},
{
name: "invalid EVM address",
address: "5a4f260A7D716c859A2736151cB38b9c58C32c64",
},
{
name: "empty address",
address: "",
},
{
name: "non EVM address",
address: "Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.EqualValues(t, tt.want, IsEVMAddress(tt.address))
})
}
}

func TestIsChecksumAddress(t *testing.T) {
tests := []struct {
name string
address string
want bool
}{
{
name: "checksum address",
address: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
want: true,
},
{
name: "invalid checksum address",
address: "0x5a4f260a7D716c859A2736151CB38b9c58C32c64",
},
{
name: "all lowercase",
address: "0x5a4f260a7d716c859a2736151cb38b9c58c32c64",
},
{
name: "all uppercase",
address: "0x5A4F260A7D716C859A2736151CB38B9C58C32C64",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.EqualValues(t, tt.want, IsChecksumAddress(tt.address))
})
}
}

func TestToChecksumAddress(t *testing.T) {
tests := []struct {
name string
address string
want string
}{
{
name: "checksum address",
address: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
want: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
},
{
name: "all lowercase",
address: "0x5a4f260a7d716c859a2736151cb38b9c58c32c64",
want: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
},
{
name: "all uppercase",
address: "0x5A4F260A7D716C859A2736151CB38B9C58C32C64",
want: "0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
},
{
name: "empty address returns null address",
address: "",
want: "0x0000000000000000000000000000000000000000",
},
{
name: "non evm address returns null address",
address: "Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr",
want: "0x0000000000000000000000000000000000000000",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.EqualValues(t, tt.want, ToChecksumAddress(tt.address))
})
}
}
1 change: 1 addition & 0 deletions testutil/sample/fungible.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func ForeignCoins(t *testing.T, address string) types.ForeignCoins {
Symbol: StringRandom(r, 32),
CoinType: coin.CoinType_ERC20,
GasLimit: r.Uint64(),
LiquidityCap: UintInRange(0, 10000000000),
}
}

Expand Down
22 changes: 19 additions & 3 deletions x/crosschain/types/message_whitelist_erc20.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
cosmoserrors "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/pkg/errors"

"github.com/zeta-chain/node/pkg/crypto"
"github.com/zeta-chain/node/x/fungible/types"
)

Expand Down Expand Up @@ -50,10 +52,10 @@ func (msg *MsgWhitelistERC20) GetSignBytes() []byte {
func (msg *MsgWhitelistERC20) ValidateBasic() error {
_, err := sdk.AccAddressFromBech32(msg.Creator)
if err != nil {
return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err)
return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "invalid creator address (%s)", err.Error())
}
if msg.Erc20Address == "" {
return cosmoserrors.Wrapf(sdkerrors.ErrInvalidAddress, "empty asset address")
if err := validateAssetAddress(msg.Erc20Address); err != nil {
return cosmoserrors.Wrapf(types.ErrInvalidAddress, "invalid asset address (%s)", err.Error())
}
if msg.Decimals > 128 {
return cosmoserrors.Wrapf(types.ErrInvalidDecimals, "invalid decimals (%d)", msg.Decimals)
Expand All @@ -63,3 +65,17 @@ func (msg *MsgWhitelistERC20) ValidateBasic() error {
}
return nil
}

func validateAssetAddress(address string) error {
if address == "" {
return errors.New("empty asset address")
}

// if the address is an evm address, check if it is in checksum format
if crypto.IsEVMAddress(address) && !crypto.IsChecksumAddress(address) {
return errors.New("evm address is not in checksum format")
}

// currently no specific check is implemented for other address format
return nil
}
30 changes: 28 additions & 2 deletions x/crosschain/types/message_whitelist_erc20_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,36 @@ func TestMsgWhitelistERC20_ValidateBasic(t *testing.T) {
error: true,
},
{
name: "valid",
name: "evm asset address with invalid checksum format",
msg: types.NewMsgWhitelistERC20(
sample.AccAddress(),
sample.EthAddress().Hex(),
"0x5a4f260a7d716c859a2736151cb38b9c58c32c64",
1,
"name",
"symbol",
6,
10,
),
error: true,
},
{
name: "valid message with evm asset address",
msg: types.NewMsgWhitelistERC20(
sample.AccAddress(),
"0x5a4f260A7D716c859A2736151cB38b9c58C32c64",
1,
"name",
"symbol",
6,
10,
),
error: false,
},
{
name: "valid message with solana asset address",
msg: types.NewMsgWhitelistERC20(
sample.AccAddress(),
"Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr",
1,
"name",
"symbol",
Expand Down
24 changes: 24 additions & 0 deletions x/fungible/keeper/migrator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"

v3 "github.com/zeta-chain/node/x/fungible/migrations/v3"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
fungibleKeeper Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper Keeper) Migrator {
return Migrator{
fungibleKeeper: keeper,
}
}

// Migrate2to3 migrates the store from consensus version 2 to 3
func (m Migrator) Migrate2to3(ctx sdk.Context) error {
return v3.MigrateStore(ctx, m.fungibleKeeper)
}
30 changes: 30 additions & 0 deletions x/fungible/migrations/v3/migrate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package v3

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/node/pkg/crypto"
"github.com/zeta-chain/node/x/fungible/types"
)

type fungibleKeeper interface {
GetAllForeignCoins(ctx sdk.Context) (list []types.ForeignCoins)
SetForeignCoins(ctx sdk.Context, foreignCoins types.ForeignCoins)
}

// MigrateStore migrates the x/fungible module state from the consensus version 2 to 3
// It updates all existing address in ForeignCoin to use checksum format if the address is EVM type
func MigrateStore(ctx sdk.Context, fungibleKeeper fungibleKeeper) error {
fcs := fungibleKeeper.GetAllForeignCoins(ctx)
for _, fc := range fcs {
if fc.Asset != "" && crypto.IsEVMAddress(fc.Asset) && !crypto.IsChecksumAddress(fc.Asset) {
checksumAddress := crypto.ToChecksumAddress(fc.Asset)
ctx.Logger().Info("Patching zrc20 asset", "zrc20", fc.Symbol, "old", fc.Asset, "new", checksumAddress)

fc.Asset = checksumAddress
fungibleKeeper.SetForeignCoins(ctx, fc)
}
}

return nil
}
Loading

0 comments on commit 6bbe9ea

Please sign in to comment.