Reservations epic -> dev tracking - #4282
Draft
piotr-roslaniec wants to merge 100 commits into
Draft
piotr-roslaniec wants to merge 100 commits into
piotr-roslaniec wants to merge 100 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.
ReservationAnchorProposal/ReservationReanchorProposal's Marshal panicked on a nil *big.Int field (3 fields across the two types), and the two Unmarshal implementations disagreed on zero-hash rejection: re-anchor rejected an all-zero TargetWalletPublicKeyHash but anchor accepted an all-zero DepositFundingTxHash. Both proposal types were also the only wire-format methods in the file missing the doc-comment convention every sibling type follows.
ReservationsActivationBlock was a single compile-time constant (26,500,000) with no per-network override, unlike DepositSweepEveryWindowActivationBlock's graceful testnet degrade - this gate's degrade path was 'feature entirely absent' on every lower-tip chain for years. Threaded ethereumNetwork through node.NewNode (mirroring the existing groupParameters/chain plumbing) down to the activation-block lookup, and copied DepositSweepEveryWindowActivationBlock's upgrade-precondition warning onto this gate's comment: a genuinely-old binary's coordination messages fail to unmarshal, causing followers to spuriously fault an honest, upgraded leader for the entire rollout window. Also wires the reservation metrics recorder into the proposal generator alongside the existing redemption metrics wiring, so cached coordination executors created before metrics are set still pick up the reservation gauges (see pkg/clientinfo saturation gauges commit).
The maintainer already warned when its reservation flag was on but the client's was off; the client emitted no mirrored warning for the reverse case, where the client originates reservation actions but the maintainer never proves them on-chain, guaranteeing action timeouts. Emit the paired warning naming the maintainer flag it depends on.
Free-slot and occupancy monitors were named as B-specific operational duties and leading indicators of the reservation saturation cliff, but only per-action execution counters existed - nobody could see reservation capacity approaching its cap before acceptances silently stopped. Registers four gauges (active_reservations_count, max_active_reservations, live_wallets_count, wallet_reservations_count) gated on the same reservationsEnabled flag as the existing wallet action counters, sourced from chain calls the acceptance/re-anchor tasks already make. Wires a metrics recorder into both tasks via the proposal generator, mirroring the existing redemption metrics wiring.
…ut memoization The reservation proof loop's retry eviction (after 3 failed GetReservationAction passes) deleted the pending event without rewinding the scan cursor, which had already advanced past the event's block - a transient RPC outage permanently stranded an already-confirmed Bitcoin anchor/re-anchor transaction's SPV proof. Eviction now rewinds the cursor behind the lost event's block so the next pass re-fetches and rediscovers it, applied symmetrically to both the acceptance and re-anchor paths. The stale-deposit watcher's deriveTimeoutFromReveal re-ran a full 216,000-block eth_getLogs scan every poll tick for every deposit with no requested action yet, with no memoization of the derived, immutable timeout. Added a per-deposit-key memoization map so subsequent ticks skip straight to the timeout comparison. Also completes the reanchor-proof submission counters (2 of 10 return paths in submitReservationActionProof were still uncounted) and adds TestIsReservedDeposit_PointerIdentity, pinning the fix for this PR's own historical pointer-identity map-key bug. Several P2/P3 findings in this cluster (M-7 nonce-aware timeout check, WalletMembersResolver architecture, dead-code cluster removal, tautological guard removal, unused alias, startStaleDepositPoll testability) remain deferred - see spv-cluster-followups.md. A first attempt combining these with the structural M-16 redesign broke 3 existing tests and was reverted; only the independently-safe P1 subset above is included here.
…ce reconciliation, caps Reservation acceptance: - findReservationAcceptanceCandidate ran an unbounded eth_getLogs scan from genesis per candidate deposit per coordination window - errors on RPC providers that cap eth_getLogs ranges (silently skipping the candidate forever), and made the RequestNonce+1 retry branch dead code. Bounded the scan to ReservationAcceptanceLookBackBlocks, matching every other event scan in the diff. - Added a vault-not-configured guard against the actual zero-address hex string the chain.Address converter produces (the prior check compared against "", which the converter never returns). - RequestNonce is predicted client-side before the on-chain write and was never reconciled by re-reading the reservation afterward; a follower would hard-fail GetReservationAction forever if the Bridge assigned a different nonce. Added hasPendingAction, a re-read-and- compare guard against duplicate in-flight requests, shared by both the acceptance and re-anchor paths. - maxActiveReservations==0 (the global circuit-breaker cap) was treated as unlimited instead of failing closed; added boundary tests at the exact ==cap/==cap+1 edge for all three amount-based caps. - Fixed the typed-nil interface return on the below-minimum path. Reservation re-anchor (M-27, resolved via tbtc-v2 source): the below-dust trigger requested a re-anchor from the client's ordinary operator key even though tbtc-v2's on-chain requestReservationReanchor caps that path to privileged callers, making the trigger dead code as shipped. Removed the privileged-caller trigger; wired NotifyMovingFundsBelowDust (permissionless, already present in the Bridge ABI but never called from this client) so a wallet whose reservations have already fully drained closes once its last reservation's own re-anchor completes and its main UTXO computes below the moving-funds dust threshold. Added a pre-check (AssembleReservationReanchorTransaction, build-and-discard) before requesting re-anchor, mirroring the acceptance path's existing pre-check. Moved hasPendingAction to reservation_acceptance.go, its only real caller, and corrected its doc comment. Extends the shared Chain interface (NotifyMovingFundsBelowDust) and its LocalChain/TbtcChain implementations to support the above.
…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.
…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.
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.
…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.
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.
#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.
…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.
PR #4280 is merged and explicitly deferred these two tests (per its own description) - it did not land the missing go-ethereum simulated-backend infrastructure. The prior wording ('blocked on that infra landing. See PR #4280') read as if #4280 was the pending vehicle for that infra, which is now misleading since #4280 shipped without it. Point instead at the real state: the infra is unbuilt and has no owning PR.
piotr-roslaniec
force-pushed
the
reservations-epic
branch
from
September 8, 2026 15:40
9c0a612 to
4390a79
Compare
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.