diff --git a/cmd/start.go b/cmd/start.go index 47a82626cf..f2eba399c2 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -196,6 +196,11 @@ func start(cmd *cobra.Command) error { // fatal: the operator opted into reservations, so a missing // watcher would silently strand anchors. if clientConfig.Tbtc.Reservations.Enabled { + if !clientConfig.Maintainer.Spv.Reservations.Enabled { + logger.Warnf("Client reservation proposal generation is enabled; " + + "ensure the paired Maintainer.Spv.Reservations.Enabled flag is also " + + "enabled in the maintainer config for end-to-end operation") + } if err := spv.WireReservationWatchers( ctx, tbtcChain, diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index 5f02098d1b..d252b92a67 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -2537,6 +2537,47 @@ func (tc *TbtcChain) ValidateReservationAnchorProposal( FundingTx *bitcoin.Transaction }, ) error { + abiProposal, abiExtraInfo := buildReservationAnchorProposalAbi( + walletPublicKeyHash, + proposal, + depositExtraInfo, + ) + + valid, err := tc.walletProposalValidator.ValidateReservationAnchorProposal( + abiProposal, + abiExtraInfo, + ) + if err != nil { + return fmt.Errorf("validation failed: [%v]", err) + } + + // Should never happen because `validateReservationAnchorProposal` + // returns true or reverts (returns an error) but do the check just in + // case. + if !valid { + return fmt.Errorf("unexpected validation result") + } + + return nil +} + +// buildReservationAnchorProposalAbi constructs the ABI-struct arguments +// for WalletProposalValidator.ValidateReservationAnchorProposal from their +// application-level representations. Extracted as a pure function from +// ValidateReservationAnchorProposal so the field mapping can be unit +// tested directly, mirroring the reverse-direction converters below +// (convertReservationFromAbiType et al.). +func buildReservationAnchorProposalAbi( + walletPublicKeyHash [20]byte, + proposal *tbtc.ReservationAnchorProposal, + depositExtraInfo struct { + *tbtc.Deposit + FundingTx *bitcoin.Transaction + }, +) ( + tbtcabi.WalletProposalValidatorReservationAnchorProposal, + tbtcabi.WalletProposalValidatorDepositExtraInfo, +) { // WalletProposalValidator's DepositExtraInfo.FundingTx is typed as // BitcoinTxInfo2 because the BitcoinTxInfo struct is renamed via the // collision hook in gen/Makefile (Bridge keeps the un-suffixed name; @@ -2569,22 +2610,7 @@ func (tc *TbtcChain) ValidateReservationAnchorProposal( AnchorTxFee: proposal.AnchorTxFee, } - valid, err := tc.walletProposalValidator.ValidateReservationAnchorProposal( - abiProposal, - abiExtraInfo, - ) - if err != nil { - return fmt.Errorf("validation failed: [%v]", err) - } - - // Should never happen because `validateReservationAnchorProposal` - // returns true or reverts (returns an error) but do the check just in - // case. - if !valid { - return fmt.Errorf("unexpected validation result") - } - - return nil + return abiProposal, abiExtraInfo } // ValidateReservationReanchorProposal asks the WalletProposalValidator @@ -2595,12 +2621,10 @@ func (tc *TbtcChain) ValidateReservationReanchorProposal( sourceWalletPublicKeyHash [20]byte, proposal *tbtc.ReservationReanchorProposal, ) error { - abiProposal := tbtcabi.WalletProposalValidatorReservationReanchorProposal{ - SourceWalletPubKeyHash: sourceWalletPublicKeyHash, - ReservationKey: proposal.ReservationKey, - TargetWalletPubKeyHash: proposal.TargetWalletPublicKeyHash, - ReanchorTxFee: proposal.ReanchorTxFee, - } + abiProposal := buildReservationReanchorProposalAbi( + sourceWalletPublicKeyHash, + proposal, + ) valid, err := tc.walletProposalValidator.ValidateReservationReanchorProposal( abiProposal, @@ -2619,6 +2643,23 @@ func (tc *TbtcChain) ValidateReservationReanchorProposal( return nil } +// buildReservationReanchorProposalAbi constructs the ABI-struct argument +// for WalletProposalValidator.ValidateReservationReanchorProposal from its +// application-level representation. Extracted as a pure function from +// ValidateReservationReanchorProposal so the field mapping can be unit +// tested directly. +func buildReservationReanchorProposalAbi( + sourceWalletPublicKeyHash [20]byte, + proposal *tbtc.ReservationReanchorProposal, +) tbtcabi.WalletProposalValidatorReservationReanchorProposal { + return tbtcabi.WalletProposalValidatorReservationReanchorProposal{ + SourceWalletPubKeyHash: sourceWalletPublicKeyHash, + ReservationKey: proposal.ReservationKey, + TargetWalletPubKeyHash: proposal.TargetWalletPublicKeyHash, + ReanchorTxFee: proposal.ReanchorTxFee, + } +} + // convertReservationFromAbiType converts the ReservationRouter-specific // Reservation.ReservationRequest ABI struct to the TBTC application // `tbtc.Reservation` representation. @@ -2840,6 +2881,7 @@ func (tc *TbtcChain) RequestReservationAcceptance( return err } + // Here we add a 20% margin to overcome the gas problems. gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) _, err = tc.reservationRouter.RequestReservationAcceptance( @@ -2868,6 +2910,7 @@ func (tc *TbtcChain) RequestReservationReanchor( return err } + // Here we add a 20% margin to overcome the gas problems. gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) _, err = tc.reservationRouter.RequestReservationReanchor( @@ -2961,6 +3004,7 @@ func (tc *TbtcChain) NotifyReservationActionTimeout( return err } + // Here we add a 20% margin to overcome the gas problems. gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) _, err = tc.reservationRouter.NotifyReservationActionTimeout( @@ -2986,6 +3030,7 @@ func (tc *TbtcChain) NotifyStaleReservedDeposit( return err } + // Here we add a 20% margin to overcome the gas problems. gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) _, err = tc.reservationRouter.NotifyStaleReservedDeposit( @@ -3010,6 +3055,7 @@ func (tc *TbtcChain) NotifyReservationStranded( return err } + // Here we add a 20% margin to overcome the gas problems. gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) _, err = tc.reservationRouter.NotifyReservationStranded( @@ -3022,6 +3068,49 @@ func (tc *TbtcChain) NotifyReservationStranded( return err } +// NotifyMovingFundsBelowDust notifies the Bridge that the given wallet's +// main UTXO has fallen below the moving funds dust threshold, ending the +// moving funds process and starting wallet closing immediately. This call +// is permissionless on-chain (MovingFunds.sol's notifyMovingFundsBelowDust +// carries no caller restriction), so it is submitted directly through the +// Bridge rather than routed through MaintainerProxy for reimbursement, +// mirroring the other reservation notify/request calls in this file. +func (tc *TbtcChain) NotifyMovingFundsBelowDust( + walletPublicKeyHash [20]byte, + mainUtxo *bitcoin.UnspentTransactionOutput, +) error { + var utxo tbtcabi.BitcoinTxUTXO + if mainUtxo != nil { + utxo = tbtcabi.BitcoinTxUTXO{ + TxHash: mainUtxo.Outpoint.TransactionHash, + TxOutputIndex: mainUtxo.Outpoint.OutputIndex, + TxOutputValue: uint64(mainUtxo.Value), + } + } + + gasEstimate, err := tc.bridge.NotifyMovingFundsBelowDustGasEstimate( + walletPublicKeyHash, + utxo, + ) + if err != nil { + return err + } + + // Here we add a 20% margin to overcome the gas problems, mirroring the + // other reservation notify calls in this file. + gasEstimateWithMargin := float64(gasEstimate) * float64(1.2) + + _, err = tc.bridge.NotifyMovingFundsBelowDust( + walletPublicKeyHash, + utxo, + ethutil.TransactionOptions{ + GasLimit: uint64(gasEstimateWithMargin), + }, + ) + + return err +} + // ReservationCaps returns the cap parameters that gate reservation // acceptance via the reservationRouter binding (see reservationRouterBinding). func (tc *TbtcChain) ReservationCaps() ( diff --git a/pkg/chain/ethereum/tbtc_test.go b/pkg/chain/ethereum/tbtc_test.go index ea6e9edb11..1614297fda 100644 --- a/pkg/chain/ethereum/tbtc_test.go +++ b/pkg/chain/ethereum/tbtc_test.go @@ -800,3 +800,122 @@ func TestConvertReservationParametersFromAbiType(t *testing.T) { ) } } + +func TestBuildReservationAnchorProposalAbi(t *testing.T) { + walletPublicKeyHash := [20]byte{ + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, + 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, + } + fundingTxHash := bitcoin.Hash{0x21, 0x22, 0x23} + + proposal := &tbtc.ReservationAnchorProposal{ + DepositFundingTxHash: fundingTxHash, + DepositFundingOutputIndex: 7, + AnchorTxFee: big.NewInt(1500), + } + + fundingTx := &bitcoin.Transaction{ + Version: 2, + Inputs: []*bitcoin.TransactionInput{{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x31}, + OutputIndex: 3, + }, + }}, + Outputs: []*bitcoin.TransactionOutput{{ + Value: 42000, + PublicKeyScript: []byte{0x00, 0x14}, + }}, + Locktime: 600000, + } + + deposit := &tbtc.Deposit{ + BlindingFactor: [8]byte{0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48}, + WalletPublicKeyHash: [20]byte{0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x60, 0x61, 0x62, 0x63, 0x64}, + RefundPublicKeyHash: [20]byte{0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x78, 0x79, 0x7a, 0x7b, 0x7c, 0x7d, 0x7e, 0x7f, 0x80, 0x81, 0x82, 0x83, 0x84}, + RefundLocktime: [4]byte{0x91, 0x92, 0x93, 0x94}, + } + + depositExtraInfo := struct { + *tbtc.Deposit + FundingTx *bitcoin.Transaction + }{Deposit: deposit, FundingTx: fundingTx} + + abiProposal, abiExtraInfo := buildReservationAnchorProposalAbi( + walletPublicKeyHash, + proposal, + depositExtraInfo, + ) + + expectedProposal := tbtcabi.WalletProposalValidatorReservationAnchorProposal{ + WalletPubKeyHash: walletPublicKeyHash, + DepositKey: tbtcabi.WalletProposalValidatorDepositKey{ + FundingTxHash: fundingTxHash, + FundingOutputIndex: 7, + }, + AnchorTxFee: big.NewInt(1500), + } + if !reflect.DeepEqual(expectedProposal, abiProposal) { + t.Errorf( + "unexpected abi proposal\nexpected: [%+v]\nactual: [%+v]\n", + expectedProposal, + abiProposal, + ) + } + + expectedExtraInfo := tbtcabi.WalletProposalValidatorDepositExtraInfo{ + FundingTx: tbtcabi.BitcoinTxInfo2{ + Version: fundingTx.SerializeVersion(), + InputVector: fundingTx.SerializeInputs(), + OutputVector: fundingTx.SerializeOutputs(), + Locktime: fundingTx.SerializeLocktime(), + }, + BlindingFactor: deposit.BlindingFactor, + WalletPubKeyHash: deposit.WalletPublicKeyHash, + RefundPubKeyHash: deposit.RefundPublicKeyHash, + RefundLocktime: deposit.RefundLocktime, + } + if !reflect.DeepEqual(expectedExtraInfo, abiExtraInfo) { + t.Errorf( + "unexpected abi extra info\nexpected: [%+v]\nactual: [%+v]\n", + expectedExtraInfo, + abiExtraInfo, + ) + } +} + +func TestBuildReservationReanchorProposalAbi(t *testing.T) { + sourceWalletPublicKeyHash := [20]byte{ + 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8, 0xa9, 0xaa, + 0xab, 0xac, 0xad, 0xae, 0xaf, 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, + } + targetWalletPublicKeyHash := [20]byte{ + 0xc1, 0xc2, 0xc3, 0xc4, 0xc5, 0xc6, 0xc7, 0xc8, 0xc9, 0xca, + 0xcb, 0xcc, 0xcd, 0xce, 0xcf, 0xd0, 0xd1, 0xd2, 0xd3, 0xd4, + } + + proposal := &tbtc.ReservationReanchorProposal{ + ReservationKey: big.NewInt(54321), + TargetWalletPublicKeyHash: targetWalletPublicKeyHash, + ReanchorTxFee: big.NewInt(1700), + } + + abiProposal := buildReservationReanchorProposalAbi( + sourceWalletPublicKeyHash, + proposal, + ) + + expected := tbtcabi.WalletProposalValidatorReservationReanchorProposal{ + SourceWalletPubKeyHash: sourceWalletPublicKeyHash, + ReservationKey: big.NewInt(54321), + TargetWalletPubKeyHash: targetWalletPublicKeyHash, + ReanchorTxFee: big.NewInt(1700), + } + if !reflect.DeepEqual(expected, abiProposal) { + t.Errorf( + "unexpected abi proposal\nexpected: [%+v]\nactual: [%+v]\n", + expected, + abiProposal, + ) + } +} diff --git a/pkg/clientinfo/performance.go b/pkg/clientinfo/performance.go index 54de8005f5..c817bb9d9a 100644 --- a/pkg/clientinfo/performance.go +++ b/pkg/clientinfo/performance.go @@ -329,6 +329,15 @@ func (pm *PerformanceMetrics) registerAllMetrics() { MetricRAMUtilizationPercent, MetricSwapUtilizationPercent, } + if pm.reservationsEnabled { + gauges = append( + gauges, + MetricReservationActiveReservationsCount, + MetricReservationMaxActiveReservations, + MetricReservationLiveWalletsCount, + MetricReservationWalletReservationsCount, + ) + } // First, initialize all gauges in the map pm.gaugesMutex.Lock() @@ -715,6 +724,16 @@ const ( MetricCPULoadPercent = "cpu_load_percent" MetricRAMUtilizationPercent = "ram_utilization_percent" MetricSwapUtilizationPercent = "swap_utilization_percent" + + // Reservation Metrics (m1 reservations feature; only registered when + // reservationsEnabled - see NewPerformanceMetrics). These are leading + // indicators of the §4.1 saturation cliff: without them, an operator + // cannot see reservation capacity approaching its cap before + // acceptances silently stop. + MetricReservationActiveReservationsCount = "active_reservations_count" + MetricReservationMaxActiveReservations = "max_active_reservations" + MetricReservationLiveWalletsCount = "live_wallets_count" + MetricReservationWalletReservationsCount = "wallet_reservations_count" ) // Network join request failure reasons. These are the low-cardinality diff --git a/pkg/clientinfo/performance_test.go b/pkg/clientinfo/performance_test.go index 622b1f86e5..f5a0eb4d06 100644 --- a/pkg/clientinfo/performance_test.go +++ b/pkg/clientinfo/performance_test.go @@ -541,3 +541,76 @@ func TestWalletActionMetricsNotRegisteredWhenReservationsDisabled(t *testing.T) } } } + +// TestReservationGaugesRegistered verifies the four reservation saturation +// gauges (active_reservations_count, max_active_reservations, +// live_wallets_count, wallet_reservations_count) are registered upfront +// with a 0 value when reservations are enabled. +func TestReservationGaugesRegistered(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + registry := &Registry{keepclientinfo.NewRegistry(), ctx} + pm := NewPerformanceMetrics(ctx, registry, true) + + reservationGauges := []string{ + MetricReservationActiveReservationsCount, + MetricReservationMaxActiveReservations, + MetricReservationLiveWalletsCount, + MetricReservationWalletReservationsCount, + } + + for _, name := range reservationGauges { + pm.gaugesMutex.RLock() + g, exists := pm.gauges[name] + pm.gaugesMutex.RUnlock() + if !exists { + t.Errorf("gauge %s should be registered upfront", name) + continue + } + g.mutex.RLock() + value := g.value + g.mutex.RUnlock() + if value != 0 { + t.Errorf("gauge %s should start at 0, got %v", name, value) + } + } + + // SetGauge (as the reservation tasks do) must update the registered + // gauge, not silently no-op. + pm.SetGauge(MetricReservationActiveReservationsCount, 42) + if got := pm.GetGaugeValue(MetricReservationActiveReservationsCount); got != 42 { + t.Errorf("expected active_reservations_count = 42, got %v", got) + } +} + +// TestReservationGaugesNotRegisteredWhenReservationsDisabled verifies the +// four reservation saturation gauges are absent (not just zero) when the +// m1 reservations feature is disabled, mirroring the wallet-action-metrics +// gating in TestWalletActionMetricsNotRegisteredWhenReservationsDisabled. +func TestReservationGaugesNotRegisteredWhenReservationsDisabled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + registry := &Registry{keepclientinfo.NewRegistry(), ctx} + pm := NewPerformanceMetrics(ctx, registry, false) + + reservationGauges := []string{ + MetricReservationActiveReservationsCount, + MetricReservationMaxActiveReservations, + MetricReservationLiveWalletsCount, + MetricReservationWalletReservationsCount, + } + + for _, name := range reservationGauges { + pm.gaugesMutex.RLock() + _, exists := pm.gauges[name] + pm.gaugesMutex.RUnlock() + if exists { + t.Errorf( + "gauge %s should not be registered when reservations are disabled", + name, + ) + } + } +} diff --git a/pkg/maintainer/spv/chain_test.go b/pkg/maintainer/spv/chain_test.go index 0973b1204f..33a52039aa 100644 --- a/pkg/maintainer/spv/chain_test.go +++ b/pkg/maintainer/spv/chain_test.go @@ -9,6 +9,7 @@ import ( "fmt" "math/big" "sync" + "testing" "github.com/ethereum/go-ethereum/common" @@ -1249,3 +1250,22 @@ func (lc *localChain) BuildDepositKey( key := buildDepositRequestKey(fundingTxHash, fundingOutputIndex) return new(big.Int).SetBytes(key[:]) } +func TestIsReservedDeposit_PointerIdentity(t *testing.T) { + spvChain := newLocalChain() + + // Set reserved with one pointer + key1 := big.NewInt(123) + wallet := [20]byte{1, 2, 3} + spvChain.setReservedDeposit(key1, wallet, true) + + // Check reserved with another pointer with same value + key2 := big.NewInt(123) + isReserved, err := spvChain.IsReservedDeposit(key2) + + if err != nil { + t.Fatal(err) + } + if !isReserved { + t.Fatal("expected deposit to be reserved") + } +} diff --git a/pkg/maintainer/spv/reservation_proof_loop.go b/pkg/maintainer/spv/reservation_proof_loop.go index 71dd35fe40..065c8941cb 100644 --- a/pkg/maintainer/spv/reservation_proof_loop.go +++ b/pkg/maintainer/spv/reservation_proof_loop.go @@ -425,6 +425,9 @@ func proveReservationAcceptanceActions( // Re-check every tracked event's on-chain action state, evict settled/stale // ones, and group still-pending events by wallet public key hash. + // nextScannedBlock starts at currentBlock but is pulled back by any + // eviction below, so a rewind survives the final cursor update. + nextScannedBlock := currentBlock walletEvents := make(map[[20]byte][]*tbtc.ReservationAcceptanceRequestedEvent) for key, event := range state.pendingAcceptanceEvents { action, err := spvChain.GetReservationAction( @@ -434,13 +437,27 @@ func proveReservationAcceptanceActions( if err != nil { state.acceptanceRetries[key]++ if state.acceptanceRetries[key] >= maxReservationActionLoadRetries { + // Eviction is non-lossy: rewind the cursor behind the + // lost event's block so the next pass re-fetches the + // range and rediscovers the event. Without this, a + // transient RPC outage that hits `maxRetries` consecutive + // times permanently strands the action generation: the + // cursor has already advanced past the event, no other + // code path re-creates the pending entry, and the + // already-confirmed Bitcoin anchor's SPV proof goes + // unsubmitted until action timeout slashes. + if event.BlockNumber > 0 && + (nextScannedBlock == 0 || event.BlockNumber-1 < nextScannedBlock) { + nextScannedBlock = event.BlockNumber - 1 + } logger.Errorf( "failed to load reservation acceptance action [%v]/%d: [%v]; "+ - "exceeded max retries (%d), evicting event", + "exceeded max retries (%d), evicting event and rewinding cursor to [%d]", event.ReservationKey, event.RequestNonce, err, maxReservationActionLoadRetries, + nextScannedBlock, ) delete(state.pendingAcceptanceEvents, key) delete(state.acceptanceRetries, key) @@ -530,7 +547,7 @@ func proveReservationAcceptanceActions( } } - state.acceptanceLastScannedBlock = currentBlock + state.acceptanceLastScannedBlock = nextScannedBlock return nil } @@ -641,9 +658,11 @@ func proveReservationReanchorActions( state.pendingReanchorEvents[key] = event } - // Re-check every tracked event's on-chain action state and drop the // Re-check every tracked event's on-chain action state, evict settled/stale // ones, and group still-pending events by source wallet public key hash. + // nextScannedBlock starts at currentBlock but is pulled back by any + // eviction below, so a rewind survives the final cursor update. + nextScannedBlock := currentBlock walletEvents := make(map[[20]byte][]*tbtc.ReservationReanchorRequestedEvent) for key, event := range state.pendingReanchorEvents { action, err := spvChain.GetReservationAction( @@ -653,13 +672,22 @@ func proveReservationReanchorActions( if err != nil { state.reanchorRetries[key]++ if state.reanchorRetries[key] >= maxReservationActionLoadRetries { + // Eviction is non-lossy: rewind the cursor behind the + // lost event's block so the next pass re-fetches the + // range and rediscovers the event. See the mirrored + // comment in proveReservationAcceptanceActions above. + if event.BlockNumber > 0 && + (nextScannedBlock == 0 || event.BlockNumber-1 < nextScannedBlock) { + nextScannedBlock = event.BlockNumber - 1 + } logger.Errorf( "failed to load reservation re-anchor action [%v]/%d: [%v]; "+ - "exceeded max retries (%d), evicting event", + "exceeded max retries (%d), evicting event and rewinding cursor to [%d]", event.ReservationKey, event.RequestNonce, err, maxReservationActionLoadRetries, + nextScannedBlock, ) delete(state.pendingReanchorEvents, key) delete(state.reanchorRetries, key) @@ -764,7 +792,7 @@ func proveReservationReanchorActions( } } - state.reanchorLastScannedBlock = currentBlock + state.reanchorLastScannedBlock = nextScannedBlock return nil } diff --git a/pkg/maintainer/spv/reservation_proof_loop_test.go b/pkg/maintainer/spv/reservation_proof_loop_test.go index ead2209ce8..c5c304d13c 100644 --- a/pkg/maintainer/spv/reservation_proof_loop_test.go +++ b/pkg/maintainer/spv/reservation_proof_loop_test.go @@ -866,6 +866,173 @@ func TestProveReservationAcceptanceActions(t *testing.T) { }) } +// TestProveReservationAcceptanceActions_EvictionRewindsCursor is a +// regression test for the eviction cursor-loss bug: when GetReservationAction +// fails maxReservationActionLoadRetries times in a row for a tracked event, +// the event is evicted from the pending map, but the scan cursor must be +// rewound behind the evicted event's block. Without the rewind, the cursor +// has already advanced past the event on every failed pass, so once the RPC +// recovers there is no code path left that rediscovers the event and its +// already-confirmed Bitcoin anchor transaction never gets its SPV proof +// submitted. +func TestProveReservationAcceptanceActions_EvictionRewindsCursor(t *testing.T) { + const proofStart = 790270 + diff := func(d int64) *big.Int { return big.NewInt(d) } + + spvChain := newLocalChain() + btcChain := newLocalBitcoinChain() + + if err := populateBlockHeaders( + btcChain, + proofStart, + proofStart+19, + func(uint) *big.Int { return diff(32) }, + ); err != nil { + t.Fatal(err) + } + spvChain.setTxProofDifficultyFactor(big.NewInt(6)) + spvChain.setCurrentEpoch(392) + spvChain.setCurrentAndPrevEpochDifficulty(diff(32), diff(16)) + + blockCounter := newMockBlockCounter() + blockCounter.SetCurrentBlock(1000) + spvChain.setBlockCounter(blockCounter) + + fundingTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{Value: 150000}}, + } + if err := btcChain.BroadcastTransaction(fundingTx); err != nil { + t.Fatal(err) + } + fundingTxHash := fundingTx.Hash() + reservationKey := spvChain.BuildDepositKey(fundingTxHash, 0) + const requestNonce = 1 + + spvChain.setDepositRequest(fundingTxHash, 0, &tbtc.DepositChainRequest{ + Amount: 150000, + }) + + walletPublicKeyHash := [20]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20} + walletScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + transaction := &bitcoin.Transaction{ + Inputs: []*bitcoin.TransactionInput{{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTxHash, + OutputIndex: 0, + }, + }}, + Outputs: []*bitcoin.TransactionOutput{{ + Value: 100000, + PublicKeyScript: walletScript, + }}, + } + if err := btcChain.BroadcastTransaction(transaction); err != nil { + t.Fatal(err) + } + if err := btcChain.addTransactionConfirmations( + transaction.Hash(), + 20, + ); err != nil { + t.Fatal(err) + } + btcChain.setCoinbaseTxHash(transaction.Hash()) + + spvChain.addReservationAcceptanceRequestedEvent(&tbtc.ReservationAcceptanceRequestedEvent{ + ReservationKey: reservationKey, + RequestNonce: requestNonce, + WalletPublicKeyHash: walletPublicKeyHash, + BlockNumber: 500, + }) + + // Simulate a persistent RPC outage on GetReservationAction. + spvChain.getReservationActionErr = fmt.Errorf("simulated chain read failure") + + submissions := 0 + spvChain.submitReservationProofHook = func( + proofType uint8, + txInfo *tbtc.BitcoinTxInfo, + proof *tbtc.BitcoinTxProof, + mainUtxo *tbtc.BitcoinTxUTXO, + reservationKey *big.Int, + requestNonce uint64, + ) error { + submissions++ + return nil + } + + config := Config{TransactionLimit: 100} + scanState := newReservationProofScanState() + key := reservationEventKey(reservationKey, requestNonce) + + for i := uint(1); i < maxReservationActionLoadRetries; i++ { + if err := proveReservationAcceptanceActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on pass %d: %v", i, err) + } + if _, exists := scanState.pendingAcceptanceEvents[key]; !exists { + t.Fatalf("expected event to remain pending on pass %d", i) + } + } + + // Final failing pass: exceeds max retries, evicts the event, and must + // rewind the cursor behind block 500 rather than leaving it at 1000. + if err := proveReservationAcceptanceActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on eviction pass: %v", err) + } + if _, exists := scanState.pendingAcceptanceEvents[key]; exists { + t.Fatal("expected event to be evicted after exceeding max retries") + } + if scanState.acceptanceLastScannedBlock >= 500 { + t.Fatalf( + "expected cursor to be rewound behind block 500, got %d", + scanState.acceptanceLastScannedBlock, + ) + } + + // RPC recovers and the action is genuinely pending. + spvChain.getReservationActionErr = nil + spvChain.setReservationAction( + reservationKey, + requestNonce, + &tbtc.ReservationAction{ + State: tbtc.ReservationActionStatePending, + ActionType: tbtc.ReservationActionTypeAcceptance, + TargetWalletPublicKeyHash: walletPublicKeyHash, + }, + ) + + // Next pass must rediscover the event via the rewound cursor and submit + // its proof; without the rewind fix the cursor would already be at 1000 + // and the event's block 500 would never be re-scanned. + if err := proveReservationAcceptanceActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on recovery pass: %v", err) + } + if submissions != 1 { + t.Fatalf("expected event to be rediscovered and proved after cursor rewind, got %d submissions", submissions) + } +} + // TestProveReservationReanchorActions is an end-to-end test of the // top-level orchestration function wired into production via // runReservationProofLoop: it seeds a requested event, a matching @@ -1143,6 +1310,180 @@ func TestProveReservationReanchorActions(t *testing.T) { }) } +// TestProveReservationReanchorActions_EvictionRewindsCursor mirrors +// TestProveReservationAcceptanceActions_EvictionRewindsCursor for the +// re-anchor path: after eviction on exceeded retries, the scan cursor must +// be rewound behind the evicted event's block so it is rediscovered once +// the RPC recovers. +func TestProveReservationReanchorActions_EvictionRewindsCursor(t *testing.T) { + const proofStart = 790270 + diff := func(d int64) *big.Int { return big.NewInt(d) } + + spvChain := newLocalChain() + btcChain := newLocalBitcoinChain() + + if err := populateBlockHeaders( + btcChain, + proofStart, + proofStart+19, + func(uint) *big.Int { return diff(32) }, + ); err != nil { + t.Fatal(err) + } + spvChain.setTxProofDifficultyFactor(big.NewInt(6)) + spvChain.setCurrentEpoch(392) + spvChain.setCurrentAndPrevEpochDifficulty(diff(32), diff(16)) + + blockCounter := newMockBlockCounter() + blockCounter.SetCurrentBlock(1000) + spvChain.setBlockCounter(blockCounter) + + reservationKey := big.NewInt(424242) + const requestNonce = 2 + + priorAnchorTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{ + {Value: 10000}, + {Value: 600000}, + }, + } + if err := btcChain.BroadcastTransaction(priorAnchorTx); err != nil { + t.Fatal(err) + } + anchorTxHash := priorAnchorTx.Hash() + anchorUtxo := &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: anchorTxHash, + OutputIndex: 1, + }, + Value: 600000, + } + + sourceWalletPublicKeyHash := [20]byte{21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40} + walletScript, err := bitcoin.PayToWitnessPublicKeyHash(sourceWalletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + transaction := &bitcoin.Transaction{ + Inputs: []*bitcoin.TransactionInput{{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: anchorTxHash, + OutputIndex: 1, + }, + }}, + Outputs: []*bitcoin.TransactionOutput{{ + Value: 590000, + PublicKeyScript: walletScript, + }}, + } + if err := btcChain.BroadcastTransaction(transaction); err != nil { + t.Fatal(err) + } + if err := btcChain.addTransactionConfirmations( + transaction.Hash(), + 20, + ); err != nil { + t.Fatal(err) + } + btcChain.setCoinbaseTxHash(transaction.Hash()) + + spvChain.addReservationReanchorRequestedEvent(&tbtc.ReservationReanchorRequestedEvent{ + ReservationKey: reservationKey, + RequestNonce: requestNonce, + SourceWalletPublicKeyHash: sourceWalletPublicKeyHash, + TargetWalletPublicKeyHash: sourceWalletPublicKeyHash, + BlockNumber: 500, + }) + spvChain.setReservation(reservationKey, &tbtc.Reservation{ + AnchorUtxo: anchorUtxo, + }) + + // Simulate a persistent RPC outage on GetReservationAction. + spvChain.getReservationActionErr = fmt.Errorf("simulated chain read failure") + + submissions := 0 + spvChain.submitReservationProofHook = func( + proofType uint8, + txInfo *tbtc.BitcoinTxInfo, + proof *tbtc.BitcoinTxProof, + mainUtxo *tbtc.BitcoinTxUTXO, + reservationKey *big.Int, + requestNonce uint64, + ) error { + submissions++ + return nil + } + + config := Config{TransactionLimit: 100} + scanState := newReservationProofScanState() + key := reservationEventKey(reservationKey, requestNonce) + + for i := uint(1); i < maxReservationActionLoadRetries; i++ { + if err := proveReservationReanchorActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on pass %d: %v", i, err) + } + if _, exists := scanState.pendingReanchorEvents[key]; !exists { + t.Fatalf("expected event to remain pending on pass %d", i) + } + } + + // Final failing pass: exceeds max retries, evicts the event, and must + // rewind the cursor behind block 500 rather than leaving it at 1000. + if err := proveReservationReanchorActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on eviction pass: %v", err) + } + if _, exists := scanState.pendingReanchorEvents[key]; exists { + t.Fatal("expected event to be evicted after exceeding max retries") + } + if scanState.reanchorLastScannedBlock >= 500 { + t.Fatalf( + "expected cursor to be rewound behind block 500, got %d", + scanState.reanchorLastScannedBlock, + ) + } + + // RPC recovers and the action is genuinely pending. + spvChain.getReservationActionErr = nil + spvChain.setReservationAction( + reservationKey, + requestNonce, + &tbtc.ReservationAction{ + State: tbtc.ReservationActionStatePending, + ActionType: tbtc.ReservationActionTypeReanchor, + TargetWalletPublicKeyHash: sourceWalletPublicKeyHash, + }, + ) + + // Next pass must rediscover the event via the rewound cursor and submit + // its proof; without the rewind fix the cursor would already be at 1000 + // and the event's block 500 would never be re-scanned. + if err := proveReservationReanchorActions( + scanState, + config, + spvChain, + spvChain, + btcChain, + ); err != nil { + t.Fatalf("unexpected error on recovery pass: %v", err) + } + if submissions != 1 { + t.Fatalf("expected event to be rediscovered and proved after cursor rewind, got %d submissions", submissions) + } +} + // TestSubmitReservationReanchorActionProof_UsesTargetWallet verifies that // submitReservationReanchorActionProof re-checks the action generation // against event.TargetWalletPublicKeyHash, not diff --git a/pkg/maintainer/spv/reservation_reanchor_proof.go b/pkg/maintainer/spv/reservation_reanchor_proof.go index f37da53e64..10b6646b0f 100644 --- a/pkg/maintainer/spv/reservation_reanchor_proof.go +++ b/pkg/maintainer/spv/reservation_reanchor_proof.go @@ -238,9 +238,15 @@ func submitReservationActionProof( return fmt.Errorf("provided required confirmations count must be greater than 0") } if reservationKey == nil { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("reservation key is required") } if requestNonce == 0 { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("request nonce must be > 0") } @@ -266,6 +272,9 @@ func submitReservationActionProof( action, err := spvChain.GetReservationAction(reservationKey, requestNonce) if err != nil { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("cannot fetch reservation action generation: [%v]", err) } @@ -278,10 +287,16 @@ func submitReservationActionProof( } if action.ActionType != expectedActionType { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("reservation action generation is not expected type") } if action.State != tbtc.ReservationActionStatePending { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("reservation action generation is not pending") } @@ -297,8 +312,15 @@ func submitReservationActionProof( reservationKey, requestNonce, ); err != nil { + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_failed_total", 1) + } return fmt.Errorf("failed to submit reservation proof: [%v]", err) } + if metricsRecorder != nil { + metricsRecorder.IncrementCounter(metricsPrefix+"_submissions_succeeded_total", 1) + } + return nil } diff --git a/pkg/maintainer/spv/reservation_stale_deposit_watch.go b/pkg/maintainer/spv/reservation_stale_deposit_watch.go index 51ac7655bf..0ebb20896c 100644 --- a/pkg/maintainer/spv/reservation_stale_deposit_watch.go +++ b/pkg/maintainer/spv/reservation_stale_deposit_watch.go @@ -40,8 +40,9 @@ const ( // because the anchor can never be produced. The watcher is the backstop that // flips the deposit's bookkeeping when the wallet never shows up. type ReservationStaleDepositWatcher struct { - spvChain Chain - notified map[string]struct{} + spvChain Chain + notified map[string]struct{} + memoizedTimeout map[string]uint32 } // NewReservationStaleDepositWatcher constructs a stale-deposit watcher @@ -50,8 +51,9 @@ func NewReservationStaleDepositWatcher( spvChain Chain, ) *ReservationStaleDepositWatcher { return &ReservationStaleDepositWatcher{ - spvChain: spvChain, - notified: make(map[string]struct{}), + spvChain: spvChain, + notified: make(map[string]struct{}), + memoizedTimeout: make(map[string]uint32), } } @@ -259,6 +261,10 @@ func (rsdw *ReservationStaleDepositWatcher) deriveTimeoutFromReveal( depositKey *big.Int, walletPublicKeyHash [20]byte, ) (uint32, error) { + if timeout, ok := rsdw.memoizedTimeout[depositKey.String()]; ok { + return timeout, nil + } + blockCounter, err := rsdw.spvChain.BlockCounter() if err != nil { return 0, fmt.Errorf( @@ -343,6 +349,7 @@ func (rsdw *ReservationStaleDepositWatcher) deriveTimeoutFromReveal( ) } - return uint32(depositRequest.RevealedAt.Unix()) + - params.ReservationActionTimeout, nil + result := uint32(depositRequest.RevealedAt.Unix()) + params.ReservationActionTimeout + rsdw.memoizedTimeout[depositKey.String()] = result + return result, nil } diff --git a/pkg/net/local/broadcast_channel_manager_test.go b/pkg/net/local/broadcast_channel_manager_test.go index 173a449759..c6c5c8216f 100644 --- a/pkg/net/local/broadcast_channel_manager_test.go +++ b/pkg/net/local/broadcast_channel_manager_test.go @@ -88,6 +88,16 @@ func TestReleaseBroadcastChannel(t *testing.T) { ReleaseBroadcastChannel(name) + // NewTimeTicker's piping goroutine selects between an already-elapsed + // timerTick.C and ctx.Done(); if both are ready when cancel() runs, Go's + // pseudo-random select can let exactly one straggler tick through before + // the goroutine observes cancellation and exits. That single straggler + // is a harmless, already-in-flight retransmission of a message already + // sent, not a sign the ticker "kept firing" - so absorb it in a short + // settle window before asserting the real invariant this test cares + // about: no further deliveries once release has taken effect. + drain(ch1Deliveries, RetransmissionTick) + if got := drain(ch1Deliveries, RetransmissionTick*3); got != 0 { t.Errorf("expected no deliveries after release, got %d", got) } diff --git a/pkg/tbtc/coordination.go b/pkg/tbtc/coordination.go index 5bc3e02fe3..c478a0ffb3 100644 --- a/pkg/tbtc/coordination.go +++ b/pkg/tbtc/coordination.go @@ -13,6 +13,7 @@ import ( "go.uber.org/zap" "golang.org/x/exp/slices" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-core/pkg/internal/pb" "golang.org/x/sync/semaphore" @@ -68,15 +69,34 @@ const ( // upgrade to a binary containing this constant before the activation block // is reached. DepositSweepEveryWindowActivationBlock = uint64(24559289) - // ReservationsActivationBlock is the Ethereum block height at which - // reservation actions (anchor, re-anchor) become available in the - // coordination checklist. + // reservationsActivationBlocks maps each Ethereum network to the block + // height at which reservation actions (anchor, re-anchor) become + // available in the coordination checklist. All operators must upgrade + // to a binary containing this table before a network's activation + // block is reached, mirroring DepositSweepEveryWindowActivationBlock's + // precondition above. Networks without an explicit entry (e.g. local/ + // dev chains) activate the feature immediately at block 0 instead of + // having it silently disabled for years on a lower-tip chain. // - // NOTE: This value is a placeholder that MUST be set to the real mainnet - // rollout height before release and must stay ahead of the chain tip. - ReservationsActivationBlock = uint64(26500000) + // NOTE: The mainnet value is a placeholder that MUST be set to the + // real mainnet rollout height before release and must stay ahead of + // the chain tip. ) +// reservationsActivationBlocks maps each Ethereum network to its +// reservations activation block. See the doc comment above. +var reservationsActivationBlocks = map[ethereum.Network]uint64{ + ethereum.Mainnet: 26500000, +} + +// reservationsActivationBlock returns the reservations activation block +// height for the given network. Networks without an explicit entry in +// reservationsActivationBlocks (e.g. local/dev chains) return 0, meaning +// reservation actions are active immediately. +func reservationsActivationBlock(network ethereum.Network) uint64 { + return reservationsActivationBlocks[network] +} + // errCoordinationExecutorBusy is an error returned when the coordination // executor cannot execute the requested coordination due to an ongoing one. var errCoordinationExecutorBusy = fmt.Errorf("coordination executor is busy") @@ -299,7 +319,8 @@ func (cm *coordinationMessage) Type() string { type coordinationExecutor struct { lock *semaphore.Weighted - chain Chain + chain Chain + ethereumNetwork ethereum.Network coordinatedWallet wallet membersIndexes []group.MemberIndex @@ -325,6 +346,7 @@ type coordinationExecutor struct { // given wallet. func newCoordinationExecutor( chain Chain, + ethereumNetwork ethereum.Network, coordinatedWallet wallet, membersIndexes []group.MemberIndex, operatorAddress chain.Address, @@ -337,6 +359,7 @@ func newCoordinationExecutor( return &coordinationExecutor{ lock: semaphore.NewWeighted(1), chain: chain, + ethereumNetwork: ethereumNetwork, coordinatedWallet: coordinatedWallet, membersIndexes: membersIndexes, operatorAddress: operatorAddress, @@ -649,13 +672,11 @@ func (ce *coordinationExecutor) getActionsChecklist( // throughput-driven DepositSweep/MovedFundsSweep/MovingFunds actions // above: a delayed reservation acceptance or re-anchor risks the // on-chain ReservationActionTimeout backstop firing before the wallet - // subsystem gets a chance to act. There is no per-operator enable - // flag here; the activation block is a network-wide constant, which - // keeps leader and follower checklists in agreement without relying - // on local config (a follower whose local config diverged from the - // leader's would otherwise fault an honest leader's reservation - // proposal as FaultLeaderMistake because the action would not appear - // in its own checklist). + // subsystem gets a chance to act. The activation block is a per-network + // table (reservationsActivationBlock), not a single global constant, but + // it is still config-independent and globally observable from chain + // height alone -- which is what keeps leader and follower checklists in + // agreement without relying on local config. // // Note: because getActionsChecklist appends reservation actions after // Redemption/DepositSweep/MovedFundsSweep/MovingFunds and @@ -664,7 +685,7 @@ func (ce *coordinationExecutor) getActionsChecklist( // traffic can still delay reservation acceptance/re-anchor even // though the checklist entry itself is unconditional. This is an // accepted tradeoff bounded by ReservationActionTimeout, not a bug. - if coordinationBlock >= ReservationsActivationBlock { + if coordinationBlock >= reservationsActivationBlock(ce.ethereumNetwork) { actions = append(actions, ActionReservationAnchor) actions = append(actions, ActionReservationReanchor) } diff --git a/pkg/tbtc/coordination_test.go b/pkg/tbtc/coordination_test.go index 365e72c508..e9a15309a1 100644 --- a/pkg/tbtc/coordination_test.go +++ b/pkg/tbtc/coordination_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/go-test/deep" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "golang.org/x/exp/slices" "github.com/keep-network/keep-core/pkg/bitcoin" @@ -321,6 +322,7 @@ func runCoordinationRound( go func(operatorIndex int, op *coordinationOperatorFixture) { executor := newCoordinationExecutor( op.chain, + ethereum.Unknown, coordinatedWallet, coordinatedWallet.membersByOperator(op.address), op.address, @@ -874,7 +876,7 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { // loop since it does not vary per subtest. var _ uint64 = DepositSweepEveryWindowActivationBlock - executor := &coordinationExecutor{} + executor := &coordinationExecutor{ethereumNetwork: ethereum.Mainnet} for testName, test := range tests { t.Run( @@ -1018,7 +1020,7 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { // be typed as uint64. var _ uint64 = DepositSweepEveryWindowActivationBlock - executor := &coordinationExecutor{} + executor := &coordinationExecutor{ethereumNetwork: ethereum.Mainnet} for testName, test := range tests { t.Run( @@ -1080,17 +1082,17 @@ func TestCoordinationExecutor_GetActionsChecklist_Reservations(t *testing.T) { expectedReservationActions []WalletActionType }{ "below activation": { - coordinationBlock: ReservationsActivationBlock - 1, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet] - 1, windowIndex: 4, expectedReservationActions: nil, }, "at activation, non-4th window": { - coordinationBlock: ReservationsActivationBlock, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet], windowIndex: 5, expectedReservationActions: []WalletActionType{ActionReservationAnchor, ActionReservationReanchor}, }, "at activation, 4th window": { - coordinationBlock: ReservationsActivationBlock, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet], windowIndex: 4, expectedReservationActions: []WalletActionType{ActionReservationAnchor, ActionReservationReanchor}, }, @@ -1098,7 +1100,7 @@ func TestCoordinationExecutor_GetActionsChecklist_Reservations(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - executor := &coordinationExecutor{} + executor := &coordinationExecutor{ethereumNetwork: ethereum.Mainnet} // We don't care about the seed for this test, as it only affects // the ActionHeartbeat which is not the focus here. @@ -1127,15 +1129,16 @@ func TestCoordinationExecutor_GetActionsChecklist_Reservations(t *testing.T) { func TestReservationsActivationBlock_SanityCheck(t *testing.T) { // Reference Ethereum mainnet block height as of 2026-09-02 (~25,880,000). - // ReservationsActivationBlock must be set to a future block height ahead - // of chain tip before release. If this test fails, both the reference - // height and ReservationsActivationBlock must be updated. + // reservationsActivationBlocks[ethereum.Mainnet] must be set to a future + // block height ahead of chain tip before release. If this test fails, + // both the reference height and that value must be updated. const referenceMainnetBlockHeight = uint64(25880000) - if ReservationsActivationBlock <= referenceMainnetBlockHeight { + mainnetActivationBlock := reservationsActivationBlocks[ethereum.Mainnet] + if mainnetActivationBlock <= referenceMainnetBlockHeight { t.Errorf( - "ReservationsActivationBlock [%d] must be ahead of the reference mainnet block height [%d]", - ReservationsActivationBlock, + "mainnet reservationsActivationBlock [%d] must be ahead of the reference mainnet block height [%d]", + mainnetActivationBlock, referenceMainnetBlockHeight, ) } diff --git a/pkg/tbtc/inactivity_test.go b/pkg/tbtc/inactivity_test.go index ce8762a455..7eab1eebba 100644 --- a/pkg/tbtc/inactivity_test.go +++ b/pkg/tbtc/inactivity_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "golang.org/x/crypto/sha3" "github.com/keep-network/keep-core/internal/testutils" @@ -171,6 +172,7 @@ func setupInactivityClaimExecutorScenario(t *testing.T) ( ) node, err := newNode( + ethereum.Unknown, groupParameters, localChain, newLocalBitcoinChain(), diff --git a/pkg/tbtc/marshaling.go b/pkg/tbtc/marshaling.go index ffddebf4b2..bee270cadb 100644 --- a/pkg/tbtc/marshaling.go +++ b/pkg/tbtc/marshaling.go @@ -493,8 +493,12 @@ func validateMemberIndex(protoIndex uint32) error { return nil } -// Marshal converts the reservationAnchorProposal to a byte array. +// Marshal converts the ReservationAnchorProposal to a byte array. func (rap *ReservationAnchorProposal) Marshal() ([]byte, error) { + if rap.AnchorTxFee == nil { + return nil, fmt.Errorf("anchor transaction fee is required") + } + return proto.Marshal( &pb.ReservationAnchorProposal{ DepositFundingTxHash: rap.DepositFundingTxHash[:], @@ -504,7 +508,7 @@ func (rap *ReservationAnchorProposal) Marshal() ([]byte, error) { }) } -// Unmarshal converts a byte array back to the reservationAnchorProposal. +// Unmarshal converts a byte array back to the ReservationAnchorProposal. func (rap *ReservationAnchorProposal) Unmarshal(data []byte) error { pbMsg := pb.ReservationAnchorProposal{} if err := proto.Unmarshal(data, &pbMsg); err != nil { @@ -529,6 +533,9 @@ func (rap *ReservationAnchorProposal) Unmarshal(data []byte) error { len(pbMsg.DepositFundingTxHash), ) } + if [32]byte(pbMsg.DepositFundingTxHash) == [32]byte{} { + return fmt.Errorf("deposit funding tx hash is required") + } copy(rap.DepositFundingTxHash[:], pbMsg.DepositFundingTxHash) rap.DepositFundingOutputIndex = pbMsg.DepositFundingOutputIndex @@ -538,8 +545,15 @@ func (rap *ReservationAnchorProposal) Unmarshal(data []byte) error { return nil } -// Marshal converts the reservationReanchorProposal to a byte array. +// Marshal converts the ReservationReanchorProposal to a byte array. func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { + if rrp.ReservationKey == nil { + return nil, fmt.Errorf("reservation key is required") + } + if rrp.ReanchorTxFee == nil { + return nil, fmt.Errorf("re-anchor transaction fee is required") + } + return proto.Marshal( &pb.ReservationReanchorProposal{ ReservationKey: rrp.ReservationKey.Bytes(), @@ -549,7 +563,7 @@ func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { }) } -// Unmarshal converts a byte array back to the reservationReanchorProposal. +// Unmarshal converts a byte array back to the ReservationReanchorProposal. func (rrp *ReservationReanchorProposal) Unmarshal(data []byte) error { pbMsg := pb.ReservationReanchorProposal{} if err := proto.Unmarshal(data, &pbMsg); err != nil { diff --git a/pkg/tbtc/marshaling_test.go b/pkg/tbtc/marshaling_test.go index 5fa3692918..adb264c6da 100644 --- a/pkg/tbtc/marshaling_test.go +++ b/pkg/tbtc/marshaling_test.go @@ -505,6 +505,44 @@ func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithNoopProposal(t *testing } } +func TestReservationAnchorProposal_Marshal_NilPanic(t *testing.T) { + proposal := &ReservationAnchorProposal{ + AnchorTxFee: nil, + } + + _, err := proposal.Marshal() + if err == nil { + t.Fatal("expected error when marshaling proposal with nil AnchorTxFee") + } + if err.Error() != "anchor transaction fee is required" { + t.Errorf("unexpected error: [%v]", err) + } +} + +func TestReservationAnchorProposal_Unmarshal_ZeroHash(t *testing.T) { + proposal := &ReservationAnchorProposal{ + DepositFundingTxHash: [32]byte{}, + } + // Manually construct the protobuf message to bypass nil check + pbMsg := &pb.ReservationAnchorProposal{ + DepositFundingTxHash: proposal.DepositFundingTxHash[:], + DepositFundingOutputIndex: 0, + RequestNonce: 1, + AnchorTxFee: big.NewInt(1000).Bytes(), + } + data, err := proto.Marshal(pbMsg) + if err != nil { + t.Fatal(err) + } + + err = proposal.Unmarshal(data) + if err == nil { + t.Fatal("expected error when unmarshaling proposal with zero hash") + } + if err.Error() != "deposit funding tx hash is required" { + t.Errorf("unexpected error: [%v]", err) + } +} func TestFuzzCoordinationMessage_Unmarshaler(t *testing.T) { pbutils.FuzzUnmarshaler(&coordinationMessage{}) } diff --git a/pkg/tbtc/node.go b/pkg/tbtc/node.go index a41913d2a0..5a96077995 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -10,6 +10,7 @@ import ( "github.com/keep-network/keep-core/pkg/chain" "github.com/keep-network/keep-core/pkg/clientinfo" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-common/pkg/persistence" "github.com/keep-network/keep-core/pkg/generator" @@ -44,6 +45,7 @@ const ( // node represents the current state of an ECDSA node. type node struct { + ethereumNetwork ethereum.Network groupParameters *GroupParameters chain Chain @@ -121,6 +123,7 @@ type node struct { } func newNode( + ethereumNetwork ethereum.Network, groupParameters *GroupParameters, chain Chain, btcChain bitcoin.Chain, @@ -145,6 +148,7 @@ func newNode( node := &node{ groupParameters: groupParameters, chain: chain, + ethereumNetwork: ethereumNetwork, btcChain: btcChain, netProvider: netProvider, walletRegistry: walletRegistry, @@ -230,6 +234,18 @@ func (n *node) setPerformanceMetrics(metrics interface { pg.SetRedemptionMetricsRecorder(metrics) } + // Wire reservation metrics to proposal generator if it supports it, + // mirroring the redemption wiring above. A no-op on non-reservation + // deployments since SetReservationMetricsRecorder finds no reservation + // tasks in that case. + if pg, ok := n.proposalGenerator.(interface { + SetReservationMetricsRecorder(recorder interface { + SetGauge(name string, value float64) + }) + }); ok { + pg.SetReservationMetricsRecorder(metrics) + } + // Update metrics recorder for all cached coordination executors // This is important because executors may be created before metrics are set n.coordinationExecutorsMutex.Lock() diff --git a/pkg/tbtc/node_executors.go b/pkg/tbtc/node_executors.go index 8a33854ef6..fee41512e4 100644 --- a/pkg/tbtc/node_executors.go +++ b/pkg/tbtc/node_executors.go @@ -201,6 +201,7 @@ func (n *node) getCoordinationExecutor( executor := newCoordinationExecutor( n.chain, + n.ethereumNetwork, wallet, membersIndexes, operatorAddress, diff --git a/pkg/tbtc/node_test.go b/pkg/tbtc/node_test.go index 2a6820fb16..bc3e41b5c2 100644 --- a/pkg/tbtc/node_test.go +++ b/pkg/tbtc/node_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-common/pkg/persistence" "github.com/keep-network/keep-core/internal/testutils" "github.com/keep-network/keep-core/pkg/bitcoin" @@ -52,6 +53,7 @@ func TestNode_GetSigningExecutor(t *testing.T) { keyStorePersistence := createMockKeyStorePersistence(t, signer) node, err := newNode( + ethereum.Unknown, groupParameters, localChain, newLocalBitcoinChain(), @@ -184,6 +186,7 @@ func TestNode_GetCoordinationExecutor(t *testing.T) { keyStorePersistence := createMockKeyStorePersistence(t, signer) node, err := newNode( + ethereum.Unknown, groupParameters, localChain, newLocalBitcoinChain(), @@ -321,6 +324,7 @@ func TestNode_RunCoordinationLayer(t *testing.T) { keyStorePersistence := createMockKeyStorePersistence(t, signer) n, err := newNode( + ethereum.Unknown, groupParameters, localChain, newLocalBitcoinChain(), @@ -1122,6 +1126,7 @@ func setupNodeForClosureTests(t *testing.T) (*node, *signer, *localChain) { }) n, err := newNode( + ethereum.Unknown, groupParameters, lc, newLocalBitcoinChain(), @@ -1301,6 +1306,7 @@ func setupNodeWithChain(t *testing.T) (*node, *signer, *localChain) { }) n, err := newNode( + ethereum.Unknown, groupParameters, lc, newLocalBitcoinChain(), diff --git a/pkg/tbtc/reservation.go b/pkg/tbtc/reservation.go index 2a36f08787..8cecef2958 100644 --- a/pkg/tbtc/reservation.go +++ b/pkg/tbtc/reservation.go @@ -444,9 +444,14 @@ func (raa *reservationAnchorAction) execute() error { // be block-range narrowed the way the deposit sweep validation path // narrows it via DepositsRevealBlocks. Narrow by wallet PKH instead and // match the exact funding outpoint among the returned events. + eventsStartBlock := uint64(0) + if raa.startBlock > reservationLookBackBlocks { + eventsStartBlock = raa.startBlock - reservationLookBackBlocks + } + events, err := raa.chain.PastDepositRevealedEvents(&DepositRevealedEventFilter{ WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - StartBlock: raa.startBlock - reservationLookBackBlocks, + StartBlock: eventsStartBlock, }) if err != nil { return fmt.Errorf("cannot fetch deposit revealed events: [%v]", err) @@ -491,6 +496,9 @@ func (raa *reservationAnchorAction) execute() error { if action.ActionType != ReservationActionTypeAcceptance || action.State != ReservationActionStatePending { return fmt.Errorf("reservation action is not a pending acceptance") } + if action.TargetWalletPublicKeyHash != walletPublicKeyHash { + return fmt.Errorf("reservation action targets a different wallet") + } err = raa.chain.ValidateReservationAnchorProposal( walletPublicKeyHash, diff --git a/pkg/tbtc/reservation_test.go b/pkg/tbtc/reservation_test.go index 21bdb5fe41..0153d81aad 100644 --- a/pkg/tbtc/reservation_test.go +++ b/pkg/tbtc/reservation_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/btcec" "go.uber.org/zap" "google.golang.org/protobuf/proto" @@ -466,6 +467,194 @@ func TestAssembleReservationTransactions_FeeBoundaries(t *testing.T) { } } +// TestAssembleReservationTransactions_HappyPathShape verifies the actual +// shape of a successfully assembled reservation anchor/re-anchor +// transaction: exactly one input, exactly one output, the output value +// equal to input value minus fee, and a P2WPKH locking script paying the +// target wallet. Existing tests only exercise error/boundary paths; none +// assert the happy-path output shape. +func TestAssembleReservationTransactions_HappyPathShape(t *testing.T) { + targetWalletPublicKeyHash := [20]byte{ + 0x8d, 0xb5, 0x0e, 0xb5, 0x20, 0x63, 0xea, 0x9d, 0x98, 0xb3, + 0xea, 0xc9, 0x14, 0x89, 0xa9, 0x0f, 0x73, 0x89, 0x86, 0xf6, + } + const fee = int64(1500) + const depositValue = int64(100000) + + expectedOutputScript, err := bitcoin.PayToWitnessPublicKeyHash( + targetWalletPublicKeyHash, + ) + if err != nil { + t.Fatal(err) + } + + btcecKey, err := btcec.NewPrivateKey(btcec.S256()) + if err != nil { + t.Fatal(err) + } + signingKey := (*ecdsa.PrivateKey)(btcecKey) + + t.Run("anchor transaction", func(t *testing.T) { + bitcoinChain := newLocalBitcoinChain() + + deposit := &Deposit{ + Depositor: "0x934b98637ca318a4d6e7ca6ffd1690b8e77df637", + WalletPublicKeyHash: [20]byte{0xaa}, + RefundPublicKeyHash: [20]byte{0xbb}, + RefundLocktime: [4]byte{0x60, 0xbc, 0xea, 0x61}, + } + depositScript, err := deposit.Script() + if err != nil { + t.Fatal(err) + } + scriptHash := sha256.Sum256(depositScript) + fundingOutputScript, err := bitcoin.PayToWitnessScriptHash(scriptHash) + if err != nil { + t.Fatal(err) + } + + fundingTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: depositValue, + PublicKeyScript: fundingOutputScript, + }}, + } + if err := bitcoinChain.BroadcastTransaction(fundingTx); err != nil { + t.Fatal(err) + } + + deposit.Utxo = &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTx.Hash(), + OutputIndex: 0, + }, + Value: depositValue, + } + + builder, err := AssembleReservationAnchorTransaction( + bitcoinChain, + deposit, + targetWalletPublicKeyHash, + &ReservationAction{TxMaxFee: 2000}, + fee, + ) + if err != nil { + t.Fatal(err) + } + + signedTx := signReservationTransaction( + t, + builder, + &signingKey.PublicKey, + signingKey.D, + ) + + assertReservationTransactionShape( + t, + signedTx, + depositValue, + fee, + expectedOutputScript, + ) + }) + + t.Run("re-anchor transaction", func(t *testing.T) { + bitcoinChain := newLocalBitcoinChain() + + anchorOutputScript, err := bitcoin.PayToWitnessPublicKeyHash( + [20]byte{0xcc}, + ) + if err != nil { + t.Fatal(err) + } + + anchorFundingTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: depositValue, + PublicKeyScript: anchorOutputScript, + }}, + } + if err := bitcoinChain.BroadcastTransaction(anchorFundingTx); err != nil { + t.Fatal(err) + } + + anchorUtxo := &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: anchorFundingTx.Hash(), + OutputIndex: 0, + }, + Value: depositValue, + } + + builder, err := AssembleReservationReanchorTransaction( + bitcoinChain, + anchorUtxo, + targetWalletPublicKeyHash, + &ReservationAction{TxMaxFee: 2000}, + fee, + ) + if err != nil { + t.Fatal(err) + } + + signedTx := signReservationTransaction( + t, + builder, + &signingKey.PublicKey, + signingKey.D, + ) + + assertReservationTransactionShape( + t, + signedTx, + depositValue, + fee, + expectedOutputScript, + ) + }) +} + +// assertReservationTransactionShape asserts the invariants a successfully +// assembled and signed reservation anchor/re-anchor transaction must +// satisfy: exactly 1 input, exactly 1 output, output value == inputValue - +// fee, and the output's locking script matches expectedOutputScript exactly. +func assertReservationTransactionShape( + t *testing.T, + transaction *bitcoin.Transaction, + inputValue int64, + fee int64, + expectedOutputScript bitcoin.Script, +) { + t.Helper() + + if len(transaction.Inputs) != 1 { + t.Fatalf("expected exactly 1 input, got %d", len(transaction.Inputs)) + } + if len(transaction.Outputs) != 1 { + t.Fatalf("expected exactly 1 output, got %d", len(transaction.Outputs)) + } + + expectedValue := inputValue - fee + if transaction.Outputs[0].Value != expectedValue { + t.Errorf( + "unexpected output value\nexpected: %d\nactual: %d", + expectedValue, + transaction.Outputs[0].Value, + ) + } + + if !reflect.DeepEqual( + []byte(transaction.Outputs[0].PublicKeyScript), + []byte(expectedOutputScript), + ) { + t.Errorf( + "unexpected output locking script\nexpected: %x\nactual: %x", + expectedOutputScript, + transaction.Outputs[0].PublicKeyScript, + ) + } +} + // reservationTestWallet returns a wallet with a real ECDSA public key so // bitcoin.PublicKeyHash (called at the top of both execute() methods) // doesn't panic on a nil key. @@ -637,18 +826,19 @@ func TestReservationAnchorAction_Execute(t *testing.T) { RevealedAt: time.Now(), }) chain.setReservationAction(&ReservationAction{ - ActionType: ReservationActionTypeAcceptance, - State: ReservationActionStatePending, - TxMaxFee: 2000, + ActionType: ReservationActionTypeAcceptance, + State: ReservationActionStatePending, + TargetWalletPublicKeyHash: walletPublicKeyHash, + TxMaxFee: 2000, }) action := newAction(chain, btcChain, fundingTxHash) // Below reservationActionSigningTimeoutSafetyMarginBlocks (300): // every real upstream step (event match, deposit request fetch, - // reservation key derivation, action load, on-chain validation, - // transaction assembly) must succeed before this guard is - // reached and rejects the proposal - reaching this exact error - // is the test's proof that all of it worked. + // reservation key derivation, action load, target wallet match, + // on-chain validation, transaction assembly) must succeed before + // this guard is reached and rejects the proposal - reaching this + // exact error is the test's proof that all of it worked. action.expiryBlock = 100 err = action.execute() @@ -659,6 +849,73 @@ func TestReservationAnchorAction_Execute(t *testing.T) { ) } }) + + t.Run("target wallet mismatch is rejected before signing", func(t *testing.T) { + chain := Connect() + btcChain := newLocalBitcoinChain() + + depositForScript := &Deposit{ + Depositor: "0x0000000000000000000000000000000000000001", + WalletPublicKeyHash: walletPublicKeyHash, + } + depositScript, err := depositForScript.Script() + if err != nil { + t.Fatal(err) + } + scriptHash := sha256.Sum256(depositScript) + fundingOutputScript, err := bitcoin.PayToWitnessScriptHash(scriptHash) + if err != nil { + t.Fatal(err) + } + + fundingTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: 100000, + PublicKeyScript: fundingOutputScript, + }}, + } + if err := btcChain.BroadcastTransaction(fundingTx); err != nil { + t.Fatal(err) + } + fundingTxHash := fundingTx.Hash() + + if err := chain.setPastDepositRevealedEvents( + &DepositRevealedEventFilter{ + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + StartBlock: 300000 - reservationLookBackBlocks, + }, + []*DepositRevealedEvent{{ + FundingTxHash: fundingTxHash, + FundingOutputIndex: fundingOutputIndex, + WalletPublicKeyHash: walletPublicKeyHash, + Amount: 100000, + Depositor: "0x0000000000000000000000000000000000000001", + }}, + ); err != nil { + t.Fatal(err) + } + chain.setDepositRequest(fundingTxHash, fundingOutputIndex, &DepositChainRequest{ + Amount: 100000, + RevealedAt: time.Now(), + }) + chain.setReservationAction(&ReservationAction{ + ActionType: ReservationActionTypeAcceptance, + State: ReservationActionStatePending, + TargetWalletPublicKeyHash: [20]byte{0xff}, // does not match the signing wallet + TxMaxFee: 2000, + }) + + action := newAction(chain, btcChain, fundingTxHash) + action.expiryBlock = 100 + + err = action.execute() + if err == nil || err.Error() != "reservation action targets a different wallet" { + t.Errorf( + "unexpected error\nexpected: [reservation action targets a different wallet]\nactual: [%v]", + err, + ) + } + }) } func TestReservationReanchorAction_Execute(t *testing.T) { diff --git a/pkg/tbtc/signing_test.go b/pkg/tbtc/signing_test.go index 3e7367fa43..f83fd61827 100644 --- a/pkg/tbtc/signing_test.go +++ b/pkg/tbtc/signing_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/keep-network/keep-common/pkg/chain/ethereum" + "github.com/keep-network/keep-core/internal/testutils" "github.com/keep-network/keep-core/pkg/bitcoin" "github.com/keep-network/keep-core/pkg/chain" @@ -261,6 +263,7 @@ func setupSigningExecutor(t *testing.T) *signingExecutor { keyStorePersistence := createMockKeyStorePersistence(t, signers...) node, err := newNode( + ethereum.Unknown, groupParameters, localChain, newLocalBitcoinChain(), diff --git a/pkg/tbtc/tbtc.go b/pkg/tbtc/tbtc.go index 7baa5f2995..0e9ae700e8 100644 --- a/pkg/tbtc/tbtc.go +++ b/pkg/tbtc/tbtc.go @@ -184,6 +184,7 @@ func Initialize( } node, err := newNode( + ethereumNetwork, groupParameters, chain, btcChain, diff --git a/pkg/tbtcpg/chain.go b/pkg/tbtcpg/chain.go index 80f3809b76..d3736268e1 100644 --- a/pkg/tbtcpg/chain.go +++ b/pkg/tbtcpg/chain.go @@ -201,6 +201,17 @@ type Chain interface { targetWalletPublicKeyHash [20]byte, ) error + // NotifyMovingFundsBelowDust notifies the Bridge that the given wallet's + // main UTXO has fallen below the moving funds dust threshold, ending + // the moving funds process and starting wallet closing immediately. + // mainUtxo may be nil when the wallet has no main UTXO at all; the + // Bridge only uses it to verify the on-chain balance it already holds + // for the wallet, so it is ignored in that case. + NotifyMovingFundsBelowDust( + walletPublicKeyHash [20]byte, + mainUtxo *bitcoin.UnspentTransactionOutput, + ) error + // GetReservation gets the on-chain reservation record for the given // reservation key. Returns an error if the reservation was not found. GetReservation(reservationKey *big.Int) (*tbtc.Reservation, error) diff --git a/pkg/tbtcpg/chain_test.go b/pkg/tbtcpg/chain_test.go index 69de27fa20..8ad1e985b9 100644 --- a/pkg/tbtcpg/chain_test.go +++ b/pkg/tbtcpg/chain_test.go @@ -36,6 +36,13 @@ type reservationReanchorRequestSubmission struct { TargetWalletPublicKeyHash [20]byte } +// belowDustNotification captures a submitted NotifyMovingFundsBelowDust +// call that tests can inspect for assertion. +type belowDustNotification struct { + WalletPublicKeyHash [20]byte + MainUtxo *bitcoin.UnspentTransactionOutput +} + type LocalChain struct { mutex sync.Mutex @@ -71,7 +78,9 @@ type LocalChain struct { reservationParametersSet bool reservationProposalValidations map[[32]byte]bool reservationReanchorRequestSubmissions []*reservationReanchorRequestSubmission + belowDustNotifications []*belowDustNotification reservationWalletKeys map[[20]byte][]*big.Int + reservedDeposits map[string]bool liveWalletsCountValue uint32 liveWalletsCountSet bool } @@ -100,7 +109,9 @@ func NewLocalChain() *LocalChain { reservationActions: make(map[string]*tbtc.ReservationAction), reservationProposalValidations: make(map[[32]byte]bool), reservationReanchorRequestSubmissions: make([]*reservationReanchorRequestSubmission, 0), + belowDustNotifications: make([]*belowDustNotification, 0), reservationWalletKeys: make(map[[20]byte][]*big.Int), + reservedDeposits: make(map[string]bool), } } @@ -1446,7 +1457,19 @@ func (lc *LocalChain) RequestReservationAcceptance( defer lc.mutex.Unlock() _ = walletPublicKeyHash - _ = reservationKey + + // Mirror the on-chain Bridge's own nonce bump: GetReservation after + // this call must observe the incremented RequestNonce for the + // nonce-reconciliation check in proposeReservationAcceptance. + key := reservationKey.Text(16) + existing, ok := lc.reservations[key] + if ok && existing != nil { + updated := *existing + updated.RequestNonce++ + lc.reservations[key] = &updated + } else { + lc.reservations[key] = &tbtc.Reservation{RequestNonce: 1} + } return nil } @@ -1466,9 +1489,51 @@ func (lc *LocalChain) RequestReservationReanchor( TargetWalletPublicKeyHash: targetWalletPublicKeyHash, }, ) + + // Mirror the on-chain Bridge's own nonce bump: GetReservation after + // this call must observe the incremented RequestNonce for the + // nonce-reconciliation check in ProposeReservationReanchor. + key := reservationKey.Text(16) + if existing, ok := lc.reservations[key]; ok && existing != nil { + updated := *existing + updated.RequestNonce++ + lc.reservations[key] = &updated + } + return nil +} + +// NotifyMovingFundsBelowDust records a submitted below-dust notification +// for assertion in tests. +func (lc *LocalChain) NotifyMovingFundsBelowDust( + walletPublicKeyHash [20]byte, + mainUtxo *bitcoin.UnspentTransactionOutput, +) error { + lc.mutex.Lock() + defer lc.mutex.Unlock() + + lc.belowDustNotifications = append( + lc.belowDustNotifications, + &belowDustNotification{ + WalletPublicKeyHash: walletPublicKeyHash, + MainUtxo: mainUtxo, + }, + ) return nil } +// GetBelowDustNotifications returns the recorded NotifyMovingFundsBelowDust +// submissions for assertion. +func (lc *LocalChain) GetBelowDustNotifications() []*belowDustNotification { + lc.mutex.Lock() + defer lc.mutex.Unlock() + + copy := make([]*belowDustNotification, len(lc.belowDustNotifications)) + for i, n := range lc.belowDustNotifications { + copy[i] = n + } + return copy +} + // GetReservation returns the configured reservation record for the given // reservation key, or an error if not found. func (lc *LocalChain) GetReservation( @@ -1644,11 +1709,28 @@ func (lc *LocalChain) ActiveReservationsCount() ( return 0, 0, nil } -// IsReservedDeposit returns false by default. +// IsReservedDeposit returns false unless the deposit key was previously +// marked reserved via SetReservedDeposit. func (lc *LocalChain) IsReservedDeposit( depositKey *big.Int, ) (bool, error) { - return false, nil + lc.mutex.Lock() + defer lc.mutex.Unlock() + + if depositKey == nil { + return false, nil + } + + return lc.reservedDeposits[depositKey.Text(16)], nil +} + +// SetReservedDeposit marks the given deposit key as reserved (or not) for +// IsReservedDeposit to return. +func (lc *LocalChain) SetReservedDeposit(depositKey *big.Int, reserved bool) { + lc.mutex.Lock() + defer lc.mutex.Unlock() + + lc.reservedDeposits[depositKey.Text(16)] = reserved } // PastReservationAcceptanceRequestedEvents returns no events by default. diff --git a/pkg/tbtcpg/deposit_sweep.go b/pkg/tbtcpg/deposit_sweep.go index 42b36de377..3e54945264 100644 --- a/pkg/tbtcpg/deposit_sweep.go +++ b/pkg/tbtcpg/deposit_sweep.go @@ -208,6 +208,20 @@ func findDeposits( depositKey := chain.BuildDepositKey(event.FundingTxHash, event.FundingOutputIndex) depositKeyStr := depositKey.Text(16) + isReserved, err := chain.IsReservedDeposit(depositKey) + if err != nil { + fnLogger.Errorf( + "failed to check if deposit [%s] is reserved: [%v]", + depositKeyStr, + err, + ) + continue + } + if isReserved { + fnLogger.Infof("skipping reserved deposit [%s]", depositKeyStr) + continue + } + fnLogger.Debugf("getting details of deposit [%s]", depositKeyStr) depositRequest, found, err := chain.GetDepositRequest( diff --git a/pkg/tbtcpg/deposit_sweep_test.go b/pkg/tbtcpg/deposit_sweep_test.go index dbfd9e24ea..544fe0e08f 100644 --- a/pkg/tbtcpg/deposit_sweep_test.go +++ b/pkg/tbtcpg/deposit_sweep_test.go @@ -1108,3 +1108,82 @@ func TestFindDepositsToSweep_VaultGrouping(t *testing.T) { } }) } + +// TestFindDepositsToSweep_ExcludesReservedDeposits verifies that findDeposits +// skips deposits IsReservedDeposit reports as reserved, so a wallet's +// reservation-vault deposits never starve its ordinary deposits of +// sweeping by winning the largest-group selection in FindDepositsToSweep. +func TestFindDepositsToSweep_ExcludesReservedDeposits(t *testing.T) { + currentBlock := uint64(300000) + filterStartBlock := currentBlock - tbtcpg.DepositSweepLookBackBlocks + walletPublicKeyHash := hexToByte20( + "7670343fc00ccc2d0cd65360e6ad400697ea0fed", + ) + + tbtcChain := tbtcpg.NewLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + tbtcChain.SetBlockCounter(blockCounter) + tbtcChain.SetDepositMinAge(3600) + + // 1 ordinary (non-reserved) deposit. + ordinaryHash := setupVaultGroupingDeposit( + t, tbtcChain, btcChain, walletPublicKeyHash, filterStartBlock, + "6666666666666666666666666666666666666666666666666666666666666666", + 0, 290000, nil, + ) + + // 2 reservation-vault deposits: without exclusion this would be the + // larger group and would starve the ordinary deposit above. + reservedHash1 := setupVaultGroupingDeposit( + t, tbtcChain, btcChain, walletPublicKeyHash, filterStartBlock, + "7777777777777777777777777777777777777777777777777777777777777777", + 0, 290001, nil, + ) + reservedHash2 := setupVaultGroupingDeposit( + t, tbtcChain, btcChain, walletPublicKeyHash, filterStartBlock, + "8888888888888888888888888888888888888888888888888888888888888888", + 0, 290002, nil, + ) + + tbtcChain.SetReservedDeposit( + tbtcChain.BuildDepositKey(reservedHash1, 0), true, + ) + tbtcChain.SetReservedDeposit( + tbtcChain.BuildDepositKey(reservedHash2, 0), true, + ) + + task := tbtcpg.NewDepositSweepTask(tbtcChain, btcChain) + deposits, err := task.FindDepositsToSweep( + &testutils.MockLogger{}, + walletPublicKeyHash, + 10, + ) + if err != nil { + t.Fatal(err) + } + + if len(deposits) != 1 { + t.Fatalf( + "expected exactly 1 deposit (reserved ones excluded), got %d", + len(deposits), + ) + } + if deposits[0].FundingTxHash != ordinaryHash { + t.Errorf( + "expected the ordinary deposit %v, got %v", + ordinaryHash, + deposits[0].FundingTxHash, + ) + } + for _, d := range deposits { + if d.FundingTxHash == reservedHash1 || d.FundingTxHash == reservedHash2 { + t.Errorf( + "reserved deposit %v should have been excluded", + d.FundingTxHash, + ) + } + } +} diff --git a/pkg/tbtcpg/internal/test/testdata/reservation_reanchor_scenario_1.json b/pkg/tbtcpg/internal/test/testdata/reservation_reanchor_scenario_1.json index 913a7ec21b..408618759a 100644 --- a/pkg/tbtcpg/internal/test/testdata/reservation_reanchor_scenario_1.json +++ b/pkg/tbtcpg/internal/test/testdata/reservation_reanchor_scenario_1.json @@ -1,5 +1,5 @@ { - "Title": "below-dust trigger: Live wallet without main UTXO re-anchors", + "Title": "M-27: Live wallet without main UTXO is not eligible for re-anchor (privileged-caller gate removed)", "SourceWalletPublicKeyHash": "0xffb3f7538bfa98a511495dd96027cfbd57baf2fa", "SourceWalletState": "Live", "SourceWalletMainUtxoHash": "0000000000000000000000000000000000000000000000000000000000000000", @@ -25,10 +25,5 @@ "PendingActionState": "Unknown" } ], - "ExpectedProposal": { - "ReservationKey": "0xbbbb02", - "RequestNonce": 6, - "TargetWalletPublicKeyHash": "0x92a6ec889a8fa34f731e639edede4c75e184307c", - "ReanchorTxFee": 550 - } + "ExpectedProposal": null } diff --git a/pkg/tbtcpg/reservation_acceptance.go b/pkg/tbtcpg/reservation_acceptance.go index 62818009f1..caa7907e40 100644 --- a/pkg/tbtcpg/reservation_acceptance.go +++ b/pkg/tbtcpg/reservation_acceptance.go @@ -21,6 +21,12 @@ import ( // sweep look-back window: 30 days at 12 seconds per block. const ReservationAcceptanceLookBackBlocks = uint64(216000) +// zeroAddressHex is the Ethereum zero address as returned by the chain +// adapter's address converter (chain.Address(common.Address{}.String()) +// never produces an empty string, even for the zero address) -- used to +// detect an unconfigured reservation vault instead of comparing against "". +const zeroAddressHex = "0x0000000000000000000000000000000000000000" + // ReservationAcceptanceTask is a task that may produce a reservation // acceptance (anchor) proposal. It scans the chain for reserved deposits // revealed to the operator's wallet, validates the wallet's eligibility @@ -30,6 +36,15 @@ const ReservationAcceptanceLookBackBlocks = uint64(216000) type ReservationAcceptanceTask struct { chain Chain btcChain bitcoin.Chain + + // metricsRecorder is optional and used for recording performance + // metrics: active_reservations_count, max_active_reservations, and + // wallet_reservations_count, sourced from the chain calls this task + // already makes in findReservationAcceptanceCandidate. These are + // leading indicators of the reservation capacity saturation cliff. + metricsRecorder interface { + SetGauge(name string, value float64) + } } // NewReservationAcceptanceTask constructs a ReservationAcceptanceTask. @@ -43,6 +58,14 @@ func NewReservationAcceptanceTask( } } +// setMetricsRecorder sets the metrics recorder for the reservation +// acceptance task. +func (rat *ReservationAcceptanceTask) setMetricsRecorder(recorder interface { + SetGauge(name string, value float64) +}) { + rat.metricsRecorder = recorder +} + // Run inspects the chain for an acceptance candidate reserved deposit and, // if one passes the eligibility gate, returns the resulting anchor proposal. // The task is a no-op (proposal == nil, shouldExecute == false) when no @@ -86,6 +109,9 @@ func (rat *ReservationAcceptanceTask) Run(request *tbtc.CoordinationProposalRequ ) } + if proposal == nil { + return nil, shouldExecute, nil + } return proposal, shouldExecute, nil } @@ -124,7 +150,7 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( ) } reservationVault := reservationParameters.ReservationVault - if reservationVault == "" { + if reservationVault == "" || reservationVault == chain.Address(zeroAddressHex) { taskLogger.Info("reservation vault not configured") return nil, nil } @@ -175,6 +201,12 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( err, ) } + if rat.metricsRecorder != nil { + rat.metricsRecorder.SetGauge( + "wallet_reservations_count", + float64(walletReservationsCount), + ) + } walletReservationsAmount, err := rat.chain.WalletReservationsAmount( walletPublicKeyHash, @@ -194,6 +226,16 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( err, ) } + if rat.metricsRecorder != nil { + rat.metricsRecorder.SetGauge( + "active_reservations_count", + float64(activeReservationsCount), + ) + rat.metricsRecorder.SetGauge( + "max_active_reservations", + float64(maxActiveReservations), + ) + } depositMinAgeSeconds, err := rat.chain.GetDepositMinAge() if err != nil { @@ -204,6 +246,16 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( } depositMinAge := time.Duration(depositMinAgeSeconds) * time.Second + if uint64(reservationParameters.ReservationActionTimeout) <= uint64(depositMinAgeSeconds) { + taskLogger.Errorf( + "misconfiguration: ReservationActionTimeout [%d] <= DEPOSIT_MIN_AGE [%d]; "+ + "every reserved deposit will be marked stale before it can become "+ + "acceptance-eligible", + reservationParameters.ReservationActionTimeout, + depositMinAgeSeconds, + ) + } + filterStartBlock := uint64(0) if currentBlock > ReservationAcceptanceLookBackBlocks { filterStartBlock = currentBlock - ReservationAcceptanceLookBackBlocks @@ -334,30 +386,11 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( continue } - // Third fix (a): check for existing acceptance requested events - // and fail closed on error. - acceptanceEvents, err := rat.chain.PastReservationAcceptanceRequestedEvents( - &tbtc.ReservationAcceptanceRequestedEventFilter{ - ReservationKey: []*big.Int{depositKey}, - }, - ) - if err != nil { - taskLogger.Errorf( - "failed to get past reservation acceptance requested events for [%v]: [%v]", - depositKey, - err, - ) - continue - } - if len(acceptanceEvents) > 0 { - taskLogger.Infof( - "reservation [%v] already has acceptance requested event(s), skipping", - depositKey, - ) - continue - } - - // Second & Third fix: check reservation state and derive RequestNonce. + // Determine RequestNonce and re-request eligibility from the + // reservation's own on-chain state, which authoritatively reflects + // whether a prior generation is still pending -- not from acceptance- + // requested event history, which would still show a first generation + // that has since timed out and become eligible for retry again. var requestNonce uint64 = 1 reservation, err := rat.chain.GetReservation(depositKey) if err != nil { @@ -422,6 +455,46 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( return nil, nil } +// hasPendingAction reports whether the on-chain reservation action +// generation at the reservation's current request nonce (if any) is in a +// pending state. This guards against duplicate acceptance requests within +// findReservationAcceptanceCandidate: the Bridge rejects a new request +// while the previous generation is still in flight. The caller supplies +// the reservation record it already fetched rather than this function +// re-reading it. +func hasPendingAction( + reservationKey *big.Int, + reservation *tbtc.Reservation, + chain Chain, + taskLogger log.StandardLogger, +) bool { + if reservation.RequestNonce == 0 { + return false + } + + action, err := chain.GetReservationAction( + reservationKey, + reservation.RequestNonce, + ) + if err != nil { + // Fail safe: a lookup error is indistinguishable from "still + // pending" here, and treating it as not-pending would let the + // caller send a duplicate acceptance request that the Bridge + // rejects while a real pending generation is in flight. Skip this + // reservation for the current coordination window instead; the + // next window retries. + taskLogger.Errorf( + "cannot get reservation action for [0x%x] nonce [%d]: [%v]", + reservationKey, + reservation.RequestNonce, + err, + ) + return true + } + + return action.State == tbtc.ReservationActionStatePending +} + func checkReservationAcceptanceEligibility( taskLogger log.StandardLogger, depositRequest *tbtc.DepositChainRequest, @@ -443,8 +516,14 @@ func checkReservationAcceptanceEligibility( return false } - if maxActiveReservations > 0 && - activeReservationsCount >= maxActiveReservations { + if maxActiveReservations == 0 { + taskLogger.Errorf( + "active reservations cap (maxActiveReservations) not configured " + + "(is 0); failing closed rather than treating as unlimited", + ) + return false + } + if activeReservationsCount >= maxActiveReservations { taskLogger.Infof( "active reservations count [%d] already at max [%d]", activeReservationsCount, @@ -476,19 +555,24 @@ func checkReservationAcceptanceEligibility( return false } - if reservationParameters.ReservationMaxTotalAmount > 0 { - newTotal := reservationParameters.ReservationTotalAmount + - depositRequest.Amount - if newTotal > reservationParameters.ReservationMaxTotalAmount { - taskLogger.Infof( - "global reservation total would exceed cap "+ - "[current=%d, deposit=%d, cap=%d]", - reservationParameters.ReservationTotalAmount, - depositRequest.Amount, - reservationParameters.ReservationMaxTotalAmount, - ) - return false - } + if reservationParameters.ReservationMaxTotalAmount == 0 { + taskLogger.Errorf( + "global reservation total amount cap (ReservationMaxTotalAmount) " + + "not configured (is 0); failing closed rather than treating as unlimited", + ) + return false + } + newTotal := reservationParameters.ReservationTotalAmount + + depositRequest.Amount + if newTotal > reservationParameters.ReservationMaxTotalAmount { + taskLogger.Infof( + "global reservation total would exceed cap "+ + "[current=%d, deposit=%d, cap=%d]", + reservationParameters.ReservationTotalAmount, + depositRequest.Amount, + reservationParameters.ReservationMaxTotalAmount, + ) + return false } return true @@ -580,13 +664,13 @@ func (rat *ReservationAcceptanceTask) proposeReservationAcceptance( ) } - // Fifth fix: Note that RequestReservationAcceptance is called as a - // side effect of proposal generation itself (before coordination has - // agreed to anything), which is a known, accepted deviation from the - // read-only-during-generation pattern (also present in MovingFundsTask's - // SubmitMovingFundsCommitment). The guards against re-requesting - // acceptance for existing or pending reservations are the primary - // mitigation for spurious repeat writes. + // RequestReservationAcceptance is called as a side effect of proposal + // generation itself (before coordination has agreed to anything), which + // is a known, accepted deviation from the read-only-during-generation + // pattern (also present in MovingFundsTask's SubmitMovingFundsCommitment). + // The guards against re-requesting acceptance for existing or pending + // reservations (checked in findReservationAcceptanceCandidate) are the + // primary mitigation for spurious repeat writes. if err := rat.chain.RequestReservationAcceptance( reservationKey, walletPublicKeyHash, @@ -594,6 +678,19 @@ func (rat *ReservationAcceptanceTask) proposeReservationAcceptance( return nil, false, fmt.Errorf("cannot request reservation acceptance: [%v]", err) } + updatedReservation, err := rat.chain.GetReservation(reservationKey) + if err != nil { + return nil, false, fmt.Errorf("cannot re-read reservation: [%v]", err) + } + if updatedReservation.RequestNonce != candidate.RequestNonce { + return nil, false, fmt.Errorf( + "reservation request nonce mismatch after request: predicted [%d], on-chain [%d]", + candidate.RequestNonce, + updatedReservation.RequestNonce, + ) + } + proposal.RequestNonce = updatedReservation.RequestNonce + return proposal, true, nil } diff --git a/pkg/tbtcpg/reservation_acceptance_metrics_test.go b/pkg/tbtcpg/reservation_acceptance_metrics_test.go new file mode 100644 index 0000000000..b781067545 --- /dev/null +++ b/pkg/tbtcpg/reservation_acceptance_metrics_test.go @@ -0,0 +1,106 @@ +package tbtcpg + +import ( + "math/big" + "testing" + + "github.com/keep-network/keep-core/pkg/chain" + "github.com/keep-network/keep-core/pkg/tbtc" +) + +// fakeMetricsRecorder captures SetGauge calls for assertion, distinguishing +// "never called" from "called with a zero value" - the pre-registered gauge +// default in the real PerformanceMetrics would make a value-only assertion +// pass even if the wiring were silently dropped. +type fakeMetricsRecorder struct { + calls map[string]float64 +} + +func newFakeMetricsRecorder() *fakeMetricsRecorder { + return &fakeMetricsRecorder{calls: make(map[string]float64)} +} + +func (f *fakeMetricsRecorder) SetGauge(name string, value float64) { + f.calls[name] = value +} + +// TestReservationAcceptanceTask_RecordsSaturationGauges is a regression +// test for the M-clientinfo saturation-monitoring gap: findDeposits already +// fetches wallet_reservations_count, active_reservations_count, and +// max_active_reservations from the chain, but nothing exposed them as +// metrics, so an operator could not see reservation capacity approaching +// its cap before acceptances silently stopped. +func TestReservationAcceptanceTask_RecordsSaturationGauges(t *testing.T) { + lc := NewLocalChain() + btcChain := NewLocalBitcoinChain() + + walletPublicKeyHash := [20]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20} + + lc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, + }) + lc.SetWallet(walletPublicKeyHash, &tbtc.WalletChainData{State: tbtc.StateLive}) + lc.SetDepositMinAge(3600) + + blockCounter := NewMockBlockCounter() + blockCounter.SetCurrentBlock(300000) + lc.SetBlockCounter(blockCounter) + + // Non-zero wallet reservations count so the assertion below can + // distinguish a real wired value from a coincidental zero default. + lc.SetWalletReservations(walletPublicKeyHash, []*big.Int{big.NewInt(1), big.NewInt(2)}) + + // Run scans PastDepositRevealedEvents with a filter bounded by + // ReservationAcceptanceLookBackBlocks; register an empty match so the + // call succeeds and Run proceeds to (correctly) report no candidate, + // rather than erroring on an unregistered filter. + currentBlock := uint64(300000) + filterStartBlock := currentBlock - ReservationAcceptanceLookBackBlocks + if err := lc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: filterStartBlock, + EndBlock: ¤tBlock, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + // Targets a different vault so it is filtered out immediately + // without needing a matching deposit request/funding tx. + BlockNumber: filterStartBlock, + WalletPublicKeyHash: walletPublicKeyHash, + Vault: &[]chain.Address{chain.Address( + "0xOtherVaultAddress1234567890abcdef123456789012", + )}[0], + }, + ); err != nil { + t.Fatal(err) + } + + task := NewReservationAcceptanceTask(lc, btcChain) + recorder := newFakeMetricsRecorder() + task.setMetricsRecorder(recorder) + + if _, _, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got, ok := recorder.calls["wallet_reservations_count"]; !ok { + t.Error("expected wallet_reservations_count gauge to be recorded") + } else if got != 2 { + t.Errorf("expected wallet_reservations_count = 2, got %v", got) + } + + if _, ok := recorder.calls["active_reservations_count"]; !ok { + t.Error("expected active_reservations_count gauge to be recorded") + } + if _, ok := recorder.calls["max_active_reservations"]; !ok { + t.Error("expected max_active_reservations gauge to be recorded") + } +} diff --git a/pkg/tbtcpg/reservation_acceptance_test.go b/pkg/tbtcpg/reservation_acceptance_test.go index 1a40bbc5c5..40ea2bd138 100644 --- a/pkg/tbtcpg/reservation_acceptance_test.go +++ b/pkg/tbtcpg/reservation_acceptance_test.go @@ -511,6 +511,232 @@ func TestReservationAcceptanceTask_NoCandidates(t *testing.T) { } } +// TestReservationAcceptanceTask_VaultNotConfigured_ZeroAddress verifies +// that a zero-address ReservationVault (the actual value the production +// chain.Address converter emits for an unset vault, never an empty +// string) is correctly treated as "not configured". +func TestReservationAcceptanceTask_VaultNotConfigured_ZeroAddress(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0x0000000000000000000000000000000000000000", + ), + } + ralc.maxPerWalletAmount = 1000000 + ralc.maxSingleAmount = 5000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + currentBlock := uint64(300000) + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + proposal, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if shouldExecute { + t.Errorf("expected shouldExecute=false, got true") + } + if proposal != nil { + t.Errorf("expected nil proposal, got [%+v]", proposal) + } +} + +// TestReservationAcceptanceTask_AmountCapBoundaries verifies the three +// amount-based eligibility caps (single-deposit, wallet-aggregate, +// global-total) at their exact boundary: a deposit that would land +// exactly at the cap is accepted, one satoshi over is rejected. +func TestReservationAcceptanceTask_AmountCapBoundaries(t *testing.T) { + const depositAmount = uint64(1_000_000) + + tests := map[string]struct { + singleCap uint64 + walletCap uint64 + walletExisting uint64 + globalCap uint64 + globalExisting uint64 + expectAcceptance bool + }{ + "single-deposit cap: exactly at cap is accepted": { + singleCap: depositAmount, + walletCap: depositAmount * 10, + globalCap: depositAmount * 10, + expectAcceptance: true, + }, + "single-deposit cap: one over cap is rejected": { + singleCap: depositAmount - 1, + walletCap: depositAmount * 10, + globalCap: depositAmount * 10, + expectAcceptance: false, + }, + "wallet-aggregate cap: exactly at cap is accepted": { + singleCap: depositAmount * 10, + walletCap: depositAmount, + walletExisting: 0, + globalCap: depositAmount * 10, + expectAcceptance: true, + }, + "wallet-aggregate cap: one over cap is rejected": { + singleCap: depositAmount * 10, + walletCap: depositAmount, + walletExisting: 1, + globalCap: depositAmount * 10, + expectAcceptance: false, + }, + "global-total cap: exactly at cap is accepted": { + singleCap: depositAmount * 10, + walletCap: depositAmount * 10, + globalCap: depositAmount, + globalExisting: 0, + expectAcceptance: true, + }, + "global-total cap: one over cap is rejected": { + singleCap: depositAmount * 10, + walletCap: depositAmount * 10, + globalCap: depositAmount, + globalExisting: 1, + expectAcceptance: false, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: test.globalCap, + ReservationTotalAmount: test.globalExisting, + } + ralc.maxPerWalletAmount = test.walletCap + ralc.maxSingleAmount = test.singleCap + ralc.walletReservationsAmount = test.walletExisting + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + currentBlock := uint64(300000) + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + fundingTxHash := fundingTxHashForTestName(testName) + dummyTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: 0, + PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), + }}, + } + btcChain.SetTransaction(fundingTxHash, dummyTx) + btcChain.SetEstimateSatPerVByteFee(1, 1) + btcChain.SetTransactionConfirmations( + fundingTxHash, + tbtc.DepositSweepRequiredFundingTxConfirmations, + ) + ralc.SetDepositRequest( + fundingTxHash, + 0, + &tbtc.DepositChainRequest{ + Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), + Amount: depositAmount, + RevealedAt: time.Now().Add(-2 * time.Hour), + SweptAt: time.Unix(0, 0), + Vault: &[]chain.Address{chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + )}[0], + }, + ) + + filterStartBlock := uint64(0) + if currentBlock > tbtcpg.ReservationAcceptanceLookBackBlocks { + filterStartBlock = currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks + } + + if err := ralc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: filterStartBlock, + EndBlock: ¤tBlock, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + BlockNumber: 290000, + WalletPublicKeyHash: walletPublicKeyHash, + FundingTxHash: fundingTxHash, + FundingOutputIndex: 0, + Vault: &[]chain.Address{chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + )}[0], + }, + ); err != nil { + t.Fatal(err) + } + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + proposal, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if test.expectAcceptance { + if !shouldExecute || proposal == nil { + t.Fatalf("expected proposal to be accepted at the exact cap boundary") + } + } else { + if shouldExecute || proposal != nil { + t.Fatalf("expected proposal to be rejected one unit over the cap") + } + } + }) + } +} + +// fundingTxHashForTestName derives a unique, deterministic funding tx hash +// per subtest name so parallel/sequential subtests never collide on the +// same fixture key. +func fundingTxHashForTestName(name string) bitcoin.Hash { + sum := 0 + for _, r := range name { + sum += int(r) + } + return hashFromString(fmt.Sprintf("%064x", sum+1)) +} + // TestReservationAcceptanceTask_BoundedLookback verifies that the bounded // look-back window is applied when the current block exceeds it. func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { @@ -529,9 +755,10 @@ func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } ralc.maxPerWalletAmount = 5000000 ralc.maxSingleAmount = 5000000 @@ -858,9 +1085,10 @@ func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } ralc.maxPerWalletAmount = 5000000 ralc.maxSingleAmount = 5000000 @@ -998,9 +1226,10 @@ func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } ralc.maxPerWalletAmount = 5000000 ralc.maxSingleAmount = 5000000 @@ -1104,109 +1333,6 @@ func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { } } -// TestReservationAcceptanceTask_Stateless_PastEventsError verifies that an -// RPC failure querying past acceptance requested events fails closed (skips candidate). -func TestReservationAcceptanceTask_Stateless_PastEventsError(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() - btcChain := tbtcpg.NewLocalBitcoinChain() - - walletPublicKeyHash := hexToByte20( - "8db50eb52063ea9d98b3eac91489a90f738986f6", - ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) - - fundingTxHash := hashFromString( - "7777777777777777777777777777777777777777777777777777777777777777", - ) - dummyTx := &bitcoin.Transaction{ - Outputs: []*bitcoin.TransactionOutput{{ - Value: 0, - PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), - }}, - } - btcChain.SetTransaction(fundingTxHash, dummyTx) - btcChain.SetEstimateSatPerVByteFee(1, 1) - btcChain.SetTransactionConfirmations( - fundingTxHash, - tbtc.DepositSweepRequiredFundingTxConfirmations, - ) - - ralc.SetDepositRequest( - fundingTxHash, - 0, - &tbtc.DepositChainRequest{ - Amount: 2000000, - RevealedAt: time.Now().Add(-2 * time.Hour), - SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ) - - filterStartBlock := uint64(0) - if currentBlock > tbtcpg.ReservationAcceptanceLookBackBlocks { - filterStartBlock = currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks - } - - if err := ralc.AddPastDepositRevealedEvent( - &tbtc.DepositRevealedEventFilter{ - StartBlock: filterStartBlock, - EndBlock: ¤tBlock, - WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - }, - &tbtc.DepositRevealedEvent{ - BlockNumber: 290000, - WalletPublicKeyHash: walletPublicKeyHash, - FundingTxHash: fundingTxHash, - FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ); err != nil { - t.Fatal(err) - } - - // Force an error on PastReservationAcceptanceRequestedEvents. - ralc.acceptanceEventsErr = fmt.Errorf("rpc failure") - - task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) - request := &tbtc.CoordinationProposalRequest{ - WalletPublicKeyHash: walletPublicKeyHash, - } - - proposal, shouldExecute, err := task.Run(request) - if err != nil { - t.Fatalf("unexpected task error: [%v]", err) - } - if shouldExecute || proposal != nil { - t.Fatalf("expected candidate to be skipped when past events check fails closed") - } -} - // TestReservationAcceptanceTask_Stateless_NonEligibleReservationState verifies that // a reservation whose on-chain state is Active, ActionPending, Closed, or Stranded // is skipped from acceptance proposals. @@ -1342,9 +1468,10 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 5000000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 5000000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } ralc.maxPerWalletAmount = 50000000 ralc.maxSingleAmount = 50000000 @@ -1435,9 +1562,10 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 1000000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 1000000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } // Second run on the same task instance: deposit is now above min and proposed. @@ -1465,9 +1593,10 @@ func TestReservationAcceptanceTask_Stateless_RequestNonceIncremented(t *testing. ReservationVault: chain.Address( "0xReservationVaultAddress1234567890abcdef12345678", ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + ReservationMaxTotalAmount: 100000000, } ralc.maxPerWalletAmount = 5000000 ralc.maxSingleAmount = 5000000 diff --git a/pkg/tbtcpg/reservation_reanchor.go b/pkg/tbtcpg/reservation_reanchor.go index 2ee6959dbd..adbaa679c8 100644 --- a/pkg/tbtcpg/reservation_reanchor.go +++ b/pkg/tbtcpg/reservation_reanchor.go @@ -26,6 +26,13 @@ const ReservationReanchorLookBackBlocks = uint64(216000) type ReservationReanchorTask struct { chain Chain btcChain bitcoin.Chain + + // metricsRecorder is optional and used for recording performance + // metrics: live_wallets_count, sourced from the GetLiveWalletsCount + // chain call this task already makes. + metricsRecorder interface { + SetGauge(name string, value float64) + } } // NewReservationReanchorTask returns a new ReservationReanchorTask bound to @@ -40,20 +47,34 @@ func NewReservationReanchorTask( } } +// setMetricsRecorder sets the metrics recorder for the reservation +// re-anchor task. +func (rrt *ReservationReanchorTask) setMetricsRecorder(recorder interface { + SetGauge(name string, value float64) +}) { + rrt.metricsRecorder = recorder +} + // ActionType returns the type of wallet action this task produces. func (rrt *ReservationReanchorTask) ActionType() tbtc.WalletActionType { return tbtc.ActionReservationReanchor } // Run evaluates whether the given wallet needs to re-anchor any of its -// reservations and returns a single ReservationReanchorProposal for the first -// reservation found to be re-anchorable. A wallet is a candidate for re-anchor -// when either: -// - it has entered the StateMovingFunds state (the wallet is migrating and -// reservations must be released to a live wallet), or -// - its main UTXO has dropped below the moving funds dust threshold (a -// re-anchor frees value from the wallet's main UTXO pool into a fresh -// reservation anchor held by another live wallet). +// reservations and returns a single ReservationReanchorProposal for the +// first reservation found to be re-anchorable. A wallet is a candidate for +// re-anchor only once it has entered the StateMovingFunds state (the +// wallet is migrating and reservations must be released to a live +// wallet); tbtc-v2's Reservation.requestReservationReanchor requires a +// privileged (governance) caller for StateLive sources +// (Reservation.sol:742-746, ReservationRouter.sol:269-277), which the +// client's ordinary operator key can never satisfy, so no below-dust +// re-anchor trigger is attempted for Live wallets. +// +// Once a MovingFunds wallet's reservations are fully drained, Run also +// checks whether its main UTXO has fallen below the moving funds dust +// threshold and, if so, notifies the Bridge so wallet closing can proceed +// (see notifyMovingFundsBelowDustIfEligible). // // Returns (nil, false, nil) when no reservation is eligible; callers should // treat that as a benign no-op for the coordination window. @@ -79,24 +100,9 @@ func (rrt *ReservationReanchorTask) Run( ) } - migrating := walletChainData.State == tbtc.StateMovingFunds - if !migrating { - // Check the below-dust trigger. Re-anchor also unlocks wallet value - // when the main UTXO has fallen below the moving funds dust - // threshold, so wallets in StateLive may still need to re-anchor - // before they enter the moving funds flow. - needs, err := rrt.isBelowMovingFundsDustThreshold( - taskLogger, - walletPublicKeyHash, - ) - if err != nil { - return nil, false, err - } - - if !needs { - taskLogger.Info("wallet is not eligible for reservation re-anchor") - return nil, false, nil - } + if walletChainData.State != tbtc.StateMovingFunds { + taskLogger.Info("wallet is not eligible for reservation re-anchor") + return nil, false, nil } reservationKeys, err := rrt.chain.WalletReservations(walletPublicKeyHash) @@ -109,6 +115,7 @@ func (rrt *ReservationReanchorTask) Run( if len(reservationKeys) == 0 { taskLogger.Info("wallet has no reservations to re-anchor") + rrt.notifyMovingFundsBelowDustIfEligible(taskLogger, walletPublicKeyHash) return nil, false, nil } @@ -119,6 +126,9 @@ func (rrt *ReservationReanchorTask) Run( err, ) } + if rrt.metricsRecorder != nil { + rrt.metricsRecorder.SetGauge("live_wallets_count", float64(liveWalletsCount)) + } if liveWalletsCount == 0 { taskLogger.Info("no live wallets available for re-anchor target") @@ -234,21 +244,22 @@ func (rrt *ReservationReanchorTask) ProposeReservationReanchor( ) } - // Estimate fee if it's missing. The Bridge caps each reservation - // lifecycle transaction with its own ReservationTxMaxFee, not the - // moving-funds TxMaxTotalFee, so we use the reservation parameters - // directly. + // The Bridge caps each reservation lifecycle transaction with its own + // ReservationTxMaxFee, not the moving-funds TxMaxTotalFee, so we use + // the reservation parameters directly. Fetched unconditionally (not + // only when fee needs estimating) because the pre-check below also + // needs ReservationTxMaxFee to bound-check a caller-supplied fee. + params, err := rrt.chain.ReservationParameters() + if err != nil { + return nil, fmt.Errorf( + "cannot get reservation parameters: [%w]", + err, + ) + } + if fee <= 0 { taskLogger.Infof("estimating reservation re-anchor transaction fee") - params, err := rrt.chain.ReservationParameters() - if err != nil { - return nil, fmt.Errorf( - "cannot get reservation parameters: [%w]", - err, - ) - } - fee, err = estimateReservationReanchorFee( rrt.btcChain, params.ReservationTxMaxFee, @@ -263,6 +274,23 @@ func (rrt *ReservationReanchorTask) ProposeReservationReanchor( taskLogger.Infof("reservation re-anchor transaction fee: [%d]", fee) + feeBoundAction := &tbtc.ReservationAction{ + TxMaxFee: params.ReservationTxMaxFee, + } + + if _, err := tbtc.AssembleReservationReanchorTransaction( + rrt.btcChain, + reservation.AnchorUtxo, + targetWalletPublicKeyHash, + feeBoundAction, + fee, + ); err != nil { + return nil, fmt.Errorf( + "cannot assemble reservation re-anchor transaction: [%v]", + err, + ) + } + proposal := &tbtc.ReservationReanchorProposal{ ReservationKey: new(big.Int).Set(reservationKey), RequestNonce: requestNonce, @@ -290,6 +318,19 @@ func (rrt *ReservationReanchorTask) ProposeReservationReanchor( return nil, fmt.Errorf("cannot request reservation re-anchor: [%v]", err) } + updatedReservation, err := rrt.chain.GetReservation(reservationKey) + if err != nil { + return nil, fmt.Errorf("cannot re-read reservation: [%v]", err) + } + if updatedReservation.RequestNonce != requestNonce { + return nil, fmt.Errorf( + "reservation request nonce mismatch after request: predicted [%d], on-chain [%d]", + requestNonce, + updatedReservation.RequestNonce, + ) + } + proposal.RequestNonce = updatedReservation.RequestNonce + return proposal, nil } @@ -353,17 +394,18 @@ func (rrt *ReservationReanchorTask) findTargetWallet( return [20]byte{}, fmt.Errorf("no live wallet available for re-anchor target") } -// isBelowMovingFundsDustThreshold returns true when the wallet's main UTXO -// value is below the moving funds dust threshold. The threshold is sourced -// from the on-chain MovingFundsParameters. A wallet without a main UTXO is -// considered to have fallen below the threshold. +// isBelowMovingFundsDustThreshold returns the wallet's resolved main UTXO +// (nil if it has none) and whether its value is below the moving funds +// dust threshold. The threshold is sourced from the on-chain +// MovingFundsParameters. A wallet without a main UTXO is considered to +// have fallen below the threshold. func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( taskLogger log.StandardLogger, walletPublicKeyHash [20]byte, -) (bool, error) { +) (*bitcoin.UnspentTransactionOutput, bool, error) { params, err := rrt.chain.GetMovingFundsParameters() if err != nil { - return false, fmt.Errorf( + return nil, false, fmt.Errorf( "cannot get moving funds parameters: [%w]", err, ) @@ -371,7 +413,7 @@ func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( walletChainData, err := rrt.chain.GetWallet(walletPublicKeyHash) if err != nil { - return false, fmt.Errorf( + return nil, false, fmt.Errorf( "cannot get wallet chain data: [%w]", err, ) @@ -381,7 +423,7 @@ func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( // No main UTXO on-chain, the wallet has fully depleted its pool and // must release any reservation anchors. taskLogger.Info("wallet has no main UTXO; below dust threshold") - return true, nil + return nil, true, nil } walletMainUtxo, err := tbtc.DetermineWalletMainUtxo( @@ -390,7 +432,7 @@ func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( rrt.btcChain, ) if err != nil { - return false, fmt.Errorf( + return nil, false, fmt.Errorf( "cannot determine wallet main UTXO: [%w]", err, ) @@ -398,7 +440,7 @@ func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( if walletMainUtxo == nil { taskLogger.Info("wallet has no resolvable main UTXO; below dust threshold") - return true, nil + return nil, true, nil } below := walletMainUtxo.Value < int64(params.DustThreshold) @@ -409,46 +451,52 @@ func (rrt *ReservationReanchorTask) isBelowMovingFundsDustThreshold( params.DustThreshold, ) } - return below, nil + return walletMainUtxo, below, nil } -// hasPendingAction reports whether the on-chain reservation action -// generation at the reservation's current request nonce (if any) is in a -// pending state. This guards against duplicate re-anchor requests: the -// Bridge rejects a new request while the previous generation is still in -// flight. The caller supplies the reservation record it already fetched -// (see Run) rather than this function re-reading it. -func hasPendingAction( - reservationKey *big.Int, - reservation *tbtc.Reservation, - chain Chain, +// notifyMovingFundsBelowDustIfEligible checks whether the given (just +// drained) MovingFunds wallet's main UTXO has fallen below the moving +// funds dust threshold and, if so, notifies the Bridge so wallet closing +// can proceed. m1-b-implementation.md §5 documents this as the only +// remaining route to close a wallet that proved its funds moved while it +// still held reservation anchors: the Bridge's own automatic closing +// attempt runs once, while the reservation count is still non-zero, and is +// never retried. Errors are logged rather than propagated: a failed +// notification here must not block the coordination window, and the wallet +// remains in StateMovingFunds so the next call to Run retries. +func (rrt *ReservationReanchorTask) notifyMovingFundsBelowDustIfEligible( taskLogger log.StandardLogger, -) bool { - if reservation.RequestNonce == 0 { - return false - } - - action, err := chain.GetReservationAction( - reservationKey, - reservation.RequestNonce, + walletPublicKeyHash [20]byte, +) { + mainUtxo, below, err := rrt.isBelowMovingFundsDustThreshold( + taskLogger, + walletPublicKeyHash, ) if err != nil { - // Fail safe: a lookup error is indistinguishable from "still - // pending" here, and treating it as not-pending would let the - // caller send a duplicate re-anchor request that the Bridge - // rejects while a real pending generation is in flight. Skip this - // reservation for the current coordination window instead; the - // next window retries. taskLogger.Errorf( - "cannot get reservation action for [0x%x] nonce [%d]: [%v]", - reservationKey, - reservation.RequestNonce, + "cannot determine moving funds below-dust eligibility: [%v]", + err, + ) + return + } + if !below { + return + } + + if err := rrt.chain.NotifyMovingFundsBelowDust( + walletPublicKeyHash, + mainUtxo, + ); err != nil { + taskLogger.Errorf( + "cannot notify moving funds below dust: [%v]", err, ) - return true + return } - return action.State == tbtc.ReservationActionStatePending + taskLogger.Info( + "notified moving funds below dust; wallet has no remaining reservations", + ) } // estimateReservationReanchorFee estimates the fee for a reservation diff --git a/pkg/tbtcpg/reservation_reanchor_metrics_test.go b/pkg/tbtcpg/reservation_reanchor_metrics_test.go new file mode 100644 index 0000000000..72f543c3e7 --- /dev/null +++ b/pkg/tbtcpg/reservation_reanchor_metrics_test.go @@ -0,0 +1,60 @@ +package tbtcpg + +import ( + "math/big" + "testing" + + "github.com/keep-network/keep-core/pkg/tbtc" +) + +// TestReservationReanchorTask_RecordsLiveWalletsCountGauge is a regression +// test for the same saturation-monitoring gap in the re-anchor task: Run +// already fetches GetLiveWalletsCount but never exposed it as a metric. +func TestReservationReanchorTask_RecordsLiveWalletsCountGauge(t *testing.T) { + lc := NewLocalChain() + btcChain := NewLocalBitcoinChain() + + sourceWalletPublicKeyHash := [20]byte{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1} + targetWalletPublicKeyHash := [20]byte{2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2} + + blockCounter := NewMockBlockCounter() + blockCounter.SetCurrentBlock(1000) + lc.SetBlockCounter(blockCounter) + + if err := lc.AddPastNewWalletRegisteredEvent( + &tbtc.NewWalletRegisteredEventFilter{StartBlock: 0}, + &tbtc.NewWalletRegisteredEvent{WalletPublicKeyHash: targetWalletPublicKeyHash}, + ); err != nil { + t.Fatal(err) + } + + lc.SetWallet(sourceWalletPublicKeyHash, &tbtc.WalletChainData{State: tbtc.StateMovingFunds}) + lc.SetWallet(targetWalletPublicKeyHash, &tbtc.WalletChainData{State: tbtc.StateLive}) + // A reservation exists but has no anchor UTXO, so the wallet is left + // with nothing eligible to re-anchor. This test only cares that the + // live_wallets_count gauge fires before that failure, matching Run's + // actual call order. + reservationKey := big.NewInt(1) + lc.SetWalletReservations(sourceWalletPublicKeyHash, []*big.Int{reservationKey}) + lc.SetReservation(reservationKey, &tbtc.Reservation{ + WalletPublicKeyHash: sourceWalletPublicKeyHash, + State: tbtc.ReservationStateActive, + }) + lc.SetLiveWalletsCount(3) + + task := NewReservationReanchorTask(lc, btcChain) + recorder := newFakeMetricsRecorder() + task.setMetricsRecorder(recorder) + + if _, _, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: sourceWalletPublicKeyHash, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got, ok := recorder.calls["live_wallets_count"]; !ok { + t.Error("expected live_wallets_count gauge to be recorded") + } else if got != 3 { + t.Errorf("expected live_wallets_count = 3, got %v", got) + } +} diff --git a/pkg/tbtcpg/reservation_reanchor_test.go b/pkg/tbtcpg/reservation_reanchor_test.go index 9c1a0b03ae..56f6a20f9b 100644 --- a/pkg/tbtcpg/reservation_reanchor_test.go +++ b/pkg/tbtcpg/reservation_reanchor_test.go @@ -100,12 +100,23 @@ func TestReservationReanchorTask_Run(t *testing.T) { t.Fatal(err) } + anchorWalletScript, err := bitcoin.PayToWitnessPublicKeyHash( + r.WalletPublicKeyHash, + ) + if err != nil { + t.Fatal(err) + } + anchorOutputs := make([]*bitcoin.TransactionOutput, r.AnchorTxOutputIndex+1) + for i := range anchorOutputs { + anchorOutputs[i] = &bitcoin.TransactionOutput{Value: 1} + } + anchorOutputs[r.AnchorTxOutputIndex] = &bitcoin.TransactionOutput{ + Value: r.AnchorValue, + PublicKeyScript: anchorWalletScript, + } btcChain.SetTransaction(anchorTxHash, &bitcoin.Transaction{ Version: 1, - Outputs: []*bitcoin.TransactionOutput{{ - Value: r.AnchorValue, - PublicKeyScript: []byte{}, - }}, + Outputs: anchorOutputs, }) reservationState := r.State @@ -323,12 +334,16 @@ func TestReservationReanchorTask_TargetWalletExclusion_SharedTask(t *testing.T) "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", bitcoin.ReversedByteOrder, ) + walletAScript, err := bitcoin.PayToWitnessPublicKeyHash(walletA) + if err != nil { + t.Fatal(err) + } btcChain.SetTransaction(anchorTxHashA, &bitcoin.Transaction{ Version: 1, - Outputs: []*bitcoin.TransactionOutput{{ - Value: 100000, - PublicKeyScript: []byte{}, - }}, + Outputs: []*bitcoin.TransactionOutput{ + {Value: 1}, + {Value: 100000, PublicKeyScript: walletAScript}, + }, }) tbtcChain.SetReservation(resAKey, &tbtc.Reservation{ WalletPublicKeyHash: walletA, @@ -350,12 +365,16 @@ func TestReservationReanchorTask_TargetWalletExclusion_SharedTask(t *testing.T) "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", bitcoin.ReversedByteOrder, ) + walletBScript, err := bitcoin.PayToWitnessPublicKeyHash(walletB) + if err != nil { + t.Fatal(err) + } btcChain.SetTransaction(anchorTxHashB, &bitcoin.Transaction{ Version: 1, - Outputs: []*bitcoin.TransactionOutput{{ - Value: 200000, - PublicKeyScript: []byte{}, - }}, + Outputs: []*bitcoin.TransactionOutput{ + {Value: 1}, + {Value: 200000, PublicKeyScript: walletBScript}, + }, }) tbtcChain.SetReservation(resBKey, &tbtc.Reservation{ WalletPublicKeyHash: walletB, @@ -487,12 +506,16 @@ func TestReservationReanchorTask_Run_SkipNonActiveReservations(t *testing.T) { "1111111111111111111111111111111111111111111111111111111111111111", bitcoin.ReversedByteOrder, ) + walletAScript, err := bitcoin.PayToWitnessPublicKeyHash(walletA) + if err != nil { + t.Fatal(err) + } btcChain.SetTransaction(anchorTxHash1, &bitcoin.Transaction{ Version: 1, - Outputs: []*bitcoin.TransactionOutput{{ - Value: 100000, - PublicKeyScript: []byte{}, - }}, + Outputs: []*bitcoin.TransactionOutput{ + {Value: 1}, + {Value: 100000, PublicKeyScript: walletAScript}, + }, }) tbtcChain.SetReservation(res1Key, &tbtc.Reservation{ WalletPublicKeyHash: walletA, @@ -514,10 +537,10 @@ func TestReservationReanchorTask_Run_SkipNonActiveReservations(t *testing.T) { ) btcChain.SetTransaction(anchorTxHash2, &bitcoin.Transaction{ Version: 1, - Outputs: []*bitcoin.TransactionOutput{{ - Value: 200000, - PublicKeyScript: []byte{}, - }}, + Outputs: []*bitcoin.TransactionOutput{ + {Value: 1}, + {Value: 200000, PublicKeyScript: walletAScript}, + }, }) tbtcChain.SetReservation(res2Key, &tbtc.Reservation{ WalletPublicKeyHash: walletA, @@ -556,3 +579,100 @@ func TestReservationReanchorTask_Run_SkipNonActiveReservations(t *testing.T) { t.Errorf("expected target walletB [%x], got [%x]", walletB, proposal.TargetWalletPublicKeyHash) } } + +// TestReservationReanchorTask_Run_NotifiesMovingFundsBelowDust is a +// regression test for the NotifyMovingFundsBelowDust wiring: once a +// MovingFunds wallet has no reservations left and its main UTXO is below +// the moving funds dust threshold, Run must call NotifyMovingFundsBelowDust +// exactly once with the wallet's resolved main UTXO. A wallet above the +// dust threshold must not trigger any notification. +func TestReservationReanchorTask_Run_NotifiesMovingFundsBelowDust(t *testing.T) { + walletPublicKeyHash := hexToByte20("ffb3f7538bfa98a511495dd96027cfbd57baf2fa") + + newFixture := func(mainUtxoValue int64) (*tbtcpg.LocalChain, *tbtcpg.LocalBitcoinChain) { + tbtcChain := tbtcpg.NewLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + mainUtxoTx := &bitcoin.Transaction{ + Version: 1, + Outputs: []*bitcoin.TransactionOutput{{ + Value: mainUtxoValue, + PublicKeyScript: walletScript, + }}, + } + mainUtxoTxHash := mainUtxoTx.Hash() + btcChain.SetTransaction(mainUtxoTxHash, mainUtxoTx) + btcChain.SetTxHashesForPublicKeyHash( + walletPublicKeyHash, + []bitcoin.Hash{mainUtxoTxHash}, + ) + + mainUtxo := &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: mainUtxoTxHash, + OutputIndex: 0, + }, + Value: mainUtxoValue, + } + + tbtcChain.SetWallet(walletPublicKeyHash, &tbtc.WalletChainData{ + State: tbtc.StateMovingFunds, + MainUtxoHash: tbtcChain.ComputeMainUtxoHash(mainUtxo), + }) + tbtcChain.SetMovingFundsParameters( + 1000000, 1000000, 0, 0, nil, 0, 0, 0, 0, nil, 0, + ) + tbtcChain.SetWalletReservations(walletPublicKeyHash, nil) + + return tbtcChain, btcChain + } + + t.Run("below dust threshold: notifies exactly once", func(t *testing.T) { + tbtcChain, btcChain := newFixture(500000) + task := tbtcpg.NewReservationReanchorTask(tbtcChain, btcChain) + + prop, ok, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ok || prop != nil { + t.Fatalf("expected no proposal, got ok=%v, prop=%v", ok, prop) + } + + notifications := tbtcChain.GetBelowDustNotifications() + if len(notifications) != 1 { + t.Fatalf("expected exactly 1 below-dust notification, got %d", len(notifications)) + } + if notifications[0].WalletPublicKeyHash != walletPublicKeyHash { + t.Errorf( + "unexpected notified wallet\nexpected: %x\nactual: %x", + walletPublicKeyHash, + notifications[0].WalletPublicKeyHash, + ) + } + if notifications[0].MainUtxo == nil || notifications[0].MainUtxo.Value != 500000 { + t.Errorf("unexpected notified main UTXO: %+v", notifications[0].MainUtxo) + } + }) + + t.Run("above dust threshold: no notification", func(t *testing.T) { + tbtcChain, btcChain := newFixture(2000000) + task := tbtcpg.NewReservationReanchorTask(tbtcChain, btcChain) + + if _, _, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if notifications := tbtcChain.GetBelowDustNotifications(); len(notifications) != 0 { + t.Fatalf("expected no below-dust notifications, got %d", len(notifications)) + } + }) +} diff --git a/pkg/tbtcpg/tbtcpg.go b/pkg/tbtcpg/tbtcpg.go index d28482a548..7e2b0439e5 100644 --- a/pkg/tbtcpg/tbtcpg.go +++ b/pkg/tbtcpg/tbtcpg.go @@ -58,6 +58,23 @@ func (pg *ProposalGenerator) SetRedemptionMetricsRecorder(recorder interface { } } +// SetReservationMetricsRecorder sets the metrics recorder for the +// reservation acceptance and re-anchor tasks (registered only when +// reservationsEnabled - see NewProposalGenerator). A no-op when +// reservations are disabled since neither task is present in pg.tasks. +func (pg *ProposalGenerator) SetReservationMetricsRecorder(recorder interface { + SetGauge(name string, value float64) +}) { + for _, task := range pg.tasks { + switch t := task.(type) { + case *ReservationAcceptanceTask: + t.setMetricsRecorder(recorder) + case *ReservationReanchorTask: + t.setMetricsRecorder(recorder) + } + } +} + // NewProposalGenerator returns a new proposal generator. When // reservationsEnabled is true the proposal generator appends the reservation // acceptance (anchor) and re-anchor tasks to the standard task list so that