Merge branch 'develop' into dependabot/go_modules/golang.org/x/crypto…

…-0.17.0

changelog.md

@@ -20,6 +20,7 @@
 
 ### Fixes
 
+* [1554](https://github.com/zeta-chain/node/pull/1554) - Screen out unconfirmed UTXOs that are not created by TSS itself
 * [1560](https://github.com/zeta-chain/node/issues/1560) - Zetaclient post evm-chain outtx hashes only when receipt is available
 * [1516](https://github.com/zeta-chain/node/issues/1516) - Unprivileged outtx tracker removal
 * [1537](https://github.com/zeta-chain/node/issues/1537) - Sanity check events of ZetaSent/ZetaReceived/ZetaRevertedWithdrawn/Deposited
@@ -37,10 +38,12 @@
 * [1525](https://github.com/zeta-chain/node/pull/1525) - relax EVM chain block header length check 1024->4096
 * [1522](https://github.com/zeta-chain/node/pull/1522/files) - block `distribution` module account from receiving zeta
 * [1528](https://github.com/zeta-chain/node/pull/1528) - fix panic caused on decoding malformed BTC addresses
+* [1556](https://github.com/zeta-chain/node/pull/1556) - add emptiness check for topic array in event parsing
 * [1555](https://github.com/zeta-chain/node/pull/1555) - Reduce websocket message limit to 10MB
 
 ### Refactoring
 
+* [1552](https://github.com/zeta-chain/node/pull/1552) - requires group2 to enable header verification
 * [1211](https://github.com/zeta-chain/node/issues/1211) - use `grpc` and `msg` for query and message files
 * refactor cctx scheduler - decouple evm cctx scheduler from btc cctx scheduler
 * move tss state from crosschain to observer

x/crosschain/keeper/evm_hooks.go

@@ -275,6 +275,9 @@ func (k Keeper) ParseZRC20WithdrawalEvent(ctx sdk.Context, log ethtypes.Log) (*z
 	if err != nil {
 		return nil, err
 	}
+	if len(log.Topics) == 0 {
+		return nil, fmt.Errorf("ParseZRC20WithdrawalEvent: invalid log - no topics")
+	}
 	event, err := zrc20ZEVM.ParseWithdrawal(log)
 	if err != nil {
 		return nil, err
@@ -306,6 +309,9 @@ func ParseZetaSentEvent(log ethtypes.Log, connectorZEVM ethcommon.Address) (*con
 	if err != nil {
 		return nil, err
 	}
+	if len(log.Topics) == 0 {
+		return nil, fmt.Errorf("ParseZetaSentEvent: invalid log - no topics")
+	}
 	event, err := zetaConnectorZEVM.ParseZetaSent(log)
 	if err != nil {
 		return nil, err

x/observer/keeper/msg_server_update_crosschain_flags.go

@@ -14,13 +14,8 @@ import (
 func (k msgServer) UpdateCrosschainFlags(goCtx context.Context, msg *types.MsgUpdateCrosschainFlags) (*types.MsgUpdateCrosschainFlagsResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	requiredGroup := types.Policy_Type_group1
-	if msg.IsInboundEnabled || msg.IsOutboundEnabled || msg.GasPriceIncreaseFlags != nil {
-		requiredGroup = types.Policy_Type_group2
-	}
-
 	// check permission
-	if msg.Creator != k.GetParams(ctx).GetAdminPolicyAccount(requiredGroup) {
+	if msg.Creator != k.GetParams(ctx).GetAdminPolicyAccount(msg.GetRequiredGroup()) {
 		return &types.MsgUpdateCrosschainFlagsResponse{}, types.ErrNotAuthorizedPolicy
 	}
 

x/observer/keeper/msg_server_update_crosschain_flags_test.go

@@ -57,6 +57,7 @@ func TestMsgServer_UpdateCrosschainFlags(t *testing.T) {
 		require.Equal(t, uint32(42), flags.GasPriceIncreaseFlags.GasPriceIncreasePercent)
 		require.True(t, flags.BlockHeaderVerificationFlags.IsEthTypeChainEnabled)
 		require.False(t, flags.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled)
+
 		setAdminCrossChainFlags(ctx, k, admin, types.Policy_Type_group2)
 
 		// can update flags again
@@ -71,7 +72,7 @@ func TestMsgServer_UpdateCrosschainFlags(t *testing.T) {
 			},
 			BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
 				IsEthTypeChainEnabled: false,
-				IsBtcTypeChainEnabled: false,
+				IsBtcTypeChainEnabled: true,
 			},
 		})
 		require.NoError(t, err)
@@ -84,7 +85,8 @@ func TestMsgServer_UpdateCrosschainFlags(t *testing.T) {
 		require.Equal(t, time.Minute*43, flags.GasPriceIncreaseFlags.RetryInterval)
 		require.Equal(t, uint32(43), flags.GasPriceIncreaseFlags.GasPriceIncreasePercent)
 		require.False(t, flags.BlockHeaderVerificationFlags.IsEthTypeChainEnabled)
-		require.False(t, flags.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled)
+		require.True(t, flags.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled)
+
 		// group 1 should be able to disable inbound and outbound
 		setAdminCrossChainFlags(ctx, k, admin, types.Policy_Type_group1)
 
@@ -103,6 +105,33 @@ func TestMsgServer_UpdateCrosschainFlags(t *testing.T) {
 		require.Equal(t, int64(43), flags.GasPriceIncreaseFlags.EpochLength)
 		require.Equal(t, time.Minute*43, flags.GasPriceIncreaseFlags.RetryInterval)
 		require.Equal(t, uint32(43), flags.GasPriceIncreaseFlags.GasPriceIncreasePercent)
+		require.False(t, flags.BlockHeaderVerificationFlags.IsEthTypeChainEnabled)
+		require.True(t, flags.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled)
+
+		// group 1 should be able to disable header verification
+		setAdminCrossChainFlags(ctx, k, admin, types.Policy_Type_group1)
+
+		// if gas price increase flags is nil, it should not be updated
+		_, err = srv.UpdateCrosschainFlags(sdk.WrapSDKContext(ctx), &types.MsgUpdateCrosschainFlags{
+			Creator:           admin,
+			IsInboundEnabled:  false,
+			IsOutboundEnabled: false,
+			BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+				IsEthTypeChainEnabled: false,
+				IsBtcTypeChainEnabled: false,
+			},
+		})
+		require.NoError(t, err)
+
+		flags, found = k.GetCrosschainFlags(ctx)
+		require.True(t, found)
+		require.False(t, flags.IsInboundEnabled)
+		require.False(t, flags.IsOutboundEnabled)
+		require.Equal(t, int64(43), flags.GasPriceIncreaseFlags.EpochLength)
+		require.Equal(t, time.Minute*43, flags.GasPriceIncreaseFlags.RetryInterval)
+		require.Equal(t, uint32(43), flags.GasPriceIncreaseFlags.GasPriceIncreasePercent)
+		require.False(t, flags.BlockHeaderVerificationFlags.IsEthTypeChainEnabled)
+		require.False(t, flags.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled)
 
 		// if flags are not defined, default should be used
 		k.RemoveCrosschainFlags(ctx)

x/observer/types/message_crosschain_flags.go

@@ -67,3 +67,21 @@ func (gpf GasPriceIncreaseFlags) Validate() error {
 	}
 	return nil
 }
+
+// GetRequiredGroup returns the required group policy for the message to execute the message
+// Group 1 should only be able to stop or disable functiunalities in case of emergency
+// this concerns disabling inbound and outbound txs or block header verification
+// every other action requires group 2
+func (msg *MsgUpdateCrosschainFlags) GetRequiredGroup() Policy_Type {
+	if msg.IsInboundEnabled || msg.IsOutboundEnabled {
+		return Policy_Type_group2
+	}
+	if msg.GasPriceIncreaseFlags != nil {
+		return Policy_Type_group2
+	}
+	if msg.BlockHeaderVerificationFlags != nil && (msg.BlockHeaderVerificationFlags.IsEthTypeChainEnabled || msg.BlockHeaderVerificationFlags.IsBtcTypeChainEnabled) {
+		return Policy_Type_group2
+
+	}
+	return Policy_Type_group1
+}

x/observer/types/message_crosschain_flags_test.go

@@ -116,3 +116,118 @@ func TestGasPriceIncreaseFlags_Validate(t *testing.T) {
 		})
 	}
 }
+
+func TestMsgUpdateCrosschainFlags_GetRequiredGroup(t *testing.T) {
+	tests := []struct {
+		name string
+		msg  types.MsgUpdateCrosschainFlags
+		want types.Policy_Type
+	}{
+		{
+			name: "disabling outbound and inbound allows group 1",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:                      sample.AccAddress(),
+				IsInboundEnabled:             false,
+				IsOutboundEnabled:            false,
+				BlockHeaderVerificationFlags: nil,
+				GasPriceIncreaseFlags:        nil,
+			},
+			want: types.Policy_Type_group1,
+		},
+		{
+			name: "disabling outbound and inbound and block header verification allows group 1",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  false,
+				IsOutboundEnabled: false,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: false,
+					IsBtcTypeChainEnabled: false,
+				},
+				GasPriceIncreaseFlags: nil,
+			},
+			want: types.Policy_Type_group1,
+		},
+		{
+			name: "updating gas price increase flags requires group 2",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  false,
+				IsOutboundEnabled: false,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: false,
+					IsBtcTypeChainEnabled: false,
+				},
+				GasPriceIncreaseFlags: &types.GasPriceIncreaseFlags{
+					EpochLength:             1,
+					RetryInterval:           1,
+					GasPriceIncreasePercent: 1,
+					MaxPendingCctxs:         100,
+				},
+			},
+			want: types.Policy_Type_group2,
+		},
+		{
+			name: "enabling inbound requires group 2",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  true,
+				IsOutboundEnabled: false,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: false,
+					IsBtcTypeChainEnabled: false,
+				},
+				GasPriceIncreaseFlags: nil,
+			},
+			want: types.Policy_Type_group2,
+		},
+		{
+			name: "enabling outbound requires group 2",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  false,
+				IsOutboundEnabled: true,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: false,
+					IsBtcTypeChainEnabled: false,
+				},
+				GasPriceIncreaseFlags: nil,
+			},
+			want: types.Policy_Type_group2,
+		},
+		{
+			name: "enabling eth header verification requires group 2",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  false,
+				IsOutboundEnabled: false,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: true,
+					IsBtcTypeChainEnabled: false,
+				},
+				GasPriceIncreaseFlags: nil,
+			},
+			want: types.Policy_Type_group2,
+		},
+		{
+			name: "enabling btc header verification requires group 2",
+			msg: types.MsgUpdateCrosschainFlags{
+				Creator:           sample.AccAddress(),
+				IsInboundEnabled:  false,
+				IsOutboundEnabled: false,
+				BlockHeaderVerificationFlags: &types.BlockHeaderVerificationFlags{
+					IsEthTypeChainEnabled: false,
+					IsBtcTypeChainEnabled: true,
+				},
+				GasPriceIncreaseFlags: nil,
+			},
+			want: types.Policy_Type_group2,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, tt.msg.GetRequiredGroup())
+		})
+	}
+}

zetaclient/bitcoin_client.go

@@ -57,7 +57,7 @@ type BitcoinChainClient struct {
 
 	Mu                *sync.Mutex // lock for all the maps, utxos and core params
 	pendingNonce      uint64
-	includedTxHashes  map[string]uint64                        // key: tx hash
+	includedTxHashes  map[string]bool                          // key: tx hash
 	includedTxResults map[string]*btcjson.GetTransactionResult // key: chain-tss-nonce
 	broadcastedTx     map[string]string                        // key: chain-tss-nonce, value: outTx hash
 	utxos             []btcjson.ListUnspentResult
@@ -147,7 +147,7 @@ func NewBitcoinClient(
 
 	ob.zetaClient = bridge
 	ob.Tss = tss
-	ob.includedTxHashes = make(map[string]uint64)
+	ob.includedTxHashes = make(map[string]bool)
 	ob.includedTxResults = make(map[string]*btcjson.GetTransactionResult)
 	ob.broadcastedTx = make(map[string]string)
 	ob.params = btcCfg.ChainParams
@@ -757,16 +757,13 @@ func (ob *BitcoinChainClient) FetchUTXOS() error {
 	}
 	maxConfirmations := int(bh)
 
-	// List unspent.
+	// List all unspent UTXOs (160ms)
 	tssAddr := ob.Tss.BTCAddress()
 	address, err := common.DecodeBtcAddress(tssAddr, ob.chain.ChainId)
 	if err != nil {
 		return fmt.Errorf("btc: error decoding wallet address (%s) : %s", tssAddr, err.Error())
 	}
-	addresses := []btcutil.Address{address}
-
-	// fetching all TSS utxos takes 160ms
-	utxos, err := ob.rpcClient.ListUnspentMinMaxAddresses(0, maxConfirmations, addresses)
+	utxos, err := ob.rpcClient.ListUnspentMinMaxAddresses(0, maxConfirmations, []btcutil.Address{address})
 	if err != nil {
 		return err
 	}
@@ -782,12 +779,20 @@ func (ob *BitcoinChainClient) FetchUTXOS() error {
 		return utxos[i].Amount < utxos[j].Amount
 	})
 
-	// filter UTXOs big enough to cover the cost of spending themselves
+	// filter UTXOs good to spend for next TSS transaction
 	utxosFiltered := make([]btcjson.ListUnspentResult, 0)
 	for _, utxo := range utxos {
-		if utxo.Amount >= BtcDepositorFeeMin {
-			utxosFiltered = append(utxosFiltered, utxo)
+		// UTXOs big enough to cover the cost of spending themselves
+		if utxo.Amount < BtcDepositorFeeMin {
+			continue
+		}
+		// we don't want to spend other people's unconfirmed UTXOs as they may not be safe to spend
+		if utxo.Confirmations == 0 {
+			if !ob.isTssTransaction(utxo.TxID) {
+				continue
+			}
 		}
+		utxosFiltered = append(utxosFiltered, utxo)
 	}
 
 	ob.Mu.Lock()
@@ -797,6 +802,13 @@ func (ob *BitcoinChainClient) FetchUTXOS() error {
 	return nil
 }
 
+// isTssTransaction checks if a given transaction was sent by TSS itself.
+// An unconfirmed transaction is safe to spend only if it was sent by TSS and verified by ourselves.
+func (ob *BitcoinChainClient) isTssTransaction(txid string) bool {
+	_, found := ob.includedTxHashes[txid]
+	return found
+}
+
 // refreshPendingNonce tries increasing the artificial pending nonce of outTx (if lagged behind).
 // There could be many (unpredictable) reasons for a pending nonce lagging behind, for example:
 // 1. The zetaclient gets restarted.
@@ -1083,6 +1095,7 @@ func (ob *BitcoinChainClient) setIncludedTx(nonce uint64, getTxResult *btcjson.G
 	res, found := ob.includedTxResults[outTxID]
 
 	if !found { // not found.
+		ob.includedTxHashes[txHash] = true
 		ob.includedTxResults[outTxID] = getTxResult // include new outTx and enforce rigid 1-to-1 mapping: nonce <===> txHash
 		if nonce >= ob.pendingNonce {               // try increasing pending nonce on every newly included outTx
 			ob.pendingNonce = nonce + 1
@@ -1107,11 +1120,15 @@ func (ob *BitcoinChainClient) getIncludedTx(nonce uint64) *btcjson.GetTransactio
 	return ob.includedTxResults[ob.GetTxID(nonce)]
 }
 
-// removeIncludedTx removes included tx's result from memory
+// removeIncludedTx removes included tx from memory
 func (ob *BitcoinChainClient) removeIncludedTx(nonce uint64) {
 	ob.Mu.Lock()
 	defer ob.Mu.Unlock()
-	delete(ob.includedTxResults, ob.GetTxID(nonce))
+	txResult, found := ob.includedTxResults[ob.GetTxID(nonce)]
+	if found {
+		delete(ob.includedTxHashes, txResult.TxID)
+		delete(ob.includedTxResults, ob.GetTxID(nonce))
+	}
 }
 
 // Basic TSS outTX checks: