Skip to content

Ignore spent funding cell(s) in channel gossip message - #1489

Open
bolajahmad wants to merge 3 commits into
nervosnetwork:developfrom
bolajahmad:broadcastonlylivecells
Open

Ignore spent funding cell(s) in channel gossip message #1489
bolajahmad wants to merge 3 commits into
nervosnetwork:developfrom
bolajahmad:broadcastonlylivecells

Conversation

@bolajahmad

@bolajahmad bolajahmad commented Jun 24, 2026

Copy link
Copy Markdown

Closes #530

Description

Currently, during ChannelAnnouncement, the funding TX is fetched [using get_transaction](https://github.com/nervosnetwork/fiber/blob/develop/crates/fiber-lib/src/ckb/client.rs#L220C4-L227C6).

This approach only checks that the Funding Outpoint was Committed. But a Committed Outpoint does not guarantee availability of the Cell.

This PR adds that layer of security.

  • Ensure the Outpoint is Committed
  • Ensure that the underlying Cell is still Live
  • No point in broadcasting a dead cell.

Tests

cargo test -p fnn --all-features \
ckb::tests::test_utils::test_mock_ckb_chain_client_reports_dead_live_cell -- --exact \
 && cargo test -p fnn --all-features  \
fiber::tests::gossip::test_reject_channel_announcement_when_funding_outpoint_is_dead \ 
-- --exact

cargo fmt --all -- --check
cargo clippy -p fnn --features rocksdb --all-targets -- -D warnings
git diff --cached --check

@bolajahmad bolajahmad changed the title use get_live_cell from ckbClient to determine cell liveness Ignore spent funding cell(s) in channel gossip message Jun 24, 2026
@chainTe

chainTe commented Jun 25, 2026

Copy link
Copy Markdown

Security Review

Result: No reportable security findings in the intended change.
Reviewed range: 3c25bcf16200e5d641dcd9b79f086f391e976172..8562d79ca726099fc2cb0295f10e197241a37196

Validation:

  • Existing broad tests were not run locally; CI is expected to cover them.
  • Targeted build validation: cargo check -p fnn --features sqlite fails because crates/fiber-lib/src/fiber/gossip.rs calls is_funding_outpoint_live(...), but that function is not defined in this PR.
  • cargo check -p fnn --features rocksdb could not reach the PR code in this local environment because RocksDB binding generation failed on a missing local Clang header (stdbool.h).

Notes:

  • The added live-cell gate is security-positive in intent, but the helper implementation needs to be added (or the call updated) before this can be buildable and fully validated.

@bolajahmad
bolajahmad marked this pull request as ready for review July 1, 2026 09:01
@quake

quake commented Jul 21, 2026

Copy link
Copy Markdown
Member

@bolajahmad we added block_hash to the rpc get_live_cell in ckb 0.208 ( nervosnetwork/ckb#5269 )

I think we can use this new field to improve the existing live cell checking code.

@bolajahmad

Copy link
Copy Markdown
Author

@bolajahmad we added block_hash to the rpc get_live_cell in ckb 0.208 ( nervosnetwork/ckb#5269 )

I think we can use this new field to improve the existing live cell checking code.

If I understand correctly and based on the fixes I made, the new block_hash field in the response has the same effect as checking if the cell is Some. If anything,I wonder if it's also okay to push this change and follow up with another commit to upgrade to new v5.1.0. If I'm missing something, please let me know!

@bolajahmad
bolajahmad force-pushed the broadcastonlylivecells branch from 9575f78 to 492017a Compare August 10, 2026 13:40
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.

Ignore channel gossip message when funding cell is consumed

3 participants