From f5767337247828c651c5d5fd1cb79e9183fe0f22 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 26 Jun 2024 19:07:26 +0200 Subject: [PATCH] Refactor NewOutboundData. Reserve space for EIP-1559 --- zetaclient/chains/evm/signer/gas.go | 47 +++ zetaclient/chains/evm/signer/outbound_data.go | 301 +++++++++++------- .../chains/evm/signer/outbound_data_test.go | 163 ++++++---- zetaclient/chains/evm/signer/signer.go | 180 ++++++----- zetaclient/chains/evm/signer/signer_test.go | 30 +- 5 files changed, 445 insertions(+), 276 deletions(-) create mode 100644 zetaclient/chains/evm/signer/gas.go diff --git a/zetaclient/chains/evm/signer/gas.go b/zetaclient/chains/evm/signer/gas.go new file mode 100644 index 0000000000..ad7c5f3294 --- /dev/null +++ b/zetaclient/chains/evm/signer/gas.go @@ -0,0 +1,47 @@ +package signer + +import ( + "math/big" + + "github.com/pkg/errors" +) + +// 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) +type gas struct { + Limit uint64 + + // MaxFeePerUnit absolute maximum we're willing to pay per unit of gas to get tx included in a block. + MaxFeePerUnit *big.Int + + // PriorityFeePerUnit optional fee paid directly to validators. + PriorityFeePerUnit *big.Int +} + +func (g gas) validate() error { + if g.Limit == 0 { + return errors.New("gas limit is zero") + } + + if g.MaxFeePerUnit == nil { + return errors.New("max fee per unit is nil") + } + + if g.PriorityFeePerUnit == nil { + return errors.New("priority fee per unit is nil") + } + + return nil +} + +// isLegacy determines whether this 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.PriorityFeePerUnit.Sign() <= 1 +} diff --git a/zetaclient/chains/evm/signer/outbound_data.go b/zetaclient/chains/evm/signer/outbound_data.go index 070c73597e..877bc4e0d4 100644 --- a/zetaclient/chains/evm/signer/outbound_data.go +++ b/zetaclient/chains/evm/signer/outbound_data.go @@ -1,21 +1,19 @@ package signer import ( - "context" "encoding/base64" "encoding/hex" - "errors" "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" ) const ( @@ -27,16 +25,19 @@ const ( // 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 @@ -45,65 +46,6 @@ 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 chains.IsEthereumChain(chain.ChainId) { - suggested, err := client.SuggestGasPrice(context.Background()) - if err != nil { - return errors.Join(err, fmt.Errorf("cannot get gas price from chain %s ", chain)) - } - 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. @@ -113,75 +55,198 @@ func (txData *OutboundData) SetupGas( func NewOutboundData( cctx *types.CrossChainTx, evmObserver *observer.Observer, - evmRPC interfaces.EVMRPCClient, - logger zerolog.Logger, 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 + outboundParams := cctx.GetCurrentOutboundParam() + if outboundParams == nil { + return nil, false, errors.New("outboundParams is nil") } - toChain := chains.GetChainFromChainID(txData.toChainID.Int64()) - if toChain == nil { - return nil, true, fmt.Errorf("unknown chain: %d", txData.toChainID.Int64()) + // Check if the CCTX has already been processed + alreadyIncluded, alreadyConfirmed, err := evmObserver.IsOutboundProcessed(cctx, logger) + switch { + case err != nil: + return nil, true, errors.Wrap(err, "failed to check if outbound is processed") + case alreadyIncluded || alreadyConfirmed: + logger.Info().Msgf("CCTX already processed; skipping") + return nil, true, nil } - // Get nonce, Early return if the cctx is already processed - nonce := cctx.GetCurrentOutboundParam().TssNonce - included, confirmed, err := evmObserver.IsOutboundProcessed(cctx, logger) - if err != nil { - return nil, true, errors.New("IsOutboundProcessed failed") - } - if included || confirmed { - logger.Info().Msgf("CCTX already processed; exit signer") + // Determine destination chain and address + to, toChainID, skip := determineDestination(cctx, logger) + if skip { return nil, true, nil } - // Set up gas limit and gas price - err = txData.SetupGas(cctx, logger, evmRPC, toChain) - if err != nil { - return nil, true, err + toChain := chains.GetChainFromChainID(toChainID.Int64()) + if toChain == nil { + return nil, false, fmt.Errorf("unknown chain: %d", toChainID.Int64()) } + nonce := outboundParams.TssNonce + // Get sendHash - logger.Info(). - Msgf("chain %s minting %d to %s, nonce %d, finalized zeta bn %d", toChain, 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) + cctxIndex, err := getCCTXIndex(cctx) + if err != nil { + return nil, false, err + } + + // Determine gas fees + gas, err := determineGas(cctx, logger) + if err != nil { + return nil, false, errors.Wrap(err, "failed to determine gas fees") } - 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) + if tx := evmObserver.GetPendingTx(nonce); tx != nil { + newFeeIsLessThanPending := gas.MaxFeePerUnit.Cmp(tx.GasPrice()) <= 0 + + evt := logger.Info(). + Str("cctx.pending_tx_hash", tx.Hash().Hex()). + Uint64("cctx.pending_tx_nonce", nonce) + + if newFeeIsLessThanPending { + evt.Msg("Please wait for pending outbound to be included in the block") return nil, true, nil } + + evt. + Uint64("cctx.max_gas_fee", gas.MaxFeePerUnit.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 + logger.Info(). + Str("cctx.to_chain_name", toChain.GetChainName().String()). + Int64("cctx.to_chain_id", toChain.ChainId). + Str("cctx.to_recipient", to.Hex()). + Uint64("cctx.nonce", nonce). + Uint64("cctx.finalized_zeta_height", cctx.InboundParams.FinalizedZetaHeight). + Msg("Constructed OutboundData") + + return &OutboundData{ + outboundParams: outboundParams, + + 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, + }, 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 determineGas(cctx *types.CrossChainTx, logger zerolog.Logger) (gas, error) { + var ( + outboundParams = cctx.GetCurrentOutboundParam() + limit = outboundParams.GasLimit + ) + + switch { + case limit < MinGasLimit: + limit = MinGasLimit + logger.Warn(). + Uint64("cctx.initial_gas_limit", outboundParams.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", outboundParams.GasLimit). + Uint64("cctx.gas_limit", limit). + Msgf("Gas limit is too high; Setting to the maximum (%d)", MaxGasLimit) + } + + maxFee, ok := new(big.Int).SetString(outboundParams.GasPrice, 10) + if !ok { + return gas{}, errors.New("unable to parse gasPrice from " + outboundParams.GasPrice) + } + + // TODO RELY ONLY ON gas{} data. + // 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 chains.IsEthereumChain(chain.ChainId) { + // suggested, err := client.SuggestGasPrice(context.Background()) + // if err != nil { + // return errors.Join(err, fmt.Errorf("cannot get gas price from chain %s ", chain)) + // } + // txData.gasPrice = roundUpToNearestGwei(suggested) + // } else { + // return fmt.Errorf("cannot convert gas price %s ", cctx.GetCurrentOutboundParam().GasPrice) + // } + //} else { + // txData.gasPrice = specified + //} + + return gas{ + Limit: limit, + MaxFeePerUnit: maxFee, + PriorityFeePerUnit: big.NewInt(0), // todo! + }, nil } diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index d7df5a33d1..482c0dbf7b 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/zetaclient/chains/evm/signer/outbound_data_test.go @@ -6,102 +6,131 @@ import ( ethcommon "github.com/ethereum/go-ethereum/common" "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "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{} +func TestNewOutboundData(t *testing.T) { + // Setup evm signer + mockObserver, err := getNewEvmChainObserver(t, nil) + require.NoError(t, err) + logger := zerolog.Logger{} - t.Run("SetChainAndSender PendingRevert", func(t *testing.T) { - cctx.CctxStatus.Status = types.CctxStatus_PendingRevert - skipTx := txData.SetChainAndSender(cctx, logger) + newOutbound := func(cctx *types.CrossChainTx) (*OutboundData, bool, error) { + return NewOutboundData(cctx, mockObserver, 123, logger) + } - require.False(t, skipTx) - require.Equal(t, ethcommon.HexToAddress(cctx.InboundParams.Sender), txData.to) - require.Equal(t, big.NewInt(cctx.InboundParams.SenderChainId), txData.toChainID) - }) + t.Run("success", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) - t.Run("SetChainAndSender PendingOutbound", func(t *testing.T) { - cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - skipTx := txData.SetChainAndSender(cctx, logger) + // ACT + out, skip, err := newOutbound(cctx) - require.False(t, skipTx) - require.Equal(t, ethcommon.HexToAddress(cctx.GetCurrentOutboundParam().Receiver), txData.to) - require.Equal(t, big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId), txData.toChainID) - }) + // ASSERT + assert.NoError(t, err) + assert.False(t, skip) - 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) - }) -} + assert.NotEmpty(t, out) -func TestSigner_SetupGas(t *testing.T) { - cctx := getCCTX(t) - evmSigner, err := getNewEvmSigner(nil) - require.NoError(t, err) + assert.NotEmpty(t, out.srcChainID) + assert.NotEmpty(t, out.sender) - txData := &OutboundData{} - logger := zerolog.Logger{} + assert.NotEmpty(t, out.toChainID) + assert.NotEmpty(t, out.to) - t.Run("SetupGas_success", func(t *testing.T) { - chain := chains.BscMainnet - err := txData.SetupGas(cctx, logger, evmSigner.EvmClient(), &chain) - require.NoError(t, err) + 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) }) - 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") + t.Run("pending revert", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) + cctx.CctxStatus.Status = types.CctxStatus_PendingRevert + + // ACT + out, skip, err := newOutbound(cctx) + + // ASSERT + assert.NoError(t, err) + assert.False(t, skip) + assert.Equal(t, ethcommon.HexToAddress(cctx.InboundParams.Sender), out.to) + require.Equal(t, big.NewInt(cctx.InboundParams.SenderChainId), out.toChainID) }) -} -func TestSigner_NewOutboundData(t *testing.T) { - // Setup evm signer - evmSigner, err := getNewEvmSigner(nil) - require.NoError(t, err) + t.Run("pending outbound", func(t *testing.T) { + // ARRANGE + cctx := getCCTX(t) + cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound - mockObserver, err := getNewEvmChainObserver(t, nil) - require.NoError(t, err) + // ACT + out, skip, err := newOutbound(cctx) - t.Run("NewOutboundData success", func(t *testing.T) { + // 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("skip inbound", func(t *testing.T) { + // ARRANGE cctx := getCCTX(t) - _, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - require.False(t, skip) - require.NoError(t, err) + cctx.CctxStatus.Status = types.CctxStatus_PendingInbound + + // ACT + _, skip, err := newOutbound(cctx) + + // ASSERT + assert.NoError(t, err) + assert.True(t, skip) }) - t.Run("NewOutboundData skip", func(t *testing.T) { + t.Run("skip aborted", func(t *testing.T) { + // ARRANGE cctx := getCCTX(t) cctx.CctxStatus.Status = types.CctxStatus_Aborted - _, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - require.NoError(t, err) - require.True(t, skip) - }) - t.Run("NewOutboundData unknown chain", func(t *testing.T) { - cctx := getInvalidCCTX(t) - require.NoError(t, err) - _, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - require.ErrorContains(t, err, "unknown chain") - require.True(t, skip) + // ACT + _, skip, err := newOutbound(cctx) + + // ASSERT + assert.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) - require.NoError(t, err) cctx.GetCurrentOutboundParam().GasPrice = "invalidGasPrice" - _, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) - require.True(t, skip) - require.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, "unknown chain") }) } diff --git a/zetaclient/chains/evm/signer/signer.go b/zetaclient/chains/evm/signer/signer.go index 56d27e2834..cfef82c597 100644 --- a/zetaclient/chains/evm/signer/signer.go +++ b/zetaclient/chains/evm/signer/signer.go @@ -15,6 +15,7 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethclient" + "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/zeta-chain/protocol-contracts/pkg/contracts/evm/erc20custody.sol" @@ -150,22 +151,26 @@ func (s *Signer) GetERC20CustodyAddress() ethcommon.Address { return s.er20CustodyAddress } -// Sign given data, and metadata (gas, nonce, etc) +// Sign given data, and metadata (gas, nonce, etc.) // returns a signed transaction, sig bytes, hash bytes, and error func (s *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().Msgf("Sign: TSS signer: %s", s.TSS().Pubkey()) + s.Logger().Debug(). + Str("tss_pub_key", string(s.TSS().Pubkey())). + Msg("Signing evm transaction") + + chainID := big.NewInt(s.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 := s.ethSigner.Hash(tx).Bytes() sig, err := s.TSS().Sign(hashBytes, height, nonce, s.Chain().ChainId, "") @@ -176,11 +181,11 @@ func (s *Signer) Sign( log.Debug().Msgf("Sign: Signature: %s", hex.EncodeToString(sig[:])) pubk, err := crypto.SigToPub(hashBytes, sig[:]) if err != nil { - s.Logger().Std.Error().Err(err).Msgf("SigToPub error") + s.Logger().Error().Err(err).Msgf("SigToPub error") } addr := crypto.PubkeyToAddress(*pubk) - s.Logger().Std.Info().Msgf("Sign: Ecrecovery of signature: %s", addr.Hex()) + s.Logger().Info().Msgf("Sign: Ecrecovery of signature: %s", addr.Hex()) signedTX, err := tx.WithSignature(s.ethSigner, sig[:]) if err != nil { return nil, nil, nil, err @@ -189,6 +194,41 @@ func (s *Signer) Sign( return signedTX, sig[:], hashBytes[:], nil } +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.MaxFeePerUnit, + Gas: gas.Limit, + Nonce: nonce, + }), nil + } + + return ethtypes.NewTx(ðtypes.DynamicFeeTx{ + ChainID: chainID, + To: &to, + Value: amount, + Data: data, + GasFeeCap: gas.MaxFeePerUnit, + GasTipCap: gas.PriorityFeePerUnit, + Gas: gas.Limit, + Nonce: nonce, + }), nil +} + // Broadcast takes in signed tx, broadcast to external chain node func (s *Signer) Broadcast(tx *ethtypes.Transaction) error { ctxt, cancel := context.WithTimeout(context.Background(), 1*time.Second) @@ -208,29 +248,28 @@ func (s *Signer) Broadcast(tx *ethtypes.Transaction) error { // // ) external virtual {} func (s *Signer) SignOutbound(txData *OutboundData) (*ethtypes.Transaction, error) { - var data []byte - var err error - - data, err = s.zetaConnectorABI.Pack("onReceive", + data, err := s.zetaConnectorABI.Pack("onReceive", txData.sender.Bytes(), txData.srcChainID, txData.to, txData.amount, txData.message, txData.cctxIndex) + if err != nil { return nil, fmt.Errorf("onReceive pack error: %w", err) } - tx, _, _, err := s.Sign(data, + tx, _, _, err := s.Sign( + data, s.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) + return nil, errors.Wrap(err, "unable to sign onReceive") } return tx, nil @@ -247,10 +286,7 @@ func (s *Signer) SignOutbound(txData *OutboundData) (*ethtypes.Transaction, erro // bytes32 internalSendHash // ) external override whenNotPaused onlyTssAddress func (s *Signer) SignRevertTx(txData *OutboundData) (*ethtypes.Transaction, error) { - var data []byte - var err error - - data, err = s.zetaConnectorABI.Pack("onRevert", + data, err := s.zetaConnectorABI.Pack("onRevert", txData.sender, txData.srcChainID, txData.to.Bytes(), @@ -262,15 +298,16 @@ func (s *Signer) SignRevertTx(txData *OutboundData) (*ethtypes.Transaction, erro return nil, fmt.Errorf("onRevert pack error: %w", err) } - tx, _, _, err := s.Sign(data, + tx, _, _, err := s.Sign( + data, s.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) + return nil, errors.Wrap(err, "unable to sign onRevert") } return tx, nil @@ -278,17 +315,18 @@ func (s *Signer) SignRevertTx(txData *OutboundData) (*ethtypes.Transaction, erro // SignCancelTx signs a transaction from TSS address to itself with a zero amount in order to increment the nonce func (s *Signer) SignCancelTx(txData *OutboundData) (*ethtypes.Transaction, error) { + // todo LIMIT=evm.EthTransferGasLimit + tx, _, _, err := s.Sign( nil, s.TSS().EVMAddress(), zeroValue, // zero out the amount to cancel the tx - evm.EthTransferGasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) if err != nil { - return nil, fmt.Errorf("SignCancelTx error: %w", err) + return nil, errors.Wrap(err, "unable to sign cancellation tx") } return tx, nil @@ -296,17 +334,18 @@ func (s *Signer) SignCancelTx(txData *OutboundData) (*ethtypes.Transaction, erro // SignWithdrawTx signs a withdrawal transaction sent from the TSS address to the destination func (s *Signer) SignWithdrawTx(txData *OutboundData) (*ethtypes.Transaction, error) { + // todo LIMIT=evm.EthTransferGasLimit + tx, _, _, err := s.Sign( nil, txData.to, txData.amount, - evm.EthTransferGasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) if err != nil { - return nil, fmt.Errorf("SignWithdrawTx error: %w", err) + return nil, errors.Wrap(err, "unable to sign withdraw tx") } return tx, nil @@ -337,21 +376,20 @@ func (s *Signer) TryProcessOutbound( zetacoreClient interfaces.ZetacoreClient, height uint64, ) { - logger := s.Logger().Std.With(). - Str("outboundID", outboundID). - Str("SendHash", cctx.Index). + logger := s.Logger().With(). + Str("outbound_id", outboundID). + Str("cctx.index", cctx.Index). Logger() - logger.Info().Msgf("start processing outboundID %s", outboundID) - logger.Info().Msgf( - "EVM Chain TryProcessOutbound: %s, value %d to %s", - cctx.Index, - cctx.GetCurrentOutboundParam().Amount.BigInt(), - cctx.GetCurrentOutboundParam().Receiver, - ) - defer func() { - outboundProc.EndTryProcess(outboundID) - }() + params := cctx.GetCurrentOutboundParam() + + logger.Info(). + Str("cctx.amount", params.Amount.String()). + Str("cctx.receiver", params.Receiver). + Msgf("start processing outbound") + + defer outboundProc.EndTryProcess(outboundID) + myID := zetacoreClient.GetKeys().GetOperatorAddress() evmObserver, ok := chainObserver.(*observer.Observer) @@ -361,7 +399,7 @@ func (s *Signer) TryProcessOutbound( } // Setup Transaction input - txData, skipTx, err := NewOutboundData(cctx, evmObserver, s.client, logger, height) + txData, skipTx, err := NewOutboundData(cctx, evmObserver, height, logger) if err != nil { logger.Err(err).Msg("error setting up transaction input fields") return @@ -425,7 +463,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignWithdrawTx(txData) case coin.CoinType_ERC20: @@ -434,7 +472,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignERC20WithdrawTx(txData) case coin.CoinType_Zeta: @@ -443,7 +481,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignOutbound(txData) } @@ -458,7 +496,7 @@ func (s *Signer) TryProcessOutbound( "SignRevertTx: %d => %s, nonce %d, gasPrice %d", cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -469,7 +507,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignWithdrawTx(txData) case coin.CoinType_ERC20: @@ -477,7 +515,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignERC20WithdrawTx(txData) } @@ -491,7 +529,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) txData.srcChainID = big.NewInt(cctx.OutboundParams[0].ReceiverChainId) txData.toChainID = big.NewInt(cctx.GetCurrentOutboundParam().ReceiverChainId) @@ -507,7 +545,7 @@ func (s *Signer) TryProcessOutbound( cctx.InboundParams.SenderChainId, toChain, cctx.GetCurrentOutboundParam().TssNonce, - txData.gasPrice, + txData.gas.MaxFeePerUnit, ) tx, err = s.SignOutbound(txData) if err != nil { @@ -586,9 +624,7 @@ func (s *Signer) BroadcastOutbound( // uint256 amount, // ) external onlyTssAddress func (s *Signer) SignERC20WithdrawTx(txData *OutboundData) (*ethtypes.Transaction, error) { - var data []byte - var err error - data, err = s.erc20CustodyABI.Pack("withdraw", txData.to, txData.asset, txData.amount) + data, err := s.erc20CustodyABI.Pack("withdraw", txData.to, txData.asset, txData.amount) if err != nil { return nil, fmt.Errorf("withdraw pack error: %w", err) } @@ -597,13 +633,12 @@ func (s *Signer) SignERC20WithdrawTx(txData *OutboundData) (*ethtypes.Transactio data, s.er20CustodyAddress, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) if err != nil { - return nil, fmt.Errorf("sign withdraw error: %w", err) + return nil, errors.Wrap(err, "unable to sign withdraw") } return tx, nil @@ -645,24 +680,27 @@ func ErrorMsg(cctx *types.CrossChainTx) string { func (s *Signer) SignWhitelistERC20Cmd(txData *OutboundData, params string) (*ethtypes.Transaction, error) { outboundParams := txData.outboundParams + erc20 := ethcommon.HexToAddress(params) 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 := s.Sign( data, txData.to, zeroValue, - txData.gasLimit, - txData.gasPrice, + txData.gas, outboundParams.TssNonce, txData.height, ) @@ -677,14 +715,14 @@ func (s *Signer) SignMigrateTssFundsCmd(txData *OutboundData) (*ethtypes.Transac nil, txData.to, txData.amount, - txData.gasLimit, - txData.gasPrice, + txData.gas, txData.nonce, txData.height, ) if err != nil { - return nil, fmt.Errorf("SignMigrateTssFundsCmd error: %w", err) + return nil, errors.Wrap(err, "unable to sign migrate tss funds") } + return tx, nil } @@ -820,13 +858,3 @@ func getEVMRPC(endpoint string) (interfaces.EVMRPCClient, ethtypes.Signer, error ethSigner := ethtypes.LatestSignerForChainID(chainID) return client, ethSigner, nil } - -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 ea27152b97..e2a55d603f 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -180,7 +180,7 @@ func TestSigner_SignOutbound(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -199,7 +199,7 @@ func TestSigner_SignOutbound(t *testing.T) { // Call SignOutbound tx, err := evmSigner.SignOutbound(txData) - require.ErrorContains(t, err, "sign onReceive error") + require.ErrorContains(t, err, "unable to sign onReceive") require.Nil(t, tx) }) } @@ -214,7 +214,7 @@ func TestSigner_SignRevertTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -237,7 +237,7 @@ func TestSigner_SignRevertTx(t *testing.T) { // Call SignRevertTx tx, err := evmSigner.SignRevertTx(txData) - require.ErrorContains(t, err, "sign onRevert error") + require.ErrorContains(t, err, "unable to sign onRevert") require.Nil(t, tx) }) } @@ -252,7 +252,7 @@ func TestSigner_SignCancelTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -275,7 +275,7 @@ func TestSigner_SignCancelTx(t *testing.T) { // Call SignCancelTx tx, err := evmSigner.SignCancelTx(txData) - require.ErrorContains(t, err, "SignCancelTx error") + require.ErrorContains(t, err, "unable to sign cancellation tx") require.Nil(t, tx) }) } @@ -290,7 +290,7 @@ func TestSigner_SignWithdrawTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -312,7 +312,7 @@ func TestSigner_SignWithdrawTx(t *testing.T) { // Call SignWithdrawTx tx, err := evmSigner.SignWithdrawTx(txData) - require.ErrorContains(t, err, "SignWithdrawTx error") + require.ErrorContains(t, err, "unable to sign withdraw tx") require.Nil(t, tx) }) } @@ -326,7 +326,7 @@ func TestSigner_SignCommandTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, nil) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -371,7 +371,7 @@ func TestSigner_SignERC20WithdrawTx(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -395,7 +395,7 @@ func TestSigner_SignERC20WithdrawTx(t *testing.T) { // Call SignERC20WithdrawTx tx, err := evmSigner.SignERC20WithdrawTx(txData) - require.ErrorContains(t, err, "sign withdraw error") + require.ErrorContains(t, err, "unable to sign withdraw") require.Nil(t, tx) }) } @@ -409,7 +409,7 @@ func TestSigner_BroadcastOutbound(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, nil) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -459,7 +459,7 @@ func TestSigner_SignWhitelistERC20Cmd(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -502,7 +502,7 @@ func TestSigner_SignMigrateTssFundsCmd(t *testing.T) { cctx := getCCTX(t) mockObserver, err := getNewEvmChainObserver(t, tss) require.NoError(t, err) - txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + txData, skip, err := NewOutboundData(cctx, mockObserver, 123, zerolog.Logger{}) require.False(t, skip) require.NoError(t, err) @@ -526,7 +526,7 @@ func TestSigner_SignMigrateTssFundsCmd(t *testing.T) { // Call SignMigrateTssFundsCmd tx, err := evmSigner.SignMigrateTssFundsCmd(txData) - require.ErrorContains(t, err, "SignMigrateTssFundsCmd error") + require.ErrorContains(t, err, "unable to sign migrate tss funds") require.Nil(t, tx) }) }