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

imp(erc20): ERC-20 registration #1997

Merged
merged 32 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
b68995a
imp(erc20): ERC-20 registration
fedekunze Nov 4, 2023
83bc196
Merge branch 'main' into fedekunze/erc20-registration
MalteHerrmann Nov 10, 2023
d16fc6f
add changelog entry
MalteHerrmann Nov 10, 2023
8de2dd5
use bankkeeper import instead of erc20 types bank keeper interface in…
MalteHerrmann Nov 10, 2023
df23222
fix instantiation of erc20 keeper
MalteHerrmann Nov 10, 2023
6226516
fix checking for duplicate precompiles
MalteHerrmann Nov 10, 2023
a33420c
regenerate mocks for EVM and bank keepers using mockery
MalteHerrmann Nov 10, 2023
7423713
add instructions for mockery usage
MalteHerrmann Nov 10, 2023
110c481
run make format
MalteHerrmann Nov 10, 2023
2ed03eb
adjust comment
MalteHerrmann Nov 10, 2023
57fff81
rename IsRegistered to IsActivePrecompile and add IsAvailablePrecompi…
MalteHerrmann Nov 10, 2023
4c86af2
address linter
MalteHerrmann Nov 10, 2023
a711ec5
add IsAvailablePrecompile to interface and regenerate mock
MalteHerrmann Nov 10, 2023
bd4b98f
run make format
MalteHerrmann Nov 10, 2023
f181e1f
add tests to AddEVMExtension and GetAvailablePrecompileAddrs utility
MalteHerrmann Nov 10, 2023
c3ec0db
add tests for IsActivePrecompile
MalteHerrmann Nov 10, 2023
6dafc92
fix linter
MalteHerrmann Nov 10, 2023
1a3cbbf
check that active precompiles in params are sorted
MalteHerrmann Nov 10, 2023
85cff21
sort the active precompiles when setting parameters
MalteHerrmann Nov 10, 2023
bc63af1
improve getting available precompile addrs by pre allocating full slice
MalteHerrmann Nov 10, 2023
c541385
address linter
MalteHerrmann Nov 10, 2023
2266ac8
add test for adding multiple EVM extensions at once
MalteHerrmann Nov 10, 2023
7c869b1
add tests for RegisterERC20Extensions
MalteHerrmann Nov 10, 2023
00492f2
return with error from IterateTokenPairs instead of panic
MalteHerrmann Nov 10, 2023
3af5e66
address linter
MalteHerrmann Nov 10, 2023
410fda5
Update x/erc20/types/mocks/README.md
MalteHerrmann Nov 10, 2023
b20dc6a
fix test
MalteHerrmann Nov 10, 2023
409b071
error on iteration
fedekunze Nov 12, 2023
99fcd53
sort map addr
fedekunze Nov 12, 2023
914aeda
fix adding WERC20 precompile for evm denomination
MalteHerrmann Nov 13, 2023
71ad335
tests refactor
MalteHerrmann Nov 13, 2023
a2149c6
add test for multiple added precompiles with duplicates
MalteHerrmann Nov 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ func NewEvmos(
app.Erc20Keeper = erc20keeper.NewKeeper(
keys[erc20types.StoreKey], appCodec, authtypes.NewModuleAddress(govtypes.ModuleName),
app.AccountKeeper, app.BankKeeper, app.EvmKeeper, app.StakingKeeper, app.ClaimsKeeper,
app.AuthzKeeper, &app.TransferKeeper,
)

app.IncentivesKeeper = incentiveskeeper.NewKeeper(
Expand Down
12 changes: 9 additions & 3 deletions x/erc20/keeper/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@
suite.app.GetKey("erc20"), suite.app.AppCodec(),
authtypes.NewModuleAddress(govtypes.ModuleName),
suite.app.AccountKeeper, suite.app.BankKeeper,
mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper)
mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper,

Check failure on line 98 in x/erc20/keeper/evm_test.go

View workflow job for this annotation

GitHub Actions / test-unit-cover

cannot use mockEVMKeeper (variable of type *MockEVMKeeper) as "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper value in argument to keeper.NewKeeper: *MockEVMKeeper does not implement "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper (missing method AddEVMExtensions)

Check failure on line 98 in x/erc20/keeper/evm_test.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint

cannot use mockEVMKeeper (variable of type *MockEVMKeeper) as "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper value in argument to keeper.NewKeeper: *MockEVMKeeper does not implement "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper (missing method AddEVMExtensions)
s.app.AuthzKeeper, &s.app.TransferKeeper,
)

tc.malleate()

Expand Down Expand Up @@ -283,7 +285,9 @@
suite.app.Erc20Keeper = keeper.NewKeeper(
suite.app.GetKey("erc20"), suite.app.AppCodec(),
authtypes.NewModuleAddress(govtypes.ModuleName), suite.app.AccountKeeper,
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper)
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper,

Check failure on line 288 in x/erc20/keeper/evm_test.go

View workflow job for this annotation

GitHub Actions / test-unit-cover

cannot use mockEVMKeeper (variable of type *MockEVMKeeper) as "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper value in argument to keeper.NewKeeper: *MockEVMKeeper does not implement "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper (missing method AddEVMExtensions)
s.app.AuthzKeeper, &s.app.TransferKeeper,
)

tc.malleate()

Expand Down Expand Up @@ -372,7 +376,9 @@
suite.app.Erc20Keeper = keeper.NewKeeper(
suite.app.GetKey("erc20"), suite.app.AppCodec(),
authtypes.NewModuleAddress(govtypes.ModuleName), suite.app.AccountKeeper,
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper)
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper,

Check failure on line 379 in x/erc20/keeper/evm_test.go

View workflow job for this annotation

GitHub Actions / test-unit-cover

cannot use mockEVMKeeper (variable of type *MockEVMKeeper) as "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper value in argument to keeper.NewKeeper: *MockEVMKeeper does not implement "github.com/evmos/evmos/v15/x/erc20/types".EVMKeeper (missing method AddEVMExtensions)
s.app.AuthzKeeper, &s.app.TransferKeeper,
)

tc.malleate()

Expand Down
37 changes: 23 additions & 14 deletions x/erc20/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authzkeeper "github.com/cosmos/cosmos-sdk/x/authz/keeper"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
transferkeeper "github.com/evmos/evmos/v15/x/ibc/transfer/keeper"

"github.com/evmos/evmos/v15/x/erc20/types"
)
Expand All @@ -21,11 +24,13 @@ type Keeper struct {
// the address capable of executing a MsgUpdateParams message. Typically, this should be the x/gov module account.
authority sdk.AccAddress

accountKeeper types.AccountKeeper
bankKeeper types.BankKeeper
evmKeeper types.EVMKeeper
stakingKeeper types.StakingKeeper
claimsKeeper types.ClaimsKeeper
accountKeeper types.AccountKeeper
bankKeeper bankkeeper.Keeper
evmKeeper types.EVMKeeper
stakingKeeper types.StakingKeeper
claimsKeeper types.ClaimsKeeper
authzKeeper authzkeeper.Keeper
transferKeeper *transferkeeper.Keeper
}

// NewKeeper creates new instances of the erc20 Keeper
Expand All @@ -34,25 +39,29 @@ func NewKeeper(
cdc codec.BinaryCodec,
authority sdk.AccAddress,
ak types.AccountKeeper,
bk types.BankKeeper,
bk bankkeeper.Keeper,
evmKeeper types.EVMKeeper,
sk types.StakingKeeper,
ck types.ClaimsKeeper,
authzKeeper authzkeeper.Keeper,
transferKeeper *transferkeeper.Keeper,
) Keeper {
// ensure gov module account is set and is not nil
if err := sdk.VerifyAddressFormat(authority); err != nil {
panic(err)
}

return Keeper{
authority: authority,
storeKey: storeKey,
cdc: cdc,
accountKeeper: ak,
bankKeeper: bk,
evmKeeper: evmKeeper,
stakingKeeper: sk,
claimsKeeper: ck,
authority: authority,
storeKey: storeKey,
cdc: cdc,
accountKeeper: ak,
bankKeeper: bk,
evmKeeper: evmKeeper,
stakingKeeper: sk,
claimsKeeper: ck,
authzKeeper: authzKeeper,
transferKeeper: transferKeeper,
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/erc20/keeper/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (suite *KeeperTestSuite) TestMigrations() {
legacySubspace.GetParamSetIfExists(ctx, &outputParams)

// Added dummy keeper in order to use the test store and store key
mockKeeper := erc20keeper.NewKeeper(storeKey, nil, authtypes.NewModuleAddress(govtypes.ModuleName), nil, nil, nil, nil, nil)
mockKeeper := erc20keeper.NewKeeper(storeKey, nil, authtypes.NewModuleAddress(govtypes.ModuleName), nil, nil, nil, nil, nil, s.app.AuthzKeeper, nil)
mockSubspace := newMockSubspace(v3types.DefaultParams(), storeKey, tKey)
migrator := erc20keeper.NewMigrator(mockKeeper, mockSubspace)

Expand Down
108 changes: 81 additions & 27 deletions x/erc20/keeper/msg_server_test.go

Large diffs are not rendered by default.

64 changes: 64 additions & 0 deletions x/erc20/keeper/precompile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright Tharsis Labs Ltd.(Evmos)
// SPDX-License-Identifier:ENCL-1.0(https://github.com/evmos/evmos/blob/main/LICENSE)

package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/evmos/evmos/v15/precompiles/erc20"
"github.com/evmos/evmos/v15/precompiles/werc20"
"github.com/evmos/evmos/v15/x/erc20/types"
)

// RegisterERC20Extensions registers the ERC20 precompiles with the EVM.
func (k Keeper) RegisterERC20Extensions(ctx sdk.Context) error {
precompiles := make([]vm.PrecompiledContract, 0)
params := k.evmKeeper.GetParams(ctx)
evmDenom := params.EvmDenom
logger := ctx.Logger()

k.IterateTokenPairs(ctx, func(tokenPair types.TokenPair) bool {
// skip registration if token is native or if it has already been registered
// NOTE: this should handle failure during the selfdestruct
if tokenPair.ContractOwner != types.OWNER_MODULE ||
params.IsPrecompileRegistered(tokenPair.Erc20Address) {
return false
}
MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved

var (
err error
precompile vm.PrecompiledContract
)

if tokenPair.Denom == evmDenom {
precompile, err = werc20.NewPrecompile(tokenPair, k.bankKeeper, k.authzKeeper, *k.transferKeeper)
} else {
precompile, err = erc20.NewPrecompile(tokenPair, k.bankKeeper, k.authzKeeper, *k.transferKeeper)
}

if err != nil {
panic(fmt.Errorf("failed to instantiate ERC-20 precompile for denom %s: %w", tokenPair.Denom, err))
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

address := tokenPair.GetERC20Contract()

// try selfdestruct ERC20 contract

// NOTE(@fedekunze): From now on, the contract address will map to a precompile instead
// of the ERC20MinterBurner contract. We try to force a selfdestruct to remove the unnecessary
// code and storage from the state machine. In any case, the precompiles are handled in the EVM
// before the regular contracts so not removing them doesn't create any issues in the implementation.
if err := k.evmKeeper.DeleteAccount(ctx, address); err != nil {
logger.Debug("failed to selfdestruct account", "error", err)
}

precompiles = append(precompiles, precompile)
return false
})

// add the ERC20s to the EVM active and available precompiles
return k.evmKeeper.AddEVMExtensions(ctx, precompiles...)
}
4 changes: 3 additions & 1 deletion x/erc20/keeper/proposals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ func (suite KeeperTestSuite) TestRegisterERC20() { //nolint:govet // we can copy
suite.app.Erc20Keeper = keeper.NewKeeper(
suite.app.GetKey("erc20"), suite.app.AppCodec(),
authtypes.NewModuleAddress(govtypes.ModuleName), suite.app.AccountKeeper,
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper)
suite.app.BankKeeper, mockEVMKeeper, suite.app.StakingKeeper, suite.app.ClaimsKeeper,
suite.app.AuthzKeeper, &suite.app.TransferKeeper,
)

mockEVMKeeper.On("EstimateGasInternal", mock.Anything, mock.Anything, mock.Anything).Return(&evmtypes.EstimateGasResponse{Gas: uint64(200)}, nil)
mockEVMKeeper.On("ApplyMessage", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("forced ApplyMessage error"))
Expand Down
17 changes: 2 additions & 15 deletions x/erc20/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
Expand All @@ -28,20 +27,6 @@ type AccountKeeper interface {
GetAccount(sdk.Context, sdk.AccAddress) authtypes.AccountI
}

// BankKeeper defines the expected interface needed to retrieve account balances.
type BankKeeper interface {
SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error
SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
IsSendEnabledCoin(ctx sdk.Context, coin sdk.Coin) bool
BlockedAddr(addr sdk.AccAddress) bool
GetDenomMetaData(ctx sdk.Context, denom string) (banktypes.Metadata, bool)
SetDenomMetaData(ctx sdk.Context, denomMetaData banktypes.Metadata)
HasSupply(ctx sdk.Context, denom string) bool
GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin
}

MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved
// StakingKeeper defines the expected interface needed to retrieve the staking denom.
type StakingKeeper interface {
BondDenom(ctx sdk.Context) string
Expand All @@ -53,6 +38,8 @@ type EVMKeeper interface {
GetAccountWithoutBalance(ctx sdk.Context, addr common.Address) *statedb.Account
EstimateGasInternal(c context.Context, req *evmtypes.EthCallRequest, fromType evmtypes.CallType) (*evmtypes.EstimateGasResponse, error)
ApplyMessage(ctx sdk.Context, msg core.Message, tracer vm.EVMLogger, commit bool) (*evmtypes.MsgEthereumTxResponse, error)
AddEVMExtensions(ctx sdk.Context, precompiles ...vm.PrecompiledContract) error
DeleteAccount(ctx sdk.Context, addr common.Address) error
}

// StakingKeeper defines the expected interface needed to retrieve the staking denom.
Expand Down
41 changes: 41 additions & 0 deletions x/evm/keeper/precompiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ package keeper

import (
"fmt"
"sort"

"golang.org/x/exp/maps"

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

sdk "github.com/cosmos/cosmos-sdk/types"
authzkeeper "github.com/cosmos/cosmos-sdk/x/authz/keeper"
distributionkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
Expand All @@ -23,6 +25,7 @@ import (
stakingprecompile "github.com/evmos/evmos/v15/precompiles/staking"
vestingprecompile "github.com/evmos/evmos/v15/precompiles/vesting"
erc20Keeper "github.com/evmos/evmos/v15/x/erc20/keeper"
"github.com/evmos/evmos/v15/x/evm/types"
transferkeeper "github.com/evmos/evmos/v15/x/ibc/transfer/keeper"
vestingkeeper "github.com/evmos/evmos/v15/x/vesting/keeper"
)
Expand Down Expand Up @@ -110,3 +113,41 @@ func (k Keeper) Precompiles(

return activePrecompileMap
}

// AddEVMExtension adds the given precompiles to the list of active precompiles in the EVM parameters
MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved
// and to the available precompiles map in the Keeper. This function returns an error if
// the precompiles are invalid or duplicated.
func (k *Keeper) AddEVMExtensions(ctx sdk.Context, precompiles ...vm.PrecompiledContract) error {
params := k.GetParams(ctx)

addresses := make([]string, len(precompiles))
precompilesMap := maps.Clone(k.precompiles)

for i, precompile := range precompiles {
// add to active precompiles
address := precompile.Address()
addresses[i] = address.String()
// add to available precompiles
precompilesMap[address] = precompile
}

params.ActivePrecompiles = append(params.ActivePrecompiles, addresses...)

// sort precompile addresses prior to validation
sort.Slice(params.ActivePrecompiles, func(i, j int) bool {
return params.ActivePrecompiles[i] < params.ActivePrecompiles[j]
})

// error if the precompiled address is already registered
if err := types.ValidatePrecompiles(params.ActivePrecompiles); err != nil {
return err
}

if err := k.SetParams(ctx, params); err != nil {
return err
}

// update the pointer to the map with the newly added EVM Extensions
k.precompiles = precompilesMap
return nil
}
23 changes: 18 additions & 5 deletions x/evm/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package types
import (
"fmt"
"math/big"
"sort"
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -101,7 +103,7 @@ func (p Params) Validate() error {
return err
}

return validatePrecompiles(p.ActivePrecompiles)
return ValidatePrecompiles(p.ActivePrecompiles)
}

// EIPs returns the ExtraEIPS as a int slice
Expand All @@ -128,6 +130,16 @@ func (p Params) GetActivePrecompilesAddrs() []common.Address {
return precompiles
}

// IsPrecompileRegistered returns true if the given precompile address is
// registered as an active precompile.
func (p Params) IsPrecompileRegistered(address string) bool {
_, found := sort.Find(len(p.ActivePrecompiles), func(i int) int {
return strings.Compare(address, p.ActivePrecompiles[i])
})
MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved

return found
}

func validateEVMDenom(i interface{}) error {
denom, ok := i.(string)
if !ok {
Expand Down Expand Up @@ -176,23 +188,24 @@ func validateChainConfig(i interface{}) error {
return cfg.Validate()
}

func validatePrecompiles(i interface{}) error {
// ValidatePrecompiles checks if the precompile addresses are valid and unique.
func ValidatePrecompiles(i interface{}) error {
precompiles, ok := i.([]string)
if !ok {
return fmt.Errorf("invalid precompile slice type: %T", i)
}

seenPrecompiles := make(map[string]bool)
seenPrecompiles := make(map[string]struct{})
for _, precompile := range precompiles {
if seenPrecompiles[precompile] {
if _, ok := seenPrecompiles[precompile]; !ok {
return fmt.Errorf("duplicate precompile %s", precompile)
}

if err := types.ValidateAddress(precompile); err != nil {
return fmt.Errorf("invalid precompile %s", precompile)
}

seenPrecompiles[precompile] = true
seenPrecompiles[precompile] = struct{}{}
MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
Expand Down