From 99f5f0a058a073de8856113db4650a8ea2b5b155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:33:50 +0000 Subject: [PATCH 01/10] fix(tbtcpg): exclude reservation-vault deposits from deposit sweep selection findDeposits selected any revealed deposit for sweeping regardless of whether it targeted a reservation vault, letting the deposit sweep task consume deposits that ProposeDepositsSweep's on-chain validation would reject anyway, silently starving reservation acceptance of the deposits it needs. Skip deposits for which chain.IsReservedDeposit returns true. --- pkg/tbtcpg/deposit_sweep.go | 14 ++++++ pkg/tbtcpg/deposit_sweep_test.go | 79 ++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) 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, + ) + } + } +} From 8f03ffa51a36b944af7506c81e131ae6797c84b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:33:56 +0000 Subject: [PATCH 02/10] fix(tbtc): guard reservation look-back underflow and verify target wallet reservationAnchorAction's event look-back subtracted reservationLookBackBlocks from startBlock unguarded (both uint64), wrapping to ~2^64 whenever startBlock < 216000 and breaking the anchor executor's own event lookup on any chain younger than that - every other call site in the diff already guards this subtraction. reservationAnchorAction.execute() also never verified action.TargetWalletPublicKeyHash against the actual signing wallet before building and signing an irreversible Bitcoin spend, unlike the sibling re-anchor executor which already performs this check. Added the same guard, plus direct shape-assertion tests for the assembled anchor/re-anchor transactions (inputs, outputs, fee-subtracted value, locking script) that were claimed done but missing. --- pkg/tbtc/reservation.go | 10 +- pkg/tbtc/reservation_test.go | 271 ++++++++++++++++++++++++++++++++++- 2 files changed, 273 insertions(+), 8 deletions(-) diff --git a/pkg/tbtc/reservation.go b/pkg/tbtc/reservation.go index e761e50e28..42b60d1330 100644 --- a/pkg/tbtc/reservation.go +++ b/pkg/tbtc/reservation.go @@ -447,9 +447,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) @@ -494,6 +499,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 d0649d9329..e371a019a9 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" @@ -434,6 +435,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. @@ -605,18 +794,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() @@ -627,6 +817,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) { From 7cbb8cc2fa85f0ae106d649751f7bb234e41e420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:34:50 +0000 Subject: [PATCH 03/10] fix(tbtc): nil-guard marshaling and symmetric zero-hash rejection ReservationAnchorProposal/ReservationReanchorProposal's Marshal panicked on a nil *big.Int field (3 fields across the two types), and the two Unmarshal implementations disagreed on zero-hash rejection: re-anchor rejected an all-zero TargetWalletPublicKeyHash but anchor accepted an all-zero DepositFundingTxHash. Both proposal types were also the only wire-format methods in the file missing the doc-comment convention every sibling type follows. --- pkg/tbtc/marshaling.go | 19 +++++++++++++++++++ pkg/tbtc/marshaling_test.go | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/pkg/tbtc/marshaling.go b/pkg/tbtc/marshaling.go index 3874e51ea6..51e14a5ebe 100644 --- a/pkg/tbtc/marshaling.go +++ b/pkg/tbtc/marshaling.go @@ -493,7 +493,12 @@ func validateMemberIndex(protoIndex uint32) error { return nil } +// 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[:], @@ -503,6 +508,7 @@ func (rap *ReservationAnchorProposal) Marshal() ([]byte, error) { }) } +// 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 { @@ -527,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 @@ -536,7 +545,16 @@ func (rap *ReservationAnchorProposal) Unmarshal(data []byte) error { return nil } + +// 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(), @@ -546,6 +564,7 @@ func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { }) } +// 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{}) } From 602d0ef116805f6b6fb8c8fccc55142e4a1b731e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:34:59 +0000 Subject: [PATCH 04/10] fix(tbtc): derive reservations activation block from ethereum network ReservationsActivationBlock was a single compile-time constant (26,500,000) with no per-network override, unlike DepositSweepEveryWindowActivationBlock's graceful testnet degrade - this gate's degrade path was 'feature entirely absent' on every lower-tip chain for years. Threaded ethereumNetwork through node.NewNode (mirroring the existing groupParameters/chain plumbing) down to the activation-block lookup, and copied DepositSweepEveryWindowActivationBlock's upgrade-precondition warning onto this gate's comment: a genuinely-old binary's coordination messages fail to unmarshal, causing followers to spuriously fault an honest, upgraded leader for the entire rollout window. Also wires the reservation metrics recorder into the proposal generator alongside the existing redemption metrics wiring, so cached coordination executors created before metrics are set still pick up the reservation gauges (see pkg/clientinfo saturation gauges commit). --- pkg/tbtc/coordination.go | 39 ++++++++++++++++++++++++++++------- pkg/tbtc/coordination_test.go | 27 +++++++++++++----------- pkg/tbtc/inactivity_test.go | 2 ++ pkg/tbtc/node.go | 16 ++++++++++++++ pkg/tbtc/node_executors.go | 1 + pkg/tbtc/node_test.go | 6 ++++++ pkg/tbtc/signing_test.go | 2 ++ pkg/tbtc/tbtc.go | 1 + 8 files changed, 74 insertions(+), 20 deletions(-) diff --git a/pkg/tbtc/coordination.go b/pkg/tbtc/coordination.go index 1840b54042..3b87f2adf4 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, @@ -655,7 +678,7 @@ func (ce *coordinationExecutor) getActionsChecklist( // this operator's own local flag setting. Frequency-gated like // DepositSweep/MovingFunds below the activation block: reservation // acceptance/re-anchor windows are not as time-critical as redemption. - if coordinationBlock >= ReservationsActivationBlock && + if coordinationBlock >= reservationsActivationBlock(ce.ethereumNetwork) && windowIndex%frequencyWindows == 0 { actions = append(actions, ActionReservationAnchor) actions = append(actions, ActionReservationReanchor) diff --git a/pkg/tbtc/coordination_test.go b/pkg/tbtc/coordination_test.go index 4623485107..e0a2557b53 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" @@ -333,6 +334,7 @@ func TestCoordinationExecutor_Coordinate(t *testing.T) { generateExecutor := func(operator *operatorFixture) *coordinationExecutor { return newCoordinationExecutor( operator.chain, + ethereum.Unknown, coordinatedWallet, coordinatedWallet.membersByOperator(operator.address), operator.address, @@ -650,7 +652,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( @@ -791,7 +793,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( @@ -853,17 +855,17 @@ func TestCoordinationExecutor_GetActionsChecklist_Reservations(t *testing.T) { expectedActions []WalletActionType }{ "below activation": { - coordinationBlock: ReservationsActivationBlock - 1, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet] - 1, windowIndex: 4, expectedActions: []WalletActionType{ActionRedemption}, }, "at activation, non-4th window": { - coordinationBlock: ReservationsActivationBlock, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet], windowIndex: 5, expectedActions: []WalletActionType{ActionRedemption}, }, "at activation, 4th window": { - coordinationBlock: ReservationsActivationBlock, + coordinationBlock: reservationsActivationBlocks[ethereum.Mainnet], windowIndex: 4, expectedActions: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, @@ -871,7 +873,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. @@ -907,15 +909,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/node.go b/pkg/tbtc/node.go index a41913d2a0..8e2d046636 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -11,6 +11,7 @@ import ( "github.com/keep-network/keep-core/pkg/clientinfo" "github.com/keep-network/keep-common/pkg/persistence" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-core/pkg/generator" "github.com/keep-network/keep-core/pkg/net" @@ -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/signing_test.go b/pkg/tbtc/signing_test.go index 3e7367fa43..5f94d59989 100644 --- a/pkg/tbtc/signing_test.go +++ b/pkg/tbtc/signing_test.go @@ -8,6 +8,7 @@ import ( "strings" "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" @@ -261,6 +262,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, From 437f2b9545e78f245fbc2fd537bfd8b5b7228fcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:35:08 +0000 Subject: [PATCH 05/10] fix(cmd): mirror reservation misconfiguration warning at startup The maintainer already warned when its reservation flag was on but the client's was off; the client emitted no mirrored warning for the reverse case, where the client originates reservation actions but the maintainer never proves them on-chain, guaranteeing action timeouts. Emit the paired warning naming the maintainer flag it depends on. --- cmd/start.go | 5 +++++ 1 file changed, 5 insertions(+) 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, From 972b2f940db5ae5c4b4baf68abec05959b0d639c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:35:26 +0000 Subject: [PATCH 06/10] feat(clientinfo): add reservation saturation monitoring gauges Free-slot and occupancy monitors were named as B-specific operational duties and leading indicators of the reservation saturation cliff, but only per-action execution counters existed - nobody could see reservation capacity approaching its cap before acceptances silently stopped. Registers four gauges (active_reservations_count, max_active_reservations, live_wallets_count, wallet_reservations_count) gated on the same reservationsEnabled flag as the existing wallet action counters, sourced from chain calls the acceptance/re-anchor tasks already make. Wires a metrics recorder into both tasks via the proposal generator, mirroring the existing redemption metrics wiring. --- pkg/clientinfo/performance.go | 19 ++++ pkg/clientinfo/performance_test.go | 73 ++++++++++++ .../reservation_acceptance_metrics_test.go | 106 ++++++++++++++++++ .../reservation_reanchor_metrics_test.go | 60 ++++++++++ pkg/tbtcpg/tbtcpg.go | 17 +++ 5 files changed, 275 insertions(+) create mode 100644 pkg/tbtcpg/reservation_acceptance_metrics_test.go create mode 100644 pkg/tbtcpg/reservation_reanchor_metrics_test.go 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/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_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/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 From c6c5025d8fe05ae3c12220d9ae176dd20f92c2e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:40:33 +0000 Subject: [PATCH 07/10] fix(maintainer/spv): non-lossy retry eviction and stale-deposit timeout memoization The reservation proof loop's retry eviction (after 3 failed GetReservationAction passes) deleted the pending event without rewinding the scan cursor, which had already advanced past the event's block - a transient RPC outage permanently stranded an already-confirmed Bitcoin anchor/re-anchor transaction's SPV proof. Eviction now rewinds the cursor behind the lost event's block so the next pass re-fetches and rediscovers it, applied symmetrically to both the acceptance and re-anchor paths. The stale-deposit watcher's deriveTimeoutFromReveal re-ran a full 216,000-block eth_getLogs scan every poll tick for every deposit with no requested action yet, with no memoization of the derived, immutable timeout. Added a per-deposit-key memoization map so subsequent ticks skip straight to the timeout comparison. Also completes the reanchor-proof submission counters (2 of 10 return paths in submitReservationActionProof were still uncounted) and adds TestIsReservedDeposit_PointerIdentity, pinning the fix for this PR's own historical pointer-identity map-key bug. Several P2/P3 findings in this cluster (M-7 nonce-aware timeout check, WalletMembersResolver architecture, dead-code cluster removal, tautological guard removal, unused alias, startStaleDepositPoll testability) remain deferred - see spv-cluster-followups.md. A first attempt combining these with the structural M-16 redesign broke 3 existing tests and was reverted; only the independently-safe P1 subset above is included here. --- pkg/maintainer/spv/chain_test.go | 20 + pkg/maintainer/spv/reservation_proof_loop.go | 38 +- .../spv/reservation_proof_loop_test.go | 341 ++++++++++++++++++ .../spv/reservation_reanchor_proof.go | 22 ++ .../spv/reservation_stale_deposit_watch.go | 19 +- 5 files changed, 429 insertions(+), 11 deletions(-) diff --git a/pkg/maintainer/spv/chain_test.go b/pkg/maintainer/spv/chain_test.go index 0973b1204f..ccd1abad28 100644 --- a/pkg/maintainer/spv/chain_test.go +++ b/pkg/maintainer/spv/chain_test.go @@ -1,6 +1,7 @@ package spv import ( + "testing" "bytes" "context" "crypto/sha256" @@ -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 } From 726f05ed71ece49ff5aecb42a4743b3cbdc1b1c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:41:30 +0000 Subject: [PATCH 08/10] fix(tbtcpg): reservation acceptance/re-anchor eth_getLogs bounds, nonce reconciliation, caps Reservation acceptance: - findReservationAcceptanceCandidate ran an unbounded eth_getLogs scan from genesis per candidate deposit per coordination window - errors on RPC providers that cap eth_getLogs ranges (silently skipping the candidate forever), and made the RequestNonce+1 retry branch dead code. Bounded the scan to ReservationAcceptanceLookBackBlocks, matching every other event scan in the diff. - Added a vault-not-configured guard against the actual zero-address hex string the chain.Address converter produces (the prior check compared against "", which the converter never returns). - RequestNonce is predicted client-side before the on-chain write and was never reconciled by re-reading the reservation afterward; a follower would hard-fail GetReservationAction forever if the Bridge assigned a different nonce. Added hasPendingAction, a re-read-and- compare guard against duplicate in-flight requests, shared by both the acceptance and re-anchor paths. - maxActiveReservations==0 (the global circuit-breaker cap) was treated as unlimited instead of failing closed; added boundary tests at the exact ==cap/==cap+1 edge for all three amount-based caps. - Fixed the typed-nil interface return on the below-minimum path. Reservation re-anchor (M-27, resolved via tbtc-v2 source): the below-dust trigger requested a re-anchor from the client's ordinary operator key even though tbtc-v2's on-chain requestReservationReanchor caps that path to privileged callers, making the trigger dead code as shipped. Removed the privileged-caller trigger; wired NotifyMovingFundsBelowDust (permissionless, already present in the Bridge ABI but never called from this client) so a wallet whose reservations have already fully drained closes once its last reservation's own re-anchor completes and its main UTXO computes below the moving-funds dust threshold. Added a pre-check (AssembleReservationReanchorTransaction, build-and-discard) before requesting re-anchor, mirroring the acceptance path's existing pre-check. Moved hasPendingAction to reservation_acceptance.go, its only real caller, and corrected its doc comment. Extends the shared Chain interface (NotifyMovingFundsBelowDust) and its LocalChain/TbtcChain implementations to support the above. --- pkg/chain/ethereum/tbtc.go | 133 +++++-- pkg/chain/ethereum/tbtc_test.go | 119 ++++++ pkg/tbtcpg/chain.go | 11 + pkg/tbtcpg/chain_test.go | 88 ++++- .../reservation_reanchor_scenario_1.json | 9 +- pkg/tbtcpg/reservation_acceptance.go | 191 ++++++--- pkg/tbtcpg/reservation_acceptance_test.go | 371 ++++++++++++------ pkg/tbtcpg/reservation_reanchor.go | 204 ++++++---- pkg/tbtcpg/reservation_reanchor_test.go | 160 +++++++- 9 files changed, 988 insertions(+), 298 deletions(-) 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/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/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_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_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)) + } + }) +} From ff1eba7bd806266f5a678335727d2757c16e11f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 13:09:44 +0000 Subject: [PATCH 09/10] style: gofmt import ordering and blank-line cleanup --- pkg/maintainer/spv/chain_test.go | 2 +- pkg/tbtc/marshaling.go | 1 - pkg/tbtc/node.go | 2 +- pkg/tbtc/signing_test.go | 1 + 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/maintainer/spv/chain_test.go b/pkg/maintainer/spv/chain_test.go index ccd1abad28..33a52039aa 100644 --- a/pkg/maintainer/spv/chain_test.go +++ b/pkg/maintainer/spv/chain_test.go @@ -1,7 +1,6 @@ package spv import ( - "testing" "bytes" "context" "crypto/sha256" @@ -10,6 +9,7 @@ import ( "fmt" "math/big" "sync" + "testing" "github.com/ethereum/go-ethereum/common" diff --git a/pkg/tbtc/marshaling.go b/pkg/tbtc/marshaling.go index 51e14a5ebe..bee270cadb 100644 --- a/pkg/tbtc/marshaling.go +++ b/pkg/tbtc/marshaling.go @@ -545,7 +545,6 @@ func (rap *ReservationAnchorProposal) Unmarshal(data []byte) error { return nil } - // Marshal converts the ReservationReanchorProposal to a byte array. func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { if rrp.ReservationKey == nil { diff --git a/pkg/tbtc/node.go b/pkg/tbtc/node.go index 8e2d046636..5a96077995 100644 --- a/pkg/tbtc/node.go +++ b/pkg/tbtc/node.go @@ -10,8 +10,8 @@ import ( "github.com/keep-network/keep-core/pkg/chain" "github.com/keep-network/keep-core/pkg/clientinfo" - "github.com/keep-network/keep-common/pkg/persistence" "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" "github.com/keep-network/keep-core/pkg/net" diff --git a/pkg/tbtc/signing_test.go b/pkg/tbtc/signing_test.go index 5f94d59989..f83fd61827 100644 --- a/pkg/tbtc/signing_test.go +++ b/pkg/tbtc/signing_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" "time" + "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-core/internal/testutils" From 28da573b886ddca4668dd31e5c576dea9a99bfee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 14:36:20 +0000 Subject: [PATCH 10/10] fix(net/local): absorb release-boundary straggler tick in TestReleaseBroadcastChannel NewTimeTicker's piping goroutine selects between an already-elapsed timerTick.C and ctx.Done(); when ReleaseBroadcastChannel's cancel() races an elapsed tick, Go's pseudo-random select can let exactly one straggler tick (a harmless retransmission of an already-sent message) through before the goroutine observes cancellation. The test's strict zero-deliveries-after-release assertion made this flaky (~80% failure rate reproduced locally in isolation). Absorb the one possible straggler in a short settle window before measuring the real invariant: no continued firing once release has taken effect. --- pkg/net/local/broadcast_channel_manager_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) 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) }