From 146f4435d835925069ade8af7b69ecf206086d84 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Thu, 31 Oct 2024 09:43:20 -0700 Subject: [PATCH 1/2] feat(e2e): add latency report to withdrawal performance tests (#3071) * feat(e2e): add latency report to withdrawal performance tests * update WaitCCTXMinedByIndex interval * address coderabbit --- e2e/e2etests/test_stress_eth_withdraw.go | 59 ++++++++++++++++-------- e2e/utils/zetacore.go | 10 ++-- go.mod | 1 + go.sum | 2 + 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/e2e/e2etests/test_stress_eth_withdraw.go b/e2e/e2etests/test_stress_eth_withdraw.go index 337a4d416d..864890d6cf 100644 --- a/e2e/e2etests/test_stress_eth_withdraw.go +++ b/e2e/e2etests/test_stress_eth_withdraw.go @@ -4,9 +4,10 @@ import ( "fmt" "math/big" "strconv" + "sync" "time" - ethtypes "github.com/ethereum/go-ethereum/core/types" + "github.com/montanaflynn/stats" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" @@ -36,6 +37,10 @@ func TestStressEtherWithdraw(r *runner.E2ERunner, args []string) { // create a wait group to wait for all the withdraws to complete var eg errgroup.Group + // store durations as float64 seconds like prometheus + withdrawDurations := []float64{} + withdrawDurationsLock := sync.Mutex{} + // send the withdraws for i := 0; i < numWithdraws; i++ { i := i @@ -49,29 +54,45 @@ func TestStressEtherWithdraw(r *runner.E2ERunner, args []string) { r.Logger.Print("index %d: starting withdraw, tx hash: %s", i, tx.Hash().Hex()) eg.Go(func() error { - return monitorEtherWithdraw(r, tx, i, time.Now()) + startTime := time.Now() + cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, tx.Hash().Hex(), r.CctxClient, r.Logger, r.ReceiptTimeout) + if cctx.CctxStatus.Status != crosschaintypes.CctxStatus_OutboundMined { + return fmt.Errorf( + "index %d: withdraw cctx failed with status %s, message %s, cctx index %s", + i, + cctx.CctxStatus.Status, + cctx.CctxStatus.StatusMessage, + cctx.Index, + ) + } + timeToComplete := time.Since(startTime) + r.Logger.Print("index %d: withdraw cctx success in %s", i, timeToComplete.String()) + + withdrawDurationsLock.Lock() + withdrawDurations = append(withdrawDurations, timeToComplete.Seconds()) + withdrawDurationsLock.Unlock() + + return nil }) } - require.NoError(r, eg.Wait()) + err = eg.Wait() - r.Logger.Print("all withdraws completed") -} + desc, descErr := stats.Describe(withdrawDurations, false, &[]float64{50.0, 75.0, 90.0, 95.0}) + if descErr != nil { + r.Logger.Print("❌ failed to calculate latency report: %v", descErr) + } -// monitorEtherWithdraw monitors the withdraw of ether, returns once the withdraw is complete -func monitorEtherWithdraw(r *runner.E2ERunner, tx *ethtypes.Transaction, index int, startTime time.Time) error { - cctx := utils.WaitCctxMinedByInboundHash(r.Ctx, tx.Hash().Hex(), r.CctxClient, r.Logger, r.ReceiptTimeout) - if cctx.CctxStatus.Status != crosschaintypes.CctxStatus_OutboundMined { - return fmt.Errorf( - "index %d: withdraw cctx failed with status %s, message %s, cctx index %s", - index, - cctx.CctxStatus.Status, - cctx.CctxStatus.StatusMessage, - cctx.Index, - ) + r.Logger.Print("Latency report:") + r.Logger.Print("min: %.2f", desc.Min) + r.Logger.Print("max: %.2f", desc.Max) + r.Logger.Print("mean: %.2f", desc.Mean) + r.Logger.Print("std: %.2f", desc.Std) + for _, p := range desc.DescriptionPercentiles { + r.Logger.Print("p%.0f: %.2f", p.Percentile, p.Value) } - timeToComplete := time.Since(startTime) - r.Logger.Print("index %d: withdraw cctx success in %s", index, timeToComplete.String()) - return nil + require.NoError(r, err) + + r.Logger.Print("all withdraws completed") } diff --git a/e2e/utils/zetacore.go b/e2e/utils/zetacore.go index 33f5d68262..0b4d5217ed 100644 --- a/e2e/utils/zetacore.go +++ b/e2e/utils/zetacore.go @@ -81,7 +81,7 @@ func WaitCctxsMinedByInboundHash( timedOut := time.Since(startTime) > timeout require.False(t, timedOut, "waiting cctx timeout, cctx not mined, inbound hash: %s", inboundHash) - time.Sleep(1 * time.Second) + time.Sleep(500 * time.Millisecond) // We use InTxHashToCctxData instead of InboundTrackerAllByChain to able to run these tests with the previous version // for the update tests @@ -90,7 +90,7 @@ func WaitCctxsMinedByInboundHash( res, err := client.InTxHashToCctxData(ctx, in) if err != nil { // prevent spamming logs - if i%10 == 0 { + if i%20 == 0 { logger.Info("Error getting cctx by inboundHash: %s", err.Error()) } continue @@ -113,7 +113,7 @@ func WaitCctxsMinedByInboundHash( cctx := cctx if !IsTerminalStatus(cctx.CctxStatus.Status) { // prevent spamming logs - if i%10 == 0 { + if i%20 == 0 { logger.Info( "waiting for cctx index %d to be mined by inboundHash: %s, cctx status: %s, message: %s", j, @@ -154,7 +154,7 @@ func WaitCCTXMinedByIndex( require.False(t, time.Since(startTime) > timeout, "waiting cctx timeout, cctx not mined, cctx: %s", cctxIndex) if i > 0 { - time.Sleep(1 * time.Second) + time.Sleep(500 * time.Millisecond) } // fetch cctx by index @@ -170,7 +170,7 @@ func WaitCCTXMinedByIndex( cctx := res.CrossChainTx if !IsTerminalStatus(cctx.CctxStatus.Status) { // prevent spamming logs - if i%10 == 0 { + if i%20 == 0 { logger.Info( "waiting for cctx to be mined from index: %s, cctx status: %s, message: %s", cctxIndex, diff --git a/go.mod b/go.mod index 8e769da887..bb5b93539c 100644 --- a/go.mod +++ b/go.mod @@ -340,6 +340,7 @@ require ( require ( github.com/decred/dcrd/crypto/blake256 v1.0.1 // indirect + github.com/montanaflynn/stats v0.7.1 // indirect github.com/oasisprotocol/curve25519-voi v0.0.0-20220328075252-7dd334e3daae // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect diff --git a/go.sum b/go.sum index d2da2120e8..06040e5b57 100644 --- a/go.sum +++ b/go.sum @@ -3321,6 +3321,8 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY github.com/modocache/gover v0.0.0-20171022184752-b58185e213c5/go.mod h1:caMODM3PzxT8aQXRPkAt8xlV/e7d7w8GM5g0fa5F0D8= github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826/go.mod h1:TaXosZuwdSHYgviHp1DAtfrULt5eUgsSMsZf+YrPgl8= github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc= +github.com/montanaflynn/stats v0.7.1 h1:etflOAAHORrCC44V+aR6Ftzort912ZU+YLiSTuV8eaE= +github.com/montanaflynn/stats v0.7.1/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow= github.com/moricho/tparallel v0.2.1/go.mod h1:fXEIZxG2vdfl0ZF8b42f5a78EhjjD5mX8qUplsoSU4k= github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= github.com/mostynb/zstdpool-freelist v0.0.0-20201229113212-927304c0c3b1 h1:mPMvm6X6tf4w8y7j9YIt6V9jfWhL6QlbEc7CCmeQlWk= From b58046fcd4cf68fc769ec35f0b0caa9185c5b1fa Mon Sep 17 00:00:00 2001 From: Lucas Bertrand Date: Thu, 31 Oct 2024 22:40:30 +0100 Subject: [PATCH 2/2] refactor: improve ZETA deposit check with max supply check (#3074) * refactor: max supply check * create internal func --- testutil/keeper/mocks/fungible/bank.go | 18 +++++ x/fungible/keeper/deposits_test.go | 4 + x/fungible/keeper/zeta.go | 27 +++++++ x/fungible/keeper/zeta_test.go | 78 +++++++++++++++++++ .../keeper/zevm_message_passing_test.go | 6 ++ x/fungible/types/errors.go | 1 + x/fungible/types/expected_keepers.go | 1 + 7 files changed, 135 insertions(+) diff --git a/testutil/keeper/mocks/fungible/bank.go b/testutil/keeper/mocks/fungible/bank.go index db14226310..1c46b35688 100644 --- a/testutil/keeper/mocks/fungible/bank.go +++ b/testutil/keeper/mocks/fungible/bank.go @@ -13,6 +13,24 @@ type FungibleBankKeeper struct { mock.Mock } +// GetSupply provides a mock function with given fields: ctx, denom +func (_m *FungibleBankKeeper) GetSupply(ctx types.Context, denom string) types.Coin { + ret := _m.Called(ctx, denom) + + if len(ret) == 0 { + panic("no return value specified for GetSupply") + } + + var r0 types.Coin + if rf, ok := ret.Get(0).(func(types.Context, string) types.Coin); ok { + r0 = rf(ctx, denom) + } else { + r0 = ret.Get(0).(types.Coin) + } + + return r0 +} + // MintCoins provides a mock function with given fields: ctx, moduleName, amt func (_m *FungibleBankKeeper) MintCoins(ctx types.Context, moduleName string, amt types.Coins) error { ret := _m.Called(ctx, moduleName, amt) diff --git a/x/fungible/keeper/deposits_test.go b/x/fungible/keeper/deposits_test.go index 3958ae191e..836ce0b61a 100644 --- a/x/fungible/keeper/deposits_test.go +++ b/x/fungible/keeper/deposits_test.go @@ -437,6 +437,10 @@ func TestKeeper_DepositCoinZeta(t *testing.T) { b := sdkk.BankKeeper.GetBalance(ctx, zetaToAddress, config.BaseDenom) require.Equal(t, int64(0), b.Amount.Int64()) errorMint := errors.New("", 1, "error minting coins") + + bankMock.On("GetSupply", ctx, mock.Anything, mock.Anything). + Return(sdk.NewCoin(config.BaseDenom, sdk.NewInt(0))). + Once() bankMock.On("MintCoins", ctx, types.ModuleName, mock.Anything).Return(errorMint).Once() err := k.DepositCoinZeta(ctx, to, amount) require.ErrorIs(t, err, errorMint) diff --git a/x/fungible/keeper/zeta.go b/x/fungible/keeper/zeta.go index bc15a06e44..cd4acfcaa7 100644 --- a/x/fungible/keeper/zeta.go +++ b/x/fungible/keeper/zeta.go @@ -1,6 +1,7 @@ package keeper import ( + "fmt" "math/big" sdk "github.com/cosmos/cosmos-sdk/types" @@ -9,9 +10,17 @@ import ( "github.com/zeta-chain/node/x/fungible/types" ) +// ZETAMaxSupplyStr is the maximum mintable ZETA in the fungible module +// 1.85 billion ZETA +const ZETAMaxSupplyStr = "1850000000000000000000000000" + // MintZetaToEVMAccount mints ZETA (gas token) to the given address // NOTE: this method should be used with a temporary context, and it should not be committed if the method returns an error func (k *Keeper) MintZetaToEVMAccount(ctx sdk.Context, to sdk.AccAddress, amount *big.Int) error { + if err := k.validateZetaSupply(ctx, amount); err != nil { + return err + } + coins := sdk.NewCoins(sdk.NewCoin(config.BaseDenom, sdk.NewIntFromBigInt(amount))) // Mint coins if err := k.bankKeeper.MintCoins(ctx, types.ModuleName, coins); err != nil { @@ -23,7 +32,25 @@ func (k *Keeper) MintZetaToEVMAccount(ctx sdk.Context, to sdk.AccAddress, amount } func (k *Keeper) MintZetaToFungibleModule(ctx sdk.Context, amount *big.Int) error { + if err := k.validateZetaSupply(ctx, amount); err != nil { + return err + } + coins := sdk.NewCoins(sdk.NewCoin(config.BaseDenom, sdk.NewIntFromBigInt(amount))) // Mint coins return k.bankKeeper.MintCoins(ctx, types.ModuleName, coins) } + +// validateZetaSupply checks if the minted ZETA amount exceeds the maximum supply +func (k *Keeper) validateZetaSupply(ctx sdk.Context, amount *big.Int) error { + zetaMaxSupply, ok := sdk.NewIntFromString(ZETAMaxSupplyStr) + if !ok { + return fmt.Errorf("failed to parse ZETA max supply: %s", ZETAMaxSupplyStr) + } + + supply := k.bankKeeper.GetSupply(ctx, config.BaseDenom) + if supply.Amount.Add(sdk.NewIntFromBigInt(amount)).GT(zetaMaxSupply) { + return types.ErrMaxSupplyReached + } + return nil +} diff --git a/x/fungible/keeper/zeta_test.go b/x/fungible/keeper/zeta_test.go index 62e41700c1..51e5fe279c 100644 --- a/x/fungible/keeper/zeta_test.go +++ b/x/fungible/keeper/zeta_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "errors" + "github.com/stretchr/testify/mock" "math/big" "testing" @@ -11,6 +12,7 @@ import ( "github.com/zeta-chain/node/cmd/zetacored/config" testkeeper "github.com/zeta-chain/node/testutil/keeper" "github.com/zeta-chain/node/testutil/sample" + "github.com/zeta-chain/node/x/fungible/keeper" "github.com/zeta-chain/node/x/fungible/types" ) @@ -29,6 +31,46 @@ func TestKeeper_MintZetaToEVMAccount(t *testing.T) { require.True(t, bal.Amount.Equal(sdk.NewInt(42))) }) + t.Run("mint the token to reach max supply", func(t *testing.T) { + k, ctx, sdkk, _ := testkeeper.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + acc := sample.Bech32AccAddress() + bal := sdkk.BankKeeper.GetBalance(ctx, acc, config.BaseDenom) + require.True(t, bal.IsZero()) + + zetaMaxSupply, ok := sdk.NewIntFromString(keeper.ZETAMaxSupplyStr) + require.True(t, ok) + + supply := sdkk.BankKeeper.GetSupply(ctx, config.BaseDenom).Amount + + newAmount := zetaMaxSupply.Sub(supply) + + err := k.MintZetaToEVMAccount(ctx, acc, newAmount.BigInt()) + require.NoError(t, err) + bal = sdkk.BankKeeper.GetBalance(ctx, acc, config.BaseDenom) + require.True(t, bal.Amount.Equal(newAmount)) + }) + + t.Run("can't mint more than max supply", func(t *testing.T) { + k, ctx, sdkk, _ := testkeeper.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + acc := sample.Bech32AccAddress() + bal := sdkk.BankKeeper.GetBalance(ctx, acc, config.BaseDenom) + require.True(t, bal.IsZero()) + + zetaMaxSupply, ok := sdk.NewIntFromString(keeper.ZETAMaxSupplyStr) + require.True(t, ok) + + supply := sdkk.BankKeeper.GetSupply(ctx, config.BaseDenom).Amount + + newAmount := zetaMaxSupply.Sub(supply).Add(sdk.NewInt(1)) + + err := k.MintZetaToEVMAccount(ctx, acc, newAmount.BigInt()) + require.ErrorIs(t, err, types.ErrMaxSupplyReached) + }) + coins42 := sdk.NewCoins(sdk.NewCoin(config.BaseDenom, sdk.NewInt(42))) t.Run("should fail if minting fail", func(t *testing.T) { @@ -36,6 +78,9 @@ func TestKeeper_MintZetaToEVMAccount(t *testing.T) { mockBankKeeper := testkeeper.GetFungibleBankMock(t, k) + mockBankKeeper.On("GetSupply", ctx, mock.Anything, mock.Anything). + Return(sdk.NewCoin(config.BaseDenom, sdk.NewInt(0))). + Once() mockBankKeeper.On( "MintCoins", ctx, @@ -55,6 +100,9 @@ func TestKeeper_MintZetaToEVMAccount(t *testing.T) { mockBankKeeper := testkeeper.GetFungibleBankMock(t, k) + mockBankKeeper.On("GetSupply", ctx, mock.Anything, mock.Anything). + Return(sdk.NewCoin(config.BaseDenom, sdk.NewInt(0))). + Once() mockBankKeeper.On( "MintCoins", ctx, @@ -76,3 +124,33 @@ func TestKeeper_MintZetaToEVMAccount(t *testing.T) { mockBankKeeper.AssertExpectations(t) }) } + +func TestKeeper_MintZetaToFungibleModule(t *testing.T) { + t.Run("should mint the token in the specified balance", func(t *testing.T) { + k, ctx, sdkk, _ := testkeeper.FungibleKeeper(t) + acc := k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName).GetAddress() + + bal := sdkk.BankKeeper.GetBalance(ctx, acc, config.BaseDenom) + require.True(t, bal.IsZero()) + + err := k.MintZetaToEVMAccount(ctx, acc, big.NewInt(42)) + require.NoError(t, err) + bal = sdkk.BankKeeper.GetBalance(ctx, acc, config.BaseDenom) + require.True(t, bal.Amount.Equal(sdk.NewInt(42))) + }) + + t.Run("can't mint more than max supply", func(t *testing.T) { + k, ctx, sdkk, _ := testkeeper.FungibleKeeper(t) + k.GetAuthKeeper().GetModuleAccount(ctx, types.ModuleName) + + zetaMaxSupply, ok := sdk.NewIntFromString(keeper.ZETAMaxSupplyStr) + require.True(t, ok) + + supply := sdkk.BankKeeper.GetSupply(ctx, config.BaseDenom).Amount + + newAmount := zetaMaxSupply.Sub(supply).Add(sdk.NewInt(1)) + + err := k.MintZetaToFungibleModule(ctx, newAmount.BigInt()) + require.ErrorIs(t, err, types.ErrMaxSupplyReached) + }) +} diff --git a/x/fungible/keeper/zevm_message_passing_test.go b/x/fungible/keeper/zevm_message_passing_test.go index f68fec1f62..f8e1e300d2 100644 --- a/x/fungible/keeper/zevm_message_passing_test.go +++ b/x/fungible/keeper/zevm_message_passing_test.go @@ -146,6 +146,9 @@ func TestKeeper_ZEVMDepositAndCallContract(t *testing.T) { }) require.NoError(t, err) errorMint := errors.New("", 10, "error minting coins") + bankMock.On("GetSupply", ctx, mock.Anything, mock.Anything). + Return(sdk.NewCoin(config.BaseDenom, sdk.NewInt(0))). + Once() bankMock.On("MintCoins", ctx, types.ModuleName, mock.Anything).Return(errorMint).Once() _, err = k.ZETADepositAndCallContract( @@ -296,6 +299,9 @@ func TestKeeper_ZEVMRevertAndCallContract(t *testing.T) { }) require.NoError(t, err) errorMint := errors.New("", 101, "error minting coins") + bankMock.On("GetSupply", ctx, mock.Anything, mock.Anything). + Return(sdk.NewCoin(config.BaseDenom, sdk.NewInt(0))). + Once() bankMock.On("MintCoins", ctx, types.ModuleName, mock.Anything).Return(errorMint).Once() _, err = k.ZETARevertAndCallContract( diff --git a/x/fungible/types/errors.go b/x/fungible/types/errors.go index cf333c9545..cb152f7ffe 100644 --- a/x/fungible/types/errors.go +++ b/x/fungible/types/errors.go @@ -34,4 +34,5 @@ var ( ErrZRC20NilABI = cosmoserrors.Register(ModuleName, 1132, "ZRC20 ABI is nil") ErrZeroAddress = cosmoserrors.Register(ModuleName, 1133, "address cannot be zero") ErrInvalidAmount = cosmoserrors.Register(ModuleName, 1134, "invalid amount") + ErrMaxSupplyReached = cosmoserrors.Register(ModuleName, 1135, "max supply reached") ) diff --git a/x/fungible/types/expected_keepers.go b/x/fungible/types/expected_keepers.go index c8dc02f013..8af00293ed 100644 --- a/x/fungible/types/expected_keepers.go +++ b/x/fungible/types/expected_keepers.go @@ -33,6 +33,7 @@ type BankKeeper interface { amt sdk.Coins, ) error MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error + GetSupply(ctx sdk.Context, denom string) sdk.Coin } type ObserverKeeper interface {