Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(observer): disable Bitcoin witness support for mainnet (release/v20) #2855

Merged
merged 2 commits into from
Sep 10, 2024
+116 −25
Merged

refactor(observer): disable Bitcoin witness support for mainnet (release/v20) #2855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0de34a
tests
lumtis Sep 10, 2024
6e7e580
update changelogs
lumtis Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
### Refactor

* [2615](https://github.com/zeta-chain/node/pull/2615) - Refactor cleanup of outbound trackers
* [2855](https://github.com/zeta-chain/node/pull/2855) - disable Bitcoin witness support for mainnet

### Tests

Expand Down
31 changes: 24 additions & 7 deletions zetaclient/chains/bitcoin/observer/inbound.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@
}

// #nosec G115 always positive
event, err := GetBtcEventWithWitness(
event, err := GetBtcEvent(

Check warning on line 269 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L269 

Added line #L269 was not covered by tests
ob.btcClient,
*tx,
tss,
Expand Down Expand Up @@ -328,7 +328,7 @@
continue // the first tx is coinbase; we do not process coinbase tx
}

inbound, err := GetBtcEventWithWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee)
inbound, err := GetBtcEvent(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee)

Check warning on line 331 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L331 

Added line #L331 was not covered by tests
if err != nil {
// unable to parse the tx, the caller should retry
return nil, errors.Wrapf(err, "error getting btc event for tx %s in block %d", tx.Txid, blockNumber)
Expand Down Expand Up @@ -390,10 +390,27 @@
return false
}

// GetBtcEvent either returns a valid BTCInboundEvent or nil
// GetBtcEvent returns a valid BTCInboundEvent or nil
// it uses witness data to extract the sender address, except for mainnet
func GetBtcEvent(
rpcClient interfaces.BTCRPCClient,
tx btcjson.TxRawResult,
tssAddress string,
blockNumber uint64,
logger zerolog.Logger,
netParams *chaincfg.Params,
depositorFee float64,
) (*BTCInboundEvent, error) {
if netParams.Name == chaincfg.MainNetParams.Name {
return GetBtcEventWithoutWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee)
}
return GetBtcEventWithWitness(rpcClient, tx, tssAddress, blockNumber, logger, netParams, depositorFee)

Check warning on line 407 in zetaclient/chains/bitcoin/observer/inbound.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/inbound.go#L407 

Added line #L407 was not covered by tests
}

// GetBtcEventWithoutWitness either returns a valid BTCInboundEvent or nil
// Note: the caller should retry the tx on error (e.g., GetSenderAddressByVin failed)
// TODO(revamp): simplify this function
func GetBtcEvent(
func GetBtcEventWithoutWitness(
rpcClient interfaces.BTCRPCClient,
tx btcjson.TxRawResult,
tssAddress string,
Expand Down Expand Up @@ -437,7 +454,7 @@
// deposit amount has to be no less than the minimum depositor fee
if vout0.Value < depositorFee {
logger.Info().
Msgf("GetBtcEvent: btc deposit amount %v in txid %s is less than depositor fee %v", vout0.Value, tx.Txid, depositorFee)
Msgf("GetBtcEventWithoutWitness: btc deposit amount %v in txid %s is less than depositor fee %v", vout0.Value, tx.Txid, depositorFee)
return nil, nil
}
value = vout0.Value - depositorFee
Expand All @@ -446,15 +463,15 @@
vout1 := tx.Vout[1]
memo, found, err = bitcoin.DecodeOpReturnMemo(vout1.ScriptPubKey.Hex, tx.Txid)
if err != nil {
logger.Error().Err(err).Msgf("GetBtcEvent: error decoding OP_RETURN memo: %s", vout1.ScriptPubKey.Hex)
logger.Error().Err(err).Msgf("GetBtcEventWithoutWitness: error decoding OP_RETURN memo: %s", vout1.ScriptPubKey.Hex)
return nil, nil
}
}
}
// event found, get sender address
if found {
if len(tx.Vin) == 0 { // should never happen
return nil, fmt.Errorf("GetBtcEvent: no input found for inbound: %s", tx.Txid)
return nil, fmt.Errorf("GetBtcEventWithoutWitness: no input found for inbound: %s", tx.Txid)
}

fromAddress, err := GetSenderAddressByVin(rpcClient, tx.Vin[0], netParams)
Expand Down
107 changes: 90 additions & 17 deletions zetaclient/chains/bitcoin/observer/inbound_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,13 @@ func TestGetSenderAddressByVinErrors(t *testing.T) {
})
}

func TestGetBtcEvent(t *testing.T) {
func TestGetBtcEventWithoutWitness(t *testing.T) {
// load archived inbound P2WPKH raw result
// https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa
txHash := "847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa"
chain := chains.BitcoinMainnet

// GetBtcEvent arguments
// GetBtcEventWithoutWitness arguments
tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false)
tssAddress := testutils.TSSAddressBTCMainnet
blockNumber := uint64(835640)
Expand Down Expand Up @@ -348,7 +348,7 @@ func TestGetBtcEvent(t *testing.T) {
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
Expand All @@ -363,7 +363,7 @@ func TestGetBtcEvent(t *testing.T) {
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
Expand All @@ -378,7 +378,7 @@ func TestGetBtcEvent(t *testing.T) {
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
Expand All @@ -393,7 +393,7 @@ func TestGetBtcEvent(t *testing.T) {
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
Expand All @@ -408,7 +408,7 @@ func TestGetBtcEvent(t *testing.T) {
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
Expand All @@ -419,7 +419,7 @@ func TestGetBtcEvent(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -430,13 +430,13 @@ func TestGetBtcEvent(t *testing.T) {

// modify the tx to have Vout[0] a P2SH output
tx.Vout[0].ScriptPubKey.Hex = strings.Replace(tx.Vout[0].ScriptPubKey.Hex, "0014", "a914", 1)
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)

// append 1 byte to script to make it longer than 22 bytes
tx.Vout[0].ScriptPubKey.Hex = tx.Vout[0].ScriptPubKey.Hex + "00"
event, err = observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err = observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -447,7 +447,7 @@ func TestGetBtcEvent(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -458,7 +458,7 @@ func TestGetBtcEvent(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -469,7 +469,7 @@ func TestGetBtcEvent(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -480,7 +480,7 @@ func TestGetBtcEvent(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Nil(t, event)
})
Expand All @@ -503,7 +503,7 @@ func TestGetBtcEventErrors(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.Error(t, err)
require.Nil(t, event)
})
Expand All @@ -514,7 +514,7 @@ func TestGetBtcEventErrors(t *testing.T) {

// get BTC event
rpcClient := mocks.NewMockBTCRPCClient()
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.Error(t, err)
require.Nil(t, event)
})
Expand All @@ -524,8 +524,81 @@ func TestGetBtcEventErrors(t *testing.T) {
rpcClient := mocks.NewMockBTCRPCClient()

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
event, err := observer.GetBtcEventWithoutWitness(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.Error(t, err)
require.Nil(t, event)
})
}

func TestGetBtcEvent(t *testing.T) {
t.Run("should not decode inbound event with witness with mainnet chain", func(t *testing.T) {
// load archived inbound P2WPKH raw result
// https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa
chain := chains.BitcoinMainnet

tssAddress := testutils.TSSAddressBTCMainnet
blockNumber := uint64(835640)
net := &chaincfg.MainNetParams
// 2.992e-05, see avgFeeRate https://mempool.space/api/v1/blocks/835640
depositorFee := bitcoin.DepositorFee(22 * clientcommon.BTCOutboundGasPriceMultiplier)

txHash2 := "37777defed8717c581b4c0509329550e344bdc14ac38f71fc050096887e535c8"
tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash2, false)

preHash := "c5d224963832fc0b9a597251c2342a17b25e481a88cc9119008e8f8296652697"
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(
rpcClient,
*tx,
tssAddress,
blockNumber,
log.Logger,
net,
depositorFee,
)
require.NoError(t, err)
require.Equal(t, (*observer.BTCInboundEvent)(nil), event)
})

t.Run("should support legacy BTC inbound event parsing for mainnet", func(t *testing.T) {
// load archived inbound P2WPKH raw result
// https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa
txHash := "847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa"
chain := chains.BitcoinMainnet

// GetBtcEventWithoutWitness arguments
tx := testutils.LoadBTCInboundRawResult(t, TestDataDir, chain.ChainId, txHash, false)
tssAddress := testutils.TSSAddressBTCMainnet
blockNumber := uint64(835640)
net := &chaincfg.MainNetParams
// 2.992e-05, see avgFeeRate https://mempool.space/api/v1/blocks/835640
depositorFee := bitcoin.DepositorFee(22 * clientcommon.BTCOutboundGasPriceMultiplier)

// expected result
memo, err := hex.DecodeString(tx.Vout[1].ScriptPubKey.Hex[4:])
require.NoError(t, err)
eventExpected := &observer.BTCInboundEvent{
FromAddress: "bc1q68kxnq52ahz5vd6c8czevsawu0ux9nfrzzrh6e",
ToAddress: tssAddress,
Value: tx.Vout[0].Value - depositorFee, // 7008 sataoshis
MemoBytes: memo,
BlockNumber: blockNumber,
TxHash: tx.Txid,
}

// https://mempool.space/tx/c5d224963832fc0b9a597251c2342a17b25e481a88cc9119008e8f8296652697
preHash := "c5d224963832fc0b9a597251c2342a17b25e481a88cc9119008e8f8296652697"
tx.Vin[0].Txid = preHash
tx.Vin[0].Vout = 2
eventExpected.FromAddress = "bc1q68kxnq52ahz5vd6c8czevsawu0ux9nfrzzrh6e"
// load previous raw tx so so mock rpc client can return it
rpcClient := createRPCClientAndLoadTx(t, chain.ChainId, preHash)

// get BTC event
event, err := observer.GetBtcEvent(rpcClient, *tx, tssAddress, blockNumber, log.Logger, net, depositorFee)
require.NoError(t, err)
require.Equal(t, eventExpected, event)
})
}
2 changes: 1 addition & 1 deletion zetaclient/chains/bitcoin/observer/witness_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestParseScriptFromWitness(t *testing.T) {
})
}

func TestGetBtcEventFromInscription(t *testing.T) {
func TestGetBtcEventWithWitness(t *testing.T) {
// load archived inbound P2WPKH raw result
// https://mempool.space/tx/847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa
txHash := "847139aa65aa4a5ee896375951cbf7417cfc8a4d6f277ec11f40cd87319f04aa"
Expand Down
Loading