diff --git a/changelog.md b/changelog.md index dbf40ef8e3..6f0fa7e632 100644 --- a/changelog.md +++ b/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 diff --git a/pkg/constant/constant.go b/pkg/constant/constant.go index 447eca98e5..9a81e1d8b2 100644 --- a/pkg/constant/constant.go +++ b/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" ) diff --git a/zetaclient/chains/evm/constant.go b/zetaclient/chains/evm/constant.go index e15453df59..b754d57f30 100644 --- a/zetaclient/chains/evm/constant.go +++ b/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 diff --git a/zetaclient/chains/evm/signer/outbound_data_test.go b/zetaclient/chains/evm/signer/outbound_data_test.go index 77d0f9a040..bd9e81061c 100644 --- a/zetaclient/chains/evm/signer/outbound_data_test.go +++ b/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) { diff --git a/zetaclient/chains/evm/signer/signer.go b/zetaclient/chains/evm/signer/signer.go index 8ab5608959..00f6f35fd5 100644 --- a/zetaclient/chains/evm/signer/signer.go +++ b/zetaclient/chains/evm/signer/signer.go @@ -39,7 +39,12 @@ import ( "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) GetERC20CustodyAddress() ethcommon.Address { func (signer *Signer) Sign( data []byte, to ethcommon.Address, + amount *big.Int, gasLimit uint64, gasPrice *big.Int, nonce uint64, @@ -147,7 +153,7 @@ func (signer *Signer) Sign( // 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 @@ func (signer *Signer) SignOutbound(txData *OutboundData) (*ethtypes.Transaction, 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 @@ func (signer *Signer) SignRevertTx(txData *OutboundData) (*ethtypes.Transaction, 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 @@ func (signer *Signer) TryProcessOutbound( 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 @@ func (signer *Signer) SignERC20WithdrawTx(txData *OutboundData) (*ethtypes.Trans 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 @@ func (signer *Signer) SignWhitelistERC20Cmd(txData *OutboundData, params string) } 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 } diff --git a/zetaclient/chains/evm/signer/signer_test.go b/zetaclient/chains/evm/signer/signer_test.go index 6039c44803..5bd1b6a967 100644 --- a/zetaclient/chains/evm/signer/signer_test.go +++ b/zetaclient/chains/evm/signer/signer_test.go @@ -1,9 +1,12 @@ package signer import ( + "math/big" "testing" sdktypes "github.com/cosmos/cosmos-sdk/types" + ethcommon "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/rs/zerolog" "github.com/stretchr/testify/require" @@ -13,6 +16,7 @@ import ( "github.com/zeta-chain/zetacore/testutil/sample" crosschaintypes "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" "github.com/zeta-chain/zetacore/zetaclient/common" "github.com/zeta-chain/zetacore/zetaclient/config" "github.com/zeta-chain/zetacore/zetaclient/context" @@ -29,16 +33,23 @@ var ( ERC20CustodyAddress = sample.EthAddress() ) -func getNewEvmSigner() (*Signer, error) { +// getNewEvmSigner creates a new EVM chain signer for testing +func getNewEvmSigner(tss interfaces.TSSSigner) (*Signer, error) { + // use default mock TSS if not provided + if tss == nil { + tss = mocks.NewTSSMainnet() + } + mpiAddress := ConnectorAddress erc20CustodyAddress := ERC20CustodyAddress logger := common.ClientLogger{} ts := &metrics.TelemetryServer{} cfg := config.NewConfig() + return NewSigner( chains.BscMainnet, mocks.EVMRPCEnabled, - mocks.NewTSSMainnet(), + tss, config.GetConnectorABI(), config.GetERC20CustodyABI(), mpiAddress, @@ -48,11 +59,16 @@ func getNewEvmSigner() (*Signer, error) { ts) } -func getNewEvmChainObserver() (*observer.Observer, error) { +// getNewEvmChainObserver creates a new EVM chain observer for testing +func getNewEvmChainObserver(tss interfaces.TSSSigner) (*observer.Observer, error) { + // use default mock TSS if not provided + if tss == nil { + tss = mocks.NewTSSMainnet() + } + logger := common.ClientLogger{} ts := &metrics.TelemetryServer{} cfg := config.NewConfig() - tss := mocks.NewTSSMainnet() evmcfg := config.EVMConfig{Chain: chains.BscMainnet, Endpoint: "http://localhost:8545"} cfg.EVMChainConfigs[chains.BscMainnet.ChainId] = evmcfg @@ -78,8 +94,31 @@ func getInvalidCCTX(t *testing.T) *crosschaintypes.CrossChainTx { return cctx } +// verifyTxSignature is a helper function to verify the signature of a transaction +func verifyTxSignature(t *testing.T, tx *ethtypes.Transaction, tssPubkey []byte, signer ethtypes.Signer) { + _, r, s := tx.RawSignatureValues() + signature := append(r.Bytes(), s.Bytes()...) + hash := signer.Hash(tx) + + verified := crypto.VerifySignature(tssPubkey, hash.Bytes(), signature) + require.True(t, verified) +} + +// verifyTxBodyBasics is a helper function to verify 'to', 'nonce' and 'amount' of a transaction +func verifyTxBodyBasics( + t *testing.T, + tx *ethtypes.Transaction, + to ethcommon.Address, + nonce uint64, + amount *big.Int, +) { + require.Equal(t, to, *tx.To()) + require.Equal(t, nonce, tx.Nonce()) + require.True(t, amount.Cmp(tx.Value()) == 0) +} + func TestSigner_SetGetConnectorAddress(t *testing.T) { - evmSigner, err := getNewEvmSigner() + evmSigner, err := getNewEvmSigner(nil) require.NoError(t, err) // Get and compare require.Equal(t, ConnectorAddress, evmSigner.GetZetaConnectorAddress()) @@ -91,7 +130,7 @@ func TestSigner_SetGetConnectorAddress(t *testing.T) { } func TestSigner_SetGetERC20CustodyAddress(t *testing.T) { - evmSigner, err := getNewEvmSigner() + evmSigner, err := getNewEvmSigner(nil) require.NoError(t, err) // Get and compare require.Equal(t, ERC20CustodyAddress, evmSigner.GetERC20CustodyAddress()) @@ -103,11 +142,11 @@ func TestSigner_SetGetERC20CustodyAddress(t *testing.T) { } func TestSigner_TryProcessOutbound(t *testing.T) { - evmSigner, err := getNewEvmSigner() + evmSigner, err := getNewEvmSigner(nil) require.NoError(t, err) cctx := getCCTX(t) processorManager := getNewOutboundProcessor() - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(nil) require.NoError(t, err) // Test with mock client that has keys @@ -121,13 +160,14 @@ func TestSigner_TryProcessOutbound(t *testing.T) { func TestSigner_SignOutbound(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(tss) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -140,23 +180,28 @@ func TestSigner_SignOutbound(t *testing.T) { // Verify Signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) + }) + t.Run("SignOutbound - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Call SignOutbound + tx, err := evmSigner.SignOutbound(txData) + require.ErrorContains(t, err, "sign onReceive error") + require.Nil(t, tx) }) } func TestSigner_SignRevertTx(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(tss) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -167,25 +212,72 @@ func TestSigner_SignRevertTx(t *testing.T) { tx, err := evmSigner.SignRevertTx(txData) require.NoError(t, err) - // Verify Signature + // Verify tx signature + tss := mocks.NewTSSMainnet() + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) + + // Verify tx body basics + // Note: Revert tx calls connector contract with 0 gas token + verifyTxBodyBasics(t, tx, evmSigner.zetaConnectorAddress, txData.nonce, big.NewInt(0)) + }) + t.Run("SignRevertTx - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() + + // Call SignRevertTx + tx, err := evmSigner.SignRevertTx(txData) + require.ErrorContains(t, err, "sign onRevert error") + require.Nil(t, tx) + }) +} + +func TestSigner_SignCancelTx(t *testing.T) { + // Setup evm signer + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) + require.NoError(t, err) + + // Setup txData struct + cctx := getCCTX(t) + mockObserver, err := getNewEvmChainObserver(tss) + require.NoError(t, err) + txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + require.False(t, skip) + require.NoError(t, err) + + t.Run("SignCancelTx - should successfully sign", func(t *testing.T) { + // Call SignRevertTx + tx, err := evmSigner.SignCancelTx(txData) + require.NoError(t, err) + + // Verify tx signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Verify tx body basics + // Note: Cancel tx sends 0 gas token to TSS self address + verifyTxBodyBasics(t, tx, evmSigner.tssSigner.EVMAddress(), txData.nonce, big.NewInt(0)) + }) + t.Run("SignCancelTx - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() + + // Call SignCancelTx + tx, err := evmSigner.SignCancelTx(txData) + require.ErrorContains(t, err, "SignCancelTx error") + require.Nil(t, tx) }) } func TestSigner_SignWithdrawTx(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(tss) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -196,25 +288,32 @@ func TestSigner_SignWithdrawTx(t *testing.T) { tx, err := evmSigner.SignWithdrawTx(txData) require.NoError(t, err) - // Verify Signature + // Verify tx signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Verify tx body basics + verifyTxBodyBasics(t, tx, txData.to, txData.nonce, txData.amount) + }) + t.Run("SignWithdrawTx - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() + + // Call SignWithdrawTx + tx, err := evmSigner.SignWithdrawTx(txData) + require.ErrorContains(t, err, "SignWithdrawTx error") + require.Nil(t, tx) }) } func TestSigner_SignCommandTx(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + evmSigner, err := getNewEvmSigner(nil) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(nil) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -227,14 +326,13 @@ func TestSigner_SignCommandTx(t *testing.T) { tx, err := evmSigner.SignCommandTx(txData, cmd, params) require.NoError(t, err) - // Verify Signature + // Verify tx signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Verify tx body basics + // Note: Revert tx calls erc20 custody contract with 0 gas token + verifyTxBodyBasics(t, tx, txData.to, txData.nonce, big.NewInt(0)) }) t.Run("SignCommandTx CmdMigrateTssFunds", func(t *testing.T) { @@ -243,25 +341,24 @@ func TestSigner_SignCommandTx(t *testing.T) { tx, err := evmSigner.SignCommandTx(txData, cmd, "") require.NoError(t, err) - // Verify Signature + // Verify tx signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Verify tx body basics + verifyTxBodyBasics(t, tx, txData.to, txData.nonce, txData.amount) }) } func TestSigner_SignERC20WithdrawTx(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(tss) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -272,25 +369,34 @@ func TestSigner_SignERC20WithdrawTx(t *testing.T) { tx, err := evmSigner.SignERC20WithdrawTx(txData) require.NoError(t, err) - // Verify Signature + // Verify tx signature tss := mocks.NewTSSMainnet() - _, r, s := tx.RawSignatureValues() - signature := append(r.Bytes(), s.Bytes()...) - hash := evmSigner.EvmSigner().Hash(tx) + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) - verified := crypto.VerifySignature(tss.Pubkey(), hash.Bytes(), signature) - require.True(t, verified) + // Verify tx body basics + // Note: Withdraw tx calls erc20 custody contract with 0 gas token + verifyTxBodyBasics(t, tx, evmSigner.er20CustodyAddress, txData.nonce, big.NewInt(0)) + }) + + t.Run("SignERC20WithdrawTx - should fail if keysign fails", func(t *testing.T) { + // pause tss to make keysign fail + tss.Pause() + + // Call SignERC20WithdrawTx + tx, err := evmSigner.SignERC20WithdrawTx(txData) + require.ErrorContains(t, err, "sign withdraw error") + require.Nil(t, tx) }) } func TestSigner_BroadcastOutbound(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + evmSigner, err := getNewEvmSigner(nil) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(nil) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) @@ -334,18 +440,82 @@ func TestSigner_SignerErrorMsg(t *testing.T) { func TestSigner_SignWhitelistERC20Cmd(t *testing.T) { // Setup evm signer - evmSigner, err := getNewEvmSigner() + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) require.NoError(t, err) // Setup txData struct cctx := getCCTX(t) - mockObserver, err := getNewEvmChainObserver() + mockObserver, err := getNewEvmChainObserver(tss) require.NoError(t, err) txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) require.False(t, skip) require.NoError(t, err) - tx, err := evmSigner.SignWhitelistERC20Cmd(txData, "") - require.Nil(t, tx) - require.ErrorContains(t, err, "invalid erc20 address") + t.Run("SignWhitelistERC20Cmd - should successfully sign", func(t *testing.T) { + // Call SignWhitelistERC20Cmd + tx, err := evmSigner.SignWhitelistERC20Cmd(txData, sample.EthAddress().Hex()) + require.NoError(t, err) + require.NotNil(t, tx) + + // Verify tx signature + tss := mocks.NewTSSMainnet() + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) + + // Verify tx body basics + verifyTxBodyBasics(t, tx, txData.to, txData.nonce, zeroValue) + }) + t.Run("SignWhitelistERC20Cmd - should fail on invalid erc20 address", func(t *testing.T) { + tx, err := evmSigner.SignWhitelistERC20Cmd(txData, "") + require.Nil(t, tx) + require.ErrorContains(t, err, "invalid erc20 address") + }) + t.Run("SignWhitelistERC20Cmd - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() + + // Call SignWhitelistERC20Cmd + tx, err := evmSigner.SignWhitelistERC20Cmd(txData, sample.EthAddress().Hex()) + require.ErrorContains(t, err, "sign whitelist error") + require.Nil(t, tx) + }) +} + +func TestSigner_SignMigrateTssFundsCmd(t *testing.T) { + // Setup evm signer + tss := mocks.NewTSSMainnet() + evmSigner, err := getNewEvmSigner(tss) + require.NoError(t, err) + + // Setup txData struct + cctx := getCCTX(t) + mockObserver, err := getNewEvmChainObserver(tss) + require.NoError(t, err) + txData, skip, err := NewOutboundData(cctx, mockObserver, evmSigner.EvmClient(), zerolog.Logger{}, 123) + require.False(t, skip) + require.NoError(t, err) + + t.Run("SignMigrateTssFundsCmd - should successfully sign", func(t *testing.T) { + // Call SignMigrateTssFundsCmd + tx, err := evmSigner.SignMigrateTssFundsCmd(txData) + require.NoError(t, err) + require.NotNil(t, tx) + + // Verify tx signature + tss := mocks.NewTSSMainnet() + verifyTxSignature(t, tx, tss.Pubkey(), evmSigner.EvmSigner()) + + // Verify tx body basics + verifyTxBodyBasics(t, tx, txData.to, txData.nonce, txData.amount) + }) + + t.Run("SignMigrateTssFundsCmd - should fail if keysign fails", func(t *testing.T) { + // Pause tss to make keysign fail + tss.Pause() + + // Call SignMigrateTssFundsCmd + tx, err := evmSigner.SignMigrateTssFundsCmd(txData) + require.ErrorContains(t, err, "SignMigrateTssFundsCmd error") + require.Nil(t, tx) + }) } diff --git a/zetaclient/testutils/mocks/tss_signer.go b/zetaclient/testutils/mocks/tss_signer.go index 6226200d08..87d2f2ac3b 100644 --- a/zetaclient/testutils/mocks/tss_signer.go +++ b/zetaclient/testutils/mocks/tss_signer.go @@ -31,6 +31,8 @@ var _ interfaces.TSSSigner = (*TSS)(nil) // TSS is a mock of TSS signer for testing type TSS struct { + paused bool + // set evmAddress/btcAddress if just want to mock EVMAddress()/BTCAddress() chain chains.Chain evmAddress string @@ -42,6 +44,7 @@ type TSS struct { func NewMockTSS(chain chains.Chain, evmAddress string, btcAddress string) *TSS { return &TSS{ + paused: false, chain: chain, evmAddress: evmAddress, btcAddress: btcAddress, @@ -65,6 +68,11 @@ func (s *TSS) WithPrivKey(privKey *ecdsa.PrivateKey) *TSS { // Sign uses test key unrelated to any tss key in production func (s *TSS) Sign(data []byte, _ uint64, _ uint64, _ *chains.Chain, _ string) ([65]byte, error) { + // return error if tss is paused + if s.paused { + return [65]byte{}, fmt.Errorf("tss is paused") + } + signature, err := crypto.Sign(data, s.PrivKey) if err != nil { return [65]byte{}, err @@ -161,3 +169,14 @@ func (s *TSS) btcAddressPubkey() *btcutil.AddressPubKey { } return testnet3Addr } + +// ---------------------------------------------------------------------------- +// methods to control the mock for testing +// ---------------------------------------------------------------------------- +func (s *TSS) Pause() { + s.paused = true +} + +func (s *TSS) Unpause() { + s.paused = false +}