Skip to content

Commit

Permalink
error handling improvement and renaming
Browse files Browse the repository at this point in the history
  • Loading branch information
ws4charlie committed Sep 24, 2024
1 parent 9ff3feb commit 7c5fbb9
Show file tree
Hide file tree
Showing 16 changed files with 94 additions and 52 deletions.
3 changes: 2 additions & 1 deletion cmd/zetaclientd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ func start(_ *cobra.Command, _ []string) error {
btcChainIDs[i] = chain.ID()
}

// Make sure the TSS EVM/BTC addresses are well formed
// Make sure the TSS EVM/BTC addresses are well formed.
// Zetaclient should not start if TSS addresses cannot be properly derived.
tss.CurrentPubkey = currentTss.TssPubkey
err = tss.ValidateAddresses(btcChainIDs)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ func TestKeeper_CheckMigration(t *testing.T) {
}

err := k.CheckIfTSSMigrationTransfer(ctx, &msg)
require.ErrorContains(t, err, "no Bitcoin net params for chain ID: 999")
require.ErrorContains(t, err, "no Bitcoin network params for chain ID: 999")
})

t.Run("fails if gateway is not observer ", func(t *testing.T) {
Expand Down
12 changes: 8 additions & 4 deletions zetaclient/chains/base/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,17 @@ func (ob *Observer) WithTSS(tss interfaces.TSSSigner) *Observer {
return ob
}

// TSSAddress returns the TSS address for the chain.
// TSSAddressString returns the TSS address for the chain.
//
// Note: all chains uses TSS EVM address except Bitcoin chain.
func (ob *Observer) TSSAddress() string {
func (ob *Observer) TSSAddressString() string {
switch ob.chain.Consensus {
case chains.Consensus_bitcoin:
return ob.tss.BTCAddress(ob.Chain().ChainId).EncodeAddress()
address, err := ob.tss.BTCAddress(ob.Chain().ChainId)
if err != nil {
return ""
}
return address.EncodeAddress()
default:
return ob.tss.EVMAddress().String()
}
Expand Down Expand Up @@ -298,7 +302,7 @@ func (ob *Observer) WithHeaderCache(cache *lru.Cache) *Observer {
// OutboundID returns a unique identifier for the outbound transaction.
// The identifier is now used as the key for maps that store outbound related data (e.g. transaction, receipt, etc).
func (ob *Observer) OutboundID(nonce uint64) string {
tssAddress := ob.TSSAddress()
tssAddress := ob.TSSAddressString()
return fmt.Sprintf("%d-%s-%d", ob.chain.ChainId, tssAddress, nonce)
}

Expand Down
21 changes: 18 additions & 3 deletions zetaclient/chains/base/observer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,14 @@ func TestObserverGetterAndSetter(t *testing.T) {
})
}

func TestTSSAddress(t *testing.T) {
func TestTSSAddressString(t *testing.T) {
testConfig := sdk.GetConfig()
testConfig.SetBech32PrefixForAccount(cmd.Bech32PrefixAccAddr, cmd.Bech32PrefixAccPub)

tests := []struct {
name string
chain chains.Chain
forceError bool
addrExpected string
}{
{
Expand All @@ -298,6 +299,12 @@ func TestTSSAddress(t *testing.T) {
chain: chains.SolanaDevnet,
addrExpected: testutils.TSSAddressEVMMainnet,
},
{
name: "should return empty address for unknown BTC chain",
chain: chains.BitcoinMainnet,
forceError: true,
addrExpected: "",
},
}

// run tests
Expand All @@ -306,8 +313,16 @@ func TestTSSAddress(t *testing.T) {
// create observer
ob := createObserver(t, tt.chain, defaultAlertLatency)

// force error if needed
if tt.forceError {
// pause TSS to cause error
tss := mocks.NewTSSMainnet()
tss.Pause()
ob = ob.WithTSS(tss)
}

// get TSS address
addr := ob.TSSAddress()
addr := ob.TSSAddressString()
require.Equal(t, tt.addrExpected, addr)
})
}
Expand Down Expand Up @@ -390,7 +405,7 @@ func TestOutboundID(t *testing.T) {
outboundID := ob.OutboundID(tt.nonce)

// expected outbound id
exepctedID := fmt.Sprintf("%d-%s-%d", tt.chain.ChainId, ob.TSSAddress(), tt.nonce)
exepctedID := fmt.Sprintf("%d-%s-%d", tt.chain.ChainId, ob.TSSAddressString(), tt.nonce)
require.Equal(t, exepctedID, outboundID)
})
}
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/bitcoin/observer/inbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (ob *Observer) ObserveInbound(ctx context.Context) error {
// add block header to zetacore
if len(res.Block.Tx) > 1 {
// filter incoming txs to TSS address
tssAddress := ob.TSSAddress()
tssAddress := ob.TSSAddressString()

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L149 was not covered by tests

// #nosec G115 always positive
events, err := FilterAndParseIncomingTx(
Expand Down
4 changes: 2 additions & 2 deletions zetaclient/chains/bitcoin/observer/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,8 @@ func (ob *Observer) FetchUTXOs(ctx context.Context) error {
maxConfirmations := int(bh)

// List all unspent UTXOs (160ms)
tssAddr := ob.TSS().BTCAddress(ob.Chain().ChainId)
if tssAddr == nil {
tssAddr, err := ob.TSS().BTCAddress(ob.Chain().ChainId)

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/observer.go#L372

Added line #L372 was not covered by tests
if err != nil {
return fmt.Errorf("error getting bitcoin tss address")

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/observer.go#L374

Added line #L374 was not covered by tests
}
utxos, err := ob.btcClient.ListUnspentMinMaxAddresses(0, maxConfirmations, []btcutil.Address{tssAddr})

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

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/observer.go#L376

Added line #L376 was not covered by tests
Expand Down
6 changes: 3 additions & 3 deletions zetaclient/chains/bitcoin/observer/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func (ob *Observer) getOutboundIDByNonce(ctx context.Context, nonce uint64, test

// findNonceMarkUTXO finds the nonce-mark UTXO in the list of UTXOs.
func (ob *Observer) findNonceMarkUTXO(nonce uint64, txid string) (int, error) {
tssAddress := ob.TSSAddress()
tssAddress := ob.TSSAddressString()
amount := chains.NonceMarkAmount(nonce)
for i, utxo := range ob.utxos {
sats, err := bitcoin.GetSatoshis(utxo.Amount)
Expand Down Expand Up @@ -599,7 +599,7 @@ func (ob *Observer) checkTSSVout(params *crosschaintypes.OutboundParams, vouts [
}

nonce := params.TssNonce
tssAddress := ob.TSSAddress()
tssAddress := ob.TSSAddressString()
for _, vout := range vouts {
// decode receiver and amount from vout
receiverExpected := tssAddress
Expand Down Expand Up @@ -658,7 +658,7 @@ func (ob *Observer) checkTSSVoutCancelled(params *crosschaintypes.OutboundParams
}

nonce := params.TssNonce
tssAddress := ob.TSSAddress()
tssAddress := ob.TSSAddressString()
for _, vout := range vouts {
// decode receiver and amount from vout
receiverVout, amount, err := bitcoin.DecodeTSSVout(vout, tssAddress, ob.Chain())
Expand Down
32 changes: 18 additions & 14 deletions zetaclient/chains/bitcoin/observer/outbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,29 @@ func createObserverWithPrivateKey(t *testing.T) *Observer {
func createObserverWithUTXOs(t *testing.T) *Observer {
// Create Bitcoin observer
ob := createObserverWithPrivateKey(t)
tssAddress := ob.TSS().BTCAddress(chains.BitcoinTestnet.ChainId).EncodeAddress()
tssAddress, err := ob.TSS().BTCAddress(chains.BitcoinTestnet.ChainId)
require.NoError(t, err)

// Create 10 dummy UTXOs (22.44 BTC in total)
ob.utxos = make([]btcjson.ListUnspentResult, 0, 10)
amounts := []float64{0.01, 0.12, 0.18, 0.24, 0.5, 1.26, 2.97, 3.28, 5.16, 8.72}
for _, amount := range amounts {
ob.utxos = append(ob.utxos, btcjson.ListUnspentResult{Address: tssAddress, Amount: amount})
ob.utxos = append(ob.utxos, btcjson.ListUnspentResult{Address: tssAddress.EncodeAddress(), Amount: amount})
}
return ob
}

func mineTxNSetNonceMark(ob *Observer, nonce uint64, txid string, preMarkIndex int) {
func mineTxNSetNonceMark(t *testing.T, ob *Observer, nonce uint64, txid string, preMarkIndex int) {
// Mine transaction
outboundID := ob.OutboundID(nonce)
ob.includedTxResults[outboundID] = &btcjson.GetTransactionResult{TxID: txid}

// Set nonce mark
tssAddress := ob.TSS().BTCAddress(chains.BitcoinTestnet.ChainId).EncodeAddress()
tssAddress, err := ob.TSS().BTCAddress(chains.BitcoinTestnet.ChainId)
require.NoError(t, err)
nonceMark := btcjson.ListUnspentResult{
TxID: txid,
Address: tssAddress,
Address: tssAddress.EncodeAddress(),
Amount: float64(chains.NonceMarkAmount(nonce)) * 1e-8,
}
if preMarkIndex >= 0 { // replace nonce-mark utxo
Expand Down Expand Up @@ -268,7 +270,7 @@ func TestSelectUTXOs(t *testing.T) {
require.Nil(t, result)
require.Zero(t, amount)
require.Equal(t, "getOutboundIDByNonce: cannot find outbound txid for nonce 0", err.Error())
mineTxNSetNonceMark(ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0
mineTxNSetNonceMark(t, ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0

// Case3: nonce = 1, should pass now
// input: utxoCap = 5, amount = 0.5, nonce = 1
Expand All @@ -277,7 +279,7 @@ func TestSelectUTXOs(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 0.55002, amount)
require.Equal(t, ob.utxos[0:5], result)
mineTxNSetNonceMark(ob, 1, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 1
mineTxNSetNonceMark(t, ob, 1, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 1

// Case4:
// input: utxoCap = 5, amount = 1.0, nonce = 2
Expand All @@ -286,7 +288,7 @@ func TestSelectUTXOs(t *testing.T) {
require.NoError(t, err)
require.InEpsilon(t, 1.05002001, amount, 1e-8)
require.Equal(t, ob.utxos[0:6], result)
mineTxNSetNonceMark(ob, 2, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 2
mineTxNSetNonceMark(t, ob, 2, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 2

// Case5: should include nonce-mark utxo on the LEFT
// input: utxoCap = 5, amount = 8.05, nonce = 3
Expand All @@ -296,7 +298,7 @@ func TestSelectUTXOs(t *testing.T) {
require.InEpsilon(t, 8.25002002, amount, 1e-8)
expected := append([]btcjson.ListUnspentResult{ob.utxos[0]}, ob.utxos[4:9]...)
require.Equal(t, expected, result)
mineTxNSetNonceMark(ob, 24105431, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 24105431
mineTxNSetNonceMark(t, ob, 24105431, dummyTxID, 0) // mine a transaction and set nonce-mark utxo for nonce 24105431

// Case6: should include nonce-mark utxo on the RIGHT
// input: utxoCap = 5, amount = 0.503, nonce = 24105432
Expand All @@ -306,7 +308,7 @@ func TestSelectUTXOs(t *testing.T) {
require.InEpsilon(t, 0.79107431, amount, 1e-8)
expected = append([]btcjson.ListUnspentResult{ob.utxos[4]}, ob.utxos[0:4]...)
require.Equal(t, expected, result)
mineTxNSetNonceMark(ob, 24105432, dummyTxID, 4) // mine a transaction and set nonce-mark utxo for nonce 24105432
mineTxNSetNonceMark(t, ob, 24105432, dummyTxID, 4) // mine a transaction and set nonce-mark utxo for nonce 24105432

// Case7: should include nonce-mark utxo in the MIDDLE
// input: utxoCap = 5, amount = 1.0, nonce = 24105433
Expand Down Expand Up @@ -348,7 +350,7 @@ func TestUTXOConsolidation(t *testing.T) {

t.Run("should not consolidate", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0
mineTxNSetNonceMark(t, ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0

// input: utxoCap = 10, amount = 0.01, nonce = 1, rank = 10
// output: [0.00002, 0.01], 0.01002
Expand All @@ -362,7 +364,7 @@ func TestUTXOConsolidation(t *testing.T) {

t.Run("should consolidate 1 utxo", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0
mineTxNSetNonceMark(t, ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0

// input: utxoCap = 9, amount = 0.01, nonce = 1, rank = 9
// output: [0.00002, 0.01, 0.12], 0.13002
Expand All @@ -376,7 +378,7 @@ func TestUTXOConsolidation(t *testing.T) {

t.Run("should consolidate 3 utxos", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0
mineTxNSetNonceMark(t, ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0

// input: utxoCap = 5, amount = 0.01, nonce = 0, rank = 5
// output: [0.00002, 0.014, 1.26, 0.5, 0.2], 2.01002
Expand All @@ -395,7 +397,7 @@ func TestUTXOConsolidation(t *testing.T) {

t.Run("should consolidate all utxos using rank 1", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0
mineTxNSetNonceMark(t, ob, 0, dummyTxID, -1) // mine a transaction and set nonce-mark utxo for nonce 0

// input: utxoCap = 12, amount = 0.01, nonce = 0, rank = 1
// output: [0.00002, 0.01, 8.72, 5.16, 3.28, 2.97, 1.26, 0.5, 0.24, 0.18, 0.12], 22.44002
Expand All @@ -415,6 +417,7 @@ func TestUTXOConsolidation(t *testing.T) {
t.Run("should consolidate 3 utxos sparse", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(
t,
ob,
24105431,
dummyTxID,
Expand All @@ -438,6 +441,7 @@ func TestUTXOConsolidation(t *testing.T) {
t.Run("should consolidate all utxos sparse", func(t *testing.T) {
ob := createObserverWithUTXOs(t)
mineTxNSetNonceMark(
t,
ob,
24105431,
dummyTxID,
Expand Down
7 changes: 6 additions & 1 deletion zetaclient/chains/bitcoin/observer/rpc_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ func (ob *Observer) watchRPCStatus(_ context.Context) error {

// checkRPCStatus checks the RPC status of the Bitcoin chain
func (ob *Observer) checkRPCStatus() {
tssAddress := ob.TSS().BTCAddress(ob.Chain().ChainId)
tssAddress, err := ob.TSS().BTCAddress(ob.Chain().ChainId)
if err != nil {
ob.Logger().Chain.Error().Err(err).Msg("unable to get TSS BTC address")
return

Check warning on line 35 in zetaclient/chains/bitcoin/observer/rpc_status.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/observer/rpc_status.go#L32-L35

Added lines #L32 - L35 were not covered by tests
}

blockTime, err := rpc.CheckRPCStatus(ob.btcClient, tssAddress)
if err != nil {
ob.Logger().Chain.Error().Err(err).Msg("CheckRPCStatus failed")
Expand Down
5 changes: 4 additions & 1 deletion zetaclient/chains/bitcoin/signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ func (signer *Signer) AddWithdrawTxOutputs(
}

// 1st output: the nonce-mark btc to TSS self
tssAddrP2WPKH := signer.TSS().BTCAddress(signer.Chain().ChainId)
tssAddrP2WPKH, err := signer.TSS().BTCAddress(signer.Chain().ChainId)
if err != nil {
return err

Check warning on line 159 in zetaclient/chains/bitcoin/signer/signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/chains/bitcoin/signer/signer.go#L159

Added line #L159 was not covered by tests
}
payToSelfScript, err := bitcoin.PayToAddrScript(tssAddrP2WPKH)
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion zetaclient/chains/bitcoin/signer/signer_keysign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func (suite *BTCSignTestSuite) SetupTest() {
suite.testSigner = &mocks.TSS{ // fake TSS
PrivKey: privateKey.ToECDSA(),
}
addr := suite.testSigner.BTCAddress(chains.BitcoinTestnet.ChainId)
addr, err := suite.testSigner.BTCAddress(chains.BitcoinTestnet.ChainId)
suite.Require().NoError(err)
suite.T().Logf("segwit addr: %s", addr)
}

Expand Down
3 changes: 2 additions & 1 deletion zetaclient/chains/bitcoin/signer/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ func TestAddWithdrawTxOutputs(t *testing.T) {
require.NoError(t, err)

// tss address and script
tssAddr := signer.TSS().BTCAddress(chains.BitcoinTestnet.ChainId)
tssAddr, err := signer.TSS().BTCAddress(chains.BitcoinTestnet.ChainId)
require.NoError(t, err)
tssScript, err := bitcoin.PayToAddrScript(tssAddr)
require.NoError(t, err)
fmt.Printf("tss address: %s", tssAddr.EncodeAddress())
Expand Down
2 changes: 1 addition & 1 deletion zetaclient/chains/interfaces/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,6 @@ type TSSSigner interface {

EVMAddress() ethcommon.Address
EVMAddressList() []ethcommon.Address
BTCAddress(chainID int64) *btcutil.AddressWitnessPubKeyHash
BTCAddress(chainID int64) (*btcutil.AddressWitnessPubKeyHash, error)
PubKeyCompressedBytes() []byte
}
21 changes: 13 additions & 8 deletions zetaclient/testutils/mocks/tss_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,26 +112,31 @@ func (s *TSS) EVMAddressList() []ethcommon.Address {
return []ethcommon.Address{s.EVMAddress()}
}

func (s *TSS) BTCAddress(_ int64) *btcutil.AddressWitnessPubKeyHash {
func (s *TSS) BTCAddress(_ int64) (*btcutil.AddressWitnessPubKeyHash, error) {
// return error if tss is paused
if s.paused {
return nil, fmt.Errorf("tss is paused")
}

// force use static btcAddress if set
if s.btcAddress != "" {
net, err := chains.GetBTCChainParams(s.chain.ChainId)
if err != nil {
return nil
return nil, err
}
addr, err := btcutil.DecodeAddress(s.btcAddress, net)
if err != nil {
return nil
return nil, err
}
return addr.(*btcutil.AddressWitnessPubKeyHash)
return addr.(*btcutil.AddressWitnessPubKeyHash), nil
}
// if privkey is set, use it to generate a segwit address
if s.PrivKey != nil {
pkBytes := crypto.FromECDSAPub(&s.PrivKey.PublicKey)
pk, err := btcec.ParsePubKey(pkBytes)
if err != nil {
fmt.Printf("error parsing pubkey: %v", err)
return nil
return nil, err
}

// witness program: https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#Witness_program
Expand All @@ -142,12 +147,12 @@ func (s *TSS) BTCAddress(_ int64) *btcutil.AddressWitnessPubKeyHash {
)
if err != nil {
fmt.Printf("error NewAddressWitnessPubKeyHash: %v", err)
return nil
return nil, err
}

return addrWPKH
return addrWPKH, nil
}
return nil
return nil, nil
}

// PubKeyCompressedBytes returns 33B compressed pubkey
Expand Down
Loading

0 comments on commit 7c5fbb9

Please sign in to comment.