Make RGB acceptance transactional and crash-safe - #80
Conversation
e26a5a3 to
6768dec
Compare
dea7b29 to
d02dcdd
Compare
dcorral
left a comment
There was a problem hiding this comment.
Transactional acceptance core is right and well-tested. Two separable pieces should come out so this PR is only the crash-safety fix, plus one minor. Details inline.
| /// <div class="warning">This method is meant for protocol integrations that provide their own | ||
| /// durable commit decision.</div> | ||
| #[cfg(any(feature = "electrum", feature = "esplora"))] | ||
| pub fn color_psbt_and_consume_for_operation( |
There was a problem hiding this comment.
Dead. Nothing calls it, not even the tests here. Sender colors through color_psbt_and_consume and stages through prepare_consume_fascia. Drop it.
There was a problem hiding this comment.
Retained after revalidating the final dependency stack. color_psbt_and_consume_for_operation is called by rust-lightning #32 at lightning/src/rgb_utils/mod.rs:667 while the promoted acceptance journal owns the stock. Removing it would either break #32 or require an unsafe ordinary-wallet access path, so the final #80 description records this required downstream caller.
| } | ||
|
|
||
| #[cfg(any(feature = "electrum", feature = "esplora"))] | ||
| pub(crate) fn prefetch_consignment_witnesses<const TRANSFER: bool>( |
There was a problem hiding this comment.
Split this out: prefetch_consignment_witnesses + OperationResolver and its whole apparatus (high_history_perf.rs, perf-fixture/*, signet-profile-proxy.mjs, the Dockerfiles, compose.yaml, regtest.sh, the workflow, chain.rs). ~1.1k lines of validation throughput, not acceptance. The staged path is correct with the plain OffchainResolver, so nothing here needs it. Own PR, own perf review.
There was a problem hiding this comment.
Addressed at head 22c7673. OperationResolver, witness prefetching, performance fixtures, proxy tooling, Docker harnesses, and performance workflows were removed. #80 now contains only transactional acceptance and the exact operation-owner APIs required by the downstream funding boundary.
| } | ||
|
|
||
| #[cfg(any(feature = "electrum", feature = "esplora"))] | ||
| fn write_all_checkpointed( |
There was a problem hiding this comment.
Minor, non-blocking: this chunks every write into 4096-byte file.write() calls in release too, only for test crash granularity. Gate the chunking behind the test cfg and write_all in prod.
There was a problem hiding this comment.
Addressed at head 22c7673. Production persistence uses write_all; 4 KiB chunking is compiled only under cfg(test) for deterministic crash injection.
0d310c4 to
2aeb3d4
Compare
2aeb3d4 to
22c7673
Compare
|
Final current-head update:
The required order remains #80, #32 repinned to the official #80 merge commit, #139 repinned to the official #32 merge commit, then #140 rebased onto merged #139. Could you re-review the current head? |
Summary
Make protocol-owned RGB acceptance transactional and recoverable across process crashes, while keeping witness-throughput work out of this PR.
Current head:
22c76737894db67caa2b0743e4c258ba8c2422f0. The PR is one commit on currentdev(62a8c3a), changing 17 files.Safety model
A protocol operation prepares and validates an isolated stock snapshot, syncs staged files, records the journal, and only then promotes the snapshot. Until the embedding protocol records a deterministic commit decision, ordinary RGB mutation is rejected with the owning operation ID.
Recovery is explicit and idempotent:
finalizekeeps promoted state and removes recovery evidence only after durable completionrollbackrestores the exact pre-operation snapshot and removes the journal only after the restored state is durableProduction persistence uses
write_all. The 4 KiB write granularity exists only undercfg(test)for deterministic kill injection.Review scope
The reviewer-requested throughput split is complete.
OperationResolver, witness prefetch, high-history fixtures, Signet proxy tooling, performance workflows, and related Docker harness changes are not in this PR.color_psbt_and_consume_for_operationis intentionally retained because the final rust-lightning #32 receiver boundary calls it while the promoted acceptance journal owns the stock. Removing it would either break #32 or force an unsafe ordinary-wallet access path. The exact-owner witness API is covered by the real send-operation integration test, including ordinary-access rejection and wrong-owner rejection.Witness-resolution throughput remains separate work in rgb-ops #18 and rgb-ops #20.
Validation
Dependency and merge order
Release gates
This PR remains draft until those gates are complete.