-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: wait for new blocks when build is in progress #18831
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: wait for new blocks when build is in progress #18831
Conversation
There was a problem hiding this 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
48cf7f9
to
c43faed
Compare
There was a problem hiding this 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
/// Signals when a block build is in progress | ||
build_state_tx: watch::Sender<bool>, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates/optimism/rpc/src/eth/mod.rs
Outdated
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)); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good for another round of reviews @RomanHodulak :) |
756f1ca
to
ffb1ea3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* 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) ...
Closes #18799.