Reservations epic -> dev tracking - #4282
Draft
piotr-roslaniec wants to merge 127 commits into
Draft
Conversation
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.
…cceptance task and fix gofmt
…aware confirmation lookups)
Replace the placeholder getter/no-op submitter for tbtc.ActionReservationReanchor with a real implementation: - getUnprovenReservationReanchorTransactions discovers unproven re-anchor Bitcoin transactions by walking ReservationReanchorRequested events, skipping settled/timed-out action generations, and matching candidate transactions against the still-registered anchor outpoint via ReservationByAnchorUtxo. - reservationReanchorTransactionProofSubmitter re-derives the (reservationKey, requestNonce) pair the generic proof-loop signature cannot carry, then submits via the existing SubmitReservationReanchorProof path. Extends the spv.Chain interface with PastReservationReanchorRequestedEvents and ReservationByAnchorUtxo (already present on TbtcChain); adds matching localChain test fakes. Reservation acceptance proof submission remains a documented placeholder pending its own watcher integration - out of scope here. Covers the discovery precision paths (shape mismatch, anchor mismatch, settled-action skip) and the submitter's nonce/key derivation with new unit tests.
Replace the placeholder Run() (guard checks only, no loop) with a real background poller: - WatchWallet registers a wallet public key hash for polling; dedupes registrations under a mutex-protected set. - Run(ctx) now blocks, checking every watched wallet immediately and then every poll interval, until ctx is done. Each iteration walks each watched wallet's reservations (WalletReservations) and calls the existing CheckReservationActionTimeouts per reservation. A failure checking one wallet is logged and does not abort the iteration or stop the loop. - startActionTimeoutRun (reservation_wiring.go) now threads ctx into Run(ctx) instead of discarding it; context.Canceled is treated as the expected shutdown path, not a failure to log. Run's signature changes from Run() to Run(ctx context.Context); the only call site (startActionTimeoutRun) is updated. Wallet discovery (who calls WatchWallet with which wallets) remains a separate, pre-existing gap shared by all three reservation watchers - see the 'PR H placeholder' comments on subscribeReservationWalletClosed and subscribeReservationActionTimedOut - and is out of scope here. Covers WatchWallet dedup, all three Run precondition guards, and an end-to-end test that Run notifies a timed-out action on its first (immediate) iteration and returns promptly on ctx cancellation.
…e proof submission submitDiscoveredReservationReanchorProof previously re-derived the submission nonce by reading the reservation's current RequestNonce. That field tracks the reservation's live action generation, which can have moved on since the discovered transaction was built - e.g. the original re-anchor action times out and a new, unrelated action generation becomes current while the SPV maintainer is still waiting out requiredConfirmations on the old transaction. Submitting the live nonce in that case pairs a stale, unrelated transaction with the wrong action generation. Fix: before submitting, fetch the action generation at the reservation's current nonce and require it to still be a Pending Reanchor action targeting the exact wallet the discovered transaction actually pays. Any mismatch is reported as an error so the proof loop treats the transaction as not-yet-submittable instead of silently misattributing the proof. Adds two regression tests: one for the action-no-longer-pending case, one for the pending-but-different-target-wallet case.
The prior comment framed the race as spanning proveTransactions waiting out requiredConfirmations. In fact the getter and submitter run back-to-back within the same proveTransactions call for a given transaction (spv.go:229 getter, :286 submitter); under-confirmed transactions are skipped and re-discovered on the next tick, not held. The staleness window is the narrow same-call gap between the getter's per-event Pending check and the submitter call, not a multi-block confirmation wait. The fix itself (verify the current action generation before submitting) is unchanged and still correct - only the severity/likelihood framing in the comment was wrong.
… nonce An error returned from transactionProofSubmitter propagates out of proveTransactions (spv.go:292-293), aborting the entire proving round for every other in-flight transaction across every proof type that tick, then restarting the whole SPV maintainer after the backoff. That is disproportionate for the two mismatch branches added in the prior commit (stale/superseded action generation, mismatched target wallet): both are an expected, if rare, outcome of a narrow same-tick race, not an infrastructure failure. Both branches now log a warning and return nil instead of an error, so proveTransactions treats the transaction as handled and moves on to the next one - it will simply not be rediscovered on the next tick since its action generation is no longer Pending. Flips both regression tests to assert a nil error and that the submission hook was not called, matching the corrected behavior.
Both skip branches previously claimed the transaction 'will simply not be rediscovered' on later ticks. That relied on an unverified assumption - that the Bridge allows at most one Pending action per reservation at a time - which is asserted nowhere in this Go client and could not be confirmed against the on-chain source for this action-generation model (not available in the local tbtc-v2 checkout). If that assumption is false, getUnprovenReservation- ReanchorTransactions' per-event Pending check would keep returning the same transaction and both branches would log the same warning every tick. Replaced with the honest, verifiable termination condition: this outpoint stops matching once the reservation's current generation lands its own correct re-anchor proof, at which point the existing 'no reservation is anchored at the spent outpoint' branch takes over instead. No behavior change - comment accuracy only.
…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
Closes gap-analysis Major row 1 and implementation-plan.md M1 row 3. ReservationAnchorProposal, ReservedRedemptionProposal, ReservationReanchorProposal, and ReservationDissolutionProposal previously used a JSON Marshal/Unmarshal placeholder, unlike every other CoordinationProposal type in this package (Heartbeat, DepositSweep, Redemption, MovingFunds, MovedFundsSweep), which all marshal via pkg/tbtc/gen/pb. Added the four missing message types to message.proto and regenerated message.pb.go (protoc 3.21.12 installed for this). Moved the four proposals' Marshal/Unmarshal from reservation.go's JSON stubs into marshaling.go, matching the existing proto-based implementations' structure and field-encoding conventions (big.Int fees via .Bytes()/SetBytes(), fixed-size hashes/pubkey-hashes via byte-slice copy with a length check). Preserved the original JSON stubs' validation intent under proto3's zero-value-is-absence semantics: a request nonce of 0, or empty fee/reservation-key/hash bytes, are rejected the same way an explicitly-missing JSON field was. The original '== nil' checks on *big.Int fields don't carry over as-is - SetBytes never returns nil - so they're now byte-length checks on the wire field instead, which is the pattern every other proto-based proposal in this file already uses. Testing: extended the existing table-driven TestCoordinationMessage_MarshalingRoundtrip with the four new types (exact field-for-field equality through the wire, matching the existing test's own precision, not just the fuzz-style tests already covering every sibling type) plus four new TestFuzzCoordinationMessage_MarshalingRoundtrip_With<X>Proposal crash-safety tests, matching the one-per-type convention. Rewrote the pre-existing TestReservationProposals_UnmarshalRejectsMissingIntegers (now TestReservationProposals_UnmarshalRejectsInvalidFields) to construct real protobuf payloads instead of JSON string literals, porting every original missing-field case plus two new structural cases (invalid hash/pubkey-hash length) that fall out of the new wire format. go test ./pkg/tbtc/...: 15/15 new/changed tests pass, full package suite passes (146s), -race clean (156s). gofmt/vet clean on all 6 changed files.
Blocker gap found while preparing the M3 multi-signer integration test: pkg/tbtc/coordination.go's getActionsChecklist decides which WalletActionTypes a coordination round even considers, and it never emitted ActionReservationAnchor or ActionReservationReanchor. pkg/tbtcpg.ProposalGenerator.Generate only runs a task whose ActionType() appears in the checklist it's handed (tbtcpg.go:124-135) - it never iterates pg.tasks directly. NewReservationAcceptanceTask and NewReservationReanchorTask are registered when config.Reservations.Enabled=true (tbtcpg.go:88-92), but with no checklist entry, Generate's per-window loop never selected them. Both tasks were structurally unreachable in production regardless of PR #4276/#4277's fixes. Every existing unit test for these tasks (reservation_acceptance_test.go, reservation_reanchor_test.go) calls task.Run(request) directly, bypassing getActionsChecklist/Generate entirely - which is why this was never caught by any prior PR's test suite. Fix: ActionReservationAnchor and ActionReservationReanchor are now appended unconditionally, checked on every coordination window like ActionRedemption (both are custody-critical - an unaccepted reservation or a stale re-anchor risks stranding, not just reduced throughput - unlike the frequency-gated sweep/moving-funds actions). A node with reservations disabled safely no-ops on these: Generate already treats a checklist action with no matching registered task as 'unsupported' and skips it without error (tbtcpg.go:131-135), the same mechanism that already gates every other optional per-node task. Testing: - Updated TestCoordinationExecutor_GetActionsChecklist and its _PostActivation sibling: every non-nil expected checklist now includes both new actions right after ActionRedemption, matching the real append order. Extended assertChecklistOrdering's priority map accordingly (Redemption=0, ReservationAnchor=1, ReservationReanchor=2, then the existing sweep/moving-funds/ heartbeat priorities shifted). - Added TestCoordinationExecutor_GetActionsChecklist_ReservationActionsAlwaysPresent, a dedicated regression guard asserting both actions are present across pre/post-activation and 4th/non-4th windows, decoupled from the large table-driven test - would fail on its own if this wiring regresses. - go test ./pkg/tbtc/... ./pkg/tbtcpg/...: 476/476 pass. - go build ./... && go test ./...: full repo, 49 packages, zero FAIL. - gofmt/vet clean on both changed files.
) ## Summary Reservation actions (`ActionReservationAnchor`/`ActionReservationReanchor`) were already appended to the coordination checklist by a separate commit that landed directly on `reservations-epic` (`757c6d88f`) while this branch was in flight -- that part of the original blocker this PR set out to fix is already resolved on the base branch. `757c6d88f` gated those reservation entries to every 4th coordination window, though, contradicting this branch's original custody-critical/every-window design. A later merge-conflict resolution (`b85294809`) silently kept `757c6d88f`'s gated version without anyone re-deciding the tradeoff on its merits, and this PR's description was never updated to match. This PR is a deliberate reversal of `757c6d88f`'s gate, not an accidental divergence from it. ## What's in this diff (5 files) - `pkg/tbtc/coordination.go` -- `getActionsChecklist` now checks reservation actions on every coordination window once the activation block is reached (same priority as `ActionRedemption`, no frequency gate), since a delayed acceptance/re-anchor risks the on-chain `ReservationActionTimeout` backstop firing before the wallet subsystem acts. Also fixes two comments: the reservation-gate rationale no longer describes a per-operator config flag that doesn't exist in the actual gate, and the Redemption-priority comment no longer overstates which actions are throughput-gated below it. - `pkg/tbtc/coordination_test.go` -- Test tables updated to match the unconditional gate (`TestCoordinationExecutor_GetActionsChecklist_PostActivation`'s 8-case table and `TestCoordinationExecutor_GetActionsChecklist_Reservations`). `assertChecklistOrdering`'s doc comment corrected to the real canonical priority order (Redemption < DepositSweep < MovedFundsSweep < MovingFunds < ReservationAnchor < ReservationReanchor < Heartbeat). `TestCoordinationExecutor_GetActionsChecklist_Reservations` simplified: its assertion filters both sides to reservation actions only before comparing, so the `ActionRedemption` entries in every `expectedActions` literal were inert -- renamed the field to `expectedReservationActions` and dropped them. - `pkg/tbtc/marshaling.go` -- Adds one-line godoc comments on `ReservationAnchorProposal`/`ReservationReanchorProposal`'s `Marshal`/`Unmarshal` methods, matching the convention already used by the other proposal types in this file. Also removes dead code in `ReservationReanchorProposal.Unmarshal`: the length check earlier in the function already guarantees `copy(...)` always returns 20, so the `== 0` disjunct in `if copy(...) == 0 || hash == [20]byte{}` could never be true; kept only the live `[20]byte{}` zero-value check with an explicit copy beforehand. - `pkg/tbtc/marshaling_test.go` -- Adds `ReservationAnchorProposal`/`ReservationReanchorProposal` cases to the existing marshaling-roundtrip test table, plus two new fuzz round-trip tests (`TestFuzzCoordinationMessage_MarshalingRoundtrip_WithReservationAnchorProposal`/`...WithReservationReanchorProposal`) following the same pattern as this file's existing fuzz tests for the other proposal types. - `pkg/tbtc/reservation_test.go` -- Adds two `Unmarshal` negative-test cases for invalid hash lengths (deposit funding tx hash on the anchor side, target wallet hash on the re-anchor side). Renames `TestReservationProposals_UnmarshalRejectsMissingIntegers` to `TestReservationProposals_UnmarshalRejectsInvalidPayloads` since its table now covers invalid-length cases too, not only missing integers, and adds a regression case (`"re-anchor zero-value target wallet hash"`) pinning the all-zero-hash rejection the marshaling.go dead-code disjunct was masking. ## Known accepted tradeoff (documented, not fixed here) `ProposalGenerator.Generate` (`pkg/tbtcpg/tbtcpg.go`) returns on the first checklist action that yields a proposal, and reservation actions sit last in the checklist (after Redemption, DepositSweep, MovedFundsSweep, MovingFunds). A wallet with steady redemption/sweep traffic can still delay reservation acceptance/re-anchor even though the checklist entry itself is now unconditional. This is called out in a new comment at the reservation-gate site in `coordination.go`; it is bounded by the on-chain `ReservationActionTimeout` backstop and is not addressed by this PR. ## Testing - `gofmt -l ./pkg/tbtc/`: clean. - `go vet ./pkg/tbtc/...`: clean. (Repo-wide `go vet ./...` surfaces one pre-existing, unrelated warning in `pkg/tecdsa/signing/protocol.go` -- a file this PR does not touch.) - `go build ./...`: succeeds. - `go test ./...`: 1846 tests passed across 89 packages, zero failures. - `go test ./pkg/tbtc/ -run 'GetActionsChecklist|ReservationProposals'`: 43 tests passed, verifying every changed test case individually. ## Not in this PR - The `docs/spec/reservations/m1-keep-core-readiness/01-gap-analysis.md` gap-analysis doc referenced by an earlier version of this description does not exist in this repo and is not added here. - Milestone 2 test-coverage backfill and the M3 multi-signer integration test are tracked as separate follow-up work.
…vation-review-fixes # Conflicts: # pkg/tbtc/coordination.go # pkg/tbtc/coordination_test.go # pkg/tbtc/marshaling.go
…BroadcastChannel 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.
A candidate deposit that cleared the gross ReservationMinAmount but failed the net-of-fee check (or didn't cover the anchor fee at all) was returned as the selected candidate and then aborted proposal generation with a hard error. Nothing marked the deposit ineligible after the abort, so the same doomed deposit was re-selected on every subsequent Run(), permanently blocking the wallet's reservation queue until the deposit-reveal event aged out of the ~30-day look-back window. Move both fee viability checks (anchor fee coverage and net-of-fee minimum) into findReservationAcceptanceCandidate's selection loop, so a candidate that fails either check is skipped in favor of the next one instead of halting the pipeline. proposeReservationAcceptance now reuses the fee already computed during selection instead of recomputing and re-validating it. Also documents (does not change) the pre-existing fail-open handling of GetReservation errors: its interface contract signals "not found" as an error, so failing closed there would reject every brand-new candidate; a RequestNonce==1 assertion was added to TestReservationAcceptanceTask_GetReservationError to pin the intentional fallback.
Fixes 20 P1/P2/P3 findings from a multi-agent review of this PR's own test coverage: pkg/tbtcpg/reservation_acceptance_test.go: - Narrow the PastDepositRevealedEvents override to swallow only its claimed sentinel; add error-injection coverage. - Assert AnchorTransactionAssembly's returned proposal fields against the task-derived candidate, not just a hand-built fixture object; make the fixture's ValidateReservationAnchorProposal genuinely check the funding outpoint instead of always returning nil. - Delete the fixture's shadow ReservationParameters override/field; route through the base LocalChain's already-correct value-copy setter/getter instead. - Fix a test comment overclaiming exclusion is "strictly attributable" to one filter when multiple fixture gaps independently cause it. - Make the freshness-control test's RequestReservationAcceptance override actually record an event, so the dedup guard it's meant to exercise engages for real. - Pin RequestNonce==1 for the documented GetReservation fail-open path. - Make BoundaryChecks' three cap fields pointer-typed so a row can express production's real "0 = unlimited" semantic; add rows proving it. - Delete the dead reservedDeposits field/IsReservedDeposit override (zero production readers). - Add TestReservationAcceptanceTask_ValidateProposalError covering the previously-unexercised validateErr wrapping path. - Fix two misleading comments (wrong function attribution for the ReservationMinAmount gate; wrong description of the GetWallet error contract). - Add testReservationVaultAddress const, replacing 30+ inline literal duplicates. - Extract newBoundaryTestChain helper, replacing a ~13-line setup block duplicated across 16 call sites. pkg/tbtcpg/chain_test.go: - Fix binary.BigEndian.PutUint64 writing EndBlock bytes into the startBlock buffer instead of endBlock across 5 call sites, so EndBlock actually contributes to the event-filter cache key. pkg/chain/ethereum/tbtc_test.go: - Replace a stale tbtc.go line-range doc citation with a symbol reference. - Fold TestConvertReservationFromAbiType_DropsCumulativeReanchorFee into TestConvertReservationFromAbiType as a subtest, matching this file's one-test-per-converter convention. pkg/chain/ethereum/tbtc.go: - Add an in-tree TODO marking ValidateReservationAnchorProposal's deferred test coverage, since the docs describing that deferral don't exist in this checkout.
…nt review of #4282) (#4283) ## Summary Remediation for the 37 confirmed findings from a multi-agent review of PR #4282 (`dev` <- `reservations-epic`, i.e. the accumulated content of #4274+#4276+#4277). 37 raised -> 37 confirmed -> 0 dropped after arbitration and validation. - **P1 (6 of 7 fully fixed, 1 partially fixed):** deposit-sweep reservation-vault exclusion, reservation look-back underflow + target-wallet check, reservation acceptance `eth_getLogs` bounds + nonce reconciliation + caps, SPV proof-loop retry-eviction data loss (symptom fixed, structural root cause deferred - see below), stale-deposit timeout memoization, below-dust re-anchor trigger removal (M-27, resolved via tbtc-v2 source after user escalation). - **P2/P3 (22 of 30 fixed, 8 explicitly deferred):** see "Deferred" below. Full-repo `go build`, `go vet`, and `go test ./...` all pass with these fixes applied (verified after every commit and once more at closeout). ## Deferred (1 P1 architectural root-cause + 7 P2/P3 symptoms/hygiene) An arbiter-recommended structural fix for M-16 (remove the SPV proof loop's persistent-cursor design entirely in favor of the stateless bounded-rescan pattern every sibling proof type already uses) was attempted together with the M-7 nonce-aware timeout fix and an M-14 dead-code removal. That combined change broke three existing tests and was reverted rather than debugged under time pressure. Only a narrower, independently-safe subset landed: a surgical patch for M-3 (non-lossy cursor rewind) plus unrelated memoization/metrics/test fixes. **M-16's own P1 rating is only partially addressed** - the persistent-cursor design itself, and the M-7/M-14 symptoms it also breeds, remain unremoved. 1. **M-16 (P1)** `pkg/maintainer/spv/reservation_proof_loop.go:227-246` - `reservationProofScanState`'s persistent cursor is the structural root cause of M-3 (fixed surgically) and M-7 (below). Removing it in favor of the stateless bounded-rescan pattern is what broke 3 tests on first attempt and remains unimplemented. 2. **M-7 (P2)** `reservation_action_timeout_watch.go:260-281` - `CheckReservationActionTimeouts` deletes `pendingActions` entries on 3 of 4 non-notifying outcomes without asserting the tracked `requestNonce` against the freshly-derived one; same root cause as M-3. 3. **P2** `reservation_action_timeout_watch.go:370` + `reservation_wiring.go:38-49` - the timeout watcher's `WalletMembersResolver` only resolves wallets the local operator co-signs; an offline/disabled/colluding wallet's own operators get zero independent timeout coverage. 4. **P2** dead-code cluster in `reservation_proof_loop.go` / `reservation_proof_loop_test.go` - `findReservationAcceptanceTransaction`, `findReservationReanchorTransaction`, and their wrapper helpers have zero production callers; 14 tests exercise the unused wrapper instead of the `isMatching*` predicates actually called in production. 5. **P2** `reservation_proof_loop.go:612,~817` - two tautological guards are algebraically always-false, masking that the real enforced constraint is only `0 < fee <= TxMaxFee`. 6. **P2** `reservation_wiring.go:237-320` `startStaleDepositPoll` - the entire loop body runs untested inside a goroutine; existing tests assert only that the goroutine starts. 7. **P3** `reservation_action_timeout_watch.go:18-20` - unused "backward-compatibility alias" constant, zero references. 8. **P3** `reservation_proof_loop.go:644` - duplicated, truncated comment fragment left by a merge. ## Known conflicts with other open PRs in this stack - read before merging This branched from `reservations-epic` at `bb3dcb398`. Three other efforts are in flight against overlapping code and were **not** reconciled here, since they belong to PRs this one doesn't own: ### 1. `pkg/tbtc/coordination.go` vs #4278 (hard conflict, not cosmetic) #4278 ("remove frequency gate on reservation checklist actions") drops `&& windowIndex%frequencyWindows == 0` from the reservation-actions checklist gate (custody-critical, should run every window like `ActionRedemption`) but its diff still references the old single `ReservationsActivationBlock` constant. This PR's `602d0ef11` independently rewrote that same `if` into `reservationsActivationBlock(ce.ethereumNetwork)`, a per-network table lookup (`ethereum.Mainnet: 26500000`, everything else defaults to 0). **A conflict resolution that naively favors this PR's side of that hunk silently reinstates the frequency gate #4278 deliberately removed.** Combined resolution (verified against both intents): ```go // Reservation actions (acceptance, re-anchor) are custody-critical like // Redemption and are checked on every coordination window once the // activation block is reached, not frequency-gated like the // throughput-driven DepositSweep/MovedFundsSweep/MovingFunds actions // above: a delayed reservation acceptance or re-anchor risks the // on-chain ReservationActionTimeout backstop firing before the wallet // subsystem gets a chance to act. The activation block is a per-network // table (reservationsActivationBlock), not a single global constant, but // it is still config-independent and globally observable from chain // height alone -- which is what keeps leader and follower checklists in // agreement without relying on local config. if coordinationBlock >= reservationsActivationBlock(ce.ethereumNetwork) { actions = append(actions, ActionReservationAnchor) actions = append(actions, ActionReservationReanchor) } ``` ### 2. `pkg/tbtc/marshaling.go` vs #4278 (duplicate, this PR's version wins) #4278 independently adds the same 4 missing `Marshal`/`Unmarshal` doc comments this PR's `7cbb8cc2f` adds, but comment-only and with a capitalization bug (lowercases the exported type name, e.g. `"...converts the reservationAnchorProposal..."`). This PR's version is a superset: correctly capitalized comments plus the actual nil-guard/zero-hash-rejection logic #4278 doesn't have. On merge, take this PR's 4 lines, drop #4278's. ### 3. `pkg/tbtcpg/reservation_acceptance_test.go` vs #4280 (whole-file conflict + one real design decision) #4280 ("M2 test-coverage backfill") independently rewrote large parts of the same shared test harness this PR's `726f05ed7` touched - the same `reservationAcceptanceLocalChain` type, constructor, and ~14 shared methods, plus `scenarioReservationAcceptanceChain`/`registerReservedDeposits`/ `expectedAnchorsEqual`. This is a heavy line-level conflict across the whole file, not just redundant test names. Specifics: - `TestReservationAcceptanceTask_AmountCapBoundaries` (this PR, cap boundaries only) is a strict subset of #4280's `TestReservationAcceptanceTask_BoundaryChecks` (adds `MaxReservationsPerWallet`, net-of-fee `ReservationMinAmount`, `ActiveReservationsCount`). Left in place rather than deleted preemptively - #4280 is still open and two-deep-stacked (on #4278, also open) and could stall or be reworked; delete this PR's version only in the merge that actually lands #4280. - This PR's `TestReservationAcceptanceTask_VaultNotConfigured_ZeroAddress` (finding: dead vault-not-configured guard) has no equivalent on #4280's side - a "just take #4280's file" resolution silently drops it. - **Real design decision, not just a merge conflict:** #4280's `TestReservationAcceptanceTask_Stateless_PastEventsError` exercises `PastReservationAcceptanceRequestedEvents` returning an error and asserts fail-closed skip-on-error. This PR's `hasPendingAction` (from `726f05ed7`) no longer calls `PastReservationAcceptanceRequestedEvents` at all - it uses a different, generation-scoped pending-action check instead. Ported onto this PR's code as-is, that test would either pass vacuously or fail for an unrelated reason. **This PR intentionally left `PastReservationAcceptanceRequestedEvents` on the `tbtcpg.Chain` interface (`chain.go:263`) and the test double's `acceptanceEvents`/`acceptanceEventsErr` fields in place, undeleted, even though they now have zero production callers** - removing them here would have foreclosed reconciling #4280's test against whichever pending-action mechanism is ultimately kept. Whoever merges this PR and #4280 needs to pick one mechanism and either delete the losing side's interface method/test or keep both if there's a reason for two independent checks. ## Testing - `go build ./...`, `go vet ./...`: clean. - `go test ./...`: full repo suite, 0 failures (verified at closeout after every commit landed).
…eBroadcastChannel The settle-window drain added to absorb the expected single straggler tick discarded its count unchecked, so a genuine regression where the ticker fires more than once after release would only be caught by the second, stricter window - not at the settle step itself, where it's easier to diagnose. Assert the settle window sees at most one tick.
…adcastChannel (#4284) Follow-up to #4283. That PR fixed `TestReleaseBroadcastChannel`'s flake (reproduced pre-existing on clean `origin/reservations-epic` at the time, ~2/5 failure rate in isolation - #4279's bug, not introduced by #4283's merge) by absorbing the one straggler tick `NewTimeTicker`'s cancel-vs-elapsed-timer race can let through after `ReleaseBroadcastChannel`. That fix's settle-window drain discarded its count unchecked, so a genuine regression where the ticker fires more than once after release would only surface at the second, stricter assertion - not at the settle step itself, where the failure is easier to diagnose. This bounds the settle window: at most one straggler, asserted explicitly. Verified 20/20 locally (`go test ./pkg/net/local/... -run TestReleaseBroadcastChannel -count=1`, repeated); full `go build`/`go vet`/`gofmt -l` clean.
…vation-test-coverage-backfill # Conflicts: # pkg/tbtcpg/reservation_acceptance.go # pkg/tbtcpg/reservation_acceptance_test.go
The 'records acceptance request, skipping duplicate on subsequent run' test relied on hasPendingAction's fail-closed default on a missing GetReservationAction record, not on the intended pending-state detection path - its comment still described the removed PastReservationAcceptanceRequestedEvents mechanism. Explicitly set the action record to Pending after the first run so the second run's dedup assertion genuinely exercises hasPendingAction's happy path.
## Summary Implements `implementation-plan.md` Milestone 2's test-coverage backfill: 7 of the 8 listed items (item 8's scope narrowed - see below). One earlier-planned item, a golden-value dedup test for `AssembleReservationAnchorTransaction`, was obsoleted when `proposeReservationAcceptance` was switched to call the exported `tbtc.AssembleReservationAnchorTransaction` directly, removing the second, unexported copy the dedup test would have compared against; it was not silently dropped. Merged up to date with `m1/reservation-multisigner-integration-test` ([#4279](#4279)), tip `9e42103e8`. The 8th item (`ValidateReservationAnchorProposal`/ `ValidateReservationReanchorProposal` tests) is explicitly deferred - it needs `go-ethereum` simulated-backend test infrastructure that doesn't exist anywhere in `pkg/chain/ethereum` today, well beyond the plan's 0.5-day estimate. See `docs/spec/reservations/m1-keep-core-readiness/01-gap-analysis.md`'s new Minor row for the full finding. ## Change **`pkg/tbtc/reservation_test.go`** - `TestAssembleReservationAnchorTransaction`: happy-path output shape (1-in-1-out, deposit value minus fee, P2WPKH to the target wallet). - `TestAssembleReservationReanchorTransaction`: same shape assertion for the re-anchor sibling. **`pkg/chain/ethereum/tbtc_test.go`** - `TestConvertReservationParametersFromAbiType`: full 10-tuple field mapping, every field a distinct non-zero value so a swapped or dropped field can't hide behind a shared zero default. - `TestConvertReservationFromAbiType_DropsCumulativeReanchorFee`: pins the intentional `CumulativeReanchorFee` omission and verifies every other field maps correctly around it. **`pkg/tbtcpg/reservation_acceptance_test.go`** - `TestReservationAcceptanceTask_BoundaryChecks`: table covering at-limit/one-over-limit boundary crossings for `MaxReservationsPerWallet`, `ReservationMinAmount`, `ReservationMaxTotalAmount`, `ReservationMaxSingleAmount`, `MaxReservationsAmountPerWallet`, and `ActiveReservationsCount`, plus the net-of-fee minimum check in `proposeReservationAcceptance` - `TestReservationAcceptanceTask_BoundedLookback` only ever used these fields as fixture data, never at the actual boundary. - `TestReservationAcceptanceTask_ReservationParametersFetchedLive`: runs the same task twice against the same deposit, mutating `ReservationMinAmount` between calls - verifies a governance-driven parameter change takes effect on the very next `Run()` call, with no leftover value from a prior run observable in the eligibility decision. - `TestReservationAcceptanceTask_AnchorTransactionAssembly`: end-to-end wiring test - runs the task to get a `ReservationAnchorProposal`, then re-assembles and signs the anchor transaction via the exported `tbtc.AssembleReservationAnchorTransaction`, and asserts the resulting signed transaction is a valid 1-input-1-output transaction paying the correct wallet P2WPKH output script with value equal to deposit amount minus the anchor fee. ## Testing - `go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/chain/ethereum/...`: 280/280 pass. - `go build ./...` && `go test ./...`: full repo, 49 packages, zero `FAIL`. - `gofmt -l` / `go vet`: clean on all changed/new files. ## Not in this PR - `ValidateReservationAnchorProposal`/`ValidateReservationReanchorProposal` tests - deferred, documented in the gap-analysis doc.
…mable candidate scan - GetReservation read errors now skip the deposit for this window instead of falling through with a fabricated nonce and bypassed eligibility/pending-action gates; only a successful read reporting State==Unknown && RequestNonce==0 is treated as 'not yet created'. - Pre-write assemble/validate failures in proposeReservationAcceptance no longer abort the whole coordination window: Run retries the next candidate via a skip-set instead of starving every other deposit on the wallet behind one permanently-rejected one. Post-write failures still abort as before. - Add an incremental per-wallet reveal cursor so each window scans only the delta since the last run instead of the full ~30-day look-back, plus a hard cap on candidates examined per run. - Add scenario_8.json: two simultaneous candidates (one ineligible, one eligible) proving the scan doesn't stop at the first ineligible deposit, guarding against a regression of the head-of-line-blocking fix. - Drop the stale merge-history comment in tbtcpg.go.
…n metric gating - ReservationReanchorTask.Run now breaks instead of continuing once a post-write post-condition check fails after RequestReservationReanchor already authorized an action, guaranteeing at most one on-chain authorization per pass. - findTargetWallet falls back to an unbounded registration-event scan when the bounded ~30-day window yields no live wallet, instead of returning no proposal indefinitely while live wallets exist. - Publish live_wallets_count unconditionally every Run, matching the sibling saturation gauges, instead of only after guards that are false in steady state. - Register wallet_action_reservation_* counters/histograms unconditionally: reservation action execution is not gated on Tbtc.Reservations.Enabled, so gating their registration silently dropped observability for exactly the operators most likely to be surprised by unconditional execution.
…he growth - CheckStaleReservedDeposit now returns Keep instead of Drop for a reserved deposit on a Live wallet, so the poller re-evaluates it after any wallet state change instead of permanently orphaning it the first time it observes a Live wallet. - notified/memoizedTimeout caches are now cleared when a deposit resolves (Drop/Notified), instead of growing for the process lifetime; the memoized deadline is invalidated when the governance ReservationActionTimeout parameter changes instead of silently reusing an earlier, shorter cached deadline. - Doc comment no longer claims the check is 'intentionally pure' given its Bridge-notification side effect and receiver-map mutations.
…ations - pollPendingActions no longer deletes a tracked action after a successful check; a submitted-but-dropped/reverted notification previously vanished from tracking with err==nil and was never retried. Eviction now relies solely on the existing state-driven branch (action no longer Pending on-chain). - Reuse the action already loaded during the poll pass in the timeout-notification path instead of re-reading it a second time. - Bound repeated GetReservationAction read failures with a retry counter so permanently unreadable entries are eventually evicted instead of accumulating forever. - Delete the unreferenced backward-compatibility alias constant. - Fix placeholder-era doc comments describing a synchronous driving integration that was never wired; production only ever starts the fixed-interval background Run loop.
…d eviction cycle - Delete the tautological Outputs[0].Value != amount-fee comparisons in the acceptance and re-anchor transaction matchers (fee is itself defined as amount - Outputs[0].Value on the preceding line, so the check could never be true); the fee<=TxMaxFee bound is the real guard. - Remove the retry-count/cursor-rewind-on-eviction machinery: on the third consecutive read failure it rewound the scan cursor and deleted the retry counter, so the next pass re-fetched the identical event with its retry count reset to zero, an unbounded evict-rediscover-retry-from-zero cycle. Errors are now logged and the event stays pending until a successful read shows a terminal state. - Delete the unused wallet-event adapter types and their test-only production wrapper functions; retarget their tests at the candidate-map/predicate logic actually used in production. - Enforce the same fee<=TxMaxFee bound in the acceptance matcher's not-found branch as the found branch, instead of only OutputValue>0.
…rnals WireReservationWatchers is the only production composition point for this watcher; its constructor, type, and check method had no external callers. Make them package-private, matching this package's other watcher internals.
…ch-error handling - Bound WireReservationWatchers' startup wallet-registration scan to a ~30-day look-back instead of scanning full chain history (StartBlock:0) on every reservation-enabled client boot; older wallets remain covered by the live subscription and the existing per-wallet close re-check. - Compare a revealed deposit's event.Vault against the locally-known ReservationVault before calling IsReservedDeposit, skipping the eth_call entirely for deposits targeting an unrelated vault. - Fix the stale-deposit poll's batch-error handling: a single failed IsReservedDeposit call previously broke out of the whole event batch and then advanced lastSeenBlock past the entire window regardless, silently dropping every deposit after the failed one; it now skips only the failed deposit and still examines the rest of the window. - Wire up the stranding watcher's now-package-private constructor and type, and fix its placeholder-era poll-interval doc comment.
buildReservationProofMainUtxo's naming/comments read as if it resolves a wallet main UTXO, but ReservationRouter.sol's own devdoc marks the parameter unused until milestone 2. Document that explicitly and pin the current (harmless) encoding with a regression test so a future milestone-2 activation of this parameter doesn't silently inherit the wrong value.
- Rewrite the Reservations config field docs: the flag gates proposal generation, watcher wiring, and metrics registration only; execution and co-signing are unconditional past the activation block by design, not 'side-effect free' as the comment previously claimed. - Add an explicit ethereum.Sepolia entry to the reservations activation-block table; restrict the activate-at-0 fallback to ethereum.Developer/Unknown so no other public network silently inherits immediate activation. - Replace a misleading coordination_test.go block constant that implied mainnet-gate significance it didn't have in that test. - Note in ParseWalletActionType why wire slots 7 and 9 are rejected: intentionally incomplete until M2 action types are implemented.
…m steps COPY ./ci-shims/tbtc-artifacts in the Dockerfile sourced a directory excluded by .gitignore and created only by client.yml's shim step; release.yml builds the identical Dockerfile with no equivalent step, so a fresh-checkout release build hard-failed on the missing COPY source. Un-ignore the directory and track a placeholder so it always exists regardless of which workflow builds the image. Also delete the client.yml steps gated on github.base_ref == 'reservations-epic': PR #4282's base is dev, so the gate never fires for this PR, and once the epic lands on dev it can never fire again. The gen/Makefile fallback rules already supply the same artifact surface and remain the only reachable mechanism.
The gitkeep placeholder tracked in the prior commit made 'ls -A /tmp/tbtc-artifacts' always non-empty, so the shim-copy guard fired unconditionally on every development build even with no shim JSON present, and 'cp /tmp/tbtc-artifacts/*.json ...' then failed on the unmatched glob. Test for *.json specifically instead of any file in the directory.
An invented finite block height for Sepolia silently becomes live behavior if nobody edits it before release - the exact rollout risk the review finding was about. Sepolia now has no map entry and falls through to the existing math.MaxUint64 never-activate default, provably inert until a real rollout height is chosen and added explicitly.
…laked Skipping a deposit outright on a single IsReservedDeposit error left it permanently unscanned once lastSeenBlock advanced past its event's block range. Track it in pending instead: CheckStaleReservedDeposit performs its own independent IsReservedDeposit re-check every tick, so speculatively tracking it is safe and lets the next tick resolve it correctly.
…ification The prior notified-bool fix stopped deleting the entry but never retried it either: once notified was set, pollPendingActions skipped the entry forever regardless of whether the notification tx actually landed, leaving a dropped/reverted notification permanently unretried - the same end state as the eviction bug it replaced, just with a leaked map entry instead of a missing one. Replace the bool with a notifiedAt timestamp and re-offer the action to NotifyReservationActionTimeout once actionTimeoutRenotifyInterval (10 minutes, matching this package's existing backoff convention) has elapsed since the last attempt and the action is still Pending. Add a test that drives nowFn past the backoff window and asserts the retry notification is actually submitted.
# Conflicts: # pkg/chain/ethereum/tbtc.go # pkg/chain/ethereum/tbtc_test.go # pkg/clientinfo/performance.go # pkg/maintainer/spv/config.go # pkg/tbtc/tbtc.go # pkg/tbtcpg/chain_test.go # pkg/tbtcpg/deposit_sweep.go # pkg/tbtcpg/fee.go # pkg/tbtcpg/fee_test.go
…ervation proofs The reservation acceptance/re-anchor proof loop passed the hardcoded package default to getProofInfo, so an operator raising Maintainer.Spv.MaxProofHeaders (needed on testnet4-style extended minimum-difficulty runs) was silently ignored. Thread config through proveReservationTransaction and pass config.MaxProofHeaders; the viper flag default (cmd/flags.go) keeps zero-valued configs at 144. Loop-level tests now set the field explicitly since they construct Config directly.
The 144 bound is applied by flag registration, so Config values built programmatically (tests, wiring paths that bypass cmd/flags.go) yielded a cap of 0 and getProofInfo skipped every proof as proofSkipExceededMaxHeaders. Normalize once at proveReservationTransaction and pin the behavior with a subtest that submits under an explicit 0 - it fails if the guard is ever removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reservations epic -> dev tracking
This PR aggregates the UTXO reservations epic (
reservations-epic) and tracks its promotion todev.Merge stack (#4274 chain) — COMPLETE
m1/keep-core-client->reservations-epic— feat(tbtc): wire reservation executors and watchers (merged)m1/reservation-readiness-fixes->reservations-epic— fix(spv): re-verify reservation action generation before SPV proof submission (merged)m1/reservation-protobuf-marshaling->reservations-epic— test(tbtc): add reservation proposal marshaling coverage (merged)m1/reservation-coordination-checklist->reservations-epic— fix(tbtc): remove frequency gate on reservation checklist actions (merged)m1/reservation-multisigner-integration-test->m1/reservation-coordination-checklist— test(tbtc): multi-signer simulated integration test for reservation coordination (merged into fix(tbtc): remove frequency gate on reservation checklist actions #4278's branch prior to fix(tbtc): remove frequency gate on reservation checklist actions #4278 landing; content is inreservations-epic)m1/reservation-test-coverage-backfill->reservations-epic— test(reservations): M2 test-coverage backfill (7 of 8 items) (merged)The entire #4274 stack is now merged into
reservations-epic.Other PRs targeting
reservations-epicfeat/utxo-reservation-wallet-support->reservations-epic— draft: UTXO reservation wallet-side foundations (parallel branch, independent of the feat(tbtc): wire reservation executors and watchers #4274 stack; still unmerged)How to use this PR
reservations-epic(or stack onto an open reservations PR).reservations-epic -> devgate.devintoreservations-epicto resolve the current conflict, then re-run CI on the epic branch before merging this PR intodev.Status
Stack complete: #4274, #4276, #4277, #4278, #4279, #4280 all merged into
reservations-epic. This PR is blocked on resolvingreservations-epic's divergence fromdev(109 ahead / 68 behind, currentlyCONFLICTING). #4238 remains separately unmerged and untouched by this stack.