fix(tbtc): reservation review remediation (37 findings from multi-agent review of #4282)

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,

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() (

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,
+		)
+	}
+}

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