Skip to content

refactor: block_pool will forward txs to block discovery#1037

Merged
roberts-pumpurs merged 27 commits into
masterfrom
rob/proxy-txs
Dec 8, 2025
Merged

refactor: block_pool will forward txs to block discovery#1037
roberts-pumpurs merged 27 commits into
masterfrom
rob/proxy-txs

Conversation

@roberts-pumpurs

@roberts-pumpurs roberts-pumpurs commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

Describe the changes

Transactions are now passed alongside blocks to Block Discovery to block validatoin pipeline instead of being fetched during block discovery and then re-fetched in block val.

  1. Block Discovery
  • handle_block() now accepts pre-fetched BlockTransactions (contains all txs for a block)
  • No longer fetches transactions from mempool during discovery
  • the get commitment/data txs helpers that are used across the codebase now use mempool read guard instead of mpsc channels.
  • we propagate block transactions from block discovery -> block tree -> block validatoin
    this way we don't need to keep re-fetching them from the mempool
  1. Block producer
  • Returns BlockTransactions alongside block header when producing blocks, useful for tests
  1. Block pool
  • process_block() now takes BlockTransactions parameter
  • Removed reprocess_block() becuase it was no longer used - chian_sync now uses the same process_block
  1. Gossip data handler
  • New fetch_and_build_block_transactions() - unified tx fetching from cache -> mempool -> DB -> network to build the BlockTransactoins struct
  • New fetch_missing_txs_from_network() with peer fallback logic (previously was in-lined code, now its a helper fn)
  • Batch queries instead of per-tx checks

Checklist

  • Tests have been added/updated for the changes.
  • Documentation has been updated for the changes (if applicable).
  • The code follows Rust's style guidelines.

@roberts-pumpurs roberts-pumpurs changed the title fefactor: block_pool will forward txs to block discovery refactor: block_pool will forward txs to block discovery Dec 1, 2025
@roberts-pumpurs roberts-pumpurs self-assigned this Dec 1, 2025
@roberts-pumpurs roberts-pumpurs marked this pull request as ready for review December 1, 2025 10:38
Comment thread crates/actors/src/block_producer/ledger_expiry.rs
Comment thread crates/actors/src/block_discovery.rs Outdated
}

// Check IDs match in order and signatures are valid
for (tx, expected_id) in txs.iter().zip(expected_ids.iter()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's going to be beneficial, but we probably can to a par_iter here, since we can have a lot of transactions, and this is a latency-critical path, since that's were the block gets gossiped. We should probably discuss it on a call, since it may very well be a non-issue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok this actually highlighed a few things: we have a consensus param for defining a max count of data and commitment txs per block that needs to be enforced - I added new checks for this, which was previously missing.
I did not switch to par iter, given that our mainnet and testnet use max_data_txs_per_block: 100 and same for max commitment txs.

@roberts-pumpurs

Copy link
Copy Markdown
Contributor Author

I decided to merge the changes from #1040 into this branch as it was not actually that big, and keeping both branches in sync became bothersome.

let block_hash = block.block_hash;

// Take any cached transactions for this block
let cached_txs = self.block_pool.take_cached_txs_for_block(&block_hash).await;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this list is actually always going to be empty - transactions are added to the cache in the block pool after the process_block has been called, i.e. they have to fetched first to get into the cache. I don't think anything actually needs to be changed here probably, but something just doesn't feel right when I look at it

@antouhou antouhou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@roberts-pumpurs roberts-pumpurs merged commit 4bf399a into master Dec 8, 2025
17 checks passed
@roberts-pumpurs roberts-pumpurs deleted the rob/proxy-txs branch December 8, 2025 20:24
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