From 2f35ee767344b8ca968d0b45356a02c51cd7a8bf Mon Sep 17 00:00:00 2001 From: Francisco de Borja Aranda Castillejo Date: Mon, 28 Oct 2024 18:56:25 +0100 Subject: [PATCH] apply reviews --- cmd/zetacored/config/prefixes.go | 3 +++ e2e/e2etests/test_precompiles_distribute.go | 19 ++++++++-------- ...precompiles_distribute_through_contract.go | 16 ++++++-------- e2e/runner/runner.go | 4 ++++ precompiles/bank/bank.go | 1 + precompiles/staking/method_distribute.go | 22 ++++++++----------- precompiles/staking/staking.go | 1 + precompiles/types/coin.go | 6 ++--- .../keeper/zrc20_cosmos_coins_mapping.go | 1 + 9 files changed, 38 insertions(+), 35 deletions(-) diff --git a/cmd/zetacored/config/prefixes.go b/cmd/zetacored/config/prefixes.go index a96a4c57bc..5fcd9218a5 100644 --- a/cmd/zetacored/config/prefixes.go +++ b/cmd/zetacored/config/prefixes.go @@ -6,6 +6,9 @@ const ( // Bech32Prefix defines the Bech32 prefix used for Cronos Accounts Bech32Prefix = "zeta" + // ZRC20DenomPrefix defines the prefix for ZRC20 tokens when converted to sdk.Coin. + ZRC20DenomPrefix = "zrc20/" + // Bech32PrefixAccAddr defines the Bech32 prefix of an account's address Bech32PrefixAccAddr = Bech32Prefix // Bech32PrefixAccPub defines the Bech32 prefix of an account's public key diff --git a/e2e/e2etests/test_precompiles_distribute.go b/e2e/e2etests/test_precompiles_distribute.go index 7e2daa581b..7f98518f23 100644 --- a/e2e/e2etests/test_precompiles_distribute.go +++ b/e2e/e2etests/test_precompiles_distribute.go @@ -4,7 +4,6 @@ import ( "math/big" "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/stretchr/testify/require" @@ -22,7 +21,6 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { var ( spenderAddress = r.EVMAddress() distributeContractAddress = staking.ContractAddress - feeCollectorAddress = authtypes.NewModuleAddress("fee_collector") lockerAddress = bank.ContractAddress zrc20Address = r.ERC20ZRC20Addr @@ -40,7 +38,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { r.ZEVMAuth.GasLimit = 10_000_000 // Set the test to reset the state after it finishes. - defer resetTest(r, lockerAddress, previousGasLimit, fiveHundred) + defer resetDistributionTest(r, lockerAddress, previousGasLimit, fiveHundred) // Get ERC20ZRC20. txHash := r.DepositERC20WithAmountAndMessage(spenderAddress, oneThousand, []byte{}) @@ -57,7 +55,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // Check initial balances. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 0, checkZRC20Balance(r, lockerAddress)) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) tx, err := dstrContract.Distribute(r.ZEVMAuth, zrc20Address, oneThousand) require.NoError(r, err) @@ -67,7 +65,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 0, checkZRC20Balance(r, lockerAddress)) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Allow 500. approveAllowance(r, distributeContractAddress, fiveHundred) @@ -81,7 +79,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 0, checkZRC20Balance(r, lockerAddress)) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Raise the allowance to 1000. approveAllowance(r, distributeContractAddress, oneThousand) @@ -95,7 +93,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 0, checkZRC20Balance(r, lockerAddress)) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Should be able to distribute 500, which is within balance and allowance. tx, err = dstrContract.Distribute(r.ZEVMAuth, zrc20Address, fiveHundred) @@ -105,7 +103,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { balanceShouldBe(r, 500, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 500, checkZRC20Balance(r, lockerAddress)) - balanceShouldBe(r, 500, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 500, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) eventDitributed, err := dstrContract.ParseDistributed(*receipt.Logs[0]) require.NoError(r, err) @@ -115,7 +113,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // After one block the rewards should have been distributed and fee collector should have 0 ZRC20 balance. r.WaitForBlocks(1) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // DO NOT REMOVE THE FOLLOWING CODE // This section is commented until a following PR introduces the ability to withdraw delegator rewards. @@ -165,6 +163,7 @@ func TestPrecompilesDistribute(r *runner.E2ERunner, args []string) { // fmt.Printf("Validator 1 commission: %+v\n", res3) } +// checkCosmosBalance checks the cosmos coin balance for an address. The coin is specified by its denom. func checkCosmosBalance(r *runner.E2ERunner, address types.AccAddress, denom string) *big.Int { bal, err := r.BankClient.Balance( r.Ctx, @@ -175,7 +174,7 @@ func checkCosmosBalance(r *runner.E2ERunner, address types.AccAddress, denom str return bal.Balance.Amount.BigInt() } -func resetTest(r *runner.E2ERunner, lockerAddress common.Address, previousGasLimit uint64, amount *big.Int) { +func resetDistributionTest(r *runner.E2ERunner, lockerAddress common.Address, previousGasLimit uint64, amount *big.Int) { r.ZEVMAuth.GasLimit = previousGasLimit // Reset the allowance to 0; this is needed when running upgrade tests where this test runs twice. diff --git a/e2e/e2etests/test_precompiles_distribute_through_contract.go b/e2e/e2etests/test_precompiles_distribute_through_contract.go index dd4366d2e2..0fccf7ce21 100644 --- a/e2e/e2etests/test_precompiles_distribute_through_contract.go +++ b/e2e/e2etests/test_precompiles_distribute_through_contract.go @@ -3,7 +3,6 @@ package e2etests import ( "math/big" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/stretchr/testify/require" @@ -22,7 +21,6 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string var ( spenderAddress = r.EVMAddress() distributeContractAddress = staking.ContractAddress - feeCollectorAddress = authtypes.NewModuleAddress("fee_collector") lockerAddress = bank.ContractAddress zrc20Address = r.ERC20ZRC20Addr @@ -52,12 +50,12 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string r.ZEVMAuth.GasLimit = 10_000_000 // Set the test to reset the state after it finishes. - defer resetTest(r, lockerAddress, previousGasLimit, fiveHundred) + defer resetDistributionTest(r, lockerAddress, previousGasLimit, fiveHundred) // Check initial balances. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 500, checkZRC20Balance(r, lockerAddress)) // Carries 500 from distribute e2e. - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) receipt = distributeThroughContract(r, testDstrContract, zrc20Address, oneThousand) utils.RequiredTxFailed(r, receipt, "distribute should fail when there's no allowance") @@ -65,7 +63,7 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 500, checkZRC20Balance(r, lockerAddress)) // Carries 500 from distribute e2e. - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Allow 500. approveAllowance(r, distributeContractAddress, fiveHundred) @@ -76,7 +74,7 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 500, checkZRC20Balance(r, lockerAddress)) // Carries 500 from distribute e2e. - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Raise the allowance to 1000. approveAllowance(r, distributeContractAddress, oneThousand) @@ -88,7 +86,7 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string // Balances shouldn't change after a failed attempt. balanceShouldBe(r, 1000, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 500, checkZRC20Balance(r, lockerAddress)) // Carries 500 from distribute e2e. - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) // Should be able to distribute 500, which is within balance and allowance. receipt = distributeThroughContract(r, testDstrContract, zrc20Address, fiveHundred) @@ -96,7 +94,7 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string balanceShouldBe(r, 500, checkZRC20Balance(r, spenderAddress)) balanceShouldBe(r, 1000, checkZRC20Balance(r, lockerAddress)) // Carries 500 from distribute e2e. - balanceShouldBe(r, 500, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 500, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) eventDitributed, err := dstrContract.ParseDistributed(*receipt.Logs[0]) require.NoError(r, err) @@ -106,7 +104,7 @@ func TestPrecompilesDistributeThroughContract(r *runner.E2ERunner, args []string // After one block the rewards should have been distributed and fee collector should have 0 ZRC20 balance. r.WaitForBlocks(1) - balanceShouldBe(r, 0, checkCosmosBalance(r, feeCollectorAddress, zrc20Denom)) + balanceShouldBe(r, 0, checkCosmosBalance(r, r.FeeCollectorAddress, zrc20Denom)) } func distributeThroughContract( diff --git a/e2e/runner/runner.go b/e2e/runner/runner.go index c3dcb74637..cf7a8e6f02 100644 --- a/e2e/runner/runner.go +++ b/e2e/runner/runner.go @@ -9,6 +9,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/rpcclient" + "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -75,6 +76,7 @@ type E2ERunner struct { SolanaDeployerAddress solana.PublicKey TONDeployer *tonrunner.Deployer TONGateway *toncontracts.Gateway + FeeCollectorAddress types.AccAddress // all clients. // a reference to this type is required to enable creating a new E2ERunner. @@ -189,6 +191,8 @@ func NewE2ERunner( Account: account, + FeeCollectorAddress: authtypes.NewModuleAddress(authtypes.FeeCollectorName), + Clients: clients, ZEVMClient: clients.Zevm, diff --git a/precompiles/bank/bank.go b/precompiles/bank/bank.go index 9dbb23ef5b..22f2258928 100644 --- a/precompiles/bank/bank.go +++ b/precompiles/bank/bank.go @@ -74,6 +74,7 @@ func NewIBankContract( // This avoids instantiating it every time deposit or withdraw are called. zrc20ABI, err := zrc20.ZRC20MetaData.GetAbi() if err != nil { + ctx.Logger().Error("bank contract failed to get ZRC20 ABI", "error", err) return nil } diff --git a/precompiles/staking/method_distribute.go b/precompiles/staking/method_distribute.go index 84e966d936..3fd282a8cf 100644 --- a/precompiles/staking/method_distribute.go +++ b/precompiles/staking/method_distribute.go @@ -1,7 +1,6 @@ package staking import ( - "fmt" "math/big" sdk "github.com/cosmos/cosmos-sdk/types" @@ -10,14 +9,11 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/vm" + "github.com/zeta-chain/node/precompiles/bank" ptypes "github.com/zeta-chain/node/precompiles/types" fungibletypes "github.com/zeta-chain/node/x/fungible/types" ) -var ( - zrc20lockerAddress = common.HexToAddress("0x0000000000000000000000000000000000000067") -) - // function distribute(address zrc20, uint256 amount) external returns (bool success) func (c *Contract) distribute( ctx sdk.Context, @@ -27,10 +23,10 @@ func (c *Contract) distribute( args []interface{}, ) ([]byte, error) { if len(args) != 2 { - return nil, &(ptypes.ErrInvalidNumberOfArgs{ + return nil, &ptypes.ErrInvalidNumberOfArgs{ Got: len(args), Expect: 2, - }) + } } // Unpack arguments and check if they are valid. @@ -56,7 +52,7 @@ func (c *Contract) distribute( // - spender is the staking contract address (c.Address()). // - owner is the caller address. // - locker is the bank address. Assets are locked under this address to prevent liquidity fragmentation. - if err := c.fungibleKeeper.LockZRC20(ctx, c.zrc20ABI, zrc20Addr, c.Address(), caller, zrc20lockerAddress, amount); err != nil { + if err := c.fungibleKeeper.LockZRC20(ctx, c.zrc20ABI, zrc20Addr, c.Address(), caller, bank.ContractAddress, amount); err != nil { return nil, &ptypes.ErrUnexpected{ When: "LockZRC20InBank", Got: err.Error(), @@ -81,11 +77,11 @@ func (c *Contract) distribute( // Log similar message as in abci DistributeValidatorRewards function. ctx.Logger().Info( - fmt.Sprintf("Distributing ZRC20 Validator Rewards Total:%s To FeeCollector : %s, Denom: %s", - amount.String(), - authtypes.FeeCollectorName, - ptypes.ZRC20ToCosmosDenom(zrc20Addr), - )) + "Distributing ZRC20 Validator Rewards", + "Total", amount.String(), + "Fee_collector", authtypes.FeeCollectorName, + "Denom", ptypes.ZRC20ToCosmosDenom(zrc20Addr), + ) if err := c.addDistributeLog(ctx, evm.StateDB, caller, zrc20Addr, amount); err != nil { return nil, &ptypes.ErrUnexpected{ diff --git a/precompiles/staking/staking.go b/precompiles/staking/staking.go index dbb1b6d482..16fc2f1f98 100644 --- a/precompiles/staking/staking.go +++ b/precompiles/staking/staking.go @@ -88,6 +88,7 @@ func NewIStakingContract( // This avoids instantiating it every time deposit or withdraw are called. zrc20ABI, err := zrc20.ZRC20MetaData.GetAbi() if err != nil { + ctx.Logger().Error("staking contract failed to get ZRC20 ABI", "error", err) return nil } diff --git a/precompiles/types/coin.go b/precompiles/types/coin.go index 0c95eb108b..7c0bc25317 100644 --- a/precompiles/types/coin.go +++ b/precompiles/types/coin.go @@ -6,14 +6,14 @@ import ( "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum/common" -) -const ZRC20DenomPrefix = "zrc20/" + "github.com/zeta-chain/node/cmd/zetacored/config" +) // ZRC20ToCosmosDenom returns the cosmos coin address for a given ZRC20 address. // This is converted to "zrc20/{ZRC20Address}". func ZRC20ToCosmosDenom(ZRC20Address common.Address) string { - return ZRC20DenomPrefix + ZRC20Address.String() + return config.ZRC20DenomPrefix + ZRC20Address.String() } func CreateCoinSet(tokenDenom string, amount *big.Int) (sdk.Coins, error) { diff --git a/x/fungible/keeper/zrc20_cosmos_coins_mapping.go b/x/fungible/keeper/zrc20_cosmos_coins_mapping.go index 140dc52aab..7ac3d7ef22 100644 --- a/x/fungible/keeper/zrc20_cosmos_coins_mapping.go +++ b/x/fungible/keeper/zrc20_cosmos_coins_mapping.go @@ -24,6 +24,7 @@ func (k Keeper) LockZRC20( amount *big.Int, ) error { // owner is the EOA owner of the ZRC20 tokens. + // spender is the EOA allowed to spend ZRC20 on owner's behalf. // locker is the address that will lock the ZRC20 tokens, i.e: bank precompile. if err := k.CheckZRC20Allowance(ctx, zrc20ABI, owner, spender, zrc20Address, amount); err != nil { return errors.Wrap(err, "failed allowance check")