Skip to content

[Draft] refactor(tx-pool): unify transaction lifecycle under sharded authority - #5316

Draft
zhangsoledad wants to merge 3 commits into
developfrom
zhangsoledad/txpool-true-shard-authority
Draft

[Draft] refactor(tx-pool): unify transaction lifecycle under sharded authority#5316
zhangsoledad wants to merge 3 commits into
developfrom
zhangsoledad/txpool-true-shard-authority

Conversation

@zhangsoledad

@zhangsoledad zhangsoledad commented Sep 2, 2026

Copy link
Copy Markdown
Member

[!IMPORTANT]

Draft for early architecture review.

This PR is intentionally opened before final acceptance so maintainers can
still influence the ownership model, atomicity boundary, compatibility
contract, and review decomposition. It is not requesting final merge,
performance, security, or Product Acceptance sign-off.

Start here

  • Why: on develop, transaction lifecycle facts live in several mutable
    subsystems. Individual bugs can be patched, but isolated patches cannot prove
    the cross-route ownership and atomicity invariant.
  • What: the design intends one TxPoolAuthority to own every retained
    transaction. Mutation is split into non-mutating validation and planning,
    one total exact-shard Apply, and effects after authority release.
  • Why sharding: semantically independent ordinary commits can overlap on
    disjoint physical support; genuinely coupled facts are ordered only at their
    smallest authoritative cut. There is no ordinary global/outer serial
    fallback.
  • What feedback is requested now: architecture, invariants, compatibility,
    and review boundaries—not final acceptance.

For a first pass, read ARCHITECTURE.md
sections 1–4, state.rs, and
shard.rs. The three initial questions are:

  1. Is the proposed lifecycle owner actually unique?
  2. Is the ordinary/rare barrier distinction legitimate?
  3. Does semantic support—not shard identity alone—decide concurrency?

Why this needs an architectural change

CKB tx-pool ownership interacts with dependency resolution, duplicate and
conflict identity, RBF closure, Pending/Gap/Proposed state, resource and worker
accounting, relay effects, reorg, template publication, persistence, and
shutdown.

On develop@17d7db5bb423a1b2177e14a132a41d5a91a515f3, retained lifecycle
state is distributed across VerifyQueue, OrphanPool, TxPool / PoolMap,
workers, and asynchronous follow-up paths. Those locations have different
locks and transition rules.

flowchart LR
    I[Ingress] --> V[VerifyQueue]
    V --> W[Resolve / verify workers]
    W --> O[OrphanPool]
    W --> P[TxPool / PoolMap]
    R[RBF / reorg / clear] --> O
    R --> P
    V --> Q{One atomic owner + graph + charge + effect cut?}
    O --> Q
    P --> Q
    Q --> N[Not established by isolated local patches]
Loading

A concrete witness is PoolMap::check_and_record_ancestors in the frozen
develop baseline. Its FIXME records a path where an RBF candidate can remove
conflicts before a later ancestor-limit failure, requiring rollback. The
retained regression
uak_develop_stale_parent_eviction_shape_is_total_rejection_without_mutation
requires that legal shape to reject without partial mutation.

Prechecking that condition can fix that witness. It does not prove the same
property for every duplicate, RBF, dependency, capacity, reorg, cancellation,
and effect exit. The precise claim of this PR is therefore:

Individual develop defects are patchable. Finding-shaped local patches do
not close the cross-route invariant family while lifecycle authority remains
fragmented. A complete fix needs an equivalent single-owner, atomic-transition
redesign.

Proposed design constraints

TxPoolAuthority.entries, keyed by full transaction hash, is intended to be
the sole transaction-lifecycle owner. Scheduler rows, dependency relations,
indexes, resource totals, queues, and effect records should remain projections
or move-owned capabilities—not secondary transaction owners.

flowchart LR
    I[Bounded input] --> V[Validate<br/>no mutation]
    V --> C[Resolve / verify<br/>outside authority]
    C --> P[Plan<br/>one coherent cut]
    P --> R[Preflight<br/>allocation + capacity + OCC]
    R -->|stale / reject / pressure| T[Typed outcome<br/>no owner mutation]
    R --> A[Apply<br/>smallest exact shard cut]
    A --> X[Release authority<br/>move-owned receipt]
    X --> E[Relay / callback / reply / I/O]

    classDef commit fill:#dbeafe,stroke:#1d4ed8,stroke-width:2px;
    class A commit;
Loading

The transition rule reviewers should verify is: fallible policy and allocation
work finishes before mutation; Apply revalidates freshness and commits owner,
projection, accounting, clocks, and required effects atomically; external I/O
runs afterward and cannot veto the commit.

Why true sharding matters

The current implementation uses 64 routed authority shards. The intended rule
is that ordinary mutations acquire the exact canonically ordered support
derived from their typed delta. Physical separation alone does not imply
semantic independence.

flowchart TB
    A[Transaction A] --> SA[Exact support A]
    B[Transaction B] --> SB[Exact support B]
    SA --> AA[Apply cut A]
    SB --> AB[Apply cut B]
    AA --> O[May overlap when supports commute]
    AB --> O

    C[Coupled transaction C] --> U[Union of shared authoritative facts]
    D[Coupled transaction D] --> U
    U --> M[One minimum canonical cut]

    G[Chain / generation / close] --> X[Rare named lifecycle barrier]
Loading

The architectural value is the combination, not the number 64 or a specific
Rust wrapper:

  1. one lifecycle owner per transaction;
  2. one coherent Plan and one total Apply;
  3. atomic owner, dependency, projection, resource, clock, and effect changes;
  4. real overlap for semantically independent ordinary Apply cuts;
  5. minimum explicit cuts for coupled facts;
  6. bounded work and move-owned capabilities;
  7. rare global barriers that cannot become ordinary fallbacks.

The number of shards and current file layout are implementation choices.
Alternative designs are welcome if they establish the same cross-route
ownership, atomicity, bounded-work, and disjoint-progress properties.

Current status and non-claims

Area Status
develop reconciliation and history compression Complete
Ordinary outer-write fallback census 0 production routes
Terminal correctness review Open: 8 clusters / 11 candidates
Current correctness focus Held shard-cut and policy-read coherence
Static invariant qualification Not started
Controlled performance comparison with develop Not started
Final security, cold-reviewer, and Product Acceptance Not started

The committed control head is
0e0678282db0defd4b21bdea35279ab26d0b88d2; the
frozen production subject is 51d282345d1d83119c46cdde8f1115f14561b4ac with
tree 1e19719c764c7349a178d7ac0b7bf4999542966f.

Historical observation, not current-head evidence: make integration at
archive commit 0d553de760aa71a3b42dbc8f81f27ddcc775be4c stopped at
66 passed / 1 failed / 110 not started; RbfConcurrency became unreachable
before its first tx_pool_info observation. The relevant production
implementation was later transferred unchanged to 51d282345, but the full
suite has not been rerun on that identity. Standalone and four-spec replays
passed, so the root cause remains open.

This PR does not currently claim global terminal correctness, measured
performance leadership, final trusted-code-surface minimality, final security
completion, merge readiness, or that develop is literally impossible to fix.

How to review this PR

Please review the three intent-separated commits instead of starting from the
unified diff:

  1. 51d282345 — atomic true-shard architecture implementation;
  2. 33c7b1b26 — post-merge terminal-audit checkpoint and evidence;
  3. 0e0678282 — repository-owned Primary control, stable agent instructions,
    and live-state binding.

Deeper correctness review

Start from the TP-OWN-001, TP-COMMIT-001, TP-POOL-001, and
TP-EFFECT-001 rows in
REVIEW_GUIDE.md, then inspect:

Ask:

  • Can an ordinary failure occur after irreversible owner mutation starts?
  • Does every owner have exactly one matching charge and projection?
  • Are stale and cancelled capabilities returned exactly once?
  • Are required effects committed with the owner transition and published only
    after release?
  • Do public observations bind the same authority cut?
Useful focused witnesses
  • uak_duplicate_and_promotion_never_create_second_owner
  • uak_dropped_prepared_apply_is_semantically_mutation_free
  • uak_rbf_replaces_the_complete_descendant_closure_atomically
  • uak_terminal_outcome_and_effect_commit_together
  • uak_two_compatible_direct_candidates_overlap_inside_their_owner_cuts
  • uak_disjoint_accepted_local_removals_overlap_inside_the_real_runtime_cut
  • runtime_disjoint_malformed_peer_revocations_overlap_under_outer_read
Deep dive: Ready, dependency, and RBF

Read scheduler.rs, dependency.rs, plan/membership.rs, and
plan/membership/rbf.rs. Start from TP-RBF-001, TP-DEP-001, and the
matching entries in FINDINGS_LEDGER.json.

Deep dive: chain, template, persistence, and shutdown

Read plan/chain_transition.rs, template_driver.rs, read.rs, topology.rs,
and service.rs. Check that lifecycle fences remain explicit and that rare
barriers do not spread into ordinary mutation.

Compatibility decisions

The compatibility intent is to preserve CKB consensus, CKB-VM script
semantics, hardfork selection, and wire transaction semantics. The public Rust
API change is an intentional major-version migration. Persistence v2 accepts
legacy v1 input; legacy configuration is translated through validated
defaults. No compatibility facade may recreate mutable transaction or policy
authority.

Why the PR is large—and how it will get easier to review

The ownership replacement necessarily crosses ingress, validation,
dependencies, membership, resources, effects, reads, templates, reorg, and
shutdown. A half-migrated state would retain a second semantic engine or a
serial fallback—the two outcomes this design is intended to remove.

Diff inventory against the frozen develop base

The committed branch currently changes 343 files (+161,757 / -9,946). The
review slices below are mechanically classified by path:

Slice Files Additions Deletions
Production, build, and integration surfaces 126 79,947 7,193
Tests, models, benchmarks, and audit evidence 175 69,667 2,750
Documentation, control, and checker tooling 42 12,143 3

The first commit carries the architecture. The next two commits are primarily
audit, review, and repository-control material. This table explains the size;
it does not claim that the remaining production surface is already minimal.

The current size is not proposed as the final maintenance cost. Before moving
out of Draft, I will:

  • reproduce or refute every blocking candidate with source-bound evidence;
  • repair semantic classes rather than individual symptoms;
  • delete superseded routes and evidence in the same slice;
  • reduce or justify every broad type, lock, task, channel, cache, and adapter;
  • report the production trusted-code surface separately from tests and audit
    artifacts;
  • bind architecture, review guide, checker, evidence, and final source identity;
  • publish focused and aggregate gates without hiding failures;
  • measure against develop only after correctness and static qualification.

Feedback requested now

Please comment early on:

  • the one-owner and linearization model;
  • exact ordinary shard cuts versus rare lifecycle barriers;
  • shared facts that appear too broad or too narrow;
  • missing compatibility or operational contracts;
  • additional counterexamples or proof obligations;
  • the subsystem that should receive the next focused walkthrough.

The goal of this Draft is not to make reviewers accept a permanently enormous
cognitive surface. It is to let design feedback shape the architecture while
that surface is still being reduced.

@zhangsoledad
zhangsoledad force-pushed the zhangsoledad/txpool-true-shard-authority branch from a254415 to ad355d6 Compare September 2, 2026 12:12
@zhangsoledad zhangsoledad changed the title [Draft] refactor(tx-pool): introduce a true-sharded single-authority kernel [Draft] refactor(tx-pool): unify transaction lifecycle under sharded authority Sep 2, 2026
@chainTe

chainTe commented Sep 3, 2026

Copy link
Copy Markdown

Security review completed against 17d7db5bb423a1b2177e14a132a41d5a91a515f3..ad355d65afe2eca471587a744004be98da89aee2. I found four new issues: one high-severity transaction-pool authority deadlock, two medium-severity freshness races in chain reconciliation and dependency settlement, and one low-severity stale-snapshot race in clear_tx_pool.

The blocking themes are: avoid routed lock acquisition while an all-shard read cut is live; fence or version all causal sources across chain receipt capture and apply; distinguish reserved ApplySequence values from a committed-prefix watermark; and make clear-current-pool use execution-time snapshot state or reject stale intent.

cargo fmt --all -- --check and the tx-pool security-manifest checker pass. A targeted lock-fairness test could not compile in the review environment because Clang could not locate stdbool.h; CI should run the full test suite and deterministic concurrency canaries should be added for each race before merge.

@zhangsoledad

Copy link
Copy Markdown
Member Author

Security review completed against 17d7db5bb423a1b2177e14a132a41d5a91a515f3..ad355d65afe2eca471587a744004be98da89aee2. I found four new issues: one high-severity transaction-pool authority deadlock, two medium-severity freshness races in chain reconciliation and dependency settlement, and one low-severity stale-snapshot race in clear_tx_pool.

The blocking themes are: avoid routed lock acquisition while an all-shard read cut is live; fence or version all causal sources across chain receipt capture and apply; distinguish reserved ApplySequence values from a committed-prefix watermark; and make clear-current-pool use execution-time snapshot state or reject stale intent.

cargo fmt --all -- --check and the tx-pool security-manifest checker pass. A targeted lock-fairness test could not compile in the review environment because Clang could not locate stdbool.h; CI should run the full test suite and deterministic concurrency canaries should be added for each race before merge.

This is a known open issue: C2 is tracked in HELD_SHARD_CUT_AND_POLICY_READ_COHERENCE, one of eight open terminal-correctness clusters. We are first minimizing the architecture and retiring the composite/alternate routes so the lock graph and correctness TCB are smaller. We will then reopen C2 against the frozen simplified source and close it root-wise with deterministic production-bound, no-sleep interleaving evidence and a lock-order DAG. Until then, C2 and terminal correctness remain explicitly open; we will not apply a finding-specific patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants