Skip to content

feat(tbtc): wire reservation executors and watchers - #4274

Draft
piotr-roslaniec wants to merge 39 commits into
reservations-epicfrom
m1/keep-core-client
Draft

feat(tbtc): wire reservation executors and watchers#4274
piotr-roslaniec wants to merge 39 commits into
reservations-epicfrom
m1/keep-core-client

Conversation

@piotr-roslaniec

@piotr-roslaniec piotr-roslaniec commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extends PR #4238 (proposal structs, marshaling, chain-interface stubs — no executor) with the missing pieces: real ABI bindings for the ReservationRouter, chain-interface implementations against those bindings, the acceptance and re-anchor executors (proposal generation + SPV proof submission), three monitoring watchers (stranding, stale-deposit, action-timeout), and operator wiring gated behind config.Reservations.Enabled.

Repo: threshold-network/keep-core
Branch: m1/keep-core-client
Base: reservations-epic
Diffstat (this PR vs reservations-epic, working tree): 84 files changed, 25818 insertions(+), 72 deletions(-)

Build pipeline (serial gates, parallel middle)

Ten commits, four sequential stages — later stages depend on earlier ones' output, so this was not built as one flat diff:

  1. 277865cb4 — regenerate Go ABI bindings for the reservation router surface (abi/gen, cmd/gen, contract/genReservationRouter.go files, ~9,800 lines total across the three layers; plus incremental diffs to Bridge.go/RedemptionWatchtower.go/$WalletProposalValidator.go` for the reservation-adjacent methods those contracts already exposed).
  2. e49e954e8 — reservation read/validate methods against the real bindings, on pkg/tbtc.Chain.
  3. 4ba2f2267 (CurrentMonkey) — reservation write methods, remaining views, event subscriptions on pkg/tbtc.Chain.
  4. 053577925 (gate 2.5, manager-run — narrow mechanical scope, not delegated) — extends the same reservation methods onto pkg/tbtcpg.Chain and pkg/maintainer/spv.Chain. This gate caught and fixed a real defect first — see below.
  5. Three genuinely independent builders, each its own worktree/branch off 053577925, then merged sequentially:
    • e37211b33 (AcceptanceBuilder) — pkg/tbtcpg/reservation_acceptance.go (candidate selection + proposal assembly) + pkg/maintainer/spv/reservation_acceptance_proof.go (SPV proof submission).
    • 603ad0d54 (ReanchorBuilder) — the re-anchor equivalents.
    • ac58650a1 (WatchersBuilder) — pkg/maintainer/spv/reservation_{stranding,stale_deposit,action_timeout}_watch.go.
    • Merges: 2bd8731ce (acceptance, clean), daeb6afaf (re-anchor, clean), c29de0b8f (watchers — one real conflict, see below).
  6. 48985451d (HilariousTermite, serial operator wiring) — registers both proposal tasks in pkg/tbtcpg/tbtcpg.go, both proof tasks in pkg/maintainer/spv/spv.go, and the three watcher event subscriptions in pkg/tbtc/tbtc.go, all gated behind config.Reservations.Enabled.
  7. b14a38851 — merge: pull in reservations-epic base updates (bitcoin.Chain context-aware confirmation lookups) to fix local build breakage.

What ships (current state)

Area Files Content
ABI bindings (generated) pkg/chain/ethereum/tbtc/gen/{abi,cmd,contract}/ReservationRouter.go + incremental diffs to Bridge.go, RedemptionWatchtower.go, WalletProposalValidator.go, LightRelay*.go ~9,800 new lines, generated from the ABI, not hand-written
Chain interface pkg/chain/ethereum/tbtc.go (+1479), pkg/tbtc/chain.go (+495/-…), pkg/tbtcpg/chain.go (+124), pkg/maintainer/spv/chain.go (+111) Read/write/event methods against the real router surface. All reservation reads/writes/event subscriptions target the Bridge address via fallback delegatecall, never the router's own deployed address (see PR G's invariant note)
Coordination dispatch pkg/tbtc/node_coordination.go (+18), pkg/tbtc/node_proposals.go (+128), pkg/tbtc/coordination.go (+27) Wires agreed ActionReservationAnchor/ActionReservationReanchor proposals to handleReservationAnchorProposal/handleReservationReanchorProposal, which assemble, sign, and broadcast the anchor/re-anchor Bitcoin transaction; checklist gating uses a chain-derived activation block, not local config alone
Wallet action assembly pkg/tbtc/reservation.go (+1032) assembleReservationAnchorTransaction/assembleReservationReanchorTransaction and the wallet action types the dispatch above invokes
Acceptance executor pkg/tbtcpg/reservation_acceptance.go (+714) ReservationAcceptanceTask — candidate selection, eligibility checks, proposal assembly
Re-anchor executor pkg/tbtcpg/reservation_reanchor.go (+472) Re-anchor proposal generation
SPV proof submission pkg/maintainer/spv/reservation_acceptance_proof.go (+109), reservation_reanchor_proof.go (+312), reservation_proof_loop.go (+505) Builds and submits the SPV proof for each proposal type; the proof loop is the driver that scans wallet transaction history and dispatches to both submit functions
Watchers pkg/maintainer/spv/reservation_stranding_watch.go (+106), reservation_stale_deposit_watch.go (+299), reservation_action_timeout_watch.go (+422) Permissionless monitoring — notify stranded/stale/timed-out reservations
Operator wiring pkg/tbtcpg/tbtcpg.go (+24/-…), pkg/maintainer/spv/spv.go (+12), pkg/maintainer/spv/reservation_wiring.go (+417), pkg/tbtc/tbtc.go (+42/-…), pkg/maintainer/spv/config.go (+23), cmd/start.go (+29/-…) Task/proof registration and event-subscription wiring, all behind config.Reservations.Enabled
Metrics pkg/clientinfo/performance.go (+61) Reservation action-type metric names, gated so a non-reservation deployment's registered metric surface is unchanged
Tests pkg/tbtc/{reservation,coordination,chain}_test.go, pkg/tbtcpg/{reservation_acceptance,reservation_reanchor,chain,tbtcpg,fee,bitcoin_chain}_test.go, pkg/maintainer/spv/{chain,reservation_acceptance_proof,reservation_proof_loop,reservation_wiring,reservation_action_timeout_watch,reservation_reanchor_proof,reservation_stale_deposit_watch,reservation_stranding_watch}_test.go, pkg/chain/ethereum/tbtc_test.go, pkg/tbtcpg/internal/test/{marshaling,reservation_acceptance,tbtcpgtest}.go, 16 JSON test-scenario fixtures, config/config_test.go + test/config.{json,toml,yaml} Unit coverage for every new production file above, including the pure helpers in the SPV proof loop and wiring layer

Two real bugs found and fixed mid-build

  1. Pointer-identity map-key bug (AcceptanceBuilder’s output): the test-double reservationAcceptanceLocalChain.reservedDeposits field was typed map[*big.Int]bool. Go compares *big.Int map keys by pointer identity, not value, so IsReservedDeposit always missed even deposits the test had explicitly marked reserved, because BuildDepositKey allocates a fresh pointer on every call. Fixed by re-keying on depositKey.Text(16) (string).
  2. Zero-output funding-transaction stub: the happy-path and bounded-lookback test fixtures stubbed the funding Bitcoin transaction as &bitcoin.Transaction{} (zero outputs), but ReservationAcceptanceTask.Run genuinely calls assembleReservationAnchorTransaction, which reads the funding output's locking script to validate P2SH/P2WSH — so it panicked with "output index out of range", then "not P2SH/P2WSH" once the first fix landed. Fixed by giving the stub transactions a real single P2WSH output (0x0020 + 32 zero bytes) and adding the missing ReservationTxMaxFee to the bounded-lookback scenario's ReservationParameters (was defaulting to 0, so any nonzero anchor fee tripped the "exceeds configured max" guard).

Defects caught during the build (both fixed, neither shipped)

  • Gate 2.5 subagent transiently malformed pkg/tbtc/chain.go while probing interface satisfaction (duplicate ReservationChain block, methods declared after the interface's closing brace) and deleted the tracked root main.go, then yielded a PARTIAL_COMPLETION reporting a bogus SPV build failure it could not explain. Root-caused by the manager: pkg/tbtc/chain.go was already clean at yield time; the main.go deletion was the only lasting damage, restored via git restore --source=HEAD -- main.go. The two interface extensions the subagent actually delivered (pkg/tbtcpg/chain.go +137, pkg/maintainer/spv/chain.go +97) were structurally correct; the test-double mocks were missing three Past*Events stub methods, added by hand. Committed clean…
  • Watchers merge conflict (c29de0b, in pkg/maintainer/spv/chain_test.go): two independently-built reservation test-double representations — [16]byte/[24]byte-keyed maps from the acceptance/re-anchor lineage vs map[string]-keyed maps re-added by the watchers branch — plus a duplicate setReservation/setReservationAction method pair and a dropped submitReservationProofHook field, all caught and restored during resolution.
  • Build-brief self-correction before dispatch: the initial build brief recommended binding a separate read-only "router" handle to the ReservationRouter's own deployed address for views/events. Verified wrong against ReservationRouter.sol:71-76 invariant 3 ("NO STANDALONE AUTHORITY") before any downstream subagent used it — the router's own address has empty storage, so direct reads return garbage and every event it emits carries the Bridge's address in the log, not the router's. Fixed in the persisted brief before the bindings subagent committed; a localChain unit-test mock would not have caught this since it doesn't model per-address log-emitter semantics.

Verification

  • ✅ go build ./... — clean
  • ✅ go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/maintainer/spv/... ./pkg/clientinfo/... ./pkg/chain/ethereum/... -v — 340 passed, 0 failed
  • ✅ go test ./... — full repo suite, 0 failed
  • ✅ go vet ./... — clean except one pre-existing, unrelated issue at pkg/tecdsa/signing/protocol.go:737 (not touched by this PR)
  • ✅ gofmt -l (all changed files) — empty (no formatting issues)

Review notes

  • All reservation reads/writes/event subscriptions target the Bridge address, never the router's own deployed address — this is a hard invariant (ReservationRouter.sol:71-76), not a style choice. A reviewer checking the chain-interface implementation should confirm every reservation call site uses the bridge handle.
  • Config-gated: every new task/proof/watcher registration is behind config.Reservations.Enabled; with it unset (the default), this PR changes no runtime behavior for existing (non-reservation) flows.
  • The coordination checklist gate additionally requires a chain-derived ReservationsActivationBlock alongside the local config flag, so a mixed-rollout signing group (one operator flag-on, another flag-off) can't cause a flag-off follower to fault an honest flag-on leader.
  • No behavioral changes to non-reservation code paths — the diff is additive except for the Chain interface files, coordination dispatch, and operator-wiring files, which only add reservation-specific branches.
  • Known scope limits, called out as an open follow-up rather than blocking this PR:
    • The action-timeout watcher's wallet-members resolver (pkg/maintainer/spv/reservation_wiring.go) is a stub that always errors "wallet members resolver not wired," so CheckReservationActionTimeouts never reaches its notification path yet; the stranding and stale-deposit watchers are fully live. Tracked to implement against GetOperatorID once the on-chain accessor path is confirmed, mirroring pkg/tbtc/inactivity.go's operator walk.
    • ReservedRedemptionProposal/ReservationDissolutionProposal marshal/assembly scaffolding in pkg/tbtc/reservation.go ships but is inert for m1 (validator stubs explicitly error "not exposed on the m1 bridge-integration surface"); m1 only activates Acceptance/Reanchor. Kept in this PR rather than split out since it shares the same wire-format contract and is fully tested as shipped.
    • ReservationAcceptanceTask keeps its stateful incremental-scan cache (scanState, lastScannedBlock, pendingCandidates) rather than the review-suggested redesign to a stateless full-rescan matching DepositSweepTask/RedemptionTask. The confirmed correctness bug the cache caused (candidates lost across calls) is fixed in this PR; dropping the cache entirely is a larger architectural change with its own RPC-cost tradeoff and is deferred to a follow-up rather than bundled into this fix.

mswilkison and others added 16 commits August 7, 2026 16:32
Companion of the tbtc-v2 UTXO reservation draft (threshold-network/
tbtc-v2#1088). A reservation is a deposit the wallet anchors -- spends
in a 1-input-1-output transaction into a fresh wallet-controlled output
with no refund path -- instead of sweeping, so the reserved coins never
commingle with the pooled supply and are redeemable in-kind.

Adds the wallet-side foundations:
- wallet action types for the four reservation lifecycle actions
  (anchor, reserved redemption, re-anchor, dissolution), appended after
  the existing enum values to preserve serialized compatibility,
- coordination proposal types with marshaling and factory registration
  (JSON-based for now; switching to protobuf once the reservation
  message types are added to the coordination proto definition),
- Chain interface extensions for reading reservations and parameters
  and validating the four proposal kinds via WalletProposalValidator,
- unsigned transaction assembly for all four lifecycle shapes,
  enforcing the 1-input-1-output lineage (dissolution additionally
  spends the wallet main UTXO as its second input, per the Bridge
  rules),
- tests for action parsing, proposal marshaling roundtrips, and
  assembler input validation.

The Ethereum chain implementation stubs the new interface methods with
descriptive errors: the contract bindings can only be regenerated once
the reservation Bridge API is published with the @keep-network/tbtc-v2
package. Coordination executor wiring and tbtcpg proposal generation
follow in the same step.
Repairs a pre-existing build break on main: the tbtcpg Chain interface
was refactored to return tbtc.RedemptionParameters as a struct, but the
fee-estimation call site in redemptions.go still destructured the old
8-value tuple. All other call sites already use the struct form.
Regenerates the @keep-network/tbtc-v2 ABI bindings against the
m1 bridge-integration surface (/tmp/m1-g @ 9362cda1), adding the
ReservationRouter contract to the required_contracts list and
introducing a fix_reservation_router_collision Makefile hook that
renames ReservationRouter's BitcoinTxInfo / BitcoinTxProof /
BitcoinTxUTXO structs to BitcoinTxInfo4 / BitcoinTxProof3 /
BitcoinTxUTXO4 (the next free suffixes after Bridge / WalletProposalValidator
/ MaintainerProxy).

The new abi/Bridge.go surface carries the reservation selectors exposed
on the Bridge itself -- isReservedDeposit, setReservationRouter, and
getReservationRouter -- in addition to the existing Bridge API; the
regenerated MaintainerProxy, WalletProposalValidator, RedemptionWatchtower,
and Relay bindings reflect minor ABI surface additions that landed in
the same bridge-integration commit.

The abi/ReservationRouter.go binding is the encoding source for the
six read/validate methods filled in on the next commit; its call site
is the Bridge address (Bridge.fallback routes the router selector via
delegatecall), so all reads, writes, and event/log filters must target
the Bridge address, not the router's own deployment address.
…bindings

Replaces the seven reservation read/validate stubs in tbtc.go
(those declared today in pkg/tbtc/chain.go:430-480, previously
returning "reservations not supported yet" errors) with real
implementations backed by the regenerated abigen bindings.

Three view reads (GetReservation, GetReservationAction,
ReservationParameters) are reached through tc.reservationRouter, a new
binding constructed against the Bridge address -- the router code only
executes via Bridge.fallback's delegatecall, so binding the
ReservationRouter ABI at the Bridge address is the only configuration
that gives the operator a live read path (the deployed router address
holds empty storage). Two on-chain proposal validators
(ValidateReservationAnchorProposal,
ValidateReservationReanchorProposal) are reached through the existing
tc.walletProposalValidator handle against the regenerated
WalletProposalValidatorReservation*Proposal ABI structs.

Two further validators (ValidateReservedRedemptionProposal,
ValidateReservationDissolutionProposal) remain unsupported on this
milestone's bridge-integration surface -- the WalletProposalValidator
contract does not expose those entry points -- so their bodies return
an explicit "validator not exposed on the m1 bridge-integration
surface" error. The chain.go interface declarations are satisfied so
the package compiles; downstream tasks can replace these bodies once
the missing validators land on the contract side.

Thin field-by-field abigen-to-Go converters are added for the three
view structs (convertReservationFromAbiType,
convertReservationActionFromAbiType,
convertReservationParametersFromAbiType), plus three small parsers
(parseReservationState, parseReservationActionType,
parseReservationActionState) that mirror the on-chain enum layouts.
The reservationRouter field is constructed in newTbtcChain via the
reservationRouterBinding helper, which makes the storage/address
rationale explicit at the call site rather than burying it in the
struct field comment.
…t subscriptions

Adds the second half of the PR H reservation chain-interface surface
(section 1.2 of the build brief):

* Six write methods bound to the Bridge address via the
  reservationRouter handle (RequestReservationAcceptance,
  RequestReservationReanchor, SubmitReservationProof,
  NotifyReservationActionTimeout, NotifyStaleReservedDeposit,
  NotifyReservationStranded). Submission pattern mirrors the
  existing SubmitRedemptionProofWithReimbursement flow: GasEstimate
  + 20% margin + ethutil.TransactionOptions.
* Twelve additional read/view methods (ReservationCaps,
  WalletReservationsAmount, WalletReservationsCount,
  WalletReservations, ReservationByAnchorUtxo, ReservedDepositWallet,
  PendingReservedDeposits, Reservations, ReservationActions,
  ActiveReservationsCount, ReservationRouter, IsReservedDeposit),
  plus ReservationParametersFull as an alias of ReservationParameters.
  IsReservedDeposit and ReservationRouter read via the Bridge
  binding because they map to Bridge state.
* New Go types ReservationRequest, ReservationActionRecord,
  BitcoinTxInfo, BitcoinTxProof, BitcoinTxUTXO mirror the on-chain
  ReservationRouter view structs verbatim.
* Thirteen event subscriptions and twelve filter structs for every
  reservation event listed in the brief, filtering against the
  Bridge address (delegatecall preserves the caller's address
  context so router-emitted events carry the Bridge address).
* localChain mocks for all of the above so the interface stays
  satisfiable by the test double.

The reservationRouter binding remains bound to the Bridge address
(invariant 3 of ReservationRouter.sol) - no second binding against
the router's standalone address is constructed.
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…proof submission loop

Resolves the no-op watcher/proof-submission cluster flagged in review:
WireReservationWatchers previously subscribed stranding, stale-deposit,
and action-timeout watchers to handlers that discarded their inputs, and
submitReservationAcceptanceProof was an unimplemented stub. Both now
call through to the real check/notify and proof-assembly paths.

cmd/start.go now wires the reservation watchers directly against the
Chain handle instead of threading them through tbtc.Initialize via the
now-removed ReservationWatchersWirer callback type, since cmd/start.go
already imports both tbtc and spv and there is no import-cycle reason
for the indirection. clientinfo.NewPerformanceMetrics now takes the
reservations-enabled flag so reservation action metrics are only
registered when the feature is on.

Also:
- guard empty WalletMembersResolverFunc results before notifying watchers
- fix nonce walk start in the action-timeout watcher
- align nonce-base convention across SPV watchers (1-based)
- fail-safe hasPendingAction on RPC error instead of assuming no action
- remove dead depositToReservationKey identity-copy indirection
- fix Errorf missing err argument in the stranding watcher
- delete duplicate ReservationParametersFull declarations
- add chain-error passthrough, notifier-error resilience, exact-timeout-
  boundary, and nil-notifier coverage for the stranding and stale-deposit
  watchers
…flag

NewPerformanceMetrics now takes a reservationsEnabled flag and only
registers the reservation-specific wallet action metrics (anchor,
reservation_anchor, reservation_reanchor, reserved_redemption,
reservation_dissolution) when the feature is enabled, so the /metrics
endpoint does not advertise counters for actions the deployment never
produces.
convertReservationFromAbiType, convertReservationActionFromAbiType, and
convertReservationParametersFromAbiType had zero unit tests despite
being on the hot path for every reservation read: a field-order swap in
the 10-tuple parameters struct, or a wrong action-type-to-hash-field
routing decision, would silently feed bad data into
checkReservationAcceptanceEligibility undetected.

Also removes the dead duplicate ReservationParametersFull binding left
over from the reservation router integration.
…overage

Correctness:
- populate BlindingFactor/RefundPublicKeyHash/RefundLocktime in the
  acceptance deposit instead of leaving them zeroed
- set RequestNonce in proposeReservationAcceptance
- compute the anchor/re-anchor fee dynamically (applyWalletTxFeeFloor)
  instead of a hardcoded constant
- compare the net (post-fee) deposit amount against ReservationMinAmount
- guard the pendingReservedDeposits check with MaxTotalAmount>0, then
  remove it entirely once the guard made it a strict, unreachable subset
  of the preceding global-cap check
- fix the WalletReservationsAmount map-key bug and the fillBigInt16
  truncation in the shared LocalChain test double; switch reservation
  map keys from truncated fixed-byte keys to the big.Int's full base-16
  text so distinct keys can never collide
- fix the AnchorUtxo nil-check to a value-based check, since the Go-side
  chain adapter always allocates a non-nil struct
- add a bounds check in parseReservationReanchorTransactionInput
- bound findTargetWallet's wallet-registration scan and
  findReservationAcceptanceCandidate's deposit re-scan to a look-back
  window with a per-wallet cursor instead of an unbounded eth_getLogs
  scan on every coordination window
- add reservation action types to getActionsChecklist
- remove dead reservationAcceptanceCandidate fields and the
  ReservationWatchersWirer indirection's tbtcpg-side leftovers

Test coverage:
- add scenario fixtures for the 4 previously-untested eligibility
  rejection branches (wallet reservations count cap, single-deposit cap,
  wallet aggregate cap, global total cap) plus the re-anchor task's
  no-live-wallet-target, non-Active-state skip, empty-reservations, and
  minimum-fee-floor branches
- add TestNewProposalGenerator_ReservationsEnabled proving the
  reservationsEnabled constructor flag actually wires (or omits) the
  reservation tasks, closing the gap where a regression that always or
  never appended them would go undetected
- replace the reservation acceptance/re-anchor proposal comparisons'
  reliance on deep.Equal, which silently reports no difference between
  any two distinct *big.Int values because big.Int's representation is
  entirely unexported, with explicit field-by-field comparators using
  .Cmp(); this also caught and fixes two pre-existing wrong expected
  ReanchorTxFee fixture values (1015 instead of the real computed 550)
  and adds RequestNonce to the acceptance comparison, which was silently
  never checked
…ion block

- Wire ActionReservationAnchor/ActionReservationReanchor in
  processCoordinationResult to handleReservationAnchorProposal/
  handleReservationReanchorProposal, which assemble, sign, and
  broadcast the anchor/re-anchor transaction. Previously these
  proposals fell through to default: and were only logged.
- Gate the reservation checklist entries on a chain-derived
  ReservationsActivationBlock in addition to the local config flag,
  so a mixed-rollout signing group can't cause a flag-off follower to
  fault an honest flag-on leader.
- Remove the dead, duplicated Reservations()/ReservationActions() API
  surface and its unused ReservationRequest/ReservationActionRecord
  types from pkg/tbtc.Chain; add BuildDepositKey, needed by the
  reservation anchor wallet action to derive the m1 reservation key.
…t coverage

Remove the duplicated Reservations()/ReservationActions() chain-binding
methods that mirrored pkg/tbtc.Chain's now-deleted API. Add unit tests
for the reservation action/parameter ABI-to-domain conversion helpers
(TestConvertReservationActionFromAbiType,
TestConvertReservationParametersFromAbiType) covering both valid action
states and the unrecognized/zero-value error paths.
…tered set

GetAllWalletActionTypes was exported and unconditionally returned the
four reservation action names; only registerAllMetrics applied the
reservationsEnabled filter, so any other consumer saw the enlarged set
regardless of configuration. Move the reservation names into a new
GetReservationWalletActionTypes(), keeping GetAllWalletActionTypes()
returning only the original five, and have registerAllMetrics append
the reservation set only when reservationsEnabled is true.
…ng; add coverage

- Delete the ReservationParametersFull struct and the duplicated
  WalletProposalValidator declarations; fix wallet-members resolver
  callback wiring to dodge an import cycle.
- Fix reservation reanchor loop error handling and add the reservation
  proof loop's driver (scan, match, submit) with its own test coverage
  (TestReservationProofScanStartBlock, TestFindReservationAcceptance/
  ReanchorTransaction, TestProveReservationTransaction).
- Add unit tests for resolveWalletPublicKeyHash (found/not-found/
  chain-error) and isPendingStaleDepositResolved (still-reserved,
  released/swept, wallet-now-live) in reservation_wiring.go, plus a
  chain-error injection field for PastNewWalletRegisteredEvents on the
  localChain test double.
- Add TestSubmitReservationAcceptanceProof covering the zero-
  required-confirmations error path.
- Add config test coverage and a [Tbtc.Reservations] Enabled entry to
  all three sample config files.
…erage

- Fix wrong reservation-key derivation in proposeReservationAcceptance:
  use ReservationByAnchorUtxo instead of a pre-anchor GetReservationAction
  call, since the action generation record does not exist on-chain yet
  at this point in the flow.
- Use candidate.ReservationParameters.ReservationTxMaxFee (feeBoundAction)
  directly in AssembleReservationAnchorTransaction instead of re-deriving
  it through a second ReservationParameters fetch.
- Add missing currentBlock computation in
  ReservationAcceptanceTask.Run before findReservationAcceptanceCandidate;
  add EndBlock to every AddPastDepositRevealedEvent call site so bounded
  look-back scans don't silently include events past the intended range.
- Fix TestReservationAcceptanceTask_GetWalletError and
  TestReservationAcceptanceTask_DepositNotReserved fixtures to exercise
  the reachable code paths (matching EndBlock, correct wallet chain
  data seeding) instead of failing before reaching GetWallet.
- Factor a shared estimateReservationFixedSizeTxFee helper used by both
  the acceptance and reanchor fee estimators; parameterize the
  exceeds-max error message per caller.
- Add reservation_reanchor_scenario_8/9.json covering the dust-migration
  eligibility gate and a live wallet without a main UTXO re-anchor case.
- Strengthen TestNewProposalGenerator_ReservationsEnabled to assert both
  reservation action types are dispatched independently, not just that
  Generate returns a non-nil error.
…and chain interface

- coordination.go: gate reservation checklist entries solely on the
  activation block, not the local per-operator config flag (a flag-off
  follower was wrongly faulting an honest flag-on leader); give
  ReservationsActivationBlock its own independent constant instead of
  aliasing DepositSweepEveryWindowActivationBlock
- node_coordination.go: add missing routing tests for the reservation
  anchor/reanchor dispatch cases
- reservation.go: add TargetWalletPublicKeyHash guards to both live
  execute() paths and to ReservationReanchorProposal.Unmarshal; add
  ActionType/State pending guards before assembling anchor/reanchor
  transactions; bound the anchor action's deposit-revealed event scan;
  add real execute()-path tests using localChain/localBitcoinChain
  test doubles; remove the reserved-redemption and reservation-
  dissolution proposal/marshaling/assembly scaffolding that ships with
  no producer, dispatch case, or wired validator in this milestone
- marshaling.go: migrate the two surviving reservation proposal types
  (anchor, re-anchor) from encoding/json to protobuf, matching every
  other coordination proposal type; restore the required-field
  validation the migration had dropped
- wallet.go: remove the two now-unused WalletActionType values,
  preserving their wire-format numeric slots
- chain.go: remove 17 ReservationChain interface members with zero
  production callers (unused on-chain event subscriptions and reads)
- node.go/node_executors.go/tbtc.go: remove the reservationsEnabled
  field threaded from config into coordinationExecutor - it stopped
  being read once the checklist gate above was fixed to depend only on
  the activation block, and task-registration gating already happens
  independently via tbtcpg.NewProposalGenerator
…k and repeated candidates

- reservation_acceptance.go: gate RequestReservationAcceptance behind
  an in-flight check (matching the sibling reanchor task's convention)
  so the same deposit cannot be re-requested across coordination
  windows; permanently drop candidates whose on-chain state leaves
  Unknown instead of retrying them forever; cache immutable reveal/
  request data on pending candidates and expire terminal/out-of-window
  entries so unchanged candidates stop re-incurring the full RPC chain
  every run
- reservation_reanchor.go: continue past a single reservation's
  ProposeReservationReanchor failure instead of aborting the whole
  wallet's walk, matching every other per-reservation failure in the
  same loop; cache the incrementally refreshed live-wallet lookup
  instead of rescanning 216,000 blocks per re-anchor attempt
- chain.go: remove 4 Chain interface members with zero production
  callers, mirroring the pkg/tbtc trim
- update the two cap-rejection test scenarios whose fee-estimation
  failure now logs-and-continues rather than propagating as a
  returned error
…ring gaps

- reservation_action_timeout_watch.go: resolve wallet members only
  after the pending/timeout gates pass, not before, so a misconfigured
  resolver no longer errors on every reservation on every tick; evict
  closed/terminated wallets from the discovery set instead of scanning
  an ever-growing history; scan from block 0 on the first pass so
  wallets registered before the look-back window are found; fix
  doc/behavior mismatches; add a Run-loop test driving two ticks
- reservation_stale_deposit_watch.go: stop collapsing a transient RPC
  error into the same fallback path as a genuine unknown-action state;
  cache reveal timestamps instead of rescanning from genesis on every
  poll; keep tracking a pending acceptance on a Live wallet instead of
  dropping it, so a stuck anchor is still caught; fix doc mismatches
- reservation_stranding_watch.go: invert the notification filter to an
  allow-list (State == Active only) so closed/pending/already-stranded
  reservations are no longer re-notified; fix constructor doc drift
- reservation_wiring.go: wire a real WalletMembersResolver from the
  node instead of a permanent-error stub (the watcher and the node
  share the same process, contrary to the stub's standalone-process
  justification); return real errors instead of only logging; advance
  the stale-deposit scan cursor only when the whole batch classifies
  without error; track a notified marker so a deposit isn't re-
  notified every tick; fix stale doc references
- reservation_acceptance_proof.go/reservation_reanchor_proof.go: unify
  the acceptance/re-anchor input parsers into one helper; fix the
  truncated doc comment and the dual-purpose UTXO doc; fix the
  SubmitReservationProof call's argument order to match the actual
  interface signature; add negative-path test coverage for the
  metrics recorder and the unreachable key/nonce guards
- reservation_proof_loop.go: replace the full-window rescan on every
  pass with an incremental event cursor and pending-action set, and
  reuse the generic wallet-hash dedup helper instead of a duplicate;
  add end-to-end tests for both proof paths
- chain.go: remove 3 Chain interface members with zero production
  callers, mirroring the pkg/tbtc trim
- config.go: unify the reservation-enabled config surface
…s to real resolver

- pkg/chain/ethereum/tbtc.go: remove the 17 ReservationChain method
  implementations with no production caller (mirrors the interface
  trim in pkg/tbtc, pkg/tbtcpg, pkg/maintainer/spv)
- pkg/clientinfo/performance.go: drop the metric registrations and
  dead helper for the removed redemption/dissolution action types
- cmd/start.go: propagate the real WalletMembersResolver from
  tbtc.Initialize into WireReservationWatchers instead of a stub; fix
  the WireReservationWatchers error handling to actually check the
  now-real returned error
piotr-roslaniec added a commit that referenced this pull request Sep 2, 2026
Adopts the human-authored reservation watcher/proof-submission design
that landed directly on m1/keep-core-client (commits 20cb160,
1de2631, 753d717, cb8ad1a) in parallel with this branch's own
Row1/Row2 work, superseding it: their dedicated reservation-proof loop
covers both acceptance and re-anchor proof submission (this branch's
adapter only covered re-anchor), and their timeout-watcher rewrite
replaces the admitted Run() placeholder the same way this branch's did.

Conflict resolution:
- pkg/maintainer/spv/{chain,chain_test,reservation_action_timeout_watch,
  reservation_action_timeout_watch_test,reservation_wiring,spv}.go: took
  the human's version wholesale (superseding design).
- pkg/maintainer/spv/reservation_reanchor_proof*.go: took the human's
  version. Their commit only added a 9-line bounds-check fix on top of
  the pre-existing core proof-assembly primitive (SubmitReservation-
  ReanchorProof et al., already part of PR #4274's original scope, not
  this branch's contribution) - this branch's own addition was the
  325-line generic-loop discovery adapter, now dead since the human's
  design discovers via events instead. Both PR H's original acceptance-
  proof file and the new dedicated loop call directly into the
  unmodified primitive, so it could not be deleted outright.

Ported forward: added verifyReservationActionStillProvable to
reservation_proof_loop.go plus dedicated tests - a submission-time
re-check of the action generation's Pending state and target wallet,
mirroring the safety margin this branch's now-dropped adapter had
(submitDiscoveredReservationReanchorProof's stale-nonce/mismatched-
wallet guard) that the human's design does not have on its own. Wired
into both proveReservationAcceptanceActions and
proveReservationReanchorActions.
Delete the scanState/lastScannedBlock/pendingCandidates cache and
rescan the rolling look-back window fresh every Run, matching
DepositSweepTask's pattern. Fixes stale deposit snapshots, a cursor
that never advanced on the no-candidate path, and unbounded candidate
retention with no eviction rule.

Also: derive the acceptance RequestNonce from chain state instead of
a hardcoded 1 (a hardcoded nonce stalls the deposit forever after a
timed-out retry bumps the on-chain nonce); guard against re-requesting
acceptance for an already-accepted/settled reservation by checking
past request events (fail-closed on RPC error) and reservation state;
make the below-minimum check retryable since the minimum is a live
chain parameter, and remove its now-duplicated dead copy.
The single shared target-wallet cache field let a source wallet
inherit a target cached for a different source wallet without
re-applying its own exclusion. Remove the cache and resolve the
target wallet once per Run instead of once per reservation. Drop
the unreachable hasPendingAction call already covered by the
preceding Active-state check.
estimateReservationFixedSizeTxFee had no dedicated test for the
max-fee guard, unlike the sibling deposit-sweep flow.
Require the sole output to be P2WPKH to the custody/target wallet at
the expected value, so a stale acceptance/reanchor event can't be
matched against an unrelated single-deposit sweep transaction. Add a
bounded retry counter that evicts a pending event after repeated
GetReservationAction errors instead of retrying forever. Verify the
second-pass eviction path once an action settles, index fetched wallet
history by outpoint for O(1) lookup, and align the scan EndBlock with
the recorded cursor.
discoverWallets ignored its own documented look-back constant and
scanned from block 0. Bound the first scan, track pending actions
incrementally from request events instead of re-walking every known
wallet every tick, treat a non-member wallet resolution failure as an
expected skip rather than a fault, and drop the duplicate resolver
interface and unused notifier types.
A GetReservationAction RPC error was treated the same as a confirmed
Unknown state, letting a transient error trigger a premature
stale-deposit release. Split the two branches and bound the fallback
scan. Read the request nonce from chain instead of a hardcoded 1, and
drop the unused OnDepositRevealed wrapper.
A single transient RPC error against any one wallet during the
stranding-watcher startup scan aborted client startup entirely.
Downgrade per-wallet errors to warnings and continue; reserve fatal
returns for genuine mis-wiring. Have CheckStaleReservedDeposit return
its resolution directly instead of the poller re-reading state three
times to decide the same thing. Drop the dead ReservationsConfig
alias and the single-use goroutine wrapper.
ReservationsActivationBlock was already ~1.3M blocks in the past
(copy-pasted from an unrelated feature's activation height), so the
gate protected nothing on merge. Set a genuinely future placeholder
with a sanity-check test. Correct the checklist-gate and
ReservationsConfig doc comments, which described a mechanism the code
doesn't implement. Reject fee byte slices wider than 8 bytes in the
anchor/reanchor proposal unmarshal path to avoid silent Int64
truncation. Delete four orphaned doc blocks describing functions that
no longer exist, and other small comment/logging hygiene fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants