From 0192456c7e587f10ffc688f88325cf8d73411539 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Fri, 2 Aug 2024 18:22:09 +0200 Subject: [PATCH 01/11] Add Gas struct --- zetaclient/chains/evm/signer/gas.go | 109 ++++++++++++++++++ zetaclient/chains/evm/signer/gas_test.go | 134 +++++++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 zetaclient/chains/evm/signer/gas.go create mode 100644 zetaclient/chains/evm/signer/gas_test.go diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go new file mode 100644 index 0000000000..8cc56516ef --- /dev/null +++ b/zetaclient/chains/evm/signer/gas.go @@ -0,0 +1,109 @@ +package signer + +import ( + "fmt" + "math/big" + + "github.com/pkg/errors" + "github.com/rs/zerolog" + + "github.com/zeta-chain/zetacore/x/crosschain/types" +) + +// Gas represents gas parameters for EVM transactions. +// +// This is pretty interesting because all EVM chains now support EIP-1559, but some chains do it in a specific way +// https://eips.ethereum.org/EIPS/eip-1559 +// https://www.blocknative.com/blog/eip-1559-fees +// https://github.com/bnb-chain/BEPs/blob/master/BEPs/BEP226.md (tl;dr: baseFee is always zero) +// +// However, this doesn't affect tx creation nor broadcasting +type Gas struct { + Limit uint64 + + // This is a "total" gasPrice per 1 unit of gas. + Price *big.Int + + // PriorityFee a fee paid directly to validators. + PriorityFee *big.Int +} + +func (g Gas) validate() error { + switch { + case g.Limit == 0: + return errors.New("gas limit is zero") + case g.Price == nil: + return errors.New("max fee per unit is nil") + case g.PriorityFee == nil: + return errors.New("priority fee per unit is nil") + default: + return nil + } +} + +// isLegacy determines whether the gas is meant for LegacyTx{} (pre EIP-1559) +// or DynamicFeeTx{} (post EIP-1559). +// +// Returns true if priority fee is <= 0. +func (g Gas) isLegacy() bool { + return g.PriorityFee.Sign() < 1 +} + +// makeGasFromCCTX creates Gas struct based from CCTX and priorityFee. +func makeGasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { + var ( + params = cctx.GetCurrentOutboundParam() + limit = params.GasLimit + ) + + switch { + case limit < MinGasLimit: + limit = MinGasLimit + logger.Warn(). + Uint64("cctx.initial_gas_limit", params.GasLimit). + Uint64("cctx.gas_limit", limit). + Msgf("Gas limit is too low. Setting to the minimum (%d)", MinGasLimit) + case limit > MaxGasLimit: + limit = MaxGasLimit + logger.Warn(). + Uint64("cctx.initial_gas_limit", params.GasLimit). + Uint64("cctx.gas_limit", limit). + Msgf("Gas limit is too high; Setting to the maximum (%d)", MaxGasLimit) + } + + gasPrice, err := bigIntFromString(params.GasPrice) + if err != nil { + return Gas{}, errors.Wrap(err, "unable to parse gasPrice") + } + + priorityFee, err := bigIntFromString(params.GasPriorityFee) + switch { + case err != nil: + return Gas{}, errors.Wrap(err, "unable to parse priorityFee") + case gasPrice.Cmp(priorityFee) == -1: + return Gas{}, fmt.Errorf("gasPrice (%d) is less than priorityFee (%d)", gasPrice.Int64(), priorityFee.Int64()) + } + + return Gas{ + Limit: limit, + Price: gasPrice, + PriorityFee: priorityFee, + }, nil +} + +func bigIntFromString(s string) (*big.Int, error) { + if s == "" || s == "0" { + return big.NewInt(0), nil + } + + v, ok := new(big.Int).SetString(s, 10) + if !ok { + return nil, fmt.Errorf("unable to parse %q as big.Int", s) + } + + if v.Sign() == -1 { + return nil, fmt.Errorf("big.Int is negative: %d", v.Int64()) + } + + return v, nil +} diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go new file mode 100644 index 0000000000..17c8f4e5b7 --- /dev/null +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -0,0 +1,134 @@ +package signer + +import ( + "math/big" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/zeta-chain/zetacore/x/crosschain/types" +) + +func Test_makeGasFromCCTX(t *testing.T) { + logger := zerolog.New(zerolog.NewTestWriter(t)) + + makeCCTX := func(gasLimit uint64, price, priorityFee string) *types.CrossChainTx { + cctx := getCCTX(t) + cctx.GetOutboundParams()[0].GasLimit = gasLimit + cctx.GetOutboundParams()[0].GasPrice = price + cctx.GetOutboundParams()[0].GasPriorityFee = priorityFee + + return cctx + } + + for _, tt := range []struct { + name string + cctx *types.CrossChainTx + errorContains string + assert func(t *testing.T, g Gas) + }{ + + { + name: "legacy: gas is too low", + cctx: makeCCTX(MinGasLimit-200, gwei(2).String(), ""), + assert: func(t *testing.T, g Gas) { + assert.True(t, g.isLegacy()) + assertGasEquals(t, Gas{ + Limit: MinGasLimit, + PriorityFee: gwei(0), + Price: gwei(2), + }, g) + }, + }, + { + name: "london: gas is too low", + cctx: makeCCTX(MinGasLimit-200, gwei(2).String(), gwei(1).String()), + assert: func(t *testing.T, g Gas) { + assert.False(t, g.isLegacy()) + assertGasEquals(t, Gas{ + Limit: MinGasLimit, + Price: gwei(2), + PriorityFee: gwei(1), + }, g) + }, + }, + { + name: "pre London gas logic", + cctx: makeCCTX(MinGasLimit+100, gwei(3).String(), ""), + assert: func(t *testing.T, g Gas) { + assert.True(t, g.isLegacy()) + assertGasEquals(t, Gas{ + Limit: 100_100, + Price: gwei(3), + PriorityFee: gwei(0), + }, g) + }, + }, + { + name: "post London gas logic", + cctx: makeCCTX(MinGasLimit+200, gwei(4).String(), gwei(1).String()), + assert: func(t *testing.T, g Gas) { + assert.False(t, g.isLegacy()) + assertGasEquals(t, Gas{ + Limit: 100_200, + Price: gwei(4), + PriorityFee: gwei(1), + }, g) + }, + }, + { + name: "gas is too high, force to the ceiling", + cctx: makeCCTX(MaxGasLimit+200, gwei(4).String(), gwei(1).String()), + assert: func(t *testing.T, g Gas) { + assert.False(t, g.isLegacy()) + assertGasEquals(t, Gas{ + Limit: MaxGasLimit, + Price: gwei(4), + PriorityFee: gwei(1), + }, g) + }, + }, + { + name: "priority fee is invalid", + cctx: makeCCTX(123_000, gwei(4).String(), "oopsie"), + errorContains: "unable to parse priorityFee", + }, + { + name: "priority fee is negative", + cctx: makeCCTX(123_000, gwei(4).String(), "-1"), + errorContains: "unable to parse priorityFee: big.Int is negative", + }, + { + name: "gasPrice is less than priorityFee", + cctx: makeCCTX(123_000, gwei(4).String(), gwei(5).String()), + errorContains: "gasPrice (4000000000) is less than priorityFee (5000000000)", + }, + { + name: "gasPrice is invalid", + cctx: makeCCTX(123_000, "hello", gwei(5).String()), + errorContains: "unable to parse gasPrice", + }, + } { + t.Run(tt.name, func(t *testing.T) { + g, err := makeGasFromCCTX(tt.cctx, logger) + if tt.errorContains != "" { + assert.ErrorContains(t, err, tt.errorContains) + return + } + + assert.NoError(t, err) + tt.assert(t, g) + }) + } +} + +func assertGasEquals(t *testing.T, expected, actual Gas) { + assert.Equal(t, int64(expected.Limit), int64(actual.Limit), "gas limit") + assert.Equal(t, expected.Price.Int64(), actual.Price.Int64(), "max fee per unit") + assert.Equal(t, expected.PriorityFee.Int64(), actual.PriorityFee.Int64(), "priority fee per unit") +} + +func gwei(i int64) *big.Int { + const g = 1_000_000_000 + return big.NewInt(i * g) +} From 0ad41450ec028413cf659c1d08b6205490c5a5d6 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 5 Aug 2024 17:26:18 +0200 Subject: [PATCH 02/11] Add EIP-1559 fees --- zetaclient/chains/evm/signer/gas.go | 17 +- zetaclient/chains/evm/signer/gas_test.go | 16 +- zetaclient/chains/evm/signer/outbound_data.go | 258 +++++++++--------- .../chains/evm/signer/outbound_data_test.go | 184 +++++++------ zetaclient/chains/evm/signer/signer.go | 171 +++++++----- zetaclient/chains/evm/signer/signer_test.go | 24 +- 6 files changed, 359 insertions(+), 311 deletions(-) diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go index 8cc56516ef..593f773ec5 100644 --- a/zetaclient/chains/evm/signer/gas.go +++ b/zetaclient/chains/evm/signer/gas.go @@ -10,6 +10,11 @@ import ( "github.com/zeta-chain/zetacore/x/crosschain/types" ) +const ( + minGasLimit = 100_000 + maxGasLimit = 1_000_000 +) + // Gas represents gas parameters for EVM transactions. // // This is pretty interesting because all EVM chains now support EIP-1559, but some chains do it in a specific way @@ -57,18 +62,18 @@ func makeGasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, erro ) switch { - case limit < MinGasLimit: - limit = MinGasLimit + case limit < minGasLimit: + limit = minGasLimit logger.Warn(). Uint64("cctx.initial_gas_limit", params.GasLimit). Uint64("cctx.gas_limit", limit). - Msgf("Gas limit is too low. Setting to the minimum (%d)", MinGasLimit) - case limit > MaxGasLimit: - limit = MaxGasLimit + Msgf("Gas limit is too low. Setting to the minimum (%d)", minGasLimit) + case limit > maxGasLimit: + limit = maxGasLimit logger.Warn(). Uint64("cctx.initial_gas_limit", params.GasLimit). Uint64("cctx.gas_limit", limit). - Msgf("Gas limit is too high; Setting to the maximum (%d)", MaxGasLimit) + Msgf("Gas limit is too high; Setting to the maximum (%d)", maxGasLimit) } gasPrice, err := bigIntFromString(params.GasPrice) diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go index 17c8f4e5b7..1b935081ae 100644 --- a/zetaclient/chains/evm/signer/gas_test.go +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -30,11 +30,11 @@ func Test_makeGasFromCCTX(t *testing.T) { { name: "legacy: gas is too low", - cctx: makeCCTX(MinGasLimit-200, gwei(2).String(), ""), + cctx: makeCCTX(minGasLimit-200, gwei(2).String(), ""), assert: func(t *testing.T, g Gas) { assert.True(t, g.isLegacy()) assertGasEquals(t, Gas{ - Limit: MinGasLimit, + Limit: minGasLimit, PriorityFee: gwei(0), Price: gwei(2), }, g) @@ -42,11 +42,11 @@ func Test_makeGasFromCCTX(t *testing.T) { }, { name: "london: gas is too low", - cctx: makeCCTX(MinGasLimit-200, gwei(2).String(), gwei(1).String()), + cctx: makeCCTX(minGasLimit-200, gwei(2).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ - Limit: MinGasLimit, + Limit: minGasLimit, Price: gwei(2), PriorityFee: gwei(1), }, g) @@ -54,7 +54,7 @@ func Test_makeGasFromCCTX(t *testing.T) { }, { name: "pre London gas logic", - cctx: makeCCTX(MinGasLimit+100, gwei(3).String(), ""), + cctx: makeCCTX(minGasLimit+100, gwei(3).String(), ""), assert: func(t *testing.T, g Gas) { assert.True(t, g.isLegacy()) assertGasEquals(t, Gas{ @@ -66,7 +66,7 @@ func Test_makeGasFromCCTX(t *testing.T) { }, { name: "post London gas logic", - cctx: makeCCTX(MinGasLimit+200, gwei(4).String(), gwei(1).String()), + cctx: makeCCTX(minGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ @@ -78,11 +78,11 @@ func Test_makeGasFromCCTX(t *testing.T) { }, { name: "gas is too high, force to the ceiling", - cctx: makeCCTX(MaxGasLimit+200, gwei(4).String(), gwei(1).String()), + cctx: makeCCTX(maxGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ - Limit: MaxGasLimit, + Limit: maxGasLimit, Price: gwei(4), PriorityFee: gwei(1), }, g) diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index ebeef1c5b1..012c146b29 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -4,40 +4,35 @@ import ( "context" "encoding/base64" "encoding/hex" - "fmt" "math/big" ethcommon "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" "github.com/rs/zerolog" - "github.com/zeta-chain/zetacore/pkg/chains" "github.com/zeta-chain/zetacore/pkg/coin" "github.com/zeta-chain/zetacore/x/crosschain/types" "github.com/zeta-chain/zetacore/zetaclient/chains/evm/observer" - "github.com/zeta-chain/zetacore/zetaclient/chains/interfaces" zctx "github.com/zeta-chain/zetacore/zetaclient/context" ) -const ( - MinGasLimit = 100_000 - MaxGasLimit = 1_000_000 -) - // OutboundData is a data structure containing input fields used to construct each type of transaction. // This is populated using cctx and other input parameters passed to TryProcessOutbound type OutboundData struct { srcChainID *big.Int - toChainID *big.Int sender ethcommon.Address - to ethcommon.Address - asset ethcommon.Address - amount *big.Int - gasPrice *big.Int - gasLimit uint64 - message []byte - nonce uint64 - height uint64 + + toChainID *big.Int + to ethcommon.Address + + asset ethcommon.Address + amount *big.Int + + gas Gas + nonce uint64 + height uint64 + + message []byte // cctxIndex field is the inbound message digest that is sent to the destination contract cctxIndex [32]byte @@ -46,147 +41,146 @@ type OutboundData struct { outboundParams *types.OutboundParams } -// SetChainAndSender populates the destination address and Chain ID based on the status of the cross chain tx -// returns true if transaction should be skipped -// returns false otherwise -func (txData *OutboundData) SetChainAndSender(cctx *types.CrossChainTx, logger zerolog.Logger) bool { - switch cctx.CctxStatus.Status { - case types.CctxStatus_PendingRevert: - txData.to = ethcommon.HexToAddress(cctx.InboundParams.Sender) - txData.toChainID = big.NewInt(cctx.InboundParams.SenderChainId) - logger.Info().Msgf("Abort: reverting inbound") - case types.CctxStatus_PendingOutbound: - txData.to = ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver) - txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) - default: - logger.Info().Msgf("Transaction doesn't need to be processed status: %d", cctx.CctxStatus.Status) - return true - } - return false -} - -// SetupGas sets the gas limit and price -func (txData *OutboundData) SetupGas( - cctx *types.CrossChainTx, - logger zerolog.Logger, - client interfaces.EVMRPCClient, - chain chains.Chain, -) error { - txData.gasLimit = cctx.GetCurrentOutboundParam().GasLimit - if txData.gasLimit < MinGasLimit { - txData.gasLimit = MinGasLimit - logger.Warn(). - Msgf("gasLimit %d is too low; set to %d", cctx.GetCurrentOutboundParam().GasLimit, txData.gasLimit) - } - if txData.gasLimit > MaxGasLimit { - txData.gasLimit = MaxGasLimit - logger.Warn(). - Msgf("gasLimit %d is too high; set to %d", cctx.GetCurrentOutboundParam().GasLimit, txData.gasLimit) - } - - // use dynamic gas price for ethereum chains. - // The code below is a fix for https://github.com/zeta-chain/node/issues/1085 - // doesn't close directly the issue because we should determine if we want to keep using SuggestGasPrice if no GasPrice - // we should possibly remove it completely and return an error if no GasPrice is provided because it means no fee is processed on ZetaChain - specified, ok := new(big.Int).SetString(cctx.GetCurrentOutboundParam().GasPrice, 10) - if !ok { - if chain.Network == chains.Network_eth { - suggested, err := client.SuggestGasPrice(context.Background()) - if err != nil { - return errors.Wrapf(err, "cannot get gas price from chain %s ", chain.String()) - } - txData.gasPrice = roundUpToNearestGwei(suggested) - } else { - return fmt.Errorf("cannot convert gas price %s ", cctx.GetCurrentOutboundParam().GasPrice) - } - } else { - txData.gasPrice = specified - } - return nil -} - -// NewOutboundData populates transaction input fields parsed from the cctx and other parameters -// returns -// 1. New NewOutboundData Data struct or nil if an error occurred. -// 2. bool (skipTx) - if the transaction doesn't qualify to be processed the function will return true, meaning that this -// cctx will be skipped and false otherwise. -// 3. error +// NewOutboundData creates OutboundData from the given CCTX. +// returns `bool true` when transaction should be skipped. func NewOutboundData( ctx context.Context, cctx *types.CrossChainTx, - evmObserver *observer.Observer, - evmRPC interfaces.EVMRPCClient, - logger zerolog.Logger, + observer *observer.Observer, height uint64, + logger zerolog.Logger, ) (*OutboundData, bool, error) { - txData := OutboundData{} - txData.outboundParams = cctx.GetCurrentOutboundParam() - txData.amount = cctx.GetCurrentOutboundParam().Amount.BigInt() - txData.nonce = cctx.GetCurrentOutboundParam().TssNonce - txData.sender = ethcommon.HexToAddress(cctx.InboundParams.Sender) - txData.srcChainID = big.NewInt(cctx.InboundParams.SenderChainId) - txData.asset = ethcommon.HexToAddress(cctx.InboundParams.Asset) - txData.height = height - - skipTx := txData.SetChainAndSender(cctx, logger) - if skipTx { - return nil, true, nil + var ( + outboundParams = cctx.GetCurrentOutboundParam() + nonce = outboundParams.TssNonce + ) + + if err := validateParams(outboundParams); err != nil { + return nil, false, errors.Wrap(err, "invalid outboundParams") } app, err := zctx.FromContext(ctx) if err != nil { - return nil, false, err + return nil, false, errors.Wrap(err, "unable to get app from context") } - chainID := txData.toChainID.Int64() + // recipient + destination chain + to, toChainID, skip := determineDestination(cctx, logger) + if skip { + return nil, true, nil + } - toChain, err := app.GetChain(chainID) - switch { - case err != nil: - return nil, true, errors.Wrapf(err, "unable to get chain %d from app context", chainID) - case toChain.IsZeta(): - // should not happen - return nil, true, errors.New("destination chain is Zeta") + // ensure that chain exists in app's context + if _, err := app.GetChain(toChainID.Int64()); err != nil { + return nil, false, errors.Wrapf(err, "unable to get chain %d from app context", toChainID.Int64()) } - rawChain := toChain.RawChain() + gas, err := makeGasFromCCTX(cctx, logger) + if err != nil { + return nil, false, errors.Wrap(err, "unable to make gas from CCTX") + } - // Set up gas limit and gas price - err = txData.SetupGas(cctx, logger, evmRPC, *rawChain) + cctxIndex, err := getCCTXIndex(cctx) if err != nil { - return nil, true, errors.Wrap(err, "unable to setup gas") + return nil, false, errors.Wrap(err, "unable to get cctx index") } - nonce := cctx.GetCurrentOutboundParam().TssNonce + // In case there is a pending tx, make sure this keySign is a tx replacement + if tx := observer.GetPendingTx(nonce); tx != nil { + evt := logger.Info(). + Str("cctx.pending_tx_hash", tx.Hash().Hex()). + Uint64("cctx.pending_tx_nonce", nonce) - // Get sendHash - logger.Info(). - Msgf("chain %d minting %d to %s, nonce %d, finalized zeta bn %d", toChain.ID(), cctx.InboundParams.Amount, txData.to.Hex(), nonce, cctx.InboundParams.FinalizedZetaHeight) - cctxIndex, err := hex.DecodeString(cctx.Index[2:]) // remove the leading 0x - if err != nil || len(cctxIndex) != 32 { - return nil, true, fmt.Errorf("decode CCTX %s error", cctx.Index) - } - copy(txData.cctxIndex[:32], cctxIndex[:32]) - - // In case there is a pending transaction, make sure this keysign is a transaction replacement - pendingTx := evmObserver.GetPendingTx(nonce) - if pendingTx != nil { - if txData.gasPrice.Cmp(pendingTx.GasPrice()) > 0 { - logger.Info(). - Msgf("replace pending outbound %s nonce %d using gas price %d", pendingTx.Hash().Hex(), nonce, txData.gasPrice) - } else { - logger.Info().Msgf("please wait for pending outbound %s nonce %d to be included", pendingTx.Hash().Hex(), nonce) + // new gas price is less or equal to pending tx gas + if gas.Price.Cmp(tx.GasPrice()) <= 0 { + evt.Msg("Please wait for pending outbound to be included in the block") return nil, true, nil } + + evt. + Uint64("cctx.gas_price", gas.Price.Uint64()). + Uint64("cctx.priority_fee", gas.PriorityFee.Uint64()). + Msg("Replacing pending outbound transaction with higher gas fees") } // Base64 decode message + var message []byte if cctx.InboundParams.CoinType != coin.CoinType_Cmd { - txData.message, err = base64.StdEncoding.DecodeString(cctx.RelayedMessage) - if err != nil { - logger.Err(err).Msgf("decode CCTX.Message %s error", cctx.RelayedMessage) + msg, errDecode := base64.StdEncoding.DecodeString(cctx.RelayedMessage) + if errDecode != nil { + logger.Err(err).Str("cctx.relayed_message", cctx.RelayedMessage).Msg("Unable to decode relayed message") + } else { + message = msg } } - return &txData, false, nil + return &OutboundData{ + srcChainID: big.NewInt(cctx.InboundParams.SenderChainId), + sender: ethcommon.HexToAddress(cctx.InboundParams.Sender), + + toChainID: toChainID, + to: to, + + asset: ethcommon.HexToAddress(cctx.InboundParams.Asset), + amount: outboundParams.Amount.BigInt(), + + gas: gas, + nonce: outboundParams.TssNonce, + height: height, + + message: message, + + cctxIndex: cctxIndex, + + outboundParams: outboundParams, + }, false, nil +} + +func getCCTXIndex(cctx *types.CrossChainTx) ([32]byte, error) { + cctxIndexSlice, err := hex.DecodeString(cctx.Index[2:]) // remove the leading 0x + if err != nil || len(cctxIndexSlice) != 32 { + return [32]byte{}, errors.Wrapf(err, "unable to decode cctx index %s", cctx.Index) + } + + var cctxIndex [32]byte + copy(cctxIndex[:32], cctxIndexSlice[:32]) + + return cctxIndex, nil +} + +// determineDestination picks the destination address and Chain ID based on the status of the cross chain tx. +// returns true if transaction should be skipped. +func determineDestination(cctx *types.CrossChainTx, logger zerolog.Logger) (ethcommon.Address, *big.Int, bool) { + switch cctx.CctxStatus.Status { + case types.CctxStatus_PendingRevert: + to := ethcommon.HexToAddress(cctx.InboundParams.Sender) + chainID := big.NewInt(cctx.InboundParams.SenderChainId) + + logger.Info(). + Str("cctx.index", cctx.Index). + Int64("cctx.chain_id", chainID.Int64()). + Msgf("Abort: reverting inbound") + + return to, chainID, false + case types.CctxStatus_PendingOutbound: + to := ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver) + chainID := big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) + + return to, chainID, false + } + + logger.Info(). + Str("cctx.index", cctx.Index). + Str("cctx.status", cctx.CctxStatus.String()). + Msgf("CCTX doesn't need to be processed") + + return ethcommon.Address{}, nil, true +} + +func validateParams(params *types.OutboundParams) error { + if params == nil || params.GasLimit == 0 { + return errors.New("outboundParams is empty") + } + + return nil } diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index 0d44fd3dbb..6a115b4396 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/zetaclient/chains/evm/signer/outbound_data_test.go @@ -1,7 +1,6 @@ package signer import ( - "context" "math/big" "testing" @@ -9,123 +8,142 @@ import ( "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - observertypes "github.com/zeta-chain/zetacore/x/observer/types" - "github.com/zeta-chain/zetacore/zetaclient/config" - zctx "github.com/zeta-chain/zetacore/zetaclient/context" - "github.com/zeta-chain/zetacore/zetaclient/testutils/mocks" - "github.com/zeta-chain/zetacore/pkg/chains" "github.com/zeta-chain/zetacore/x/crosschain/types" ) -func TestSigner_SetChainAndSender(t *testing.T) { - // setup inputs - cctx := getCCTX(t) - txData := &OutboundData{} - logger := zerolog.Logger{} +func TestNewOutboundData(t *testing.T) { + mockObserver, err := getNewEvmChainObserver(t, nil) + require.NoError(t, err) - t.Run("SetChainAndSender PendingRevert", func(t *testing.T) { - cctx.CctxStatus.Status = types.CctxStatus_PendingRevert - skipTx := txData.SetChainAndSender(cctx, logger) + logger := zerolog.New(zerolog.NewTestWriter(t)) - require.False(t, skipTx) - require.Equal(t, ethcommon.HexToAddress(cctx.InboundParams.Sender), txData.to) - require.Equal(t, big.NewInt(cctx.InboundParams.SenderChainId), txData.toChainID) - }) + ctx := makeCtx(t) - t.Run("SetChainAndSender PendingOutbound", func(t *testing.T) { - cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - skipTx := txData.SetChainAndSender(cctx, logger) + newOutbound := func(cctx *types.CrossChainTx) (*OutboundData, bool, error) { + return NewOutboundData(ctx, cctx, mockObserver, 123, logger) + } - require.False(t, skipTx) - require.Equal(t, ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver), txData.to) - require.Equal(t, big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId), txData.toChainID) - }) + t.Run("success", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) - t.Run("SetChainAndSender Should skip cctx", func(t *testing.T) { - cctx.CctxStatus.Status = types.CctxStatus_PendingInbound - skipTx := txData.SetChainAndSender(cctx, logger) - require.True(t, skipTx) - }) -} + // ACT + out, skip, err := newOutbound(cctx) -func TestSigner_SetupGas(t *testing.T) { - cctx := getCCTX(t) - evmSigner, err := getNewEvmSigner(nil) - require.NoError(t, err) + // ASSERT + require.NoError(t, err) + assert.False(t, skip) - txData := &OutboundData{} - logger := zerolog.Logger{} + assert.NotEmpty(t, out) - t.Run("SetupGas_success", func(t *testing.T) { - chain := chains.BscMainnet - err := txData.SetupGas(cctx, logger, evmSigner.EvmClient(), chain) - require.NoError(t, err) - }) + assert.NotEmpty(t, out.srcChainID) + assert.NotEmpty(t, out.sender) - t.Run("SetupGas_error", func(t *testing.T) { - cctx.GetCurrentOutboundParam().GasPrice = "invalidGasPrice" - chain := chains.BscMainnet - err := txData.SetupGas(cctx, logger, evmSigner.EvmClient(), chain) - require.ErrorContains(t, err, "cannot convert gas price") + assert.NotEmpty(t, out.toChainID) + assert.NotEmpty(t, out.to) + + assert.Equal(t, ethcommon.HexToAddress(cctx.InboundParams.Asset), out.asset) + assert.NotEmpty(t, out.amount) + + assert.NotEmpty(t, out.nonce) + assert.NotEmpty(t, out.height) + assert.NotEmpty(t, out.gas) + assert.Equal(t, uint64(minGasLimit), out.gas.Limit) + + assert.Empty(t, out.message) + assert.NotEmpty(t, out.cctxIndex) + assert.Equal(t, cctx.OutboundParams[0], out.outboundParams) }) -} -func TestSigner_NewOutboundData(t *testing.T) { - app := zctx.New(config.New(false), zerolog.Nop()) - ctx := zctx.WithAppContext(context.Background(), app) - - bscParams := mocks.MockChainParams(chains.BscMainnet.ChainId, 10) - - // Given app context - err := app.Update( - observertypes.Keygen{}, - []chains.Chain{chains.BscMainnet}, - nil, - map[int64]*observertypes.ChainParams{chains.BscMainnet.ChainId: &bscParams}, - "tssPubKey", - observertypes.CrosschainFlags{}, - ) - require.NoError(t, err) + t.Run("pending revert", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) + cctx.CctxStatus.Status = types.CctxStatus_PendingRevert - // Setup evm signer - evmSigner, err := getNewEvmSigner(nil) - require.NoError(t, err) + // ACT + out, skip, err := newOutbound(cctx) - mockObserver, err := getNewEvmChainObserver(t, nil) - require.NoError(t, err) + // ASSERT + require.NoError(t, err) + assert.False(t, skip) + assert.Equal(t, ethcommon.HexToAddress(cctx.InboundParams.Sender), out.to) + assert.Equal(t, big.NewInt(cctx.InboundParams.SenderChainId), out.toChainID) + }) - t.Run("NewOutboundData success", func(t *testing.T) { + t.Run("pending outbound", func(t *testing.T) { + // ARRANGE cctx := getCCTX(t) + cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound + + // ACT + out, skip, err := newOutbound(cctx) - _, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + // ASSERT assert.NoError(t, err) assert.False(t, skip) + assert.Equal(t, ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver), out.to) + assert.Equal(t, big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId), out.toChainID) }) - t.Run("NewOutboundData skip", func(t *testing.T) { + t.Run("skip inbound", func(t *testing.T) { + // ARRANGE cctx := getCCTX(t) - cctx.CctxStatus.Status = types.CctxStatus_Aborted + cctx.CctxStatus.Status = types.CctxStatus_PendingInbound - _, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - assert.NoError(t, err) + // ACT + _, skip, err := newOutbound(cctx) + + // ASSERT + require.NoError(t, err) assert.True(t, skip) }) - t.Run("NewOutboundData unknown chain", func(t *testing.T) { - cctx := getInvalidCCTX(t) + t.Run("skip aborted", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) + cctx.CctxStatus.Status = types.CctxStatus_Aborted + + // ACT + _, skip, err := newOutbound(cctx) - _, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - assert.ErrorContains(t, err, "unable to get chain 13378337 from app context: id=13378337: chain not found") + // ASSERT + require.NoError(t, err) assert.True(t, skip) }) - t.Run("NewOutboundData setup gas error", func(t *testing.T) { + t.Run("invalid gas price", func(t *testing.T) { + // ARRANGE cctx := getCCTX(t) cctx.GetCurrentOutboundParam().GasPrice = "invalidGasPrice" - _, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - assert.True(t, skip) - assert.ErrorContains(t, err, "cannot convert gas price") + // ACT + _, _, err := newOutbound(cctx) + + // ASSERT + assert.ErrorContains(t, err, "unable to parse gasPrice") + }) + + t.Run("unknown chain", func(t *testing.T) { + // ARRANGE + cctx := getInvalidCCTX(t) + + // ACT + _, _, err := newOutbound(cctx) + + // ASSERT + assert.ErrorContains(t, err, "chain not found") + }) + + t.Run("no outbound params", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) + cctx.OutboundParams = nil + + // ACT + _, _, err := newOutbound(cctx) + + // ASSERT + assert.ErrorContains(t, err, "outboundParams is empty") }) } diff --git a/zetaclient/chains/evm/signer/signer.go b/zetaclient/chains/evm/signer/signer.go index 9c928face1..ed4c04ed81 100644 --- a/zetaclient/chains/evm/signer/signer.go +++ b/zetaclient/chains/evm/signer/signer.go @@ -168,16 +168,20 @@ func (signer *Signer) Sign( data []byte, to ethcommon.Address, amount *big.Int, - gasLimit uint64, - gasPrice *big.Int, + gas Gas, nonce uint64, height uint64, ) (*ethtypes.Transaction, []byte, []byte, error) { - log.Debug().Str("tss.pub_key", signer.TSS().EVMAddress().String()).Msg("Sign: TSS signer") + signer.Logger().Std.Debug(). + Str("tss_pub_key", signer.TSS().EVMAddress().String()). + Msg("Signing evm transaction") + + chainID := big.NewInt(signer.Chain().ChainId) + tx, err := newTx(chainID, data, to, amount, gas, nonce) + if err != nil { + return nil, nil, nil, err + } - // TODO: use EIP-1559 transaction type - // https://github.com/zeta-chain/node/issues/1952 - tx := ethtypes.NewTransaction(nonce, to, amount, gasLimit, gasPrice, data) hashBytes := signer.ethSigner.Hash(tx).Bytes() sig, err := signer.TSS().Sign(ctx, hashBytes, height, nonce, signer.Chain().ChainId, "") @@ -201,11 +205,46 @@ func (signer *Signer) Sign( return signedTX, sig[:], hashBytes[:], nil } -// Broadcast takes in signed tx, broadcast to external chain node -func (signer *Signer) Broadcast(tx *ethtypes.Transaction) error { - ctxt, cancel := context.WithTimeout(context.Background(), 1*time.Second) +func newTx( + chainID *big.Int, + data []byte, + to ethcommon.Address, + amount *big.Int, + gas Gas, + nonce uint64, +) (*ethtypes.Transaction, error) { + if err := gas.validate(); err != nil { + return nil, errors.Wrap(err, "invalid gas parameters") + } + + if gas.isLegacy() { + return ethtypes.NewTx(ðtypes.LegacyTx{ + To: &to, + Value: amount, + Data: data, + GasPrice: gas.Price, + Gas: gas.Limit, + Nonce: nonce, + }), nil + } + + return ethtypes.NewTx(ðtypes.DynamicFeeTx{ + ChainID: chainID, + To: &to, + Value: amount, + Data: data, + GasFeeCap: gas.Price, + GasTipCap: gas.PriorityFee, + Gas: gas.Limit, + Nonce: nonce, + }), nil +} + +func (signer *Signer) broadcast(ctx context.Context, tx *ethtypes.Transaction) error { + ctx, cancel := context.WithTimeout(ctx, 1*time.Second) defer cancel() - return signer.client.SendTransaction(ctxt, tx) + + return signer.client.SendTransaction(ctx, tx) } // SignOutbound @@ -239,10 +278,10 @@ func (signer *Signer) SignOutbound(ctx context.Context, txData *OutboundData) (* data, signer.zetaConnectorAddress, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, - txData.height) + txData.height, + ) if err != nil { return nil, fmt.Errorf("sign onReceive error: %w", err) } @@ -261,17 +300,15 @@ func (signer *Signer) SignOutbound(ctx context.Context, txData *OutboundData) (* // bytes32 internalSendHash // ) external override whenNotPaused onlyTssAddress func (signer *Signer) SignRevertTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - var data []byte - var err error - - data, err = signer.zetaConnectorABI.Pack("onRevert", + data, err := signer.zetaConnectorABI.Pack("onRevert", txData.sender, txData.srcChainID, txData.to.Bytes(), txData.toChainID, txData.amount, txData.message, - txData.cctxIndex) + txData.cctxIndex, + ) if err != nil { return nil, fmt.Errorf("onRevert pack error: %w", err) } @@ -281,10 +318,10 @@ func (signer *Signer) SignRevertTx(ctx context.Context, txData *OutboundData) (* data, signer.zetaConnectorAddress, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, - txData.height) + txData.height, + ) if err != nil { return nil, fmt.Errorf("sign onRevert error: %w", err) } @@ -294,13 +331,13 @@ func (signer *Signer) SignRevertTx(ctx context.Context, txData *OutboundData) (* // SignCancelTx signs a transaction from TSS address to itself with a zero amount in order to increment the nonce func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { + txData.gas.Limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, signer.TSS().EVMAddress(), zeroValue, // zero out the amount to cancel the tx - evm.EthTransferGasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) @@ -313,13 +350,13 @@ func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (* // SignWithdrawTx signs a withdrawal transaction sent from the TSS address to the destination func (signer *Signer) SignWithdrawTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { + txData.gas.Limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, txData.to, txData.amount, - evm.EthTransferGasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) @@ -362,6 +399,21 @@ func (signer *Signer) TryProcessOutbound( zetacoreClient interfaces.ZetacoreClient, height uint64, ) { + var ( + params = cctx.GetCurrentOutboundParam() + myID = zetacoreClient.GetKeys().GetOperatorAddress() + logger = signer.Logger().Std.With(). + Str("method", "TryProcessOutbound"). + Int64("chain", signer.Chain().ChainId). + Uint64("nonce", params.TssNonce). + Str("cctx.index", cctx.Index). + Str("cctx.receiver", params.Receiver). + Str("cctx.amount", params.Amount.String()). + Logger() + ) + + logger.Info().Msgf("TryProcessOutbound") + app, err := zctx.FromContext(ctx) if err != nil { signer.Logger().Std.Error().Err(err).Msg("error getting app context") @@ -370,24 +422,12 @@ func (signer *Signer) TryProcessOutbound( // end outbound process on panic defer func() { - outboundProc.EndTryProcess(outboundID) - if err := recover(); err != nil { - signer.Logger().Std.Error().Msgf("EVM TryProcessOutbound: %s, caught panic error: %v", cctx.Index, err) + if r := recover(); r != nil { + logger.Error().Interface("panic", r).Msg("panic in TryProcessOutbound") } - }() - - // prepare logger - params := cctx.GetCurrentOutboundParam() - logger := signer.Logger().Std.With(). - Str("method", "TryProcessOutbound"). - Int64("chain", signer.Chain().ChainId). - Uint64("nonce", params.TssNonce). - Str("cctx", cctx.Index). - Logger() - myID := zetacoreClient.GetKeys().GetOperatorAddress() - logger.Info(). - Msgf("EVM TryProcessOutbound: %s, value %d to %s", cctx.Index, params.Amount.BigInt(), params.Receiver) + outboundProc.EndTryProcess(outboundID) + }() evmObserver, ok := chainObserver.(*observer.Observer) if !ok { @@ -396,11 +436,12 @@ func (signer *Signer) TryProcessOutbound( } // Setup Transaction input - txData, skipTx, err := NewOutboundData(ctx, cctx, evmObserver, signer.client, logger, height) + txData, skipTx, err := NewOutboundData(ctx, cctx, evmObserver, height, logger) if err != nil { logger.Err(err).Msg("error setting up transaction input fields") return } + if skipTx { return } @@ -466,7 +507,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -475,7 +516,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) case coin.CoinType_Zeta: @@ -484,7 +525,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignOutbound(ctx, txData) } @@ -499,7 +540,7 @@ func (signer *Signer) TryProcessOutbound( "SignRevertTx: %d => %d, nonce %d, gasPrice %d", cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -510,7 +551,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -518,7 +559,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) } @@ -532,7 +573,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -548,7 +589,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.Price, ) tx, err = signer.SignOutbound(ctx, txData) if err != nil { @@ -605,7 +646,7 @@ func (signer *Signer) BroadcastOutbound( backOff := broadcastBackoff for i := 0; i < broadcastRetries; i++ { time.Sleep(backOff) - err := signer.Broadcast(tx) + err := signer.broadcast(ctx, tx) if err != nil { log.Warn(). Err(err). @@ -640,9 +681,7 @@ func (signer *Signer) BroadcastOutbound( // uint256 amount, // ) external onlyTssAddress func (signer *Signer) SignERC20WithdrawTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - var data []byte - var err error - data, err = signer.erc20CustodyABI.Pack("withdraw", txData.to, txData.asset, txData.amount) + data, err := signer.erc20CustodyABI.Pack("withdraw", txData.to, txData.asset, txData.amount) if err != nil { return nil, fmt.Errorf("withdraw pack error: %w", err) } @@ -652,8 +691,7 @@ func (signer *Signer) SignERC20WithdrawTx(ctx context.Context, txData *OutboundD data, signer.er20CustodyAddress, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) @@ -706,27 +744,30 @@ func (signer *Signer) SignWhitelistERC20Cmd( if erc20 == (ethcommon.Address{}) { return nil, fmt.Errorf("SignCommandTx: invalid erc20 address %s", params) } + custodyAbi, err := erc20custody.ERC20CustodyMetaData.GetAbi() if err != nil { return nil, err } + data, err := custodyAbi.Pack("whitelist", erc20) if err != nil { return nil, fmt.Errorf("whitelist pack error: %w", err) } + tx, _, _, err := signer.Sign( ctx, data, txData.to, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, outboundParams.TssNonce, txData.height, ) if err != nil { return nil, fmt.Errorf("sign whitelist error: %w", err) } + return tx, nil } @@ -738,8 +779,7 @@ func (signer *Signer) SignMigrateTssFundsCmd(ctx context.Context, txData *Outbou nil, txData.to, txData.amount, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) @@ -881,14 +921,3 @@ func getEVMRPC(ctx context.Context, endpoint string) (interfaces.EVMRPCClient, e return client, ethSigner, nil } - -// roundUpToNearestGwei rounds up the gas price to the nearest Gwei -func roundUpToNearestGwei(gasPrice *big.Int) *big.Int { - oneGwei := big.NewInt(1_000_000_000) // 1 Gwei - mod := new(big.Int) - mod.Mod(gasPrice, oneGwei) - if mod.Cmp(big.NewInt(0)) == 0 { // gasprice is already a multiple of 1 Gwei - return gasPrice - } - return new(big.Int).Add(gasPrice, new(big.Int).Sub(oneGwei, mod)) -} diff --git a/zetaclient/chains/evm/signer/signer_test.go b/zetaclient/chains/evm/signer/signer_test.go index 95da02e648..4298ff7041 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -197,7 +197,7 @@ func TestSigner_SignOutbound(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -233,7 +233,7 @@ func TestSigner_SignRevertTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -273,7 +273,7 @@ func TestSigner_SignCancelTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -313,7 +313,7 @@ func TestSigner_SignWithdrawTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -351,7 +351,7 @@ func TestSigner_SignCommandTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, nil) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -398,7 +398,7 @@ func TestSigner_SignERC20WithdrawTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -439,7 +439,7 @@ func TestSigner_BroadcastOutbound(t *testing.T) { mockObserver, err := getNewEvmChainObserver(t, nil) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.NoError(t, err) require.False(t, skip) @@ -496,7 +496,7 @@ func TestSigner_SignWhitelistERC20Cmd(t *testing.T) { mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.NoError(t, err) require.False(t, skip) @@ -541,7 +541,7 @@ func TestSigner_SignMigrateTssFundsCmd(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -576,9 +576,11 @@ func makeCtx(t *testing.T) context.Context { err := app.Update( observertypes.Keygen{}, - []chains.Chain{chains.BscMainnet}, + []chains.Chain{chains.BscMainnet, chains.ZetaChainMainnet}, nil, - map[int64]*observertypes.ChainParams{chains.BscMainnet.ChainId: &bscParams}, + map[int64]*observertypes.ChainParams{ + chains.BscMainnet.ChainId: &bscParams, + }, "tssPubKey", observertypes.CrosschainFlags{}, ) From 3f953bac26718e2f610d48a4cf00b8eb02dcfe1c Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 5 Aug 2024 17:28:10 +0200 Subject: [PATCH 03/11] Update changelog --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index 822cefe360..b4803560cd 100644 --- a/changelog.md +++ b/changelog.md @@ -46,6 +46,7 @@ * [2524](https://github.com/zeta-chain/node/pull/2524) - add inscription envelop parsing * [2560](https://github.com/zeta-chain/node/pull/2560) - add support for Solana SOL token withdraw * [2533](https://github.com/zeta-chain/node/pull/2533) - parse memo from both OP_RETURN and inscription +* [2634](https://github.com/zeta-chain/node/pull/2634) - add support for EIP-1559 gas fees ### Refactor From dad95a519af7ff30495af112d996edc201408604 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 5 Aug 2024 18:05:34 +0200 Subject: [PATCH 04/11] Add test cases for legacy vs dynamicFee txs --- .../chains/evm/signer/outbound_data_test.go | 1 + zetaclient/chains/evm/signer/signer_test.go | 48 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index 6a115b4396..03c1329ef1 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/zetaclient/chains/evm/signer/outbound_data_test.go @@ -49,6 +49,7 @@ func TestNewOutboundData(t *testing.T) { assert.NotEmpty(t, out.nonce) assert.NotEmpty(t, out.height) assert.NotEmpty(t, out.gas) + assert.True(t, out.gas.isLegacy()) assert.Equal(t, uint64(minGasLimit), out.gas.Limit) assert.Empty(t, out.message) diff --git a/zetaclient/chains/evm/signer/signer_test.go b/zetaclient/chains/evm/signer/signer_test.go index 4298ff7041..9233dfcbe2 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -10,6 +10,7 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" observertypes "github.com/zeta-chain/zetacore/x/observer/types" zctx "github.com/zeta-chain/zetacore/zetaclient/context" @@ -197,11 +198,11 @@ func TestSigner_SignOutbound(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, zerolog.Logger{}) + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, makeLogger(t)) require.False(t, skip) require.NoError(t, err) - t.Run("SignOutbound - should successfully sign", func(t *testing.T) { + t.Run("SignOutbound - should successfully sign LegacyTx", func(t *testing.T) { // Call SignOutbound tx, err := evmSigner.SignOutbound(ctx, txData) require.NoError(t, err) @@ -209,6 +210,9 @@ func TestSigner_SignOutbound(t *testing.T) { // Verify Signature tss := mocks.NewTSSMainnet() verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) + + // check that by default tx type is legacy tx + assert.Equal(t, ethtypes.LegacyTxType, int(tx.Type())) }) t.Run("SignOutbound - should fail if keysign fails", func(t *testing.T) { // Pause tss to make keysign fail @@ -219,6 +223,42 @@ func TestSigner_SignOutbound(t *testing.T) { require.ErrorContains(t, err, "sign onReceive error") require.Nil(t, tx) }) + + t.Run("SignOutbound - should successfully sign DynamicFeeTx", func(t *testing.T) { + // ARRANGE + const ( + gwei = 1_000_000_000 + priorityFee = 1 * gwei + gasPrice = 3 * gwei + ) + + // Given a CCTX with gas price and priority fee + cctx := getCCTX(t) + cctx.OutboundParams[0].GasPrice = big.NewInt(gasPrice).String() + cctx.OutboundParams[0].GasPriorityFee = big.NewInt(priorityFee).String() + + // Given outbound data + txData, skip, err := NewOutboundData(ctx, cctx, mockObserver, 123, makeLogger(t)) + require.False(t, skip) + require.NoError(t, err) + + // Given a working TSS + tss.Unpause() + + // ACT + tx, err := evmSigner.SignOutbound(ctx, txData) + require.NoError(t, err) + + // ASSERT + verifyTxSignature(t, tx, mocks.NewTSSMainnet().Pubkey(), evmSigner.EvmSigner()) + + // check that by default tx type is a dynamic fee tx + assert.Equal(t, ethtypes.DynamicFeeTxType, int(tx.Type())) + + // check that the gasPrice & priorityFee are set correctly + assert.Equal(t, int64(gasPrice), tx.GasFeeCap().Int64()) + assert.Equal(t, int64(priorityFee), tx.GasTipCap().Int64()) + }) } func TestSigner_SignRevertTx(t *testing.T) { @@ -588,3 +628,7 @@ func makeCtx(t *testing.T) context.Context { return zctx.WithAppContext(context.Background(), app) } + +func makeLogger(t *testing.T) zerolog.Logger { + return zerolog.New(zerolog.NewTestWriter(t)) +} From fae68d5973114baa43be255f43e455f67f6f92a7 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 5 Aug 2024 19:27:58 +0200 Subject: [PATCH 05/11] Fix typo; Add E2E coverage --- e2e/e2etests/test_eth_withdraw.go | 15 +++++++++++++++ zetaclient/chains/evm/observer/observer_gas.go | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/e2e/e2etests/test_eth_withdraw.go b/e2e/e2etests/test_eth_withdraw.go index 3415becca6..aa7c0dc3f6 100644 --- a/e2e/e2etests/test_eth_withdraw.go +++ b/e2e/e2etests/test_eth_withdraw.go @@ -3,6 +3,8 @@ package e2etests import ( "math/big" + ethcommon "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/stretchr/testify/require" "github.com/zeta-chain/zetacore/e2e/runner" @@ -37,5 +39,18 @@ func TestEtherWithdraw(r *runner.E2ERunner, args []string) { utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) + withdrawalReceipt := mustFetchEthReceipt(r, cctx) + require.Equal(r, uint8(ethtypes.DynamicFeeTxType), withdrawalReceipt.Type, "receipt type mismatch") + r.Logger.Info("TestEtherWithdraw completed") } + +func mustFetchEthReceipt(r *runner.E2ERunner, cctx *crosschaintypes.CrossChainTx) *ethtypes.Receipt { + hash := cctx.GetCurrentOutboundParam().Hash + require.NotEmpty(r, hash, "outbound hash is empty") + + receipt, err := r.EVMClient.TransactionReceipt(r.Ctx, ethcommon.HexToHash(hash)) + require.NoError(r, err) + + return receipt +} diff --git a/zetaclient/chains/evm/observer/observer_gas.go b/zetaclient/chains/evm/observer/observer_gas.go index 8bbc32d3a0..311ea187b9 100644 --- a/zetaclient/chains/evm/observer/observer_gas.go +++ b/zetaclient/chains/evm/observer/observer_gas.go @@ -126,7 +126,7 @@ func (ob *Observer) supportsPriorityFee(ctx context.Context) (bool, error) { defer ob.Mu().Unlock() ob.priorityFeeConfig.checked = true - ob.priorityFeeConfig.checked = isSupported + ob.priorityFeeConfig.supported = isSupported return isSupported, nil } From d8146f13f04af9776434e41e7eaf9c1561846534 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 5 Aug 2024 19:49:40 +0200 Subject: [PATCH 06/11] Address PR comments --- zetaclient/chains/evm/signer/outbound_data.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index 012c146b29..b12d3fbbb5 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/hex" + "fmt" "math/big" ethcommon "github.com/ethereum/go-ethereum/common" @@ -50,10 +51,12 @@ func NewOutboundData( height uint64, logger zerolog.Logger, ) (*OutboundData, bool, error) { - var ( - outboundParams = cctx.GetCurrentOutboundParam() - nonce = outboundParams.TssNonce - ) + if cctx == nil { + return nil, false, errors.New("cctx is nil") + } + + outboundParams := cctx.GetCurrentOutboundParam() + nonce := outboundParams.TssNonce if err := validateParams(outboundParams); err != nil { return nil, false, errors.Wrap(err, "invalid outboundParams") @@ -137,7 +140,13 @@ func NewOutboundData( } func getCCTXIndex(cctx *types.CrossChainTx) ([32]byte, error) { - cctxIndexSlice, err := hex.DecodeString(cctx.Index[2:]) // remove the leading 0x + // `0x` + `64 chars`. Two chars ranging `00...FF` represent one byte (64 chars = 32 bytes) + if len(cctx.Index) != (2 + 64) { + return [32]byte{}, fmt.Errorf("cctx index %q is invalid", cctx.Index) + } + + // remove the leading `0x` + cctxIndexSlice, err := hex.DecodeString(cctx.Index[2:]) if err != nil || len(cctxIndexSlice) != 32 { return [32]byte{}, errors.Wrapf(err, "unable to decode cctx index %s", cctx.Index) } From 4d6ed2cf32dcf27af7bc29271e7f67ed5bae5f5c Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 6 Aug 2024 12:11:35 +0200 Subject: [PATCH 07/11] Address PR comments --- changelog.md | 2 +- zetaclient/chains/evm/signer/gas.go | 3 +-- zetaclient/chains/evm/signer/gas_test.go | 2 +- zetaclient/chains/evm/signer/outbound_data.go | 8 ++++---- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/changelog.md b/changelog.md index b4803560cd..7469c739c9 100644 --- a/changelog.md +++ b/changelog.md @@ -5,6 +5,7 @@ ### Features * [2578](https://github.com/zeta-chain/node/pull/2578) - Add Gateway address in protocol contract list +* [2634](https://github.com/zeta-chain/node/pull/2634) - add support for EIP-1559 gas fees ## v19.0.0 @@ -46,7 +47,6 @@ * [2524](https://github.com/zeta-chain/node/pull/2524) - add inscription envelop parsing * [2560](https://github.com/zeta-chain/node/pull/2560) - add support for Solana SOL token withdraw * [2533](https://github.com/zeta-chain/node/pull/2533) - parse memo from both OP_RETURN and inscription -* [2634](https://github.com/zeta-chain/node/pull/2634) - add support for EIP-1559 gas fees ### Refactor diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go index 593f773ec5..2479e5a9f6 100644 --- a/zetaclient/chains/evm/signer/gas.go +++ b/zetaclient/chains/evm/signer/gas.go @@ -54,8 +54,7 @@ func (g Gas) isLegacy() bool { return g.PriorityFee.Sign() < 1 } -// makeGasFromCCTX creates Gas struct based from CCTX and priorityFee. -func makeGasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { +func gasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { var ( params = cctx.GetCurrentOutboundParam() limit = params.GasLimit diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go index 1b935081ae..21d003769c 100644 --- a/zetaclient/chains/evm/signer/gas_test.go +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -110,7 +110,7 @@ func Test_makeGasFromCCTX(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - g, err := makeGasFromCCTX(tt.cctx, logger) + g, err := gasFromCCTX(tt.cctx, logger) if tt.errorContains != "" { assert.ErrorContains(t, err, tt.errorContains) return diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index b12d3fbbb5..399f4e00e0 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -68,7 +68,7 @@ func NewOutboundData( } // recipient + destination chain - to, toChainID, skip := determineDestination(cctx, logger) + to, toChainID, skip := getDestination(cctx, logger) if skip { return nil, true, nil } @@ -78,7 +78,7 @@ func NewOutboundData( return nil, false, errors.Wrapf(err, "unable to get chain %d from app context", toChainID.Int64()) } - gas, err := makeGasFromCCTX(cctx, logger) + gas, err := gasFromCCTX(cctx, logger) if err != nil { return nil, false, errors.Wrap(err, "unable to make gas from CCTX") } @@ -157,9 +157,9 @@ func getCCTXIndex(cctx *types.CrossChainTx) ([32]byte, error) { return cctxIndex, nil } -// determineDestination picks the destination address and Chain ID based on the status of the cross chain tx. +// getDestination picks the destination address and Chain ID based on the status of the cross chain tx. // returns true if transaction should be skipped. -func determineDestination(cctx *types.CrossChainTx, logger zerolog.Logger) (ethcommon.Address, *big.Int, bool) { +func getDestination(cctx *types.CrossChainTx, logger zerolog.Logger) (ethcommon.Address, *big.Int, bool) { switch cctx.CctxStatus.Status { case types.CctxStatus_PendingRevert: to := ethcommon.HexToAddress(cctx.InboundParams.Sender) From 2260925ee1fd4c0b4613559d6946f59882401708 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 6 Aug 2024 13:04:50 +0200 Subject: [PATCH 08/11] Use gasFeeCap formula --- zetaclient/chains/evm/signer/gas.go | 51 +++++++++++++----- zetaclient/chains/evm/signer/gas_test.go | 53 ++++++++++--------- zetaclient/chains/evm/signer/outbound_data.go | 6 +-- .../chains/evm/signer/outbound_data_test.go | 4 +- zetaclient/chains/evm/signer/signer.go | 32 +++++------ zetaclient/chains/evm/signer/signer_test.go | 9 ++-- 6 files changed, 93 insertions(+), 62 deletions(-) diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go index 2479e5a9f6..08eb0c7f3f 100644 --- a/zetaclient/chains/evm/signer/gas.go +++ b/zetaclient/chains/evm/signer/gas.go @@ -4,6 +4,7 @@ import ( "fmt" "math/big" + "cosmossdk.io/math" "github.com/pkg/errors" "github.com/rs/zerolog" @@ -24,34 +25,58 @@ const ( // // However, this doesn't affect tx creation nor broadcasting type Gas struct { - Limit uint64 + limit uint64 // This is a "total" gasPrice per 1 unit of gas. - Price *big.Int + price *big.Int // PriorityFee a fee paid directly to validators. - PriorityFee *big.Int + priorityFee *big.Int } func (g Gas) validate() error { switch { - case g.Limit == 0: + case g.limit == 0: return errors.New("gas limit is zero") - case g.Price == nil: - return errors.New("max fee per unit is nil") - case g.PriorityFee == nil: + case g.price == nil: + return errors.New("gas price per unit is nil") + case g.priorityFee == nil: return errors.New("priority fee per unit is nil") default: return nil } } -// isLegacy determines whether the gas is meant for LegacyTx{} (pre EIP-1559) +// IsLegacy determines whether the gas is meant for LegacyTx{} (pre EIP-1559) // or DynamicFeeTx{} (post EIP-1559). // // Returns true if priority fee is <= 0. -func (g Gas) isLegacy() bool { - return g.PriorityFee.Sign() < 1 +func (g Gas) IsLegacy() bool { + return g.priorityFee.Sign() < 1 +} + +func (g Gas) Limit() uint64 { + return g.limit +} + +// GasPrice returns the gas price for legacy transactions. +func (g Gas) GasPrice() *big.Int { + return g.price +} + +// PriorityFee returns the priority fee for EIP-1559 transactions. +func (g Gas) PriorityFee() *big.Int { + return g.priorityFee +} + +// GasFeeCap returns the gas fee cap for EIP-1559 transactions. +// heuristic of `2*baseFee + gasTipCap` is used. And because baseFee = `gasPrice - priorityFee`, +// it's `2*(gasPrice - priorityFee) + priorityFee` => `2*gasPrice - priorityFee` +func (g Gas) GasFeeCap() *big.Int { + return math.NewUintFromBigInt(g.price). + MulUint64(2). + Sub(math.NewUintFromBigInt(g.priorityFee)). + BigInt() } func gasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { @@ -89,9 +114,9 @@ func gasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { } return Gas{ - Limit: limit, - Price: gasPrice, - PriorityFee: priorityFee, + limit: limit, + price: gasPrice, + priorityFee: priorityFee, }, nil } diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go index 21d003769c..2f2d01581d 100644 --- a/zetaclient/chains/evm/signer/gas_test.go +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -9,7 +9,7 @@ import ( "github.com/zeta-chain/zetacore/x/crosschain/types" ) -func Test_makeGasFromCCTX(t *testing.T) { +func TestGasFromCCTX(t *testing.T) { logger := zerolog.New(zerolog.NewTestWriter(t)) makeCCTX := func(gasLimit uint64, price, priorityFee string) *types.CrossChainTx { @@ -32,11 +32,11 @@ func Test_makeGasFromCCTX(t *testing.T) { name: "legacy: gas is too low", cctx: makeCCTX(minGasLimit-200, gwei(2).String(), ""), assert: func(t *testing.T, g Gas) { - assert.True(t, g.isLegacy()) + assert.True(t, g.IsLegacy()) assertGasEquals(t, Gas{ - Limit: minGasLimit, - PriorityFee: gwei(0), - Price: gwei(2), + limit: minGasLimit, + price: gwei(2), + priorityFee: gwei(0), }, g) }, }, @@ -44,23 +44,27 @@ func Test_makeGasFromCCTX(t *testing.T) { name: "london: gas is too low", cctx: makeCCTX(minGasLimit-200, gwei(2).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.isLegacy()) + assert.False(t, g.IsLegacy()) assertGasEquals(t, Gas{ - Limit: minGasLimit, - Price: gwei(2), - PriorityFee: gwei(1), + limit: minGasLimit, + price: gwei(2), + priorityFee: gwei(1), }, g) + + // gasPrice=2, priorityFee=1, so baseFee=1 + // gasFeeCap = 2*baseFee + priorityFee = 2 + 1 = 3 + assert.Equal(t, gwei(3).Int64(), g.GasFeeCap().Int64()) }, }, { name: "pre London gas logic", cctx: makeCCTX(minGasLimit+100, gwei(3).String(), ""), assert: func(t *testing.T, g Gas) { - assert.True(t, g.isLegacy()) + assert.True(t, g.IsLegacy()) assertGasEquals(t, Gas{ - Limit: 100_100, - Price: gwei(3), - PriorityFee: gwei(0), + limit: 100_100, + price: gwei(3), + priorityFee: gwei(0), }, g) }, }, @@ -68,11 +72,11 @@ func Test_makeGasFromCCTX(t *testing.T) { name: "post London gas logic", cctx: makeCCTX(minGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.isLegacy()) + assert.False(t, g.IsLegacy()) assertGasEquals(t, Gas{ - Limit: 100_200, - Price: gwei(4), - PriorityFee: gwei(1), + limit: 100_200, + price: gwei(4), + priorityFee: gwei(1), }, g) }, }, @@ -80,11 +84,11 @@ func Test_makeGasFromCCTX(t *testing.T) { name: "gas is too high, force to the ceiling", cctx: makeCCTX(maxGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.isLegacy()) + assert.False(t, g.IsLegacy()) assertGasEquals(t, Gas{ - Limit: maxGasLimit, - Price: gwei(4), - PriorityFee: gwei(1), + limit: maxGasLimit, + price: gwei(4), + priorityFee: gwei(1), }, g) }, }, @@ -123,9 +127,10 @@ func Test_makeGasFromCCTX(t *testing.T) { } func assertGasEquals(t *testing.T, expected, actual Gas) { - assert.Equal(t, int64(expected.Limit), int64(actual.Limit), "gas limit") - assert.Equal(t, expected.Price.Int64(), actual.Price.Int64(), "max fee per unit") - assert.Equal(t, expected.PriorityFee.Int64(), actual.PriorityFee.Int64(), "priority fee per unit") + assert.Equal(t, int64(expected.Limit()), int64(actual.Limit()), "gas limit") + assert.Equal(t, expected.GasPrice().Int64(), actual.GasPrice().Int64(), "gas price") + assert.Equal(t, expected.GasFeeCap().Int64(), actual.GasFeeCap().Int64(), "max fee cap") + assert.Equal(t, expected.PriorityFee().Int64(), actual.PriorityFee().Int64(), "priority fee") } func gwei(i int64) *big.Int { diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index 399f4e00e0..22a814c8fd 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -95,14 +95,14 @@ func NewOutboundData( Uint64("cctx.pending_tx_nonce", nonce) // new gas price is less or equal to pending tx gas - if gas.Price.Cmp(tx.GasPrice()) <= 0 { + if gas.GasPrice().Cmp(tx.GasPrice()) <= 0 { evt.Msg("Please wait for pending outbound to be included in the block") return nil, true, nil } evt. - Uint64("cctx.gas_price", gas.Price.Uint64()). - Uint64("cctx.priority_fee", gas.PriorityFee.Uint64()). + Uint64("cctx.gas_price", gas.GasPrice().Uint64()). + Uint64("cctx.priority_fee", gas.PriorityFee().Uint64()). Msg("Replacing pending outbound transaction with higher gas fees") } diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index 03c1329ef1..1ffc405f65 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/zetaclient/chains/evm/signer/outbound_data_test.go @@ -49,8 +49,8 @@ func TestNewOutboundData(t *testing.T) { assert.NotEmpty(t, out.nonce) assert.NotEmpty(t, out.height) assert.NotEmpty(t, out.gas) - assert.True(t, out.gas.isLegacy()) - assert.Equal(t, uint64(minGasLimit), out.gas.Limit) + assert.True(t, out.gas.IsLegacy()) + assert.Equal(t, uint64(minGasLimit), out.gas.Limit()) assert.Empty(t, out.message) assert.NotEmpty(t, out.cctxIndex) diff --git a/zetaclient/chains/evm/signer/signer.go b/zetaclient/chains/evm/signer/signer.go index ed4c04ed81..b073433da1 100644 --- a/zetaclient/chains/evm/signer/signer.go +++ b/zetaclient/chains/evm/signer/signer.go @@ -217,13 +217,13 @@ func newTx( return nil, errors.Wrap(err, "invalid gas parameters") } - if gas.isLegacy() { + if gas.IsLegacy() { return ethtypes.NewTx(ðtypes.LegacyTx{ To: &to, Value: amount, Data: data, - GasPrice: gas.Price, - Gas: gas.Limit, + GasPrice: gas.GasPrice(), + Gas: gas.Limit(), Nonce: nonce, }), nil } @@ -233,9 +233,9 @@ func newTx( To: &to, Value: amount, Data: data, - GasFeeCap: gas.Price, - GasTipCap: gas.PriorityFee, - Gas: gas.Limit, + GasFeeCap: gas.GasFeeCap(), + GasTipCap: gas.PriorityFee(), + Gas: gas.Limit(), Nonce: nonce, }), nil } @@ -331,7 +331,7 @@ func (signer *Signer) SignRevertTx(ctx context.Context, txData *OutboundData) (* // SignCancelTx signs a transaction from TSS address to itself with a zero amount in order to increment the nonce func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - txData.gas.Limit = evm.EthTransferGasLimit + txData.gas.limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, @@ -350,7 +350,7 @@ func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (* // SignWithdrawTx signs a withdrawal transaction sent from the TSS address to the destination func (signer *Signer) SignWithdrawTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - txData.gas.Limit = evm.EthTransferGasLimit + txData.gas.limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, @@ -507,7 +507,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -516,7 +516,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) case coin.CoinType_Zeta: @@ -525,7 +525,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignOutbound(ctx, txData) } @@ -540,7 +540,7 @@ func (signer *Signer) TryProcessOutbound( "SignRevertTx: %d => %d, nonce %d, gasPrice %d", cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -551,7 +551,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -559,7 +559,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) } @@ -573,7 +573,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -589,7 +589,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.Price, + txData.gas.GasPrice(), ) tx, err = signer.SignOutbound(ctx, txData) if err != nil { diff --git a/zetaclient/chains/evm/signer/signer_test.go b/zetaclient/chains/evm/signer/signer_test.go index 9233dfcbe2..65edb51cfe 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -227,9 +227,10 @@ func TestSigner_SignOutbound(t *testing.T) { t.Run("SignOutbound - should successfully sign DynamicFeeTx", func(t *testing.T) { // ARRANGE const ( - gwei = 1_000_000_000 - priorityFee = 1 * gwei - gasPrice = 3 * gwei + gwei = 1_000_000_000 + priorityFee = 1 * gwei + gasPrice = 3 * gwei + londonGasFeeCap = 2*gasPrice - priorityFee ) // Given a CCTX with gas price and priority fee @@ -256,7 +257,7 @@ func TestSigner_SignOutbound(t *testing.T) { assert.Equal(t, ethtypes.DynamicFeeTxType, int(tx.Type())) // check that the gasPrice & priorityFee are set correctly - assert.Equal(t, int64(gasPrice), tx.GasFeeCap().Int64()) + assert.Equal(t, int64(londonGasFeeCap), tx.GasFeeCap().Int64()) assert.Equal(t, int64(priorityFee), tx.GasTipCap().Int64()) }) } From 1fe1adb2427676aec3d2a6904f7f22d09139ea8e Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 6 Aug 2024 17:51:01 +0200 Subject: [PATCH 09/11] Revert "Use gasFeeCap formula" This reverts commit 2260925ee1fd4c0b4613559d6946f59882401708. --- zetaclient/chains/evm/signer/gas.go | 51 +++++------------- zetaclient/chains/evm/signer/gas_test.go | 53 +++++++++---------- zetaclient/chains/evm/signer/outbound_data.go | 6 +-- .../chains/evm/signer/outbound_data_test.go | 4 +- zetaclient/chains/evm/signer/signer.go | 32 +++++------ zetaclient/chains/evm/signer/signer_test.go | 9 ++-- 6 files changed, 62 insertions(+), 93 deletions(-) diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go index 08eb0c7f3f..2479e5a9f6 100644 --- a/zetaclient/chains/evm/signer/gas.go +++ b/zetaclient/chains/evm/signer/gas.go @@ -4,7 +4,6 @@ import ( "fmt" "math/big" - "cosmossdk.io/math" "github.com/pkg/errors" "github.com/rs/zerolog" @@ -25,58 +24,34 @@ const ( // // However, this doesn't affect tx creation nor broadcasting type Gas struct { - limit uint64 + Limit uint64 // This is a "total" gasPrice per 1 unit of gas. - price *big.Int + Price *big.Int // PriorityFee a fee paid directly to validators. - priorityFee *big.Int + PriorityFee *big.Int } func (g Gas) validate() error { switch { - case g.limit == 0: + case g.Limit == 0: return errors.New("gas limit is zero") - case g.price == nil: - return errors.New("gas price per unit is nil") - case g.priorityFee == nil: + case g.Price == nil: + return errors.New("max fee per unit is nil") + case g.PriorityFee == nil: return errors.New("priority fee per unit is nil") default: return nil } } -// IsLegacy determines whether the gas is meant for LegacyTx{} (pre EIP-1559) +// isLegacy determines whether the gas is meant for LegacyTx{} (pre EIP-1559) // or DynamicFeeTx{} (post EIP-1559). // // Returns true if priority fee is <= 0. -func (g Gas) IsLegacy() bool { - return g.priorityFee.Sign() < 1 -} - -func (g Gas) Limit() uint64 { - return g.limit -} - -// GasPrice returns the gas price for legacy transactions. -func (g Gas) GasPrice() *big.Int { - return g.price -} - -// PriorityFee returns the priority fee for EIP-1559 transactions. -func (g Gas) PriorityFee() *big.Int { - return g.priorityFee -} - -// GasFeeCap returns the gas fee cap for EIP-1559 transactions. -// heuristic of `2*baseFee + gasTipCap` is used. And because baseFee = `gasPrice - priorityFee`, -// it's `2*(gasPrice - priorityFee) + priorityFee` => `2*gasPrice - priorityFee` -func (g Gas) GasFeeCap() *big.Int { - return math.NewUintFromBigInt(g.price). - MulUint64(2). - Sub(math.NewUintFromBigInt(g.priorityFee)). - BigInt() +func (g Gas) isLegacy() bool { + return g.PriorityFee.Sign() < 1 } func gasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { @@ -114,9 +89,9 @@ func gasFromCCTX(cctx *types.CrossChainTx, logger zerolog.Logger) (Gas, error) { } return Gas{ - limit: limit, - price: gasPrice, - priorityFee: priorityFee, + Limit: limit, + Price: gasPrice, + PriorityFee: priorityFee, }, nil } diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go index 2f2d01581d..21d003769c 100644 --- a/zetaclient/chains/evm/signer/gas_test.go +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -9,7 +9,7 @@ import ( "github.com/zeta-chain/zetacore/x/crosschain/types" ) -func TestGasFromCCTX(t *testing.T) { +func Test_makeGasFromCCTX(t *testing.T) { logger := zerolog.New(zerolog.NewTestWriter(t)) makeCCTX := func(gasLimit uint64, price, priorityFee string) *types.CrossChainTx { @@ -32,11 +32,11 @@ func TestGasFromCCTX(t *testing.T) { name: "legacy: gas is too low", cctx: makeCCTX(minGasLimit-200, gwei(2).String(), ""), assert: func(t *testing.T, g Gas) { - assert.True(t, g.IsLegacy()) + assert.True(t, g.isLegacy()) assertGasEquals(t, Gas{ - limit: minGasLimit, - price: gwei(2), - priorityFee: gwei(0), + Limit: minGasLimit, + PriorityFee: gwei(0), + Price: gwei(2), }, g) }, }, @@ -44,27 +44,23 @@ func TestGasFromCCTX(t *testing.T) { name: "london: gas is too low", cctx: makeCCTX(minGasLimit-200, gwei(2).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.IsLegacy()) + assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ - limit: minGasLimit, - price: gwei(2), - priorityFee: gwei(1), + Limit: minGasLimit, + Price: gwei(2), + PriorityFee: gwei(1), }, g) - - // gasPrice=2, priorityFee=1, so baseFee=1 - // gasFeeCap = 2*baseFee + priorityFee = 2 + 1 = 3 - assert.Equal(t, gwei(3).Int64(), g.GasFeeCap().Int64()) }, }, { name: "pre London gas logic", cctx: makeCCTX(minGasLimit+100, gwei(3).String(), ""), assert: func(t *testing.T, g Gas) { - assert.True(t, g.IsLegacy()) + assert.True(t, g.isLegacy()) assertGasEquals(t, Gas{ - limit: 100_100, - price: gwei(3), - priorityFee: gwei(0), + Limit: 100_100, + Price: gwei(3), + PriorityFee: gwei(0), }, g) }, }, @@ -72,11 +68,11 @@ func TestGasFromCCTX(t *testing.T) { name: "post London gas logic", cctx: makeCCTX(minGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.IsLegacy()) + assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ - limit: 100_200, - price: gwei(4), - priorityFee: gwei(1), + Limit: 100_200, + Price: gwei(4), + PriorityFee: gwei(1), }, g) }, }, @@ -84,11 +80,11 @@ func TestGasFromCCTX(t *testing.T) { name: "gas is too high, force to the ceiling", cctx: makeCCTX(maxGasLimit+200, gwei(4).String(), gwei(1).String()), assert: func(t *testing.T, g Gas) { - assert.False(t, g.IsLegacy()) + assert.False(t, g.isLegacy()) assertGasEquals(t, Gas{ - limit: maxGasLimit, - price: gwei(4), - priorityFee: gwei(1), + Limit: maxGasLimit, + Price: gwei(4), + PriorityFee: gwei(1), }, g) }, }, @@ -127,10 +123,9 @@ func TestGasFromCCTX(t *testing.T) { } func assertGasEquals(t *testing.T, expected, actual Gas) { - assert.Equal(t, int64(expected.Limit()), int64(actual.Limit()), "gas limit") - assert.Equal(t, expected.GasPrice().Int64(), actual.GasPrice().Int64(), "gas price") - assert.Equal(t, expected.GasFeeCap().Int64(), actual.GasFeeCap().Int64(), "max fee cap") - assert.Equal(t, expected.PriorityFee().Int64(), actual.PriorityFee().Int64(), "priority fee") + assert.Equal(t, int64(expected.Limit), int64(actual.Limit), "gas limit") + assert.Equal(t, expected.Price.Int64(), actual.Price.Int64(), "max fee per unit") + assert.Equal(t, expected.PriorityFee.Int64(), actual.PriorityFee.Int64(), "priority fee per unit") } func gwei(i int64) *big.Int { diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index 22a814c8fd..399f4e00e0 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -95,14 +95,14 @@ func NewOutboundData( Uint64("cctx.pending_tx_nonce", nonce) // new gas price is less or equal to pending tx gas - if gas.GasPrice().Cmp(tx.GasPrice()) <= 0 { + if gas.Price.Cmp(tx.GasPrice()) <= 0 { evt.Msg("Please wait for pending outbound to be included in the block") return nil, true, nil } evt. - Uint64("cctx.gas_price", gas.GasPrice().Uint64()). - Uint64("cctx.priority_fee", gas.PriorityFee().Uint64()). + Uint64("cctx.gas_price", gas.Price.Uint64()). + Uint64("cctx.priority_fee", gas.PriorityFee.Uint64()). Msg("Replacing pending outbound transaction with higher gas fees") } diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index 1ffc405f65..03c1329ef1 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/zetaclient/chains/evm/signer/outbound_data_test.go @@ -49,8 +49,8 @@ func TestNewOutboundData(t *testing.T) { assert.NotEmpty(t, out.nonce) assert.NotEmpty(t, out.height) assert.NotEmpty(t, out.gas) - assert.True(t, out.gas.IsLegacy()) - assert.Equal(t, uint64(minGasLimit), out.gas.Limit()) + assert.True(t, out.gas.isLegacy()) + assert.Equal(t, uint64(minGasLimit), out.gas.Limit) assert.Empty(t, out.message) assert.NotEmpty(t, out.cctxIndex) diff --git a/zetaclient/chains/evm/signer/signer.go b/zetaclient/chains/evm/signer/signer.go index b073433da1..ed4c04ed81 100644 --- a/zetaclient/chains/evm/signer/signer.go +++ b/zetaclient/chains/evm/signer/signer.go @@ -217,13 +217,13 @@ func newTx( return nil, errors.Wrap(err, "invalid gas parameters") } - if gas.IsLegacy() { + if gas.isLegacy() { return ethtypes.NewTx(ðtypes.LegacyTx{ To: &to, Value: amount, Data: data, - GasPrice: gas.GasPrice(), - Gas: gas.Limit(), + GasPrice: gas.Price, + Gas: gas.Limit, Nonce: nonce, }), nil } @@ -233,9 +233,9 @@ func newTx( To: &to, Value: amount, Data: data, - GasFeeCap: gas.GasFeeCap(), - GasTipCap: gas.PriorityFee(), - Gas: gas.Limit(), + GasFeeCap: gas.Price, + GasTipCap: gas.PriorityFee, + Gas: gas.Limit, Nonce: nonce, }), nil } @@ -331,7 +331,7 @@ func (signer *Signer) SignRevertTx(ctx context.Context, txData *OutboundData) (* // SignCancelTx signs a transaction from TSS address to itself with a zero amount in order to increment the nonce func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - txData.gas.limit = evm.EthTransferGasLimit + txData.gas.Limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, @@ -350,7 +350,7 @@ func (signer *Signer) SignCancelTx(ctx context.Context, txData *OutboundData) (* // SignWithdrawTx signs a withdrawal transaction sent from the TSS address to the destination func (signer *Signer) SignWithdrawTx(ctx context.Context, txData *OutboundData) (*ethtypes.Transaction, error) { - txData.gas.limit = evm.EthTransferGasLimit + txData.gas.Limit = evm.EthTransferGasLimit tx, _, _, err := signer.Sign( ctx, nil, @@ -507,7 +507,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -516,7 +516,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) case coin.CoinType_Zeta: @@ -525,7 +525,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignOutbound(ctx, txData) } @@ -540,7 +540,7 @@ func (signer *Signer) TryProcessOutbound( "SignRevertTx: %d => %d, nonce %d, gasPrice %d", cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -551,7 +551,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignWithdrawTx(ctx, txData) case coin.CoinType_ERC20: @@ -559,7 +559,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignERC20WithdrawTx(ctx, txData) } @@ -573,7 +573,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -589,7 +589,7 @@ func (signer *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain.ID(), cctx.GetCurrentOutboundParam().TssNonce, - txData.gas.GasPrice(), + txData.gas.Price, ) tx, err = signer.SignOutbound(ctx, txData) if err != nil { diff --git a/zetaclient/chains/evm/signer/signer_test.go b/zetaclient/chains/evm/signer/signer_test.go index 65edb51cfe..9233dfcbe2 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -227,10 +227,9 @@ func TestSigner_SignOutbound(t *testing.T) { t.Run("SignOutbound - should successfully sign DynamicFeeTx", func(t *testing.T) { // ARRANGE const ( - gwei = 1_000_000_000 - priorityFee = 1 * gwei - gasPrice = 3 * gwei - londonGasFeeCap = 2*gasPrice - priorityFee + gwei = 1_000_000_000 + priorityFee = 1 * gwei + gasPrice = 3 * gwei ) // Given a CCTX with gas price and priority fee @@ -257,7 +256,7 @@ func TestSigner_SignOutbound(t *testing.T) { assert.Equal(t, ethtypes.DynamicFeeTxType, int(tx.Type())) // check that the gasPrice & priorityFee are set correctly - assert.Equal(t, int64(londonGasFeeCap), tx.GasFeeCap().Int64()) + assert.Equal(t, int64(gasPrice), tx.GasFeeCap().Int64()) assert.Equal(t, int64(priorityFee), tx.GasTipCap().Int64()) }) } From 086c9af4236a1a0cf6e846e7e6df933c9fe88b15 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 6 Aug 2024 17:58:05 +0200 Subject: [PATCH 10/11] Address PR comments --- zetaclient/chains/evm/signer/gas.go | 9 ++++++++- zetaclient/chains/evm/signer/gas_test.go | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go index 2479e5a9f6..575f004eb7 100644 --- a/zetaclient/chains/evm/signer/gas.go +++ b/zetaclient/chains/evm/signer/gas.go @@ -27,9 +27,10 @@ type Gas struct { Limit uint64 // This is a "total" gasPrice per 1 unit of gas. + // GasPrice for pre EIP-1559 transactions or maxFeePerGas for EIP-1559. Price *big.Int - // PriorityFee a fee paid directly to validators. + // PriorityFee a fee paid directly to validators for EIP-1559. PriorityFee *big.Int } @@ -41,6 +42,12 @@ func (g Gas) validate() error { return errors.New("max fee per unit is nil") case g.PriorityFee == nil: return errors.New("priority fee per unit is nil") + case g.Price.Cmp(g.PriorityFee) == -1: + return fmt.Errorf( + "max fee per unit (%d) is less than priority fee per unit (%d)", + g.Price.Int64(), + g.PriorityFee.Int64(), + ) default: return nil } diff --git a/zetaclient/chains/evm/signer/gas_test.go b/zetaclient/chains/evm/signer/gas_test.go index 21d003769c..d68d8c26af 100644 --- a/zetaclient/chains/evm/signer/gas_test.go +++ b/zetaclient/chains/evm/signer/gas_test.go @@ -9,7 +9,7 @@ import ( "github.com/zeta-chain/zetacore/x/crosschain/types" ) -func Test_makeGasFromCCTX(t *testing.T) { +func TestGasFromCCTX(t *testing.T) { logger := zerolog.New(zerolog.NewTestWriter(t)) makeCCTX := func(gasLimit uint64, price, priorityFee string) *types.CrossChainTx { @@ -27,7 +27,6 @@ func Test_makeGasFromCCTX(t *testing.T) { errorContains string assert func(t *testing.T, g Gas) }{ - { name: "legacy: gas is too low", cctx: makeCCTX(minGasLimit-200, gwei(2).String(), ""), @@ -117,9 +116,20 @@ func Test_makeGasFromCCTX(t *testing.T) { } assert.NoError(t, err) + assert.NoError(t, g.validate()) tt.assert(t, g) }) } + + t.Run("empty priority fee", func(t *testing.T) { + gas := Gas{ + Limit: 123_000, + Price: gwei(4), + PriorityFee: nil, + } + + assert.Error(t, gas.validate()) + }) } func assertGasEquals(t *testing.T, expected, actual Gas) { From fdc20aded3a74a212257d229d40f81d0249a3383 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Tue, 6 Aug 2024 18:20:04 +0200 Subject: [PATCH 11/11] Fix e2e upgrade tests --- e2e/e2etests/test_eth_withdraw.go | 8 ++++++-- e2e/runner/runner.go | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/e2e/e2etests/test_eth_withdraw.go b/e2e/e2etests/test_eth_withdraw.go index aa7c0dc3f6..2d43bd095d 100644 --- a/e2e/e2etests/test_eth_withdraw.go +++ b/e2e/e2etests/test_eth_withdraw.go @@ -39,8 +39,12 @@ func TestEtherWithdraw(r *runner.E2ERunner, args []string) { utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_OutboundMined) - withdrawalReceipt := mustFetchEthReceipt(r, cctx) - require.Equal(r, uint8(ethtypes.DynamicFeeTxType), withdrawalReceipt.Type, "receipt type mismatch") + // Previous binary doesn't take EIP-1559 into account, so this will fail. + // Thus, we need to skip this check for upgrade tests + if !r.IsRunningUpgrade() { + withdrawalReceipt := mustFetchEthReceipt(r, cctx) + require.Equal(r, uint8(ethtypes.DynamicFeeTxType), withdrawalReceipt.Type, "receipt type mismatch") + } r.Logger.Info("TestEtherWithdraw completed") } diff --git a/e2e/runner/runner.go b/e2e/runner/runner.go index 2ff538584f..1dfe0d3fb8 100644 --- a/e2e/runner/runner.go +++ b/e2e/runner/runner.go @@ -42,6 +42,13 @@ import ( type E2ERunnerOption func(*E2ERunner) +// Important ENV +const ( + EnvKeyLocalnetMode = "LOCALNET_MODE" + + LocalnetModeUpgrade = "upgrade" +) + func WithZetaTxServer(txServer *txserver.ZetaTxServer) E2ERunnerOption { return func(r *E2ERunner) { r.ZetaTxServer = txServer @@ -322,6 +329,11 @@ func (r *E2ERunner) PrintContractAddresses() { r.Logger.Print("TestDappEVM: %s", r.EvmTestDAppAddr.Hex()) } +// IsRunningUpgrade returns true if the test is running an upgrade test suite. +func (r *E2ERunner) IsRunningUpgrade() bool { + return os.Getenv(EnvKeyLocalnetMode) == LocalnetModeUpgrade +} + // Errorf logs an error message. Mimics the behavior of testing.T.Errorf func (r *E2ERunner) Errorf(format string, args ...any) { r.Logger.Error(format, args...)