apply reviews

zeta-chain · Oct 28, 2024 · 2f35ee7 · 2f35ee7
1 parent 95c9237
commit 2f35ee7
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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,20 +50,20 @@ 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")
 
 	// 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,15 +86,15 @@ 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)
 	utils.RequireTxSuccessful(r, receipt, "distribute should succeed when distributing within balance and allowance")
 
 	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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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")