Reservations epic -> dev tracking - #4282
Draft
piotr-roslaniec wants to merge 170 commits into
Draft
piotr-roslaniec wants to merge 170 commits into
piotr-roslaniec wants to merge 170 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 3, 2026
…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).
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 4, 2026
…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.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 7, 2026
#4282 40 confirmed findings fixed (2 P0, 7 P1, 24 P2, 7 P3); 1 P2 finding (in-kind fee reserve watch) intentionally left unfixed - its only two remediations are building unscoped new functionality or editing a separate spec repo, both out of scope here. P0: - wire notifyReservationAcceptanceTimedOut end to end (vendored ABI, Go bindings, chain interfaces, ethereum adapter, watcher dispatch by ActionType) - acceptance-type timeout notifications could never succeed on-chain before this - gate deposit-sweep's reserved-deposit filter on the reservation vault address instead of calling isReservedDeposit unconditionally on every deposit pre-activation, which risked halting sweeping network-wide during the binary-upgrade-before-Bridge-upgrade window P1: - action-timeout watcher no longer requires wallet-member resolution to notify a Reanchor timeout, so a dead/closed wallet's stuck action is now permissionlessly resolvable - drop the load-failure eviction that permanently dropped a tracked action from timeout coverage after 3 RPC hiccups - remove the mainnet reservations-activation-block placeholder pending a real rollout height - correct GetReservation/GetReservationAction doc comments (absence is Unknown state, not an error) - propagate chain-read errors instead of masking them as no-op - memoize per-pass proof-invariant chain reads shared across proof-loop transactions - add the missing reservations-remaining below-dust regression subtest P2/P3: gauge visibility, nonce-bound preload validation, notifiedAt semantics, address-comparison case sensitivity, wallet-vault fee-floor check, dual-flag config validation, duplicate/stale comments and tests, and assorted simplifications. Also reverts an in-flight hard-fail config-validation check for the Tbtc/Spv reservations dual-flag pairing back to a warning: start's config categories never load the Maintainer section, so the check would have hard-failed startup for any legitimate split deployment running the SPV maintainer as a separate process. Self-verification follow-up (same review pass, iterated against external review of the fixes themselves): re-checked every "fixed" claim above against the actual diff and against the reasoning behind each fix, and made three further corrections: - deposit-sweep follower-side (ValidateDepositSweepProposal) never actually got the reservation-vault gate described above - only the leader side (deposit_sweep task) did, so a follower could still sign a sweep a leader had already correctly rejected. Implemented the matching gate on the follower path: fetch ReservationParameters with the same bounded retry as the leader (fewer attempts only makes a wrong "reservations aren't live" guess more likely, never less, so there is no safety argument for the follower retrying less than the leader), then hard-reject on any IsReservedDeposit error or a confirmed reservation for any deposit the cheap vault-match prefilter flags as a candidate - unlike the neighboring fee soft-check just below, which stays deliberately log-only because a merely underpriced sweep is not irreversible the way sweeping a reservation is. Added the regression test proving detection (TestValidateDepositSweepProposal_RejectsReservedDeposit) and its counterpart proving the fail-open path still sweeps normally when ReservationParameters is unavailable (TestValidateDepositSweepProposal_SweepsWhenReservationParametersUnavailable, mirroring the leader's existing test of the same shape). - the P1 loadFailures-eviction removal above initially grew a backoff mechanism to avoid re-logging every poll tick during a persistent failure, but the backoff suppressed the downstream timeout-notify check for the same tick it suppressed the retry - a single transient RPC error could have delayed a genuinely-overdue Bridge notification by up to 10 minutes, worse than the log spam it was meant to prevent (a real outage fails every tracked action identically regardless of backoff, so there was nothing for a per-entry backoff to usefully save). Removed it; kept the simpler P1 fix alone - retry every tick, evict only on an observed non-Pending state. - reservation_wiring.go's walletMembersResolver parameter was threaded through WireReservationWatchers but never read by anything downstream of the P1 fix that removed its only consumer; deleted the dead parameter from the function, its test, and the cmd/start.go call site (which used to feed it the WalletMembersResolver tbtc.Initialize returned), and corrected the call site's comment (the paired-flag validation is a warning, not a hard error, per the note above). Finished the resulting clean cutover: with cmd/start.go (Initialize's only caller anywhere in the repo) no longer reading it, tbtc.Initialize now returns only error, and the WalletMembersResolver interface plus node.ResolveWalletMembers (its one implementation) are deleted with it.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 7, 2026
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 7, 2026
…4282 Comment-only audit and fix pass across all comments this PR added or modified: drift against current code, self-containment (drop references to other repos / markdown docs that don't exist in keep-core), redundant text, and inconsistent voice. No behavioral change - gofmt/build/vet clean, diff confirmed comment-only against d1697f5.
piotr-roslaniec
force-pushed
the
reservations-epic
branch
from
September 8, 2026 14:39
ad0014d to
53cac0a
Compare
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
…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.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
#4282 40 confirmed findings fixed (2 P0, 7 P1, 24 P2, 7 P3); 1 P2 finding (in-kind fee reserve watch) intentionally left unfixed - its only two remediations are building unscoped new functionality or editing a separate spec repo, both out of scope here. P0: - wire notifyReservationAcceptanceTimedOut end to end (vendored ABI, Go bindings, chain interfaces, ethereum adapter, watcher dispatch by ActionType) - acceptance-type timeout notifications could never succeed on-chain before this - gate deposit-sweep's reserved-deposit filter on the reservation vault address instead of calling isReservedDeposit unconditionally on every deposit pre-activation, which risked halting sweeping network-wide during the binary-upgrade-before-Bridge-upgrade window P1: - action-timeout watcher no longer requires wallet-member resolution to notify a Reanchor timeout, so a dead/closed wallet's stuck action is now permissionlessly resolvable - drop the load-failure eviction that permanently dropped a tracked action from timeout coverage after 3 RPC hiccups - remove the mainnet reservations-activation-block placeholder pending a real rollout height - correct GetReservation/GetReservationAction doc comments (absence is Unknown state, not an error) - propagate chain-read errors instead of masking them as no-op - memoize per-pass proof-invariant chain reads shared across proof-loop transactions - add the missing reservations-remaining below-dust regression subtest P2/P3: gauge visibility, nonce-bound preload validation, notifiedAt semantics, address-comparison case sensitivity, wallet-vault fee-floor check, dual-flag config validation, duplicate/stale comments and tests, and assorted simplifications. Also reverts an in-flight hard-fail config-validation check for the Tbtc/Spv reservations dual-flag pairing back to a warning: start's config categories never load the Maintainer section, so the check would have hard-failed startup for any legitimate split deployment running the SPV maintainer as a separate process. Self-verification follow-up (same review pass, iterated against external review of the fixes themselves): re-checked every "fixed" claim above against the actual diff and against the reasoning behind each fix, and made three further corrections: - deposit-sweep follower-side (ValidateDepositSweepProposal) never actually got the reservation-vault gate described above - only the leader side (deposit_sweep task) did, so a follower could still sign a sweep a leader had already correctly rejected. Implemented the matching gate on the follower path: fetch ReservationParameters with the same bounded retry as the leader (fewer attempts only makes a wrong "reservations aren't live" guess more likely, never less, so there is no safety argument for the follower retrying less than the leader), then hard-reject on any IsReservedDeposit error or a confirmed reservation for any deposit the cheap vault-match prefilter flags as a candidate - unlike the neighboring fee soft-check just below, which stays deliberately log-only because a merely underpriced sweep is not irreversible the way sweeping a reservation is. Added the regression test proving detection (TestValidateDepositSweepProposal_RejectsReservedDeposit) and its counterpart proving the fail-open path still sweeps normally when ReservationParameters is unavailable (TestValidateDepositSweepProposal_SweepsWhenReservationParametersUnavailable, mirroring the leader's existing test of the same shape). - the P1 loadFailures-eviction removal above initially grew a backoff mechanism to avoid re-logging every poll tick during a persistent failure, but the backoff suppressed the downstream timeout-notify check for the same tick it suppressed the retry - a single transient RPC error could have delayed a genuinely-overdue Bridge notification by up to 10 minutes, worse than the log spam it was meant to prevent (a real outage fails every tracked action identically regardless of backoff, so there was nothing for a per-entry backoff to usefully save). Removed it; kept the simpler P1 fix alone - retry every tick, evict only on an observed non-Pending state. - reservation_wiring.go's walletMembersResolver parameter was threaded through WireReservationWatchers but never read by anything downstream of the P1 fix that removed its only consumer; deleted the dead parameter from the function, its test, and the cmd/start.go call site (which used to feed it the WalletMembersResolver tbtc.Initialize returned), and corrected the call site's comment (the paired-flag validation is a warning, not a hard error, per the note above). Finished the resulting clean cutover: with cmd/start.go (Initialize's only caller anywhere in the repo) no longer reading it, tbtc.Initialize now returns only error, and the WalletMembersResolver interface plus node.ResolveWalletMembers (its one implementation) are deleted with it.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
…4282 Comment-only audit and fix pass across all comments this PR added or modified: drift against current code, self-containment (drop references to other repos / markdown docs that don't exist in keep-core), redundant text, and inconsistent voice. No behavioral change - gofmt/build/vet clean, diff confirmed comment-only against d1697f5.
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.
piotr-roslaniec
force-pushed
the
reservations-epic
branch
from
September 8, 2026 15:03
53cac0a to
9c0a612
Compare
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
…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.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
#4282 40 confirmed findings fixed (2 P0, 7 P1, 24 P2, 7 P3); 1 P2 finding (in-kind fee reserve watch) intentionally left unfixed - its only two remediations are building unscoped new functionality or editing a separate spec repo, both out of scope here. P0: - wire notifyReservationAcceptanceTimedOut end to end (vendored ABI, Go bindings, chain interfaces, ethereum adapter, watcher dispatch by ActionType) - acceptance-type timeout notifications could never succeed on-chain before this - gate deposit-sweep's reserved-deposit filter on the reservation vault address instead of calling isReservedDeposit unconditionally on every deposit pre-activation, which risked halting sweeping network-wide during the binary-upgrade-before-Bridge-upgrade window P1: - action-timeout watcher no longer requires wallet-member resolution to notify a Reanchor timeout, so a dead/closed wallet's stuck action is now permissionlessly resolvable - drop the load-failure eviction that permanently dropped a tracked action from timeout coverage after 3 RPC hiccups - remove the mainnet reservations-activation-block placeholder pending a real rollout height - correct GetReservation/GetReservationAction doc comments (absence is Unknown state, not an error) - propagate chain-read errors instead of masking them as no-op - memoize per-pass proof-invariant chain reads shared across proof-loop transactions - add the missing reservations-remaining below-dust regression subtest P2/P3: gauge visibility, nonce-bound preload validation, notifiedAt semantics, address-comparison case sensitivity, wallet-vault fee-floor check, dual-flag config validation, duplicate/stale comments and tests, and assorted simplifications. Also reverts an in-flight hard-fail config-validation check for the Tbtc/Spv reservations dual-flag pairing back to a warning: start's config categories never load the Maintainer section, so the check would have hard-failed startup for any legitimate split deployment running the SPV maintainer as a separate process. Self-verification follow-up (same review pass, iterated against external review of the fixes themselves): re-checked every "fixed" claim above against the actual diff and against the reasoning behind each fix, and made three further corrections: - deposit-sweep follower-side (ValidateDepositSweepProposal) never actually got the reservation-vault gate described above - only the leader side (deposit_sweep task) did, so a follower could still sign a sweep a leader had already correctly rejected. Implemented the matching gate on the follower path: fetch ReservationParameters with the same bounded retry as the leader (fewer attempts only makes a wrong "reservations aren't live" guess more likely, never less, so there is no safety argument for the follower retrying less than the leader), then hard-reject on any IsReservedDeposit error or a confirmed reservation for any deposit the cheap vault-match prefilter flags as a candidate - unlike the neighboring fee soft-check just below, which stays deliberately log-only because a merely underpriced sweep is not irreversible the way sweeping a reservation is. Added the regression test proving detection (TestValidateDepositSweepProposal_RejectsReservedDeposit) and its counterpart proving the fail-open path still sweeps normally when ReservationParameters is unavailable (TestValidateDepositSweepProposal_SweepsWhenReservationParametersUnavailable, mirroring the leader's existing test of the same shape). - the P1 loadFailures-eviction removal above initially grew a backoff mechanism to avoid re-logging every poll tick during a persistent failure, but the backoff suppressed the downstream timeout-notify check for the same tick it suppressed the retry - a single transient RPC error could have delayed a genuinely-overdue Bridge notification by up to 10 minutes, worse than the log spam it was meant to prevent (a real outage fails every tracked action identically regardless of backoff, so there was nothing for a per-entry backoff to usefully save). Removed it; kept the simpler P1 fix alone - retry every tick, evict only on an observed non-Pending state. - reservation_wiring.go's walletMembersResolver parameter was threaded through WireReservationWatchers but never read by anything downstream of the P1 fix that removed its only consumer; deleted the dead parameter from the function, its test, and the cmd/start.go call site (which used to feed it the WalletMembersResolver tbtc.Initialize returned), and corrected the call site's comment (the paired-flag validation is a warning, not a hard error, per the note above). Finished the resulting clean cutover: with cmd/start.go (Initialize's only caller anywhere in the repo) no longer reading it, tbtc.Initialize now returns only error, and the WalletMembersResolver interface plus node.ResolveWalletMembers (its one implementation) are deleted with it.
piotr-roslaniec
added a commit
that referenced
this pull request
Sep 8, 2026
…4282 Comment-only audit and fix pass across all comments this PR added or modified: drift against current code, self-containment (drop references to other repos / markdown docs that don't exist in keep-core), redundant text, and inconsistent voice. No behavioral change - gofmt/build/vet clean, diff confirmed comment-only against d1697f5.
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.
…t it The drift job compiled tbtc-v2 9f8f5ef1 while the harness validator was vendored from e635e229, whose validateReservationAnchorProposal logic differs under an identical ABI, so no check could notice. - Pin TBTC_V2_REF to eec999aa (merge of tbtc-v2#1161 into reservations-upgrade); the four reservation artifacts and the Go bindings still match it. - Re-vendor WalletProposalValidator.json from an eec999aa build (code unchanged apart from the metadata hash) and record that commit. - Add verify.sh and a job step comparing the vendored validator to the compiled one: ABI exactly, creation and deployed bytecode with solc's CBOR metadata removed, since the job's unfrozen yarn install can move the metadata hash. - regenerate.sh now requires TBTC_V2_BUILD_DIR instead of defaulting to a local sibling checkout.
… lookup The in-memory in-flight map did not suppress duplicate requests across rounds: rounds are far apart, the leader rotates, restarts clear it, and the adapter never reports a mempool transaction as pending. Safety comes from re-reading chain state, since Reservation.sol only accepts a request against an Active reservation. Remove the map and its helpers, GetReservationReanchorRequestReceipt with its receipt enum, and restore RequestReservationReanchor to return only an error. Run now dispatches on chain state alone: a mined request shows up as an ActionPending reservation and takes the resume path.
…alidator harness Run the real WalletProposalValidator bytecode at the boundaries only it can check: an action timing out exactly REQUEST_TIMEOUT_SAFETY_MARGIN from now is rejected and one second later is accepted (anchor and re-anchor), a MovingFunds wallet can anchor, a re-anchor target at its reservation cap is accepted and one above it is rejected, and an anchor fee above the action's max fee or naming a wallet other than the action's target is rejected. Times use the harness block timestamp so the boundaries are exact. Also state the real reason the simulated backend is avoided: github.com/fjl/memsize's linkname to runtime.stopTheWorld is rejected by the Go 1.23+ linker.
… behind the tip All three reservation loops now share one scan-range policy: the first scan starts at the reservation activation block (the action-timeout watcher no longer uses a 30-day lookback), networks without an activation entry skip the first scan, and every scan runs through fetchPastEventsInChunks and stops a few blocks behind the tip. The chunk helper hands each chunk to a callback so callers keep progress and advance their cursor chunk by chunk; a scan error no longer skips the proving or checking of already-tracked items, and the proof loop runs the re-anchor round even when the acceptance round fails. The stranding startup registration scan is chunked as well.
The LocalChain RequestReservationReanchor fake now enforces every requestReservationReanchor precondition in Solidity's order and with its revert reasons (cooldown, MovingFunds or Closing source, Live target distinct from the source, signing window, anchor floor, count cap where zero blocks all, amount cap), reserves the target's count and amount on request, and writes exactly the action fields Solidity writes. WalletReservationsCount and WalletReservationsAmount read a ledger made of the wallet's custodied reservations plus the capacity pending re-anchor requests reserved on it; new timeout and settlement helpers update it the way the contract does. Fixtures now set an action timeout and keep anchors above ReservationTxMaxFee + ReservationMinAmount, as a real request requires.
Reservation.sol accepts a permissionless re-anchor request from a MovingFunds or Closing source, and a wallet can reach Closing while it still holds reservations. Run only acted on MovingFunds wallets, so a re-anchor missed during MovingFunds, including the resume of an already authorized generation, stayed stuck and blocked wallet closing. Accept Closing on both paths; the below-dust notification stays MovingFunds only.
…producible metadata
## Context A cross-repo review compared tbtc-v2 #1116 and keep-core #4282 against the M1 reservations specification. It raised 60 candidates; two independent validation passes confirmed 48 and dropped 7. This PR closes the keep-core code findings on top of `reservations-epic`. The three P0s made the wallet-side flow nonfunctional or discarded contract-settleable proofs: - acceptance tried to validate and request an action before the depositor-created action existed; - re-anchor validated before requesting its on-chain action; - the SPV loop evicted timed-out generations that the contract still accepts. ## Changes ### Wallet coordination - Consume an existing depositor-created Pending Acceptance action at its real nonce and snapshots. Remove the invalid operator-side acceptance request. - Request re-anchor first, track the transaction by hash, then validate and propose against the mined Pending Reanchor action. - Resume pending re-anchors across rounds. Pending receipts suppress duplicate requests; reverted or dropped requests can retry; mined requests resume without another request. - Select re-anchor targets with count and amount headroom and preserve Solidity's zero-global-cap semantics. - Accept Live and MovingFunds source wallets and mirror the validator's 7,200-second request safety margin. - Tighten LocalChain validator fakes so tests enforce the same action-state, nonce, wallet-state, fee and timeout preconditions as Solidity. ### SPV maintainers and watchers - Keep TimedOut acceptance proofs through `timeoutAt + termSeconds`; keep TimedOut re-anchor proofs without an added Go-side expiry. - Match both P2WPKH and P2PKH outputs by public-key hash. - Start proof and watcher recovery scans at the configured reservation activation block, in bounded chunks. Unknown networks skip startup scans. - Derive stale-deposit deadlines from the reveal snapshot, defer all per-deposit chain reads until that deadline, and keep tracking until the on-chain pending-deposit record clears. - Match timeout equality with Solidity and count watcher goroutine deaths without panicking on a typed-nil metrics recorder. ### ABI, chain and observability - Preserve reservation action `termSeconds` and `minAmount` snapshots and the re-anchor miner fee in domain conversions. - Remove the unused generic timeout-event model that does not exist in Solidity. - Add ReservationVault bindings and publish in-kind fee-debt and TBTC reserve gauges. - Add an in-memory EVM harness around the real WalletProposalValidator and a stub Bridge. - Pin and extend CI ABI drift checks to Bridge, both reservation validator methods, ReservationRouter and ReservationVault. Mismatches now fail the job. ## Verification - `gofmt` on every changed/new Go source: clean. - `go build ./...`: pass. - `go vet ./cmd/... ./pkg/chain/ethereum ./pkg/clientinfo ./pkg/maintainer/spv ./pkg/tbtc ./pkg/tbtcpg/...`: pass. - `go test -count=1 ./cmd/... ./pkg/chain/ethereum ./pkg/clientinfo ./pkg/maintainer/spv ./pkg/tbtc ./pkg/tbtcpg/...`: pass. - `make -C pkg/chain/ethereum/tbtc/gen verify-vendored-fallback` against freshly compiled tbtc-v2 artifacts: all four ABI comparisons pass. - Full `go test -count=1 ./...` was attempted. Every changed package passed; the run failed only in unmodified `pkg/tecdsa/dkg` when `TestTssFinalize_IncomingMessageCorrupted_WrongPayload` hit its internal TSS round-one timing guard. Re-running that test produced nondeterministic outcomes across identical source and an identical 87-package dependency closure, including a pass at the exact base commit. This PR does not suppress or modify that test. Targeted regression tests were mutation-checked: each failed when its corresponding production branch was temporarily removed, then passed after exact restoration. Covered paths include late TimedOut proofs, action snapshots, P2PKH matching, activation-block recovery, re-anchor receipt states, the timeout safety margin, and typed-nil metrics. ## Scope notes - Base: `reservations-epic` at `f66f11240` (includes #4324). - This PR does not reconcile the epic branch with `dev`; #4282 remains conflicting until that separate integration step. - File-size debt is intentionally not refactored here. Most touched files already exceeded the repository's 500-line guideline at the base, and splitting them would mix a broad mechanical rewrite into correctness fixes. The largest new exception is `pkg/chain/ethereum/tbtc_validator_harness_test.go` at 981 lines; `pkg/maintainer/spv/reservation_proof_loop_test.go` grows from 1,668 to 2,168 lines. This exception was explicitly chosen for this PR and should be paid down separately. - The existing full-repository `go vet ./...` warning in `pkg/tecdsa/signing/protocol.go` is outside this diff; changed reservation packages are vetted separately. ## Related - Tracker: #4282 - Cross-repo tracker: threshold-network/tbtc-v2#1116
Estimate the fee and assemble the re-anchor transaction against the live ReservationTxMaxFee, which Solidity snapshots into the new action, before sending RequestReservationReanchor, so a fee spike no longer leaves an unusable request holding the target's capacity. Any failure after the request was sent now ends the pass instead of moving on to another reservation; only pre-submit capacity reverts retry the next target. The mined-wait stops as soon as the nonce advances to another requester's action, and the not-confirmed error now wraps its cause.
…ttleable generations A TimedOut acceptance generation is evicted once its reservation exists, since the Bridge then rejects every other acceptance proof. Each pass walks a reservation's generations Pending first, then TimedOut by descending nonce, and stops after the first submission attempt, so two generations matching the same Bitcoin transaction no longer broadcast a second, reverting proof. A TimedOut re-anchor generation is evicted when the reservation is no longer Active, ActionPending or Stranded, its reservation record is read once per pass and reused for proving, and a TimedOut generation with no matching or provable transaction is re-checked on a doubling backoff capped at 32 passes, ended early when the source wallet's transaction list changes.
…ld revert Run now skips an Active reservation while block time is before its reanchorCooldownUntil or when its anchor is not above ReservationTxMaxFee + ReservationMinAmount, the two request-time gates Reservation.sol applies to permissionless callers. Previously such reservations re-ran the target search and a reverting gas estimate on every window.
…ts pending entry pollTick always recorded the deadline from the reveal event at discovery, so the chain-read fallback that recovered it for an unmemoized deposit (with its own unchunked 30-day reveal scan and sentinel error) never ran in production. The deadline now lives in the tracked entry and is passed to CheckStaleReservedDeposit; the memo, the fallback helpers and their tests are removed.
Run searched for a target per reservation and per capacity retry, each time re-running the bounded registration scan, the unbounded fallback, and GetWallet plus capacity reads for every wallet, exactly when caps are saturated. A per-pass target search now reads registrations and wallet states at most once, lazily on the first reservation that needs a target, rules out count-capped wallets for the rest of the pass, remembers the smallest anchor that fits nowhere, and skips already-read wallets in the unbounded fallback.
… test The end-to-end typed-nil test returned once the forced panic fired, before the deferred recover had called recordReservationWatcherDeath. A no-op hook now runs after that call and the test waits on it.
…y margin Add a Run test where only the amount pre-check rules out a partly filled target, with the boundary where the target lands exactly on the cap and the zero-cap-is-unlimited case, each asserting which target received the single request. Pin the Go copy of the validator's 7200-second request timeout safety margin.
The copy of [ethereum] Network into the SPV maintainer config moves into maintainerConfig, so a test can pin it: without it the network is Unknown and every reservation catch-up scan is skipped.
Correct the EthereumNetwork, ethNetwork, lookback-constant, stale-deposit and anchor-hash comments: the reservation proof loop and the stale-deposit and action-timeout watchers all start their first scan at the activation block and skip it on networks without an entry, the 30-day lookback now bounds only the stranding registration scan, and the Go anchor hash differs from the Bridge's for a reservation without an anchor. Drop history wording from the touched comments.
The watcher-death metric test left its recovered goroutines reading reservationWatcherPanicRecovered while the typed-nil test replaced it, a data race under -race. Both tests now install the hook through one helper and wait for both recover paths before returning.
Candidates now come from the wallet's ReservationAcceptanceRequested events within the on-chain action timeout plus a margin, instead of DepositRevealed events from the last 30 days. Only generations the chain confirms are a Pending Acceptance for this wallet count toward the per-run budget, and that check runs before any reveal search or Bitcoin lookup, so never-requested reveals cannot starve real requests. The reveal is located by a chunked backward scan from the request block bounded by the generation's term, so requests made long after the reveal are still proposed. Run performs the per-window setup and gauge publication once, then tries candidates in timeout order; a fee-estimate or proposal failure skips that candidate instead of aborting the window. The pre-write error wrapper is removed. Tests delegate to the strict anchor validator, which now also checks the funding transaction hash and deposit locking script. Fixtures use real funding transactions, future refund locktimes, the 2-hour deposit minimum age and a 90-day term. Scenarios that only rejected for lack of a pending action are removed, the below-minimum scenario becomes the reachable boundary case, and a MovingFunds scenario is added.
# fix(reservations): address validated review findings on #4343 ## Context #4343 merged into `reservations-epic` before its multi-lens review was acted on. That review raised 55 findings: 45 were confirmed (listed as 42) and 10 were dropped. This PR fixes the confirmed ones on top of the merge commit `76aaecdd3`, whose code is identical to #4343's head. It also corrects several claims in the #4343 description that did not hold. They are listed under "Corrections to #4343" below. ## Changes ### Re-anchor (`pkg/tbtcpg/reservation_reanchor.go`) - Re-anchor out of **Closing** source wallets, on both the request and the resume path. Solidity allows MovingFunds and Closing sources. Before, a wallet that entered Closing while it still held reservations could never finish closing. The below-dust notification stays MovingFunds-only. - **At most one on-chain request per pass.** The fee is estimated and the transaction assembled against the live `reservationTxMaxFee` before the request is sent. Any failure after a request is sent ends the pass. The mined-wait stops early if another requester's action takes the nonce. - **Removed the in-memory in-flight map,** the `GetReservationReanchorRequestReceipt` method and its receipt enum. `RequestReservationReanchor` returns `error` again. Every round decides from re-read chain state. A duplicate request after a restart or a leader change can still revert on-chain, which costs gas but is safe. - Skip reservations in re-anchor **cooldown** (`tbtc.Reservation.ReanchorCooldownUntil` is now converted), and skip reservations whose anchor is at or below `txMaxFee + minAmount`. Both cases revert on-chain. - The target search runs **once per pass**. Wallets that are full on count are excluded for the rest of the pass, and the smallest anchor that fits nowhere is remembered. - `TestReservationRequestTimeoutSafetyMarginMatchesValidator` pins the Go copy of the 7,200-second margin. The EVM harness pins the on-chain value. ### Acceptance (`pkg/tbtcpg/reservation_acceptance.go`, `pkg/tbtc`) - Candidates now come from `ReservationAcceptanceRequested` events for the wallet. The scan covers the on-chain action timeout plus one day, in 10,000-block chunks. - Only confirmed Pending Acceptances targeting the wallet count toward the 50-candidate budget, and that check runs before any Electrum call. Before, never-requested or fake reserved reveals could use up the budget and starve real requests. - Requests made more than 30 days after the reveal are now proposed and signed: - The proposer scans backward from the request block, bounded by the snapshotted term plus 7 days. - The signer estimates the reveal block from `RevealedAt` and widens the window once if needed. - Both use the new chunked helper `tbtc.FindDepositRevealedEvent`. - A fee-estimate or proposal error now skips that candidate instead of aborting the window. - Vault fee gauges are published once per Run and reuse the vault address that was already read. The gauge is renamed to `reservation_vault_fee_reserve_tbtc_base_units`, because its value is in 1e18 base units. ### SPV maintainers (`pkg/maintainer/spv`) - The proof loop, the stale-deposit watcher and the action-timeout watcher share one first-scan rule: - Start at the reservation activation block. - Skip the startup scan on networks without an activation entry. - Scan in 10,000-block chunks. - The stranding startup scan is chunked. It keeps its 30-day start, because wallets registered before activation can hold reservations, and it no longer falls back to scanning from genesis. - Progress is saved per chunk, so a failed chunk keeps the earlier work. A scan error no longer skips proving or checking items already tracked. Scan cursors stay 12 blocks behind the tip to cover shallow reorgs. - A TimedOut acceptance generation is evicted once its reservation is no longer Unknown. Before, it was retried for up to `termSeconds` after a newer generation settled. - Each reservation gets at most one proof per pass: Pending first, then TimedOut by descending nonce. This applies to both acceptance and re-anchor. - TimedOut re-anchor generations whose reservation cannot settle are evicted. Stranded ones are kept, because late settlement is allowed. Generations with nothing to prove back off up to 32 passes. The reservation is read once per generation per pass. - Stale-deposit deadlines are stored in the tracked entry. The unused `snapshotRefundDeadline` fallback is removed. - `cmd/maintainer.go` copies the Ethereum network into the SPV config through a tested helper. ### CI, harness and tooling - `TBTC_V2_REF` is pinned to tbtc-v2 `eec999aa`, the merge of tbtc-v2#1161 into `reservations-upgrade`. The harness `WalletProposalValidator.json` was re-vendored from that same commit. The bindings and the four ABI fallbacks are unchanged and verified against it. - New CI check: the harness validator must match the compiled one. The ABI is compared exactly, and the creation and deployed bytecode are compared with the CBOR metadata stripped. This check fails on the old `9f8f5ef1` validator, which has the same ABI but different logic. - New harness cases against the real bytecode: - timeout margin at +7200 (rejected) and +7201 (accepted); - an anchor from a MovingFunds wallet; - re-anchor target count at the cap and at cap + 1; - an anchor fee above the maximum; - an anchor naming the wrong wallet. - `verify-vendored-fallback` is now a single macro. A `checkabi` failure is reported as a tool failure, not as ABI drift. `checkabi` no longer double-spaces an `internalType` that already contains a space. - `regenerate.sh` requires `TBTC_V2_BUILD_DIR` and compiles by relative path, so regenerating is reproducible across machines. ### Tests and fakes - The LocalChain `RequestReservationReanchor` fake now mirrors `Reservation.sol` (source and target state, cooldown, anchor floor, caps where zero blocks all, reserved target capacity). It writes only the action fields Solidity writes. - All acceptance Run tests now go through the strict validator fake. That fake now also checks the funding tx hash and the deposit script. Fixtures the contracts would reject were fixed. Misleading "cap" scenarios were removed. - The SPV fakes return zero records for absent keys, as the real mappings do, and honour `EndBlock`. - The conversion tests now cover `TermSeconds`, `MinAmount` and `ReanchorCooldownUntil` with non-zero values. - Each new regression test was checked by reverting its production line, confirming the test fails, and restoring the line. ### Docs - Removed the duplicated `tbtc.go` header. - Completed the lists of fields the conversions deliberately skip. - Documented that `TermSeconds` and `MinAmount` are set only for acceptance generations. - Rewrote comments that described removed behaviour or cited internal review IDs. ## Behaviour change to note Only Developer has a reservation activation entry today. On Mainnet, Sepolia and other networks, none of the three loops scans history at startup; they process new blocks only. For the action-timeout watcher this replaces the previous 30-day lookback. It matches the other loops, and reservations are not enabled on those networks until an entry is added. When an entry is added, restart recovery covers everything since activation. ## Corrections to #4343 - "Pending receipts suppress duplicate requests": the adapter never returned Pending, and entries expired after 6 blocks while rounds are 900 blocks apart. The mechanism is removed; see Re-anchor. - "Accept Live and MovingFunds source wallets": this was true only for the acceptance wallet. Re-anchor sources were MovingFunds only; Closing is now also accepted. - "Watcher recovery scans at the activation block, in bounded chunks; unknown networks skip startup scans": the action-timeout watcher did neither. It does now. - "Preserve the re-anchor miner fee in domain conversions": no conversion existed. The unused `MinerFee` field is removed. - "Tightened LocalChain validator fakes": most acceptance Run tests bypassed them. They no longer do. - "Action snapshots" mutation-checked: the conversion test compared zero with zero. It now uses non-zero values. - The undeclared dependency on tbtc-v2#1161 is resolved by the new pin. ## Not changed - Re-anchor requests stay inside leader proposal generation. Moving them into a maintainer loop was raised as a follow-up design option, not a defect. - The test capacity ledger does not model capacity held by pending acceptances. - `StubBridge.json` is not checked in CI; it is a test stub, not an upstream contract. ## Verification - `gofmt` on every changed Go file: clean. - `go build ./...`: pass. - `go vet` on `./pkg/tbtcpg/... ./pkg/tbtc/... ./pkg/chain/ethereum/... ./pkg/maintainer/... ./cmd/... ./pkg/clientinfo/...`: pass. - `go test -count=1` on the same packages: pass. - `go test -race` on `pkg/maintainer/spv` and `pkg/tbtcpg`: pass. - The drift check and the harness check, run with `bash -e` against a local `eec999aa` build: pass. ## Related - Follow-up to #4343. Tracker: #4282. - tbtc-v2#1161 (merged into `reservations-upgrade` as `eec999aa`).
This branch has not been deployed
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.Cross-repo M1 review (2026-09-28)
A review of this tracker and tbtc-v2 #1116 against the M1 specification raised 60 candidates; two validation passes confirmed 48 (3 P0, 6 P1, 27 P2, 12 P3) and dropped 7. Remediation is split across:
docs/reservations-spec@3c5603fa5- reconciled M1 requirements, architecture, readiness and delivery status.Both fix PRs must land on the epic branches before #4282/#1116 are reconciled with
dev. They do not perform that integration step.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- superseded by the acceptedreservations-epic/ Reservations epic -> dev tracking #4282 implementation line; do not reconcile or merge this earlier parallel attempt.Cumulative diff note
The epic also carries shared
pkg/net/localrelease/handler changes inherited from its merge stack, including #4284's release-boundary drain fix. They are intentional network/test-stability work, not reservation protocol logic.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
The #4274 stack is complete. Cross-repo review remediation is open in #4343 and tbtc-v2 #1161 and must land before promotion. #4238 is superseded, not a remaining merge decision. This tracker is blocked on reconciling
reservations-epicwithdev(128 ahead / 187 behind, currentlyCONFLICTING) and then re-running the full CI suite.