Skip to content

Conversation

lean-apple
Copy link
Contributor

Closes #18799.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

cool, some suggestions

cc @loocapro

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Oct 2, 2025
Copy link
Contributor

@loocapro loocapro left a comment

Choose a reason for hiding this comment

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

Nice!

@lean-apple lean-apple force-pushed the await-progress-block branch from 48cf7f9 to c43faed Compare October 2, 2025 15:28
@lean-apple lean-apple marked this pull request as ready for review October 2, 2025 15:29
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this looks pretty good, I have one more idea that makes the index check redundant and is perhaps useful in general for awaiting

Comment on lines 55 to 56
/// Signals when a block build is in progress
build_state_tx: watch::Sender<bool>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if use Option<FbInfo> here where FbInfo is parenthash + index + blocknumber

then we can easily check from the pendingblock pov if the Fb in progress is the next index

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mattsse
mattsse previously requested changes Oct 2, 2025
Comment on lines 152 to 158
if tokio::time::timeout(MAX_WAIT_DURATION, rx_clone.changed()).await.is_ok() {
let fresh = rx_clone.borrow();
if let Some(block) = self.extract_matching_block(fresh.as_ref(), parent_hash) {
return Ok(Some(block));
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can simplify this and only need to keep tokio::time::timeout(MAX_WAIT_DURATION, rx_clone.changed()).await

because borrow always returns the most recent value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lean-apple
Copy link
Contributor Author

lean-apple commented Oct 7, 2025

It's good for another round of reviews @RomanHodulak :)

@RomanHodulak RomanHodulak dismissed stale reviews from mattsse and loocapro October 9, 2025 16:31

All addressed

Copy link
Collaborator

@RomanHodulak RomanHodulak left a comment

Choose a reason for hiding this comment

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

LGTM

@RomanHodulak RomanHodulak enabled auto-merge October 9, 2025 16:31
@RomanHodulak RomanHodulak added this pull request to the merge queue Oct 9, 2025
Merged via the queue into paradigmxyz:main with commit d2070f4 Oct 9, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 9, 2025
@lean-apple lean-apple deleted the await-progress-block branch October 9, 2025 17:18
CarlBeek added a commit to CarlBeek/reth that referenced this pull request Oct 10, 2025
* main: (280 commits)
  refactor: remove needless collect() calls in trie tests (paradigmxyz#18937)
  chore(grafana): use precompile address as legend (paradigmxyz#18913)
  perf(tree): worker pooling for storage in multiproof generation (paradigmxyz#18887)
  feat: wait for new blocks when build is in progress (paradigmxyz#18831)
  chore: align node_config threshold constant (paradigmxyz#18914)
  docs: duplicate comment in Eip4844PoolTransactionError (paradigmxyz#18858)
  ci: cache hive simulator images to reduce prepare-hive job time (paradigmxyz#18899)
  refactor: replace collect().is_empty() with next().is_none() in tests (paradigmxyz#18902)
  feat(provider): add get_account_before_block to ChangesetReader (paradigmxyz#18898)
  refactor(engine): separate concerns in on_forkchoice_updated for better maintainability (paradigmxyz#18661)
  chore(node): simplify EngineApiExt bounds by removing redundant constraints (paradigmxyz#18905)
  fix(trie): Reveal extension child when extension is last remaining child of a branch (paradigmxyz#18891)
  chore: make clippy happy (paradigmxyz#18900)
  chore: relax `ChainSpec` impls (paradigmxyz#18894)
  refactor: eliminate redundant allocation in precompile cache example (paradigmxyz#18886)
  fix(era-utils): fix off-by-one for Excluded end bound in process_iter (paradigmxyz#18731)
  docs: yellowpaper sections in consensus implementation (paradigmxyz#18881)
  feat(storage): read headers and transactions only from static files (paradigmxyz#18788)
  feat: Use generic `HeaderTy` for `reth db get static-file headers` (paradigmxyz#18870)
  fix: streamline payload conversion in custom engine API (paradigmxyz#18864)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Await in progress flashblock

4 participants