refactor: removed unused code and unified tx signing logic

changelog.md

@@ -40,6 +40,7 @@
 * [2262](https://github.com/zeta-chain/node/pull/2262) - refactor MsgUpdateZRC20 into MsgPauseZrc20 and MsgUnPauseZRC20
 * [2290](https://github.com/zeta-chain/node/pull/2290) - rename `MsgAddBlameVote` message to `MsgVoteBlame`
 * [2269](https://github.com/zeta-chain/node/pull/2269) - refactor MsgUpdateCrosschainFlags into MsgEnableCCTX, MsgDisableCCTX and MsgUpdateGasPriceIncreaseFlags
+* [2306](https://github.com/zeta-chain/node/pull/2306) - refactor zetaclient outbound transaction signing logic
 * [2296](https://github.com/zeta-chain/node/pull/2296) - move `testdata` package to `testutil` to organize test-related utilities
 
 ### Tests

pkg/constant/constant.go

@@ -4,8 +4,10 @@ const (
 	// DonationMessage is the message for donation transactions
 	// Transaction sent to the TSS or ERC20 Custody address containing this message are considered as a donation
 	DonationMessage = "I am rich!"
+
 	// CmdWhitelistERC20 is used for CCTX of type cmd to give the instruction to the TSS to whitelist an ERC20 on an exeternal chain
 	CmdWhitelistERC20 = "cmd_whitelist_erc20"
+
 	// CmdMigrateTssFunds is used for CCTX of type cmd to give the instruction to the TSS to transfer its funds on a new address
 	CmdMigrateTssFunds = "cmd_migrate_tss_funds"
 )

zetaclient/chains/evm/constant.go

@@ -12,6 +12,9 @@ const (
 	// OutboundTrackerReportTimeout is the timeout for waiting for an outbound tracker report
 	OutboundTrackerReportTimeout = 10 * time.Minute
 
+	// EthTransferGasLimit is the gas limit for a standard ETH transfer
+	EthTransferGasLimit = 21000
+
 	// TopicsZetaSent is the number of topics for a Zeta sent event
 	// [signature, zetaTxSenderAddress, destinationChainId]
 	// https://github.com/zeta-chain/protocol-contracts/blob/d65814debf17648a6c67d757ba03646415842790/contracts/evm/ZetaConnector.base.sol#L34

zetaclient/chains/evm/signer/outbound_data_test.go

@@ -45,7 +45,7 @@ func TestSigner_SetChainAndSender(t *testing.T) {
 
 func TestSigner_SetupGas(t *testing.T) {
 	cctx := getCCTX(t)
-	evmSigner, err := getNewEvmSigner()
+	evmSigner, err := getNewEvmSigner(nil)
 	require.NoError(t, err)
 
 	txData := &OutboundData{}
@@ -67,10 +67,10 @@ func TestSigner_SetupGas(t *testing.T) {
 
 func TestSigner_NewOutboundData(t *testing.T) {
 	// Setup evm signer
-	evmSigner, err := getNewEvmSigner()
+	evmSigner, err := getNewEvmSigner(nil)
 	require.NoError(t, err)
 
-	mockObserver, err := getNewEvmChainObserver()
+	mockObserver, err := getNewEvmChainObserver(nil)
 	require.NoError(t, err)
 
 	t.Run("NewOutboundData success", func(t *testing.T) {

zetaclient/chains/evm/signer/signer.go

@@ -39,7 +39,12 @@
 	"github.com/zeta-chain/zetacore/zetaclient/zetacore"
 )
 
-var _ interfaces.ChainSigner = &Signer{}
+var (
+	_ interfaces.ChainSigner = &Signer{}
+
+	// zeroValue is for outbounds that carry no ETH (gas token) value
+	zeroValue = big.NewInt(0)
+)
 
 // Signer deals with the signing EVM transactions and implements the ChainSigner interface
 type Signer struct {
@@ -138,6 +143,7 @@
 func (signer *Signer) Sign(
 	data []byte,
 	to ethcommon.Address,
+	amount *big.Int,
 	gasLimit uint64,
 	gasPrice *big.Int,
 	nonce uint64,
@@ -147,7 +153,7 @@
 
 	// TODO: use EIP-1559 transaction type
 	// https://github.com/zeta-chain/node/issues/1952
-	tx := ethtypes.NewTransaction(nonce, to, big.NewInt(0), gasLimit, gasPrice, data)
+	tx := ethtypes.NewTransaction(nonce, to, amount, gasLimit, gasPrice, data)
 
 	hashBytes := signer.ethSigner.Hash(tx).Bytes()
 
@@ -207,12 +213,13 @@
 
 	tx, _, _, err := signer.Sign(data,
 		signer.zetaConnectorAddress,
+		zeroValue,
 		txData.gasLimit,
 		txData.gasPrice,
 		txData.nonce,
 		txData.height)
 	if err != nil {
-		return nil, fmt.Errorf("onReceive sign error: %w", err)
+		return nil, fmt.Errorf("sign onReceive error: %w", err)
 	}
 
 	return tx, nil
@@ -241,74 +248,57 @@
 		txData.message,
 		txData.cctxIndex)
 	if err != nil {
-		return nil, fmt.Errorf("pack error: %w", err)
+		return nil, fmt.Errorf("onRevert pack error: %w", err)
 	}
 
 	tx, _, _, err := signer.Sign(data,
 		signer.zetaConnectorAddress,
+		zeroValue,
 		txData.gasLimit,
 		txData.gasPrice,
 		txData.nonce,
 		txData.height)
 	if err != nil {
-		return nil, fmt.Errorf("Sign error: %w", err)
+		return nil, fmt.Errorf("sign onRevert error: %w", err)
 	}
 
 	return tx, nil
 }
 
 // SignCancelTx signs a transaction from TSS address to itself with a zero amount in order to increment the nonce
-func (signer *Signer) SignCancelTx(nonce uint64, gasPrice *big.Int, height uint64) (*ethtypes.Transaction, error) {
-	// TODO: use EIP-1559 transaction type
-	// https://github.com/zeta-chain/node/issues/1952
-	tx := ethtypes.NewTransaction(nonce, signer.tssSigner.EVMAddress(), big.NewInt(0), 21000, gasPrice, nil)
-
-	hashBytes := signer.ethSigner.Hash(tx).Bytes()
-	sig, err := signer.tssSigner.Sign(hashBytes, height, nonce, signer.chain, "")
-	if err != nil {
-		return nil, err
-	}
-
-	pubk, err := crypto.SigToPub(hashBytes, sig[:])
-	if err != nil {
-		signer.logger.Std.Error().Err(err).Msgf("SigToPub error")
-	}
-
-	addr := crypto.PubkeyToAddress(*pubk)
-	signer.logger.Std.Info().Msgf("Sign: Ecrecovery of signature: %s", addr.Hex())
-	signedTX, err := tx.WithSignature(signer.ethSigner, sig[:])
+func (signer *Signer) SignCancelTx(txData *OutboundData) (*ethtypes.Transaction, error) {
+	tx, _, _, err := signer.Sign(
+		nil,
+		signer.tssSigner.EVMAddress(),
+		zeroValue, // zero out the amount to cancel the tx
+		evm.EthTransferGasLimit,
+		txData.gasPrice,
+		txData.nonce,
+		txData.height,
+	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("SignCancelTx error: %w", err)
 	}
 
-	return signedTX, nil
+	return tx, nil
 }
 
 // SignWithdrawTx signs a withdrawal transaction sent from the TSS address to the destination
 func (signer *Signer) SignWithdrawTx(txData *OutboundData) (*ethtypes.Transaction, error) {
-	// TODO: use EIP-1559 transaction type
-	// https://github.com/zeta-chain/node/issues/1952
-	tx := ethtypes.NewTransaction(txData.nonce, txData.to, txData.amount, 21000, txData.gasPrice, nil)
-
-	hashBytes := signer.ethSigner.Hash(tx).Bytes()
-	sig, err := signer.tssSigner.Sign(hashBytes, txData.height, txData.nonce, signer.chain, "")
-	if err != nil {
-		return nil, err
-	}
-
-	pubk, err := crypto.SigToPub(hashBytes, sig[:])
-	if err != nil {
-		signer.logger.Std.Error().Err(err).Msgf("SigToPub error")
-	}
-
-	addr := crypto.PubkeyToAddress(*pubk)
-	signer.logger.Std.Info().Msgf("Sign: Ecrecovery of signature: %s", addr.Hex())
-	signedTX, err := tx.WithSignature(signer.ethSigner, sig[:])
+	tx, _, _, err := signer.Sign(
+		nil,
+		txData.to,
+		txData.amount,
+		evm.EthTransferGasLimit,
+		txData.gasPrice,
+		txData.nonce,
+		txData.height,
+	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("SignWithdrawTx error: %w", err)
 	}
 
-	return signedTX, nil
+	return tx, nil
 }
 
 // SignCommandTx signs a transaction based on the given command includes:
@@ -389,7 +379,7 @@
 			cctx.GetCurrentOutboundParam().CoinType.String(),
 		)
 
-		tx, err = signer.SignCancelTx(txData.nonce, txData.gasPrice, height) // cancel the tx
+		tx, err = signer.SignCancelTx(txData) // cancel the tx
 		if err != nil {
 			logger.Warn().Err(err).Msg(ErrorMsg(cctx))
 			return
@@ -589,52 +579,20 @@
 	var err error
 	data, err = signer.erc20CustodyABI.Pack("withdraw", txData.to, txData.asset, txData.amount)
 	if err != nil {
-		return nil, fmt.Errorf("pack error: %w", err)
+		return nil, fmt.Errorf("withdraw pack error: %w", err)
 	}
 
 	tx, _, _, err := signer.Sign(
 		data,
 		signer.er20CustodyAddress,
+		zeroValue,
 		txData.gasLimit,
 		txData.gasPrice,
 		txData.nonce,
 		txData.height,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("sign error: %w", err)
-	}
-
-	return tx, nil
-}
-
-// SignWhitelistTx
-// function whitelist(
-// address asset,
-// ) external onlyTssAddress
-// function unwhitelist(
-// address asset,
-// ) external onlyTssAddress
-func (signer *Signer) SignWhitelistTx(
-	action string,
-	_ ethcommon.Address,
-	asset ethcommon.Address,
-	gasLimit uint64,
-	nonce uint64,
-	gasPrice *big.Int,
-	height uint64,
-) (*ethtypes.Transaction, error) {
-	var data []byte
-
-	var err error
-
-	data, err = signer.erc20CustodyABI.Pack(action, asset)
-	if err != nil {
-		return nil, fmt.Errorf("pack error: %w", err)
-	}
-
-	tx, _, _, err := signer.Sign(data, signer.er20CustodyAddress, gasLimit, gasPrice, nonce, height)
-	if err != nil {
-		return nil, fmt.Errorf("Sign error: %w", err)
+		return nil, fmt.Errorf("sign withdraw error: %w", err)
 	}
 
 	return tx, nil
@@ -686,34 +644,35 @@
 	}
 	data, err := custodyAbi.Pack("whitelist", erc20)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("whitelist pack error: %w", err)
 	}
 	tx, _, _, err := signer.Sign(
 		data,
 		txData.to,
+		zeroValue,
 		txData.gasLimit,
 		txData.gasPrice,
 		outboundParams.TssNonce,
 		txData.height,
 	)
 	if err != nil {
-		return nil, fmt.Errorf("sign error: %w", err)
+		return nil, fmt.Errorf("sign whitelist error: %w", err)
 	}
 	return tx, nil
 }
 
 func (signer *Signer) SignMigrateTssFundsCmd(txData *OutboundData) (*ethtypes.Transaction, error) {
-	outboundParams := txData.outboundParams
 	tx, _, _, err := signer.Sign(
 		nil,
 		txData.to,
+		txData.amount,
 		txData.gasLimit,
 		txData.gasPrice,
-		outboundParams.TssNonce,
+		txData.nonce,
 		txData.height,
 	)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("SignMigrateTssFundsCmd error: %w", err)
 	}
 	return tx, nil
 }