From aeb8d5599838a5d02793ac6d69931524587723c9 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 01:40:30 +0100 Subject: [PATCH 1/9] Improve gas price coverage --- testutil/keeper/mocks/crosschain/account.go | 2 +- testutil/keeper/mocks/crosschain/bank.go | 2 +- testutil/keeper/mocks/crosschain/fungible.go | 2 +- testutil/keeper/mocks/crosschain/observer.go | 7 +- testutil/keeper/mocks/crosschain/staking.go | 2 +- testutil/keeper/mocks/fungible/account.go | 2 +- testutil/keeper/mocks/fungible/bank.go | 2 +- testutil/keeper/mocks/fungible/evm.go | 22 +- testutil/keeper/mocks/fungible/observer.go | 9 +- x/fungible/keeper/evm_test.go | 5 +- x/fungible/keeper/gas_price.go | 16 +- x/fungible/keeper/gas_price_test.go | 238 +++++++++++++++++++ x/fungible/types/errors.go | 1 + x/fungible/types/expected_keepers.go | 1 + 14 files changed, 277 insertions(+), 34 deletions(-) diff --git a/testutil/keeper/mocks/crosschain/account.go b/testutil/keeper/mocks/crosschain/account.go index 274a79766d..c8d33e57a6 100644 --- a/testutil/keeper/mocks/crosschain/account.go +++ b/testutil/keeper/mocks/crosschain/account.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/bank.go b/testutil/keeper/mocks/crosschain/bank.go index 278cccb833..29e339bdad 100644 --- a/testutil/keeper/mocks/crosschain/bank.go +++ b/testutil/keeper/mocks/crosschain/bank.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/fungible.go b/testutil/keeper/mocks/crosschain/fungible.go index b40bd35c40..2be37b5efa 100644 --- a/testutil/keeper/mocks/crosschain/fungible.go +++ b/testutil/keeper/mocks/crosschain/fungible.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/crosschain/observer.go b/testutil/keeper/mocks/crosschain/observer.go index 5000f37e03..95316ad556 100644 --- a/testutil/keeper/mocks/crosschain/observer.go +++ b/testutil/keeper/mocks/crosschain/observer.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks @@ -843,11 +843,6 @@ func (_m *CrosschainObserverKeeper) SetNonceToCctx(ctx types.Context, nonceToCct _m.Called(ctx, nonceToCctx) } -// SetObservers provides a mock function with given fields: ctx, om -func (_m *CrosschainObserverKeeper) SetObservers(ctx types.Context, om observertypes.ObserverSet) { - _m.Called(ctx, om) -} - // SetPendingNonces provides a mock function with given fields: ctx, pendingNonces func (_m *CrosschainObserverKeeper) SetPendingNonces(ctx types.Context, pendingNonces observertypes.PendingNonces) { _m.Called(ctx, pendingNonces) diff --git a/testutil/keeper/mocks/crosschain/staking.go b/testutil/keeper/mocks/crosschain/staking.go index 772bb09971..f6bd25dd1d 100644 --- a/testutil/keeper/mocks/crosschain/staking.go +++ b/testutil/keeper/mocks/crosschain/staking.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/account.go b/testutil/keeper/mocks/fungible/account.go index 94b7a84d75..f7a2788969 100644 --- a/testutil/keeper/mocks/fungible/account.go +++ b/testutil/keeper/mocks/fungible/account.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/bank.go b/testutil/keeper/mocks/fungible/bank.go index d44da595c0..fe6b8c6b14 100644 --- a/testutil/keeper/mocks/fungible/bank.go +++ b/testutil/keeper/mocks/fungible/bank.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks diff --git a/testutil/keeper/mocks/fungible/evm.go b/testutil/keeper/mocks/fungible/evm.go index 64c041b22d..b59f7d477e 100644 --- a/testutil/keeper/mocks/fungible/evm.go +++ b/testutil/keeper/mocks/fungible/evm.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks @@ -146,6 +146,26 @@ func (_m *FungibleEVMKeeper) GetBlockBloomTransient(ctx types.Context) *big.Int return r0 } +// GetCode provides a mock function with given fields: ctx, codeHash +func (_m *FungibleEVMKeeper) GetCode(ctx types.Context, codeHash common.Hash) []byte { + ret := _m.Called(ctx, codeHash) + + if len(ret) == 0 { + panic("no return value specified for GetCode") + } + + var r0 []byte + if rf, ok := ret.Get(0).(func(types.Context, common.Hash) []byte); ok { + r0 = rf(ctx, codeHash) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + return r0 +} + // GetLogSizeTransient provides a mock function with given fields: ctx func (_m *FungibleEVMKeeper) GetLogSizeTransient(ctx types.Context) uint64 { ret := _m.Called(ctx) diff --git a/testutil/keeper/mocks/fungible/observer.go b/testutil/keeper/mocks/fungible/observer.go index a808da6d5f..606d71138a 100644 --- a/testutil/keeper/mocks/fungible/observer.go +++ b/testutil/keeper/mocks/fungible/observer.go @@ -1,11 +1,11 @@ -// Code generated by mockery v2.39.1. DO NOT EDIT. +// Code generated by mockery v2.41.0. DO NOT EDIT. package mocks import ( mock "github.com/stretchr/testify/mock" + common "github.com/zeta-chain/zetacore/common" - "github.com/zeta-chain/zetacore/common" observertypes "github.com/zeta-chain/zetacore/x/observer/types" types "github.com/cosmos/cosmos-sdk/types" @@ -185,11 +185,6 @@ func (_m *FungibleObserverKeeper) SetBallot(ctx types.Context, ballot *observert _m.Called(ctx, ballot) } -// SetObservers provides a mock function with given fields: ctx, om -func (_m *FungibleObserverKeeper) SetObservers(ctx types.Context, om observertypes.ObserverSet) { - _m.Called(ctx, om) -} - // NewFungibleObserverKeeper creates a new instance of FungibleObserverKeeper. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewFungibleObserverKeeper(t interface { diff --git a/x/fungible/keeper/evm_test.go b/x/fungible/keeper/evm_test.go index b6810a6998..42399f5104 100644 --- a/x/fungible/keeper/evm_test.go +++ b/x/fungible/keeper/evm_test.go @@ -8,7 +8,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - evmkeeper "github.com/evmos/ethermint/x/evm/keeper" evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -33,7 +32,7 @@ func getValidChainID(t *testing.T) int64 { } // require that a contract has been deployed by checking stored code is non-empty. -func assertContractDeployment(t *testing.T, k *evmkeeper.Keeper, ctx sdk.Context, contractAddress common.Address) { +func assertContractDeployment(t *testing.T, k types.EVMKeeper, ctx sdk.Context, contractAddress common.Address) { acc := k.GetAccount(ctx, contractAddress) require.NotNil(t, acc) @@ -46,7 +45,7 @@ func deploySystemContracts( t *testing.T, ctx sdk.Context, k *fungiblekeeper.Keeper, - evmk *evmkeeper.Keeper, + evmk types.EVMKeeper, ) (wzeta, uniswapV2Factory, uniswapV2Router, connector, systemContract common.Address) { var err error diff --git a/x/fungible/keeper/gas_price.go b/x/fungible/keeper/gas_price.go index 3f2ee63d28..e829f59377 100644 --- a/x/fungible/keeper/gas_price.go +++ b/x/fungible/keeper/gas_price.go @@ -12,6 +12,9 @@ import ( // SetGasPrice sets gas price on the system contract in zEVM; return the gasUsed and error code func (k Keeper) SetGasPrice(ctx sdk.Context, chainid *big.Int, gasPrice *big.Int) (uint64, error) { + if gasPrice == nil { + return 0, sdkerrors.Wrapf(types.ErrNilGasPrice, "gas price param should be set") + } system, found := k.GetSystemContract(ctx) if !found { return 0, sdkerrors.Wrapf(types.ErrContractNotFound, "system contract state variable not found") @@ -28,9 +31,6 @@ func (k Keeper) SetGasPrice(ctx sdk.Context, chainid *big.Int, gasPrice *big.Int if err != nil { return 0, sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } - if res.Failed() { - return res.GasUsed, sdkerrors.Wrapf(types.ErrContractCall, "setGasPrice tx failed") - } return res.GasUsed, nil } @@ -48,13 +48,10 @@ func (k Keeper) SetGasCoin(ctx sdk.Context, chainid *big.Int, address common.Add if err != nil { return sdkerrors.Wrapf(types.ErrABIGet, "SystemContractMetaData") } - res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasCoinZRC20", chainid, address) + _, err = k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasCoinZRC20", chainid, address) if err != nil { return sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } - if res.Failed() { - return sdkerrors.Wrapf(types.ErrContractCall, "setGasCoinZRC20 tx failed") - } return nil } @@ -72,13 +69,10 @@ func (k Keeper) SetGasZetaPool(ctx sdk.Context, chainid *big.Int, pool common.Ad if err != nil { return sdkerrors.Wrapf(types.ErrABIGet, "SystemContractMetaData") } - res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasZetaPool", chainid, pool) + _, err = k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasZetaPool", chainid, pool) if err != nil { return sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } - if res.Failed() { - return sdkerrors.Wrapf(types.ErrContractCall, "setGasZetaPool tx failed") - } return nil } diff --git a/x/fungible/keeper/gas_price_test.go b/x/fungible/keeper/gas_price_test.go index ec1bf267d8..b951bd751c 100644 --- a/x/fungible/keeper/gas_price_test.go +++ b/x/fungible/keeper/gas_price_test.go @@ -5,9 +5,13 @@ import ( "testing" ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/evmos/ethermint/x/evm/statedb" + evmtypes "github.com/evmos/ethermint/x/evm/types" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/zeta-chain/protocol-contracts/pkg/contracts/zevm/systemcontract.sol" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" + fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible" "github.com/zeta-chain/zetacore/testutil/sample" "github.com/zeta-chain/zetacore/x/fungible/keeper" "github.com/zeta-chain/zetacore/x/fungible/types" @@ -36,6 +40,78 @@ func TestKeeper_SetGasPrice(t *testing.T) { require.Equal(t, big.NewInt(42), queryGasPrice(big.NewInt(1))) } +func TestKeeper_SetGasPriceContractNotFound(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(42)) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetNilGasPrice(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.SetGasPrice(ctx, big.NewInt(1), nil) + require.ErrorIs(t, err, types.ErrNilGasPrice) +} + +func TestKeeper_SetGasPriceContractIs0x0(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + deploySystemContracts(t, ctx, k, sdkk.EvmKeeper) + k.SetSystemContract(ctx, types.SystemContract{}) + + _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(42)) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetGasPriceReverts(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) + _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_SetGasPriceRevertsOnVmError(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{ + VmError: "test error", + }, nil) + _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) + require.ErrorIs(t, err, types.ErrContractCall) +} + func TestKeeper_SetGasCoin(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -50,6 +126,71 @@ func TestKeeper_SetGasCoin(t *testing.T) { require.Equal(t, gas.Hex(), found.Hex()) } +func TestKeeper_SetGasCoinContractNotFound(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + gas := sample.EthAddress() + + err := k.SetGasCoin(ctx, big.NewInt(1), gas) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetGasCoinContractIs0x0(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + gas := sample.EthAddress() + + deploySystemContracts(t, ctx, k, sdkk.EvmKeeper) + k.SetSystemContract(ctx, types.SystemContract{}) + + err := k.SetGasCoin(ctx, big.NewInt(1), gas) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetGasCoinReverts(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) + err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_SetGasCoinRevertsOnVmError(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{ + VmError: "test error", + }, sample.ErrSample) + err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) + require.ErrorIs(t, err, types.ErrContractCall) +} + func TestKeeper_SetGasZetaPool(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -73,3 +214,100 @@ func TestKeeper_SetGasZetaPool(t *testing.T) { require.NoError(t, err) require.NotEqual(t, ethcommon.Address{}, queryZetaPool(big.NewInt(1))) } + +func TestKeeper_SetGasZetaPoolContractNotFound(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + zrc20 := sample.EthAddress() + + err := k.SetGasZetaPool(ctx, big.NewInt(1), zrc20) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetGasZetaPoolContractIs0x0(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + zrc20 := sample.EthAddress() + + deploySystemContracts(t, ctx, k, sdkk.EvmKeeper) + k.SetSystemContract(ctx, types.SystemContract{}) + + err := k.SetGasZetaPool(ctx, big.NewInt(1), zrc20) + require.ErrorIs(t, err, types.ErrContractNotFound) +} + +func TestKeeper_SetGasZetaPoolReverts(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) + err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_SetGasZetaPoolRevertsOnVmError(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.On( + "ApplyMessage", + ctx, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{ + VmError: "test error", + }, sample.ErrSample) + err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func setupMockEVMKeeperForSystemContractDeployment(t *testing.T, mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { + gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} + msgRes := &evmtypes.MsgEthereumTxResponse{} + + mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) + mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) + mockEVMKeeper.On( + "EstimateGas", + mock.Anything, + mock.Anything, + ).Return(gasRes, nil) + mockEVMKeeper.On( + "ApplyMessage", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(msgRes, nil).Times(5) + mockEVMKeeper.On( + "GetAccount", + mock.Anything, + mock.Anything, + ).Return(&statedb.Account{ + Nonce: 1, + }) + mockEVMKeeper.On( + "GetCode", + mock.Anything, + mock.Anything, + ).Return([]byte{1, 2, 3}) +} diff --git a/x/fungible/types/errors.go b/x/fungible/types/errors.go index 2652442b4a..ddcef768e5 100644 --- a/x/fungible/types/errors.go +++ b/x/fungible/types/errors.go @@ -27,4 +27,5 @@ var ( ErrCallNonContract = sdkerrors.Register(ModuleName, 1124, "can't call a non-contract address") ErrForeignCoinAlreadyExist = sdkerrors.Register(ModuleName, 1125, "foreign coin already exist") ErrInvalidHash = sdkerrors.Register(ModuleName, 1126, "invalid hash") + ErrNilGasPrice = sdkerrors.Register(ModuleName, 1127, "nil gas price") ) diff --git a/x/fungible/types/expected_keepers.go b/x/fungible/types/expected_keepers.go index c576a7cafc..3ce72397e3 100644 --- a/x/fungible/types/expected_keepers.go +++ b/x/fungible/types/expected_keepers.go @@ -65,5 +65,6 @@ type EVMKeeper interface { commit bool, ) (*evmtypes.MsgEthereumTxResponse, error) GetAccount(ctx sdk.Context, addr ethcommon.Address) *statedb.Account + GetCode(ctx sdk.Context, codeHash ethcommon.Hash) []byte SetAccount(ctx sdk.Context, addr ethcommon.Address, account statedb.Account) error } From 0a1a492a9783e7cd7e3f9956db3ef1b8f7ea561c Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 13:19:07 +0100 Subject: [PATCH 2/9] Improve msg update system contract coverage and cleanup --- x/fungible/keeper/evm_test.go | 82 ++++++++--- x/fungible/keeper/gas_coin_and_pool_test.go | 2 +- x/fungible/keeper/gas_price_test.go | 136 +----------------- .../msg_server_update_system_contract_test.go | 103 ++++++++++++- ...g_server_update_zrc20_withdraw_fee_test.go | 49 +------ 5 files changed, 175 insertions(+), 197 deletions(-) diff --git a/x/fungible/keeper/evm_test.go b/x/fungible/keeper/evm_test.go index 42399f5104..01bd93e8f9 100644 --- a/x/fungible/keeper/evm_test.go +++ b/x/fungible/keeper/evm_test.go @@ -8,6 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/evmos/ethermint/x/evm/statedb" evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -16,6 +17,7 @@ import ( "github.com/zeta-chain/zetacore/server/config" "github.com/zeta-chain/zetacore/testutil/contracts" testkeeper "github.com/zeta-chain/zetacore/testutil/keeper" + fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible" "github.com/zeta-chain/zetacore/testutil/sample" "github.com/zeta-chain/zetacore/x/fungible/keeper" fungiblekeeper "github.com/zeta-chain/zetacore/x/fungible/keeper" @@ -447,13 +449,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, &evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap}, ).Return(gasRes, nil) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(msgRes, nil) + mockEVMSuccessCallOnce(mockEVMKeeper) mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -496,13 +492,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, sdk.AccAddress(fromAddr.Bytes()), ).Return(uint64(1), nil) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(msgRes, nil) + mockEVMSuccessCallOnce(mockEVMKeeper) mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -603,7 +593,6 @@ func TestKeeper_CallEVMWithData(t *testing.T) { Data: (*hexutil.Bytes)(&data), }) gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} - msgRes := &evmtypes.MsgEthereumTxResponse{} // Set up mocked methods mockAuthKeeper.On( @@ -616,13 +605,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, &evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap}, ).Return(gasRes, nil) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(msgRes, sample.ErrSample) + mockEVMFailCallOnce(mockEVMKeeper) mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -641,3 +624,58 @@ func TestKeeper_CallEVMWithData(t *testing.T) { require.ErrorIs(t, err, sample.ErrSample) }) } + +func setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, applyMessageExpectedCounter int) { + gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} + mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) + mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) + mockEVMKeeper.On( + "EstimateGas", + mock.Anything, + mock.Anything, + ).Return(gasRes, nil) + mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMKeeper.On( + "GetAccount", + mock.Anything, + mock.Anything, + ).Return(&statedb.Account{ + Nonce: 1, + }) + mockEVMKeeper.On( + "GetCode", + mock.Anything, + mock.Anything, + ).Return([]byte{1, 2, 3}) +} + +func mockEVMSuccessCallOnce(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{}) +} + +func mockEVMSuccessCallOnceWithReturn(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, ret *evmtypes.MsgEthereumTxResponse) { + mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, ret, 1) +} + +func mockEVMSuccessCallTimesWithReturn(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, ret *evmtypes.MsgEthereumTxResponse, times int) { + if ret == nil { + ret = &evmtypes.MsgEthereumTxResponse{} + } + mockEVMKeeper.On( + "ApplyMessage", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(ret, nil).Times(times) +} + +func mockEVMFailCallOnce(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { + mockEVMKeeper.On( + "ApplyMessage", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample).Once() +} diff --git a/x/fungible/keeper/gas_coin_and_pool_test.go b/x/fungible/keeper/gas_coin_and_pool_test.go index 9bdd139fdc..b6f05431ac 100644 --- a/x/fungible/keeper/gas_coin_and_pool_test.go +++ b/x/fungible/keeper/gas_coin_and_pool_test.go @@ -19,7 +19,7 @@ func setupGasCoin( t *testing.T, ctx sdk.Context, k *fungiblekeeper.Keeper, - evmk *evmkeeper.Keeper, + evmk types.EVMKeeper, chainID int64, assetName string, symbol string, diff --git a/x/fungible/keeper/gas_price_test.go b/x/fungible/keeper/gas_price_test.go index b951bd751c..d084701447 100644 --- a/x/fungible/keeper/gas_price_test.go +++ b/x/fungible/keeper/gas_price_test.go @@ -5,13 +5,9 @@ import ( "testing" ethcommon "github.com/ethereum/go-ethereum/common" - "github.com/evmos/ethermint/x/evm/statedb" - evmtypes "github.com/evmos/ethermint/x/evm/types" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/zeta-chain/protocol-contracts/pkg/contracts/zevm/systemcontract.sol" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" - fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible" "github.com/zeta-chain/zetacore/testutil/sample" "github.com/zeta-chain/zetacore/x/fungible/keeper" "github.com/zeta-chain/zetacore/x/fungible/types" @@ -74,40 +70,10 @@ func TestKeeper_SetGasPriceReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) - _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) - require.ErrorIs(t, err, types.ErrContractCall) -} - -func TestKeeper_SetGasPriceRevertsOnVmError(t *testing.T) { - k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ - UseEVMMock: true, - }) - k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) - - mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) - - deploySystemContracts(t, ctx, k, mockEVMKeeper) - - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{ - VmError: "test error", - }, nil) + mockEVMFailCallOnce(mockEVMKeeper) _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) require.ErrorIs(t, err, types.ErrContractCall) } @@ -154,39 +120,10 @@ func TestKeeper_SetGasCoinReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) - deploySystemContracts(t, ctx, k, mockEVMKeeper) - - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) - err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) - require.ErrorIs(t, err, types.ErrContractCall) -} - -func TestKeeper_SetGasCoinRevertsOnVmError(t *testing.T) { - k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ - UseEVMMock: true, - }) - k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) - - mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{ - VmError: "test error", - }, sample.ErrSample) + mockEVMFailCallOnce(mockEVMKeeper) err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) require.ErrorIs(t, err, types.ErrContractCall) } @@ -243,71 +180,10 @@ func TestKeeper_SetGasZetaPoolReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) - deploySystemContracts(t, ctx, k, mockEVMKeeper) - - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample) - err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) - require.ErrorIs(t, err, types.ErrContractCall) -} - -func TestKeeper_SetGasZetaPoolRevertsOnVmError(t *testing.T) { - k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ - UseEVMMock: true, - }) - k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) - - mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(t, mockEVMKeeper) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMKeeper.On( - "ApplyMessage", - ctx, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{ - VmError: "test error", - }, sample.ErrSample) + mockEVMFailCallOnce(mockEVMKeeper) err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) require.ErrorIs(t, err, types.ErrContractCall) } - -func setupMockEVMKeeperForSystemContractDeployment(t *testing.T, mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { - gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} - msgRes := &evmtypes.MsgEthereumTxResponse{} - - mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) - mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) - mockEVMKeeper.On( - "EstimateGas", - mock.Anything, - mock.Anything, - ).Return(gasRes, nil) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(msgRes, nil).Times(5) - mockEVMKeeper.On( - "GetAccount", - mock.Anything, - mock.Anything, - ).Return(&statedb.Account{ - Nonce: 1, - }) - mockEVMKeeper.On( - "GetCode", - mock.Anything, - mock.Anything, - ).Return([]byte{1, 2, 3}) -} diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index b464da209c..667ee245d9 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -6,9 +6,10 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/ethereum/go-ethereum/common" + evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/require" "github.com/zeta-chain/protocol-contracts/pkg/contracts/zevm/systemcontract.sol" - "github.com/zeta-chain/protocol-contracts/pkg/contracts/zevm/zrc20.sol" + zrc20 "github.com/zeta-chain/protocol-contracts/pkg/contracts/zevm/zrc20.sol" zetacommon "github.com/zeta-chain/zetacore/common" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/sample" @@ -74,6 +75,41 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { require.Equal(t, newSystemContract.Hex(), queryZRC20SystemContract(gas2)) }) + t.Run("can update the system contract if system contract not found", func(t *testing.T) { + k, ctx, _, zk := keepertest.FungibleKeeper(t) + msgServer := keeper.NewMsgServerImpl(*k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + admin := sample.AccAddress() + setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) + + chains := zetacommon.DefaultChainsList() + require.True(t, len(chains) > 1) + require.NotNil(t, chains[0]) + require.NotNil(t, chains[1]) + + wzeta, err := k.DeployWZETA(ctx) + require.NoError(t, err) + + factory, err := k.DeployUniswapV2Factory(ctx) + require.NoError(t, err) + + router, err := k.DeployUniswapV2Router02(ctx, factory, wzeta) + require.NoError(t, err) + + // deploy a new system contracts + newSystemContract, err := k.DeployContract(ctx, systemcontract.SystemContractMetaData, wzeta, factory, router) + require.NoError(t, err) + + // can update the system contract + _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) + require.NoError(t, err) + + // can retrieve the system contract + sc, found := k.GetSystemContract(ctx) + require.True(t, found) + require.Equal(t, newSystemContract.Hex(), sc.SystemContract) + }) + t.Run("should not update the system contract if not admin", func(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) msgServer := keeper.NewMsgServerImpl(*k) @@ -109,4 +145,69 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, sdkerrors.ErrInvalidAddress) }) + + t.Run("should not update if any of 3 evm calls for foreign coin fail", func(t *testing.T) { + k, ctx, _, zk := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + msgServer := keeper.NewMsgServerImpl(*k) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 9) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + admin := sample.AccAddress() + setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) + + chains := zetacommon.DefaultChainsList() + require.True(t, len(chains) > 1) + require.NotNil(t, chains[0]) + require.NotNil(t, chains[1]) + chainID1 := chains[0].ChainId + + wzeta, factory, router, _, _ := deploySystemContracts(t, ctx, k, mockEVMKeeper) + // setup mocks and setup gas coin + var encodedAddress [32]byte + copy(encodedAddress[12:], router[:]) + uniswapMock := &evmtypes.MsgEthereumTxResponse{ + Ret: encodedAddress[:], + } + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, uniswapMock) + mockEVMSuccessCallOnce(mockEVMKeeper) + + addLiqMockReturn := &evmtypes.MsgEthereumTxResponse{ + Ret: make([]byte, 3*32), + } + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, addLiqMockReturn) + + setupGasCoin(t, ctx, k, mockEVMKeeper, chainID1, "foo", "foo") + + // deploy a new system contracts + mockEVMSuccessCallOnce(mockEVMKeeper) + newSystemContract, err := k.DeployContract(ctx, systemcontract.SystemContractMetaData, wzeta, factory, router) + require.NoError(t, err) + + // fail on first evm call + mockEVMFailCallOnce(mockEVMKeeper) + + // can update the system contract + _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) + require.ErrorIs(t, err, types.ErrContractCall) + + // fail on second evm call + mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMFailCallOnce(mockEVMKeeper) + + // can update the system contract + _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) + require.ErrorIs(t, err, types.ErrContractCall) + + // fail on third evm call + mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, nil, 2) + mockEVMFailCallOnce(mockEVMKeeper) + + // can update the system contract + _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) + require.ErrorIs(t, err, types.ErrContractCall) + }) } diff --git a/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go b/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go index 108c70c664..c34db26993 100644 --- a/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go +++ b/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "errors" "math/big" "testing" @@ -176,32 +175,14 @@ func TestKeeper_UpdateZRC20WithdrawFee(t *testing.T) { require.NoError(t, err) protocolFlatFee, err := zrc20ABI.Methods["PROTOCOL_FLAT_FEE"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - false, - ).Return(&evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}, nil) + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) gasLimit, err := zrc20ABI.Methods["GAS_LIMIT"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - false, - ).Return(&evmtypes.MsgEthereumTxResponse{Ret: gasLimit}, nil) + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) // this is the update call (commit == true) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - true, - ).Return(&evmtypes.MsgEthereumTxResponse{}, errors.New("transaction failed")) + mockEVMFailCallOnce(mockEVMKeeper) _, err = msgServer.UpdateZRC20WithdrawFee(ctx, types.NewMsgUpdateZRC20WithdrawFee( admin, @@ -239,32 +220,14 @@ func TestKeeper_UpdateZRC20WithdrawFee(t *testing.T) { require.NoError(t, err) protocolFlatFee, err := zrc20ABI.Methods["PROTOCOL_FLAT_FEE"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - false, - ).Return(&evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}, nil) + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) gasLimit, err := zrc20ABI.Methods["GAS_LIMIT"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - false, - ).Return(&evmtypes.MsgEthereumTxResponse{Ret: gasLimit}, nil) + mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) // this is the update call (commit == true) - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - true, - ).Return(&evmtypes.MsgEthereumTxResponse{}, errors.New("transaction failed")) + mockEVMFailCallOnce(mockEVMKeeper) _, err = msgServer.UpdateZRC20WithdrawFee(ctx, types.NewMsgUpdateZRC20WithdrawFee( admin, From 9f7f6f580cae5076750d5d8d9dc099624a5e6da7 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 13:38:32 +0100 Subject: [PATCH 3/9] Cleanup --- x/fungible/keeper/evm_test.go | 8 ++++++-- x/fungible/keeper/gas_price_test.go | 6 +++--- .../keeper/msg_server_update_system_contract_test.go | 5 +++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/x/fungible/keeper/evm_test.go b/x/fungible/keeper/evm_test.go index 01bd93e8f9..e1c1c7eadf 100644 --- a/x/fungible/keeper/evm_test.go +++ b/x/fungible/keeper/evm_test.go @@ -625,7 +625,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { }) } -func setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, applyMessageExpectedCounter int) { +func setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -634,7 +634,7 @@ func setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper *fungiblemocks. mock.Anything, mock.Anything, ).Return(gasRes, nil) - mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMSuccessCallTimes(mockEVMKeeper, 5) mockEVMKeeper.On( "GetAccount", mock.Anything, @@ -653,6 +653,10 @@ func mockEVMSuccessCallOnce(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{}) } +func mockEVMSuccessCallTimes(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, times int) { + mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{}, times) +} + func mockEVMSuccessCallOnceWithReturn(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, ret *evmtypes.MsgEthereumTxResponse) { mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, ret, 1) } diff --git a/x/fungible/keeper/gas_price_test.go b/x/fungible/keeper/gas_price_test.go index d084701447..6cafa7aaa6 100644 --- a/x/fungible/keeper/gas_price_test.go +++ b/x/fungible/keeper/gas_price_test.go @@ -70,7 +70,7 @@ func TestKeeper_SetGasPriceReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) deploySystemContracts(t, ctx, k, mockEVMKeeper) mockEVMFailCallOnce(mockEVMKeeper) @@ -120,7 +120,7 @@ func TestKeeper_SetGasCoinReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) deploySystemContracts(t, ctx, k, mockEVMKeeper) mockEVMFailCallOnce(mockEVMKeeper) @@ -180,7 +180,7 @@ func TestKeeper_SetGasZetaPoolReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 5) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) deploySystemContracts(t, ctx, k, mockEVMKeeper) mockEVMFailCallOnce(mockEVMKeeper) diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index 667ee245d9..66633582a1 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -154,7 +154,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper, 9) + setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) admin := sample.AccAddress() setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) @@ -172,6 +172,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { uniswapMock := &evmtypes.MsgEthereumTxResponse{ Ret: encodedAddress[:], } + mockEVMSuccessCallTimes(mockEVMKeeper, 4) mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, uniswapMock) mockEVMSuccessCallOnce(mockEVMKeeper) @@ -203,7 +204,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { require.ErrorIs(t, err, types.ErrContractCall) // fail on third evm call - mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, nil, 2) + mockEVMSuccessCallTimes(mockEVMKeeper, 2) mockEVMFailCallOnce(mockEVMKeeper) // can update the system contract From 55a78b5d3e0e65c72ff2a96b0f4682f93d0888c7 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 13:50:37 +0100 Subject: [PATCH 4/9] Improve readability of mocking evm calls methods --- testutil/keeper/fungible.go | 74 ++++++++++++++++++- x/fungible/keeper/evm_test.go | 67 +---------------- x/fungible/keeper/gas_price_test.go | 12 +-- .../msg_server_update_system_contract_test.go | 22 +++--- ...g_server_update_zrc20_withdraw_fee_test.go | 12 +-- 5 files changed, 98 insertions(+), 89 deletions(-) diff --git a/testutil/keeper/fungible.go b/testutil/keeper/fungible.go index 0ef4e981f4..5f9f2b0cfd 100644 --- a/testutil/keeper/fungible.go +++ b/testutil/keeper/fungible.go @@ -1,6 +1,7 @@ package keeper import ( + "math/big" "testing" "github.com/cosmos/cosmos-sdk/codec" @@ -8,9 +9,13 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper" + "github.com/evmos/ethermint/x/evm/statedb" + evmtypes "github.com/evmos/ethermint/x/evm/types" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" tmdb "github.com/tendermint/tm-db" fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible" + "github.com/zeta-chain/zetacore/testutil/sample" fungiblemodule "github.com/zeta-chain/zetacore/x/fungible" "github.com/zeta-chain/zetacore/x/fungible/keeper" "github.com/zeta-chain/zetacore/x/fungible/types" @@ -164,8 +169,73 @@ func GetFungibleObserverMock(t testing.TB, keeper *keeper.Keeper) *fungiblemocks return fok } -func GetFungibleEVMMock(t testing.TB, keeper *keeper.Keeper) *fungiblemocks.FungibleEVMKeeper { +func GetFungibleEVMMock(t testing.TB, keeper *keeper.Keeper) *FungibleMockEVMKeeper { fek, ok := keeper.GetEVMKeeper().(*fungiblemocks.FungibleEVMKeeper) require.True(t, ok) - return fek + return &FungibleMockEVMKeeper{ + FungibleEVMKeeper: fek, + } +} + +type FungibleMockEVMKeeper struct { + *fungiblemocks.FungibleEVMKeeper +} + +func (m *FungibleMockEVMKeeper) SetupMockEVMKeeperForSystemContractDeployment() { + gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} + m.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) + m.On("ChainID").Maybe().Return(big.NewInt(1)) + m.On( + "EstimateGas", + mock.Anything, + mock.Anything, + ).Return(gasRes, nil) + m.MockEVMSuccessCallTimes(5) + m.On( + "GetAccount", + mock.Anything, + mock.Anything, + ).Return(&statedb.Account{ + Nonce: 1, + }) + m.On( + "GetCode", + mock.Anything, + mock.Anything, + ).Return([]byte{1, 2, 3}) +} + +func (m *FungibleMockEVMKeeper) MockEVMSuccessCallOnce() { + m.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{}) +} + +func (m *FungibleMockEVMKeeper) MockEVMSuccessCallTimes(times int) { + m.MockEVMSuccessCallTimesWithReturn(&evmtypes.MsgEthereumTxResponse{}, times) +} + +func (m *FungibleMockEVMKeeper) MockEVMSuccessCallOnceWithReturn(ret *evmtypes.MsgEthereumTxResponse) { + m.MockEVMSuccessCallTimesWithReturn(ret, 1) +} + +func (m *FungibleMockEVMKeeper) MockEVMSuccessCallTimesWithReturn(ret *evmtypes.MsgEthereumTxResponse, times int) { + if ret == nil { + ret = &evmtypes.MsgEthereumTxResponse{} + } + m.On( + "ApplyMessage", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(ret, nil).Times(times) +} + +func (m *FungibleMockEVMKeeper) MockEVMFailCallOnce() { + m.On( + "ApplyMessage", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample).Once() } diff --git a/x/fungible/keeper/evm_test.go b/x/fungible/keeper/evm_test.go index e1c1c7eadf..49dcfd22ec 100644 --- a/x/fungible/keeper/evm_test.go +++ b/x/fungible/keeper/evm_test.go @@ -8,7 +8,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/evmos/ethermint/x/evm/statedb" evmtypes "github.com/evmos/ethermint/x/evm/types" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -17,7 +16,6 @@ import ( "github.com/zeta-chain/zetacore/server/config" "github.com/zeta-chain/zetacore/testutil/contracts" testkeeper "github.com/zeta-chain/zetacore/testutil/keeper" - fungiblemocks "github.com/zeta-chain/zetacore/testutil/keeper/mocks/fungible" "github.com/zeta-chain/zetacore/testutil/sample" "github.com/zeta-chain/zetacore/x/fungible/keeper" fungiblekeeper "github.com/zeta-chain/zetacore/x/fungible/keeper" @@ -449,7 +447,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, &evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap}, ).Return(gasRes, nil) - mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallOnce() mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -492,7 +490,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, sdk.AccAddress(fromAddr.Bytes()), ).Return(uint64(1), nil) - mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallOnce() mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -605,7 +603,7 @@ func TestKeeper_CallEVMWithData(t *testing.T) { mock.Anything, &evmtypes.EthCallRequest{Args: args, GasCap: config.DefaultGasCap}, ).Return(gasRes, nil) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(ctx) mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) @@ -624,62 +622,3 @@ func TestKeeper_CallEVMWithData(t *testing.T) { require.ErrorIs(t, err, sample.ErrSample) }) } - -func setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { - gasRes := &evmtypes.EstimateGasResponse{Gas: 1000} - mockEVMKeeper.On("WithChainID", mock.Anything).Maybe().Return(mock.Anything) - mockEVMKeeper.On("ChainID").Maybe().Return(big.NewInt(1)) - mockEVMKeeper.On( - "EstimateGas", - mock.Anything, - mock.Anything, - ).Return(gasRes, nil) - mockEVMSuccessCallTimes(mockEVMKeeper, 5) - mockEVMKeeper.On( - "GetAccount", - mock.Anything, - mock.Anything, - ).Return(&statedb.Account{ - Nonce: 1, - }) - mockEVMKeeper.On( - "GetCode", - mock.Anything, - mock.Anything, - ).Return([]byte{1, 2, 3}) -} - -func mockEVMSuccessCallOnce(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{}) -} - -func mockEVMSuccessCallTimes(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, times int) { - mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{}, times) -} - -func mockEVMSuccessCallOnceWithReturn(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, ret *evmtypes.MsgEthereumTxResponse) { - mockEVMSuccessCallTimesWithReturn(mockEVMKeeper, ret, 1) -} - -func mockEVMSuccessCallTimesWithReturn(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper, ret *evmtypes.MsgEthereumTxResponse, times int) { - if ret == nil { - ret = &evmtypes.MsgEthereumTxResponse{} - } - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(ret, nil).Times(times) -} - -func mockEVMFailCallOnce(mockEVMKeeper *fungiblemocks.FungibleEVMKeeper) { - mockEVMKeeper.On( - "ApplyMessage", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return(&evmtypes.MsgEthereumTxResponse{}, sample.ErrSample).Once() -} diff --git a/x/fungible/keeper/gas_price_test.go b/x/fungible/keeper/gas_price_test.go index 6cafa7aaa6..5a232928d8 100644 --- a/x/fungible/keeper/gas_price_test.go +++ b/x/fungible/keeper/gas_price_test.go @@ -70,10 +70,10 @@ func TestKeeper_SetGasPriceReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) require.ErrorIs(t, err, types.ErrContractCall) } @@ -120,10 +120,10 @@ func TestKeeper_SetGasCoinReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) require.ErrorIs(t, err, types.ErrContractCall) } @@ -180,10 +180,10 @@ func TestKeeper_SetGasZetaPoolReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() deploySystemContracts(t, ctx, k, mockEVMKeeper) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) require.ErrorIs(t, err, types.ErrContractCall) } diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index 66633582a1..cabbf337cf 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -154,7 +154,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - setupMockEVMKeeperForSystemContractDeployment(mockEVMKeeper) + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) admin := sample.AccAddress() setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) @@ -172,40 +172,40 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { uniswapMock := &evmtypes.MsgEthereumTxResponse{ Ret: encodedAddress[:], } - mockEVMSuccessCallTimes(mockEVMKeeper, 4) - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, uniswapMock) - mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallTimes(4) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(uniswapMock) + mockEVMKeeper.MockEVMSuccessCallOnce() addLiqMockReturn := &evmtypes.MsgEthereumTxResponse{ Ret: make([]byte, 3*32), } - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, addLiqMockReturn) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(addLiqMockReturn) setupGasCoin(t, ctx, k, mockEVMKeeper, chainID1, "foo", "foo") // deploy a new system contracts - mockEVMSuccessCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallOnce() newSystemContract, err := k.DeployContract(ctx, systemcontract.SystemContractMetaData, wzeta, factory, router) require.NoError(t, err) // fail on first evm call - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() // can update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) require.ErrorIs(t, err, types.ErrContractCall) // fail on second evm call - mockEVMSuccessCallOnce(mockEVMKeeper) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallOnce() + mockEVMKeeper.MockEVMFailCallOnce() // can update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) require.ErrorIs(t, err, types.ErrContractCall) // fail on third evm call - mockEVMSuccessCallTimes(mockEVMKeeper, 2) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMSuccessCallTimes(2) + mockEVMKeeper.MockEVMFailCallOnce() // can update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) diff --git a/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go b/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go index c34db26993..29a5e157ae 100644 --- a/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go +++ b/x/fungible/keeper/msg_server_update_zrc20_withdraw_fee_test.go @@ -175,14 +175,14 @@ func TestKeeper_UpdateZRC20WithdrawFee(t *testing.T) { require.NoError(t, err) protocolFlatFee, err := zrc20ABI.Methods["PROTOCOL_FLAT_FEE"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) gasLimit, err := zrc20ABI.Methods["GAS_LIMIT"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) // this is the update call (commit == true) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() _, err = msgServer.UpdateZRC20WithdrawFee(ctx, types.NewMsgUpdateZRC20WithdrawFee( admin, @@ -220,14 +220,14 @@ func TestKeeper_UpdateZRC20WithdrawFee(t *testing.T) { require.NoError(t, err) protocolFlatFee, err := zrc20ABI.Methods["PROTOCOL_FLAT_FEE"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{Ret: protocolFlatFee}) gasLimit, err := zrc20ABI.Methods["GAS_LIMIT"].Outputs.Pack(big.NewInt(42)) require.NoError(t, err) - mockEVMSuccessCallOnceWithReturn(mockEVMKeeper, &evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) + mockEVMKeeper.MockEVMSuccessCallOnceWithReturn(&evmtypes.MsgEthereumTxResponse{Ret: gasLimit}) // this is the update call (commit == true) - mockEVMFailCallOnce(mockEVMKeeper) + mockEVMKeeper.MockEVMFailCallOnce() _, err = msgServer.UpdateZRC20WithdrawFee(ctx, types.NewMsgUpdateZRC20WithdrawFee( admin, From 858192439f18112550ffc978b9041aba50aa5b25 Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 14:15:07 +0100 Subject: [PATCH 5/9] Add error tests for existing system contract tests --- x/fungible/keeper/system_contract.go | 13 +- x/fungible/keeper/system_contract_test.go | 243 ++++++++++++++++++++++ 2 files changed, 250 insertions(+), 6 deletions(-) diff --git a/x/fungible/keeper/system_contract.go b/x/fungible/keeper/system_contract.go index a6e1ae8e03..be531f1597 100644 --- a/x/fungible/keeper/system_contract.go +++ b/x/fungible/keeper/system_contract.go @@ -77,7 +77,8 @@ func (k *Keeper) GetWZetaContractAddress(ctx sdk.Context) (ethcommon.Address, er "wZetaContractAddress", ) if err != nil { - return ethcommon.Address{}, cosmoserrors.Wrapf(err, "failed to call wZetaContractAddress") + return ethcommon.Address{}, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call wZetaContractAddress (%s)", err.Error()) + } type AddressResponse struct { Value ethcommon.Address @@ -113,7 +114,7 @@ func (k *Keeper) GetUniswapV2FactoryAddress(ctx sdk.Context) (ethcommon.Address, "uniswapv2FactoryAddress", ) if err != nil { - return ethcommon.Address{}, cosmoserrors.Wrapf(err, "failed to call uniswapv2FactoryAddress") + return ethcommon.Address{}, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call uniswapv2FactoryAddress (%s)", err.Error()) } type AddressResponse struct { Value ethcommon.Address @@ -149,7 +150,7 @@ func (k *Keeper) GetUniswapV2Router02Address(ctx sdk.Context) (ethcommon.Address "uniswapv2Router02Address", ) if err != nil { - return ethcommon.Address{}, cosmoserrors.Wrapf(err, "failed to call uniswapv2Router02Address") + return ethcommon.Address{}, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call uniswapv2Router02Address (%s)", err.Error()) } type AddressResponse struct { Value ethcommon.Address @@ -185,7 +186,7 @@ func (k *Keeper) CallWZetaDeposit(ctx sdk.Context, sender ethcommon.Address, amo "deposit", ) if err != nil { - return cosmoserrors.Wrapf(err, "failed to call wzeta deposit") + return cosmoserrors.Wrapf(types.ErrContractCall, "failed to call wzeta deposit (%s)", err.Error()) } return nil } @@ -215,7 +216,7 @@ func (k *Keeper) QueryWZetaBalanceOf(ctx sdk.Context, addr ethcommon.Address) (* addr, ) if err != nil { - return nil, cosmoserrors.Wrapf(err, "failed to call balanceOf") + return nil, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call balanceOf (%s)", err.Error()) } type BigIntResponse struct { @@ -254,7 +255,7 @@ func (k *Keeper) QuerySystemContractGasCoinZRC20(ctx sdk.Context, chainid *big.I chainid, ) if err != nil { - return ethcommon.Address{}, cosmoserrors.Wrapf(err, "failed to call gasCoinZRC20ByChainId") + return ethcommon.Address{}, cosmoserrors.Wrapf(types.ErrContractCall, "failed to call gasCoinZRC20ByChainId (%s)", err.Error()) } type AddressResponse struct { diff --git a/x/fungible/keeper/system_contract_test.go b/x/fungible/keeper/system_contract_test.go index 3874f52435..f737309979 100644 --- a/x/fungible/keeper/system_contract_test.go +++ b/x/fungible/keeper/system_contract_test.go @@ -52,6 +52,44 @@ func TestKeeper_GetWZetaContractAddress(t *testing.T) { require.Equal(t, wzeta, found) } +func TestKeeper_GetWZetaContractAddressFails(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetWZetaContractAddress(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMFailCallOnce() + _, err = k.GetWZetaContractAddress(ctx) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_GetWZetaContractAddressFailsToUnpack(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetWZetaContractAddress(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMSuccessCallOnce() + _, err = k.GetWZetaContractAddress(ctx) + require.ErrorIs(t, err, types.ErrABIUnpack) +} + func TestKeeper_GetUniswapV2FactoryAddress(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -66,6 +104,44 @@ func TestKeeper_GetUniswapV2FactoryAddress(t *testing.T) { require.Equal(t, factory, found) } +func TestKeeper_GetUniswapV2FactoryAddressFails(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetUniswapV2FactoryAddress(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMFailCallOnce() + _, err = k.GetUniswapV2FactoryAddress(ctx) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_GetUniswapV2FactoryAddressFailsToUnpack(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetUniswapV2FactoryAddress(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMSuccessCallOnce() + _, err = k.GetUniswapV2FactoryAddress(ctx) + require.ErrorIs(t, err, types.ErrABIUnpack) +} + func TestKeeper_GetUniswapV2Router02Address(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -80,6 +156,44 @@ func TestKeeper_GetUniswapV2Router02Address(t *testing.T) { require.Equal(t, router, found) } +func TestKeeper_GetUniswapV2Router02AddressFails(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetUniswapV2Router02Address(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMFailCallOnce() + _, err = k.GetUniswapV2Router02Address(ctx) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_GetUniswapV2Router02AddressFailsToUnpack(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + _, err := k.GetUniswapV2Router02Address(ctx) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMSuccessCallOnce() + _, err = k.GetUniswapV2Router02Address(ctx) + require.ErrorIs(t, err, types.ErrABIUnpack) +} + func TestKeeper_CallWZetaDeposit(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -108,6 +222,93 @@ func TestKeeper_CallWZetaDeposit(t *testing.T) { require.Equal(t, big.NewInt(42), balance) } +func TestKeeper_CallWZetaDepositFails(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + // mint tokens + addr := sample.Bech32AccAddress() + ethAddr := common.BytesToAddress(addr.Bytes()) + coins := sample.Coins() + err := sdkk.BankKeeper.MintCoins(ctx, types.ModuleName, sample.Coins()) + require.NoError(t, err) + err = sdkk.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addr, coins) + require.NoError(t, err) + + // fail if no system contract + err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) + require.Error(t, err) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + // deposit + mockEVMKeeper.MockEVMFailCallOnce() + err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_QueryWZetaBalanceOfFails(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + // mint tokens + addr := sample.Bech32AccAddress() + ethAddr := common.BytesToAddress(addr.Bytes()) + coins := sample.Coins() + err := sdkk.BankKeeper.MintCoins(ctx, types.ModuleName, sample.Coins()) + require.NoError(t, err) + err = sdkk.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addr, coins) + require.NoError(t, err) + + // fail if no system contract + err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) + require.Error(t, err) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + // deposit + mockEVMKeeper.MockEVMFailCallOnce() + _, err = k.QueryWZetaBalanceOf(ctx, ethAddr) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_QueryWZetaBalanceOfFailsToUnpack(t *testing.T) { + k, ctx, sdkk, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + // mint tokens + addr := sample.Bech32AccAddress() + ethAddr := common.BytesToAddress(addr.Bytes()) + coins := sample.Coins() + err := sdkk.BankKeeper.MintCoins(ctx, types.ModuleName, sample.Coins()) + require.NoError(t, err) + err = sdkk.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addr, coins) + require.NoError(t, err) + + // fail if no system contract + err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) + require.Error(t, err) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + // deposit + mockEVMKeeper.MockEVMSuccessCallOnce() + _, err = k.QueryWZetaBalanceOf(ctx, ethAddr) + require.ErrorIs(t, err, types.ErrABIUnpack) +} + func TestKeeper_QuerySystemContractGasCoinZRC20(t *testing.T) { k, ctx, sdkk, _ := keepertest.FungibleKeeper(t) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) @@ -124,3 +325,45 @@ func TestKeeper_QuerySystemContractGasCoinZRC20(t *testing.T) { require.NoError(t, err) require.Equal(t, zrc20, found) } + +func TestKeeper_QuerySystemContractGasCoinZRC20Fails(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + chainID := getValidChainID(t) + + _, err := k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMFailCallOnce() + _, err = k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) + require.ErrorIs(t, err, types.ErrContractCall) +} + +func TestKeeper_QuerySystemContractGasCoinZRC20FailsToUnpack(t *testing.T) { + k, ctx, _, _ := keepertest.FungibleKeeperWithMocks(t, keepertest.FungibleMockOptions{ + UseEVMMock: true, + }) + mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + chainID := getValidChainID(t) + + _, err := k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) + require.Error(t, err) + require.ErrorIs(t, err, types.ErrStateVariableNotFound) + + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + deploySystemContracts(t, ctx, k, mockEVMKeeper) + + mockEVMKeeper.MockEVMSuccessCallOnce() + _, err = k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) + require.ErrorIs(t, err, types.ErrABIUnpack) +} From dbbb0feae38eaf959d4f50720e9f24f368dfca8d Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 16:07:10 +0100 Subject: [PATCH 6/9] Cleanup duplication --- x/fungible/keeper/evm_test.go | 15 +++++++-- x/fungible/keeper/foreign_coins.go | 10 +++--- x/fungible/keeper/gas_price_test.go | 9 ++--- .../msg_server_update_system_contract_test.go | 3 +- ..._server_update_zrc20_paused_status_test.go | 3 +- x/fungible/keeper/system_contract_test.go | 33 +++++++------------ 6 files changed, 34 insertions(+), 39 deletions(-) diff --git a/x/fungible/keeper/evm_test.go b/x/fungible/keeper/evm_test.go index 49dcfd22ec..df8037aa06 100644 --- a/x/fungible/keeper/evm_test.go +++ b/x/fungible/keeper/evm_test.go @@ -17,7 +17,6 @@ import ( "github.com/zeta-chain/zetacore/testutil/contracts" testkeeper "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/sample" - "github.com/zeta-chain/zetacore/x/fungible/keeper" fungiblekeeper "github.com/zeta-chain/zetacore/x/fungible/keeper" "github.com/zeta-chain/zetacore/x/fungible/types" ) @@ -40,6 +39,16 @@ func assertContractDeployment(t *testing.T, k types.EVMKeeper, ctx sdk.Context, require.NotEmpty(t, code) } +func deploySystemContractsWithMockEvmKeeper( + t *testing.T, + ctx sdk.Context, + k *fungiblekeeper.Keeper, + mockEVMKeeper *testkeeper.FungibleMockEVMKeeper, +) (wzeta, uniswapV2Factory, uniswapV2Router, connector, systemContract common.Address) { + mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() + return deploySystemContracts(t, ctx, k, mockEVMKeeper) +} + // deploySystemContracts deploys the system contracts and returns their addresses. func deploySystemContracts( t *testing.T, @@ -81,7 +90,7 @@ func deploySystemContracts( func assertExampleBarValue( t *testing.T, ctx sdk.Context, - k *keeper.Keeper, + k *fungiblekeeper.Keeper, address common.Address, expected int64, ) { @@ -98,6 +107,7 @@ func assertExampleBarValue( false, "bar", ) + require.NoError(t, err) unpacked, err := exampleABI.Unpack("bar", res.Ret) require.NoError(t, err) require.NotZero(t, len(unpacked)) @@ -262,6 +272,7 @@ func TestKeeper_DepositZRC20AndCallContract(t *testing.T) { false, "bar", ) + require.NoError(t, err) unpacked, err := exampleABI.Unpack("bar", res.Ret) require.NoError(t, err) require.NotZero(t, len(unpacked)) diff --git a/x/fungible/keeper/foreign_coins.go b/x/fungible/keeper/foreign_coins.go index faa96d2876..028c961609 100644 --- a/x/fungible/keeper/foreign_coins.go +++ b/x/fungible/keeper/foreign_coins.go @@ -1,8 +1,6 @@ package keeper import ( - "fmt" - "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" ethcommon "github.com/ethereum/go-ethereum/common" @@ -12,7 +10,7 @@ import ( // SetForeignCoins set a specific foreignCoins in the store from its index func (k Keeper) SetForeignCoins(ctx sdk.Context, foreignCoins types.ForeignCoins) { - p := types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix)) + p := types.KeyPrefix(types.ForeignCoinsKeyPrefix) store := prefix.NewStore(ctx.KVStore(k.storeKey), p) b := k.cdc.MustMarshal(&foreignCoins) store.Set(types.ForeignCoinsKey( @@ -25,7 +23,7 @@ func (k Keeper) GetForeignCoins( ctx sdk.Context, zrc20Addr string, ) (val types.ForeignCoins, found bool) { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix))) + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix)) b := store.Get(types.ForeignCoinsKey( zrc20Addr, @@ -51,7 +49,7 @@ func (k Keeper) RemoveForeignCoins( // GetAllForeignCoinsForChain returns all foreignCoins on a given chain func (k Keeper) GetAllForeignCoinsForChain(ctx sdk.Context, foreignChainID int64) (list []types.ForeignCoins) { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix))) + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix)) iterator := sdk.KVStorePrefixIterator(store, []byte{}) defer iterator.Close() @@ -68,7 +66,7 @@ func (k Keeper) GetAllForeignCoinsForChain(ctx sdk.Context, foreignChainID int64 // GetAllForeignCoins returns all foreignCoins func (k Keeper) GetAllForeignCoins(ctx sdk.Context) (list []types.ForeignCoins) { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(fmt.Sprintf("%s", types.ForeignCoinsKeyPrefix))) + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.ForeignCoinsKeyPrefix)) iterator := sdk.KVStorePrefixIterator(store, []byte{}) defer iterator.Close() diff --git a/x/fungible/keeper/gas_price_test.go b/x/fungible/keeper/gas_price_test.go index 5a232928d8..acd03d359b 100644 --- a/x/fungible/keeper/gas_price_test.go +++ b/x/fungible/keeper/gas_price_test.go @@ -70,8 +70,7 @@ func TestKeeper_SetGasPriceReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() _, err := k.SetGasPrice(ctx, big.NewInt(1), big.NewInt(1)) @@ -120,8 +119,7 @@ func TestKeeper_SetGasCoinReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() err := k.SetGasCoin(ctx, big.NewInt(1), sample.EthAddress()) @@ -180,8 +178,7 @@ func TestKeeper_SetGasZetaPoolReverts(t *testing.T) { k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() err := k.SetGasZetaPool(ctx, big.NewInt(1), sample.EthAddress()) diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index cabbf337cf..130d4a63e5 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -154,7 +154,6 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { mockEVMKeeper := keepertest.GetFungibleEVMMock(t, k) msgServer := keeper.NewMsgServerImpl(*k) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) admin := sample.AccAddress() setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) @@ -165,7 +164,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { require.NotNil(t, chains[1]) chainID1 := chains[0].ChainId - wzeta, factory, router, _, _ := deploySystemContracts(t, ctx, k, mockEVMKeeper) + wzeta, factory, router, _, _ := deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) // setup mocks and setup gas coin var encodedAddress [32]byte copy(encodedAddress[12:], router[:]) diff --git a/x/fungible/keeper/msg_server_update_zrc20_paused_status_test.go b/x/fungible/keeper/msg_server_update_zrc20_paused_status_test.go index bc4238a780..82b6beb977 100644 --- a/x/fungible/keeper/msg_server_update_zrc20_paused_status_test.go +++ b/x/fungible/keeper/msg_server_update_zrc20_paused_status_test.go @@ -156,12 +156,13 @@ func TestKeeper_UpdateZRC20PausedStatus(t *testing.T) { []string{sample.EthAddress().String()}, types.UpdatePausedStatusAction_PAUSE, )) + require.ErrorIs(t, err, sdkerrors.ErrUnauthorized) admin := sample.AccAddress() setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group1) _, err = msgServer.UpdateZRC20PausedStatus(ctx, types.NewMsgUpdateZRC20PausedStatus( - sample.AccAddress(), + admin, []string{sample.EthAddress().String()}, types.UpdatePausedStatusAction_UNPAUSE, )) diff --git a/x/fungible/keeper/system_contract_test.go b/x/fungible/keeper/system_contract_test.go index f737309979..850952e162 100644 --- a/x/fungible/keeper/system_contract_test.go +++ b/x/fungible/keeper/system_contract_test.go @@ -63,8 +63,7 @@ func TestKeeper_GetWZetaContractAddressFails(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() _, err = k.GetWZetaContractAddress(ctx) @@ -82,8 +81,7 @@ func TestKeeper_GetWZetaContractAddressFailsToUnpack(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMSuccessCallOnce() _, err = k.GetWZetaContractAddress(ctx) @@ -115,8 +113,7 @@ func TestKeeper_GetUniswapV2FactoryAddressFails(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() _, err = k.GetUniswapV2FactoryAddress(ctx) @@ -134,8 +131,7 @@ func TestKeeper_GetUniswapV2FactoryAddressFailsToUnpack(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMSuccessCallOnce() _, err = k.GetUniswapV2FactoryAddress(ctx) @@ -167,8 +163,7 @@ func TestKeeper_GetUniswapV2Router02AddressFails(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() _, err = k.GetUniswapV2Router02Address(ctx) @@ -186,8 +181,7 @@ func TestKeeper_GetUniswapV2Router02AddressFailsToUnpack(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMSuccessCallOnce() _, err = k.GetUniswapV2Router02Address(ctx) @@ -242,8 +236,7 @@ func TestKeeper_CallWZetaDepositFails(t *testing.T) { err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) require.Error(t, err) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) // deposit mockEVMKeeper.MockEVMFailCallOnce() @@ -271,8 +264,7 @@ func TestKeeper_QueryWZetaBalanceOfFails(t *testing.T) { err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) require.Error(t, err) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) // deposit mockEVMKeeper.MockEVMFailCallOnce() @@ -300,8 +292,7 @@ func TestKeeper_QueryWZetaBalanceOfFailsToUnpack(t *testing.T) { err = k.CallWZetaDeposit(ctx, ethAddr, big.NewInt(42)) require.Error(t, err) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) // deposit mockEVMKeeper.MockEVMSuccessCallOnce() @@ -339,8 +330,7 @@ func TestKeeper_QuerySystemContractGasCoinZRC20Fails(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMFailCallOnce() _, err = k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) @@ -360,8 +350,7 @@ func TestKeeper_QuerySystemContractGasCoinZRC20FailsToUnpack(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, types.ErrStateVariableNotFound) - mockEVMKeeper.SetupMockEVMKeeperForSystemContractDeployment() - deploySystemContracts(t, ctx, k, mockEVMKeeper) + deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) mockEVMKeeper.MockEVMSuccessCallOnce() _, err = k.QuerySystemContractGasCoinZRC20(ctx, big.NewInt(chainID)) From 4964b4a6f289a072a277f224a43130b3c7d112bb Mon Sep 17 00:00:00 2001 From: skosito Date: Thu, 15 Feb 2024 16:14:56 +0100 Subject: [PATCH 7/9] Cleanup comments --- x/fungible/keeper/msg_server_update_system_contract_test.go | 6 +++--- x/fungible/keeper/system_contract_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index 130d4a63e5..ab3c7a43fc 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -190,7 +190,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { // fail on first evm call mockEVMKeeper.MockEVMFailCallOnce() - // can update the system contract + // can't update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) require.ErrorIs(t, err, types.ErrContractCall) @@ -198,7 +198,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { mockEVMKeeper.MockEVMSuccessCallOnce() mockEVMKeeper.MockEVMFailCallOnce() - // can update the system contract + // can't update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) require.ErrorIs(t, err, types.ErrContractCall) @@ -206,7 +206,7 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { mockEVMKeeper.MockEVMSuccessCallTimes(2) mockEVMKeeper.MockEVMFailCallOnce() - // can update the system contract + // can't update the system contract _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) require.ErrorIs(t, err, types.ErrContractCall) }) diff --git a/x/fungible/keeper/system_contract_test.go b/x/fungible/keeper/system_contract_test.go index 850952e162..e2340cb4c1 100644 --- a/x/fungible/keeper/system_contract_test.go +++ b/x/fungible/keeper/system_contract_test.go @@ -266,7 +266,7 @@ func TestKeeper_QueryWZetaBalanceOfFails(t *testing.T) { deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) - // deposit + // query mockEVMKeeper.MockEVMFailCallOnce() _, err = k.QueryWZetaBalanceOf(ctx, ethAddr) require.ErrorIs(t, err, types.ErrContractCall) @@ -294,7 +294,7 @@ func TestKeeper_QueryWZetaBalanceOfFailsToUnpack(t *testing.T) { deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) - // deposit + // query mockEVMKeeper.MockEVMSuccessCallOnce() _, err = k.QueryWZetaBalanceOf(ctx, ethAddr) require.ErrorIs(t, err, types.ErrABIUnpack) From 3374bd839986c2817590e2a6ca2b8cc730e06df7 Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 16 Feb 2024 12:28:57 +0100 Subject: [PATCH 8/9] Fix PR comments --- x/fungible/keeper/gas_price.go | 13 ++++++++++-- .../msg_server_update_system_contract_test.go | 21 ++++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/x/fungible/keeper/gas_price.go b/x/fungible/keeper/gas_price.go index e829f59377..58ca702b69 100644 --- a/x/fungible/keeper/gas_price.go +++ b/x/fungible/keeper/gas_price.go @@ -31,6 +31,9 @@ func (k Keeper) SetGasPrice(ctx sdk.Context, chainid *big.Int, gasPrice *big.Int if err != nil { return 0, sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } + if res.Failed() { + return res.GasUsed, sdkerrors.Wrapf(types.ErrContractCall, "setGasPrice tx failed") + } return res.GasUsed, nil } @@ -48,10 +51,13 @@ func (k Keeper) SetGasCoin(ctx sdk.Context, chainid *big.Int, address common.Add if err != nil { return sdkerrors.Wrapf(types.ErrABIGet, "SystemContractMetaData") } - _, err = k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasCoinZRC20", chainid, address) + res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasCoinZRC20", chainid, address) if err != nil { return sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } + if res.Failed() { + return sdkerrors.Wrapf(types.ErrContractCall, "setGasCoinZRC20 tx failed") + } return nil } @@ -69,10 +75,13 @@ func (k Keeper) SetGasZetaPool(ctx sdk.Context, chainid *big.Int, pool common.Ad if err != nil { return sdkerrors.Wrapf(types.ErrABIGet, "SystemContractMetaData") } - _, err = k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasZetaPool", chainid, pool) + res, err := k.CallEVM(ctx, *abi, types.ModuleAddressEVM, oracle, BigIntZero, nil, true, false, "setGasZetaPool", chainid, pool) if err != nil { return sdkerrors.Wrapf(types.ErrContractCall, err.Error()) } + if res.Failed() { + return sdkerrors.Wrapf(types.ErrContractCall, "setGasZetaPool tx failed") + } return nil } diff --git a/x/fungible/keeper/msg_server_update_system_contract_test.go b/x/fungible/keeper/msg_server_update_system_contract_test.go index ab3c7a43fc..0fbc261c47 100644 --- a/x/fungible/keeper/msg_server_update_system_contract_test.go +++ b/x/fungible/keeper/msg_server_update_system_contract_test.go @@ -75,18 +75,13 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { require.Equal(t, newSystemContract.Hex(), queryZRC20SystemContract(gas2)) }) - t.Run("can update the system contract if system contract not found", func(t *testing.T) { + t.Run("can update and overwrite the system contract if system contract not found", func(t *testing.T) { k, ctx, _, zk := keepertest.FungibleKeeper(t) msgServer := keeper.NewMsgServerImpl(*k) k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) admin := sample.AccAddress() setAdminPolicies(ctx, zk, admin, observertypes.Policy_Type_group2) - chains := zetacommon.DefaultChainsList() - require.True(t, len(chains) > 1) - require.NotNil(t, chains[0]) - require.NotNil(t, chains[1]) - wzeta, err := k.DeployWZETA(ctx) require.NoError(t, err) @@ -108,6 +103,19 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { sc, found := k.GetSystemContract(ctx) require.True(t, found) require.Equal(t, newSystemContract.Hex(), sc.SystemContract) + + // deploy a new system contracts + newSystemContract, err = k.DeployContract(ctx, systemcontract.SystemContractMetaData, wzeta, factory, router) + require.NoError(t, err) + + // can overwrite the previous system contract + _, err = msgServer.UpdateSystemContract(ctx, types.NewMsgUpdateSystemContract(admin, newSystemContract.Hex())) + require.NoError(t, err) + + // can retrieve the system contract + sc, found = k.GetSystemContract(ctx) + require.True(t, found) + require.Equal(t, newSystemContract.Hex(), sc.SystemContract) }) t.Run("should not update the system contract if not admin", func(t *testing.T) { @@ -161,7 +169,6 @@ func TestKeeper_UpdateSystemContract(t *testing.T) { chains := zetacommon.DefaultChainsList() require.True(t, len(chains) > 1) require.NotNil(t, chains[0]) - require.NotNil(t, chains[1]) chainID1 := chains[0].ChainId wzeta, factory, router, _, _ := deploySystemContractsWithMockEvmKeeper(t, ctx, k, mockEVMKeeper) From 062acce9f84ebbfd29fd8133ec9dd212363c40aa Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 16 Feb 2024 12:30:20 +0100 Subject: [PATCH 9/9] Add changelog entry --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 5fa64f61a2..c327be0096 100644 --- a/changelog.md +++ b/changelog.md @@ -38,6 +38,7 @@ * [1584](https://github.com/zeta-chain/node/pull/1584) - allow to run E2E tests on any networks * [1753](https://github.com/zeta-chain/node/pull/1753) - fix gosec errors on usage of rand package +* [1762](https://github.com/zeta-chain/node/pull/1762) - improve coverage for fungibile module ### CI