Repository navigation
fix: hold cells across funding and the watchtower - #1707
LusoCryptoLabs wants to merge 3 commits into
Conversation
The watchtower picked fee cells without the cells reserved by pending funding txs, and funding without the cells of the watchtower's txs in the pool. The indexer lists a cell as live until its spending block, so either could pick a cell the other had spent. Keep both in one record of held cells and lock them in each collector.
The watchtower's hold and its test only exist with the watchtower feature; the wasm build failed clippy on them as dead code.
|
CI: the one failing test, |
Security ReviewResult: 2 confirmed Medium findings in the held-cell lifecycle. Validation:
Notes:
|
|
Thanks for the review. Both hold, as I read the code.
If you can share the evidence and the regression cases you mentioned, I'll build the tests from them. Otherwise I'll write one test per finding, each failing on the current branch first. |
The watchtower held a tx's cells only after sending it, and an opening published its inputs only after the build, so either could pick a cell the other had just picked. Both now pick through TakingCollector, which holds a cell in the same step that checks it is free, and the watchtower lets its cells go when a send fails. The holds live in memory. Each picked cell is also checked with get_live_cell and include_tx_pool, so a cell a pending tx spends is skipped after a restart too.
|
Pushed 28456f7 for both findings.
Tests in Locally CI is red here for a reason outside this PR: If your evidence covers a case these tests miss, I'll add it. |
The watchtower picks the fee cells of its revocation and settlement txs with its own
DefaultCellCollector(run_periodic_check). It does not see the cells that pending funding txs reserve (LiveCellsExclusionMap, inCkbChainActor), and funding does not see the cells of the watchtower's txs still in the pool. The indexer lists a cell as live until the block that spends it, so either can pick a cell the other already spent.Seen on our testnet LSP on 2026-10-28 (a fork with #1623; the code in question is the same on develop): a settlement tx in the pool spent a cell, a tenant's funding tx built nine seconds later put the same cell in, and the funding failed with
Resolve failed Unknown(OutPoint ...)once the settlement was committed.ckb::held_cells: one record of held cells per process. EachCkbChainActorpublishes its funding inputs there after every change to its exclusion map. The watchtower holds the inputs of each revocation and settlement it sends, for 30 minutes.A process-wide record was the smallest change that reaches the watchtower's thread. If you would rather have the watchtower ask
CkbChainActorfor the exclusions, or another shape, we can change it.We run this on our testnet LSP since 2026-10-29.
Checked on this branch:
ckb::held_cells: an opening's inputs stay locked until it lets them go, and other holders' stay; the watchtower's are locked for their hold and free after itcargo fmt,typos,cargo shear,cargo clippy -p fnn -p fiber-bin --all-targets --all-features -- -D warningscargo test -p fnn --features rocksdb: all pass exceptfiber::tests::mpp::test_mpp_waiting_ack_settlement_hands_off_to_durable_retry_queue, which fails on develop too (3 of 3 at 4777d5d)