Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion pkg/bitcoin/chain.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package bitcoin

import "context"

// Chain defines an interface meant to be used for interaction with the
// Bitcoin chain.
type Chain interface {
Expand All @@ -11,7 +13,12 @@ type Chain interface {
// GetTransactionConfirmations gets the number of confirmations for the
// transaction with the given transaction hash. If the transaction with the
// given hash was not found on the chain, this function returns an error.
GetTransactionConfirmations(transactionHash Hash) (uint, error)
// The provided context bounds the lookup; implementations should abort the
// underlying call when it is cancelled.
GetTransactionConfirmations(
ctx context.Context,
transactionHash Hash,
) (uint, error)

// BroadcastTransaction broadcasts the given transaction over the
// network of the Bitcoin chain nodes. If the broadcast action could not be
Expand Down
2 changes: 2 additions & 0 deletions pkg/bitcoin/chain_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package bitcoin

import (
"context"
"fmt"
"sync"
)
Expand Down Expand Up @@ -101,6 +102,7 @@ func (lc *localChain) GetTransactionMerkleProof(
}

func (lc *localChain) GetTransactionConfirmations(
_ context.Context,
transactionHash Hash,
) (uint, error) {
lc.transactionConfirmationsMutex.Lock()
Expand Down
25 changes: 24 additions & 1 deletion pkg/bitcoin/electrum/electrum.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ func (c *Connection) GetTransaction(
txID := transactionHash.Hex(bitcoin.ReversedByteOrder)

rawTransaction, err := requestWithRetry(
c.parentCtx,
c,
func(ctx context.Context, client *electrum.Client) (string, error) {
// We cannot use `GetTransaction` to get the the transaction details
Expand Down Expand Up @@ -130,6 +131,7 @@ func (c *Connection) GetTransaction(
// transaction with the given transaction hash. If the transaction with the
// given hash was not found on the chain, this function returns an error.
func (c *Connection) GetTransactionConfirmations(
ctx context.Context,
transactionHash bitcoin.Hash,
) (uint, error) {
txID := transactionHash.Hex(bitcoin.ReversedByteOrder)
Expand All @@ -138,7 +140,16 @@ func (c *Connection) GetTransactionConfirmations(
zap.String("txID", txID),
)

// Bound the network calls below by both the caller's context and the
// connection lifetime context, so a cancelled caller aborts the lookup while
// connection shutdown still cancels it as before.
reqCtx, cancel := context.WithCancel(ctx)
defer cancel()
stopOnParentDone := context.AfterFunc(c.parentCtx, cancel)
defer stopOnParentDone()

rawTransaction, err := requestWithRetry(
reqCtx,
Comment on lines +143 to +152

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Propagate the caller context through the latest-block lookup.

GetTransactionConfirmations bounds the raw transaction and history calls with reqCtx, but its later c.GetLatestBlockHeight() call uses c.parentCtx. A slow tip request can therefore continue past the monitor’s check budget and stall the monitoring pass. Add a context-aware internal latest-height helper and call it with reqCtx; add a regression test covering a blocked tip lookup.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/bitcoin/electrum/electrum.go` around lines 143 - 152, Update
GetTransactionConfirmations to use a context-aware internal latest-block-height
helper with reqCtx instead of c.GetLatestBlockHeight(), while preserving the
existing public method behavior. Implement the helper using the supplied context
and add a regression test that blocks the tip lookup and verifies cancellation
within the monitor check budget.

c,
func(ctx context.Context, client *electrum.Client) (string, error) {
// We cannot use `GetTransaction` to get the transaction details
Expand Down Expand Up @@ -208,6 +219,7 @@ txOutLoop:
reversedScriptHashString := hex.EncodeToString(reversedScriptHash)

scriptHashHistory, err := requestWithRetry(
reqCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -296,6 +308,7 @@ func (c *Connection) BroadcastTransaction(
var response string

response, err := requestWithRetry(
c.parentCtx,
c,
func(ctx context.Context, client *electrum.Client) (string, error) {
return client.BroadcastTransaction(ctx, rawTx)
Expand All @@ -315,6 +328,7 @@ func (c *Connection) BroadcastTransaction(
// latest block was not determined, this function returns an error.
func (c *Connection) GetLatestBlockHeight() (uint, error) {
blockHeight, err := requestWithRetry(
c.parentCtx,
c,
func(ctx context.Context, client *electrum.Client) (int32, error) {
tip, err := client.SubscribeHeadersSingle(ctx)
Expand Down Expand Up @@ -344,6 +358,7 @@ func (c *Connection) GetBlockHeader(
blockHeight uint,
) (*bitcoin.BlockHeader, error) {
getBlockHeaderResult, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -375,6 +390,7 @@ func (c *Connection) GetTransactionMerkleProof(
txID := transactionHash.Hex(bitcoin.ReversedByteOrder)

getMerkleProofResult, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -514,6 +530,7 @@ func (c *Connection) getConfirmedScriptHistory(
reversedScriptHashString := hex.EncodeToString(reversedScriptHash)

items, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -577,6 +594,7 @@ func (c *Connection) getConfirmedScriptHistory(
// block height.
func (c *Connection) GetCoinbaseTxHash(blockHeight uint) (bitcoin.Hash, error) {
txHashString, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -683,6 +701,7 @@ func (c *Connection) getScriptMempool(
reversedScriptHashString := hex.EncodeToString(reversedScriptHash)

items, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -885,6 +904,7 @@ func (c *Connection) getScriptUtxos(
reversedScriptHashString := hex.EncodeToString(reversedScriptHash)

items, err := requestWithRetry(
c.parentCtx,
c,
func(
ctx context.Context,
Expand Down Expand Up @@ -1173,6 +1193,7 @@ func (c *Connection) verifyServer() error {
}

server, err := requestWithRetry(
c.parentCtx,
c,
func(ctx context.Context, client *electrum.Client) (*Server, error) {
serverVersion, protocolVersion, err := client.ServerVersion(ctx)
Expand Down Expand Up @@ -1213,6 +1234,7 @@ func (c *Connection) keepAlive() {
select {
case <-ticker.C:
_, err := requestWithRetry(
c.parentCtx,
c,
func(ctx context.Context, client *electrum.Client) (interface{}, error) {
return nil, client.Ping(ctx)
Expand Down Expand Up @@ -1265,6 +1287,7 @@ func connectWithRetry(
}

func requestWithRetry[K interface{}](
parentCtx context.Context,
c *Connection,
requestFn func(ctx context.Context, client *electrum.Client) (K, error),
requestName string,
Expand All @@ -1275,7 +1298,7 @@ func requestWithRetry[K interface{}](
var result K

err := wrappers.DoWithDefaultRetry(
c.parentCtx,
parentCtx,
c.config.RequestRetryTimeout,
func(ctx context.Context) error {
if err := c.reconnectIfShutdown(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/bitcoin/electrum/electrum_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func TestGetTransactionConfirmations_Integration(t *testing.T) {
}
expectedConfirmations := latestBlockHeight - tx.BlockHeight

result, err := electrum.GetTransactionConfirmations(tx.TxHash)
result, err := electrum.GetTransactionConfirmations(context.Background(), tx.TxHash)
if err != nil {
t.Fatal(err)
}
Expand All @@ -252,7 +252,7 @@ func TestGetTransactionConfirmations_Negative_Integration(t *testing.T) {
electrum, cancelCtx := newTestConnection(t, testConfig.clientConfig)
defer cancelCtx()

_, err := electrum.GetTransactionConfirmations(invalidTxID)
_, err := electrum.GetTransactionConfirmations(context.Background(), invalidTxID)
if shouldSkipElectrumIntegrationError(err) {
t.Skipf("skipping due to transient electrum error: %v", err)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/bitcoin/spv_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bitcoin

import (
"bytes"
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
Expand Down Expand Up @@ -39,6 +40,7 @@ func AssembleSpvProof(
btcChain Chain,
) (*Transaction, *SpvProof, error) {
confirmations, err := btcChain.GetTransactionConfirmations(
context.Background(),
transactionHash,
)
if err != nil {
Expand Down
14 changes: 9 additions & 5 deletions pkg/clientinfo/performance.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ func (pm *PerformanceMetrics) registerAllMetrics() {
MetricWalletActionSuccessTotal,
MetricWalletActionFailedTotal,
MetricWalletHeartbeatFailuresTotal,
MetricStuckWalletTransactionsTotal,
MetricUnmonitoredWalletTransactionsTotal,
MetricCoordinationWindowsDetectedTotal,
MetricCoordinationProceduresExecutedTotal,
MetricCoordinationFailedTotal,
Expand Down Expand Up @@ -635,11 +637,13 @@ const (
MetricRedemptionProofSubmissionsFailedTotal = "redemption_proof_submissions_failed_total"

// Wallet Action Metrics (aggregate)
MetricWalletActionsTotal = "wallet_actions_total"
MetricWalletActionSuccessTotal = "wallet_action_success_total"
MetricWalletActionFailedTotal = "wallet_action_failed_total"
MetricWalletActionDurationSeconds = "wallet_action_duration_seconds"
MetricWalletHeartbeatFailuresTotal = "wallet_heartbeat_failures_total"
MetricWalletActionsTotal = "wallet_actions_total"
MetricWalletActionSuccessTotal = "wallet_action_success_total"
MetricWalletActionFailedTotal = "wallet_action_failed_total"
MetricWalletActionDurationSeconds = "wallet_action_duration_seconds"
MetricWalletHeartbeatFailuresTotal = "wallet_heartbeat_failures_total"
MetricStuckWalletTransactionsTotal = "stuck_wallet_transactions_total"
MetricUnmonitoredWalletTransactionsTotal = "unmonitored_wallet_transactions_total"

// Wallet Action Metrics (per-action type)
// These are generated dynamically using WalletActionMetricName helper function
Expand Down
2 changes: 1 addition & 1 deletion pkg/clientinfo/rpc_health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (f *fakeBitcoinChain) GetBlockHeader(blockHeight uint) (*bitcoin.BlockHeade
func (f *fakeBitcoinChain) GetTransaction(transactionHash bitcoin.Hash) (*bitcoin.Transaction, error) {
panic("not needed in rpc_health tests")
}
func (f *fakeBitcoinChain) GetTransactionConfirmations(transactionHash bitcoin.Hash) (uint, error) {
func (f *fakeBitcoinChain) GetTransactionConfirmations(_ context.Context, transactionHash bitcoin.Hash) (uint, error) {
panic("not needed in rpc_health tests")
}
func (f *fakeBitcoinChain) BroadcastTransaction(transaction *bitcoin.Transaction) error {
Expand Down
2 changes: 2 additions & 0 deletions pkg/maintainer/btcdiff/bitcoin_chain_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package btcdiff

import (
"context"
"fmt"

"github.com/keep-network/keep-core/pkg/bitcoin"
Expand All @@ -26,6 +27,7 @@ func (lbc *localBitcoinChain) GetTransaction(
// transaction with the given transaction hash. If the transaction with the
// given hash was not found on the chain, this function returns an error.
func (lbc *localBitcoinChain) GetTransactionConfirmations(
_ context.Context,
transactionHash bitcoin.Hash,
) (uint, error) {
panic("unsupported")
Expand Down
3 changes: 2 additions & 1 deletion pkg/maintainer/spv/bitcoin_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package spv

import (
"bytes"
"context"
"fmt"
"math/big"
"sync"
Expand Down Expand Up @@ -72,7 +73,7 @@ func (lbc *localBitcoinChain) GetTransaction(transactionHash bitcoin.Hash) (
return nil, fmt.Errorf("transaction not found")
}

func (lbc *localBitcoinChain) GetTransactionConfirmations(transactionHash bitcoin.Hash) (
func (lbc *localBitcoinChain) GetTransactionConfirmations(_ context.Context, transactionHash bitcoin.Hash) (
uint,
error,
) {
Expand Down
1 change: 1 addition & 0 deletions pkg/maintainer/spv/spv.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ func getProofInfo(
}

accumulatedConfirmations, err := btcChain.GetTransactionConfirmations(
context.Background(),
transactionHash,
)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/tbtc/bitcoin_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tbtc

import (
"bytes"
"context"
"fmt"
"sync"

Expand Down Expand Up @@ -39,6 +40,7 @@ func (lbc *localBitcoinChain) GetTransaction(
}

func (lbc *localBitcoinChain) GetTransactionConfirmations(
_ context.Context,
transactionHash bitcoin.Hash,
) (uint, error) {
for index, transaction := range lbc.transactions {
Expand Down
5 changes: 5 additions & 0 deletions pkg/tbtc/deposit_sweep.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tbtc

import (
"context"
"crypto/ecdsa"
"fmt"
"math/big"
Expand Down Expand Up @@ -105,6 +106,7 @@ func newDepositSweepAction(
proposalProcessingStartBlock uint64,
proposalExpiryBlock uint64,
waitForBlockFn waitForBlockFn,
transactionMonitor *transactionMonitor,
) *depositSweepAction {
transactionExecutor := newWalletTransactionExecutor(
btcChain,
Expand All @@ -113,6 +115,8 @@ func newDepositSweepAction(
waitForBlockFn,
)

transactionExecutor.setTransactionMonitor(transactionMonitor)

return &depositSweepAction{
logger: logger,
chain: chain,
Expand Down Expand Up @@ -340,6 +344,7 @@ func ValidateDepositSweepProposal(
)

confirmations, err := btcChain.GetTransactionConfirmations(
context.Background(),
depositKey.FundingTxHash,
)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/tbtc/deposit_sweep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ func TestDepositSweepAction_Execute(t *testing.T) {
func(ctx context.Context, blockHeight uint64) error {
return nil
},
nil,
)

// Modify the default parameters of the action to make
Expand Down
3 changes: 3 additions & 0 deletions pkg/tbtc/moved_funds_sweep.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func newMovedFundsSweepAction(
proposalProcessingStartBlock uint64,
proposalExpiryBlock uint64,
waitForBlockFn waitForBlockFn,
transactionMonitor *transactionMonitor,
) *movedFundsSweepAction {
transactionExecutor := newWalletTransactionExecutor(
btcChain,
Expand All @@ -115,6 +116,8 @@ func newMovedFundsSweepAction(
waitForBlockFn,
)

transactionExecutor.setTransactionMonitor(transactionMonitor)

return &movedFundsSweepAction{
logger: logger,
chain: chain,
Expand Down
1 change: 1 addition & 0 deletions pkg/tbtc/moved_funds_sweep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func TestMovedFundsSweepAction_Execute(t *testing.T) {
func(ctx context.Context, blockHeight uint64) error {
return nil
},
nil,
)

// Modify the default parameters of the action to make
Expand Down
3 changes: 3 additions & 0 deletions pkg/tbtc/moving_funds.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func newMovingFundsAction(
proposalProcessingStartBlock uint64,
proposalExpiryBlock uint64,
waitForBlockFn waitForBlockFn,
transactionMonitor *transactionMonitor,
) *movingFundsAction {
transactionExecutor := newWalletTransactionExecutor(
btcChain,
Expand All @@ -111,6 +112,8 @@ func newMovingFundsAction(
waitForBlockFn,
)

transactionExecutor.setTransactionMonitor(transactionMonitor)

return &movingFundsAction{
logger: logger,
chain: chain,
Expand Down
1 change: 1 addition & 0 deletions pkg/tbtc/moving_funds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func TestMovingFundsAction_Execute(t *testing.T) {
func(ctx context.Context, blockHeight uint64) error {
return nil
},
nil,
)

// Modify the default parameters of the action to make
Expand Down
Loading
Loading