Skip to content

Commit

Permalink
fix(btcstaking-tracker): fix race condition (#35)
Browse files Browse the repository at this point in the history
- Fixes race condition where `*btcec.PrivateKey` is accessed from
multiple go routines where read/write happen in
`NewDecyptionKeyFromBTCSK` from pkg
`github.com/babylonlabs-io/babylon/crypto/schnorr-adaptor-signature`
- Add `-race` flag to detect race conditions early
- Fixes [issue](#23)
and references
[issue](#22)
  • Loading branch information
Lazar955 authored Sep 10, 2024
1 parent 3d6f6b6 commit 8cb1b95
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 8 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ $(BUILDDIR)/:
mkdir -p $(BUILDDIR)/

test:
go test ./...
go test -race ./...

test-e2e:
cd $(TOOLS_DIR); go install -trimpath $(BABYLON_PKG);
go test -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e
go test -race -mod=readonly -timeout=25m -v $(PACKAGES_E2E) -count=1 --tags=e2e

build-docker:
$(DOCKER) build --tag babylonlabs-io/vigilante -f Dockerfile \
Expand Down
17 changes: 13 additions & 4 deletions btcstaking-tracker/btcslasher/slasher.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package btcslasher
import (
"context"
"fmt"
"github.com/babylonlabs-io/vigilante/types"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
"sync"
"time"

Expand Down Expand Up @@ -233,8 +235,8 @@ func (bs *BTCSlasher) equivocationTracker() {
// SlashFinalityProvider slashes all BTC delegations under a given finality provider
// the checkBTC option indicates whether to check the slashing tx's input is still spendable
// on Bitcoin (including mempool txs).
func (bs *BTCSlasher) SlashFinalityProvider(extractedfpBTCSK *btcec.PrivateKey) error {
fpBTCPK := bbn.NewBIP340PubKeyFromBTCPK(extractedfpBTCSK.PubKey())
func (bs *BTCSlasher) SlashFinalityProvider(extractedFpBTCSK *btcec.PrivateKey) error {
fpBTCPK := bbn.NewBIP340PubKeyFromBTCPK(extractedFpBTCSK.PubKey())
bs.logger.Infof("start slashing finality provider %s", fpBTCPK.MarshalHex())

// get all active and unbonded BTC delegations at the current BTC height
Expand All @@ -246,14 +248,19 @@ func (bs *BTCSlasher) SlashFinalityProvider(extractedfpBTCSK *btcec.PrivateKey)
return fmt.Errorf("failed to get BTC delegations under finality provider %s: %w", fpBTCPK.MarshalHex(), err)
}

// Initialize a mutex protected *btcec.PrivateKey
safeExtractedFpBTCSK := types.NewPrivateKeyWithMutex(extractedFpBTCSK)

// try to slash both staking and unbonding txs for each BTC delegation
// sign and submit slashing tx for each active delegation
// TODO: use semaphore to prevent spamming BTC node
for _, del := range activeBTCDels {
bs.wg.Add(1)
go func(d *bstypes.BTCDelegationResponse) {
defer bs.wg.Done()
bs.slashBTCDelegation(fpBTCPK, extractedfpBTCSK, d)
safeExtractedFpBTCSK.UseKey(func(key *secp256k1.PrivateKey) {
bs.slashBTCDelegation(fpBTCPK, key, d)
})
}(del)
}
// sign and submit slashing tx for each unbonded delegation
Expand All @@ -262,7 +269,9 @@ func (bs *BTCSlasher) SlashFinalityProvider(extractedfpBTCSK *btcec.PrivateKey)
bs.wg.Add(1)
go func(d *bstypes.BTCDelegationResponse) {
defer bs.wg.Done()
bs.slashBTCDelegation(fpBTCPK, extractedfpBTCSK, d)
safeExtractedFpBTCSK.UseKey(func(key *secp256k1.PrivateKey) {
bs.slashBTCDelegation(fpBTCPK, key, d)
})
}(del)
}

Expand Down
6 changes: 6 additions & 0 deletions btcstaking-tracker/btcslasher/slasher_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ func findFPIdxInWitness(fpBTCPK *bbn.BIP340PubKey, fpBtcPkList []bbn.BIP340PubKe
return 0, fmt.Errorf("the given finality provider's PK is not found in the BTC delegation")
}

// BuildSlashingTxWithWitness constructs a Bitcoin slashing transaction with the required witness data
// using the provided finality provider's private key. It handles the conversion and validation of
// various parameters needed for slashing a Bitcoin delegation, including the staking transaction,
// finality provider public keys, and covenant public keys.
// Note: this function is UNSAFE for concurrent accesses as slashTx.BuildSlashingTxWithWitness is not safe for
// concurrent access inside it's calling asig.NewDecyptionKeyFromBTCSK
func BuildSlashingTxWithWitness(
d *bstypes.BTCDelegationResponse,
bsParams *bstypes.Params,
Expand Down
13 changes: 11 additions & 2 deletions monitor/btcscanner/btc_scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Scanner interface {
}

type BtcScanner struct {
mu sync.Mutex
logger *zap.SugaredLogger

// connect to BTC node
Expand Down Expand Up @@ -94,7 +95,7 @@ func (bs *BtcScanner) Start(startHeight uint64) {
return
}

bs.BaseHeight = startHeight
bs.SetBaseHeight(startHeight)

bs.Started.Store(true)
bs.logger.Info("the BTC scanner is started")
Expand Down Expand Up @@ -143,7 +144,7 @@ func (bs *BtcScanner) Bootstrap() {
if bs.confirmedTipBlock != nil {
firstUnconfirmedHeight = uint64(bs.confirmedTipBlock.Height + 1)
} else {
firstUnconfirmedHeight = bs.BaseHeight
firstUnconfirmedHeight = bs.GetBaseHeight()
}

bs.logger.Infof("the bootstrapping starts at %d", firstUnconfirmedHeight)
Expand Down Expand Up @@ -283,5 +284,13 @@ func (bs *BtcScanner) Stop() {
}

func (bs *BtcScanner) GetBaseHeight() uint64 {
bs.mu.Lock()
defer bs.mu.Unlock()
return bs.BaseHeight
}

func (bs *BtcScanner) SetBaseHeight(h uint64) {
bs.mu.Lock()
defer bs.mu.Unlock()
bs.BaseHeight = h
}
33 changes: 33 additions & 0 deletions types/safeprivatekey.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package types

import (
"github.com/decred/dcrd/dcrec/secp256k1/v4"
"sync"
)

// PrivateKeyWithMutex wraps a btcec.PrivateKey with a mutex to ensure thread-safe access.
type PrivateKeyWithMutex struct {
mu sync.Mutex
key *secp256k1.PrivateKey
}

// NewPrivateKeyWithMutex creates a new PrivateKeyWithMutex.
func NewPrivateKeyWithMutex(key *secp256k1.PrivateKey) *PrivateKeyWithMutex {
return &PrivateKeyWithMutex{
key: key,
}
}

// GetKey safely retrieves the private key.
func (p *PrivateKeyWithMutex) GetKey() *secp256k1.PrivateKey {
p.mu.Lock()
defer p.mu.Unlock()
return p.key
}

// UseKey performs an operation with the private key in a thread-safe manner.
func (p *PrivateKeyWithMutex) UseKey(operation func(key *secp256k1.PrivateKey)) {
p.mu.Lock()
defer p.mu.Unlock()
operation(p.key)
}

0 comments on commit 8cb1b95

Please sign in to comment.