feat(sidecar): integrate local builder with builder api #97
Conversation
thedevbirb
force-pushed
the
feat/fallback-builder-api
branch
from
9116295 to
5ac37bd
Compare
| /// Poll the deadline until it is reached. | ||
| pub async fn wait(&mut self) -> Option<u64> { | ||
| let slot = poll_fn(|cx| self.poll_unpin(cx)).await; | ||
| self.sleep = None; | ||
| slot | ||
| } |
There was a problem hiding this comment.
note to reviewers: we haven't measured the efficiency of this future, in the context of being polled inside a tokio::select branch. Any suggestion is welcome
There was a problem hiding this comment.
Why not just await self.sleep directly?
There was a problem hiding this comment.
because self.sleep is an Option
| /// Poll the deadline until it is reached. | ||
| /// | ||
| /// - If already reached, the future will return `None` immediately. | ||
| /// - If not reached, the future will return `Some(slot)` when the deadline is reached. | ||
| impl Future for CommitmentDeadline { | ||
| type Output = Option<u64>; | ||
|
|
||
| fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> { | ||
| let Some(ref mut sleep) = self.sleep else { | ||
| return Poll::Ready(None); | ||
| }; | ||
|
|
||
| match sleep.as_mut().poll(cx) { | ||
| Poll::Ready(_) => Poll::Ready(Some(self.slot)), | ||
| Poll::Pending => Poll::Pending, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The idea is that we select on the Some(slot) result, meaning that it won't match if the future returns None immediately, which is the case if the sleep future is already over, until Self::new() is called again
| /// Poll the deadline until it is reached. | ||
| pub async fn wait(&mut self) -> Option<u64> { | ||
| let slot = poll_fn(|cx| self.poll_unpin(cx)).await; | ||
| self.sleep = None; | ||
| slot | ||
| } |
There was a problem hiding this comment.
Why not just await self.sleep directly?
Co-authored-by: merklefruit <[email protected]>
Co-authored-by merklefruit <[email protected]>
Co-authored-by merklefruit <[email protected]>
Co-authored-by merklefruit <[email protected]>
…lidation Co-authored-by: nicolas <[email protected]>
Co-authored-by merklefruit <[email protected]>
Co-authored-by: thedevbirb <[email protected]>
Co-authored-by: thedevbirb <[email protected]>
merklefruit
force-pushed
the
feat/fallback-builder-api
branch
from
f2020f0 to
bf76777
Compare
|
@coderabbitai review |
Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes to the Changes
Sequence Diagram(s)N/A Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range comments (7)
bolt-sidecar/src/client/commit_boost.rs (2)
Line range hint
35-49: Improve error handling in the spawned task.The function
CommitBoostClient::newcorrectly initializes a newCommitBoostClientinstance. However, the error handling in the spawned task could be improved by adding proper error handling instead of usingexpect.- tokio::spawn(async move { - this.load_pubkeys().await.expect("failed to load pubkeys"); - }); + tokio::spawn(async move { + if let Err(e) = this.load_pubkeys().await { + tracing::error!(error = ?e, "failed to load pubkeys"); + } + });
Line range hint
51-74: Add a retry limit to the loop.The function
load_pubkeyscorrectly loads public keys from a remote client and stores them locally. However, it could benefit from a retry limit to avoid potential infinite loops.- loop { + let mut attempts = 0; + while attempts < 10 { + attempts += 1;bolt-sidecar/src/api/builder.rs (3)
Line range hint
87-94: LGTM!The
register_validatorsfunction is well-structured.Reminder: Intercept this request to register Bolt validators on-chain as well.
The TODO comment indicates future work.
Do you want me to generate the code to intercept this request or open a GitHub issue to track this task?
Line range hint
106-150: LGTM!The
get_headerfunction is well-structured and handles errors appropriately.Reminder: Handle failure when no local payload is produced.
The TODO comment indicates a need to handle this case.
Do you want me to generate the code to handle this failure or open a GitHub issue to track this task?
Line range hint
188-230: LGTM!The
get_payloadfunction is well-structured and handles errors appropriately.Reminder: Handle failures when submitting the signed blinded block.
The TODO comment indicates a need to handle this case.
Do you want me to generate the code to handle these failures or open a GitHub issue to track this task?
bolt-sidecar/src/state/execution.rs (1)
Line range hint
117-171: LGTM!The
try_commitfunction is well-structured and handles validation appropriately.Reminder: Check
max_fee_per_blob_gasagainst theblob_base_fee.The TODO comment indicates a need to handle this case.
Do you want me to generate the code to handle this check or open a GitHub issue to track this task?
bolt-sidecar/src/builder/payload_builder.rs (1)
Line range hint
103-237: LGTM!The
build_fallback_payloadfunction is well-structured and handles the payload building process appropriately.Reminder: Handle potential reorgs and refactor once
ConsensusStateis ready.The TODO comments indicate a need to handle these cases.
Do you want me to generate the code to handle potential reorgs and refactor or open a GitHub issue to track these tasks?
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
bolt-sidecar/Cargo.lockis excluded by!**/*.lock
Files selected for processing (29)
- Justfile (2 hunks)
- bolt-sidecar/Cargo.toml (1 hunks)
- bolt-sidecar/bin/sidecar.rs (1 hunks)
- bolt-sidecar/src/api/builder.rs (10 hunks)
- bolt-sidecar/src/api/spec.rs (3 hunks)
- bolt-sidecar/src/builder/compat.rs (5 hunks)
- bolt-sidecar/src/builder/mod.rs (2 hunks)
- bolt-sidecar/src/builder/payload_builder.rs (9 hunks)
- bolt-sidecar/src/builder/signature.rs (1 hunks)
- bolt-sidecar/src/builder/template.rs (6 hunks)
- bolt-sidecar/src/client/commit_boost.rs (2 hunks)
- bolt-sidecar/src/client/mevboost.rs (1 hunks)
- bolt-sidecar/src/client/pubsub.rs (1 hunks)
- bolt-sidecar/src/common.rs (4 hunks)
- bolt-sidecar/src/config.rs (5 hunks)
- bolt-sidecar/src/crypto/bls.rs (2 hunks)
- bolt-sidecar/src/crypto/mod.rs (1 hunks)
- bolt-sidecar/src/json_rpc/api.rs (5 hunks)
- bolt-sidecar/src/json_rpc/mod.rs (1 hunks)
- bolt-sidecar/src/lib.rs (2 hunks)
- bolt-sidecar/src/primitives/commitment.rs (4 hunks)
- bolt-sidecar/src/primitives/constraint.rs (2 hunks)
- bolt-sidecar/src/primitives/mod.rs (7 hunks)
- bolt-sidecar/src/state/consensus.rs (6 hunks)
- bolt-sidecar/src/state/execution.rs (8 hunks)
- bolt-sidecar/src/state/fetcher.rs (7 hunks)
- bolt-sidecar/src/state/head_tracker.rs (1 hunks)
- bolt-sidecar/src/state/mod.rs (7 hunks)
- bolt-sidecar/src/test_util.rs (4 hunks)
Additional comments not posted (102)
bolt-sidecar/src/crypto/mod.rs (1)
3-3: Exported items look good.The added exports for
SignerBLSandSignerBLSAsyncare correctly implemented and consistent with the rest of the code.bolt-sidecar/src/client/pubsub.rs (1)
17-17: Implementation ofPubsubClientlooks good.The
PubsubClientimplementation is correctly implemented and consistent with the rest of the code.bolt-sidecar/src/lib.rs (4)
9-10: Exported items look good.The added exports for
start_builder_proxy_serverandBuilderProxyConfigare correctly implemented and consistent with the rest of the code.
24-24: Exported item looks good.The added export for
LocalBuilderis correctly implemented and consistent with the rest of the code.
28-28: Exported items look good.The added exports for
Chain,Config, andOptsare correctly implemented and consistent with the rest of the code.
35-35: Exported item looks good.The added export for
start_rpc_serveris correctly implemented and consistent with the rest of the code.bolt-sidecar/Cargo.toml (1)
17-18: Added dependencies look good.The added dependencies for
tree_hashandtree_hash_deriveare correctly implemented and consistent with the rest of the code.bolt-sidecar/src/common.rs (3)
2-2: Added import looks good.The added import for
TransactionSignedis correctly implemented and consistent with the rest of the code.
29-32: Implementation ofmax_transaction_costlooks good.The
max_transaction_costfunction is correctly implemented and consistent with the rest of the code.
Line range hint
43-47: Implementation ofvalidate_transactionlooks good.The
validate_transactionfunction is correctly implemented and consistent with the rest of the code.bolt-sidecar/src/json_rpc/mod.rs (3)
22-22: LGTM!The
start_rpc_serverfunction is well-structured and follows good practices for setting up a server.
Line range hint
41-41: LGTM!The
handle_rpc_requestfunction handles parsing and method dispatching appropriately. Error handling is in place.
Line range hint
66-66: LGTM!The
handle_rejectionfunction handles different types of rejections and provides meaningful error messages.bolt-sidecar/src/primitives/constraint.rs (2)
92-92: LGTM!The change to use
encode_envelopedinstead ofencode_2718is consistent with the new encoding method.
Line range hint
102-102: LGTM!The
as_bytesfunction is well-implemented.bolt-sidecar/src/state/head_tracker.rs (3)
36-37: LGTM!The change to use
beacon_api_urlinstead ofbeacon_urlimproves clarity and is consistent with naming conventions.
Line range hint
72-72: LGTM!The
stopfunction is well-implemented.
Line range hint
78-78: LGTM!The
next_headfunction is well-implemented.bolt-sidecar/src/crypto/bls.rs (3)
39-42: LGTM!The
SignerBLStrait is well-defined and provides a clear interface for synchronous BLS signature generation.
46-49: LGTM!The
SignerBLSAsynctrait is well-defined and provides a clear interface for asynchronous BLS signature generation.
Line range hint
54-54: LGTM!The
Signerstruct implements theSignerBLSandSignerBLSAsynctraits and provides methods for BLS signature generation and verification.Justfile (1)
81-83: LGTM!The
kill-buildertask is well-implemented and follows good practices for stopping a Docker container.bolt-sidecar/src/test_util.rs (5)
38-44: LGTM!The function
try_get_engine_api_urlcorrectly checks the reachability of the test engine client.
63-79: LGTM!The function
get_test_configcorrectly constructs the test configuration and handles potential errors.
87-95: LGTM!The function
default_test_transactioncorrectly creates a default transaction template for tests and allows an optional nonce parameter.
Line range hint
97-99: LGTM!The function
test_bls_secret_keycorrectly creates a default BLS secret key.
Line range hint
102-132: LGTM!The struct
TestSignableDataand its implementations forSignableBLSandSignableECDSAare correctly defined and implemented.bolt-sidecar/src/client/commit_boost.rs (1)
Line range hint
67-82: LGTM!The function
signcorrectly signs data asynchronously using BLS signatures.bolt-sidecar/src/builder/template.rs (7)
38-49: LGTM!The function
add_transactioncorrectly adds a transaction to the block template and updates the state diff.
61-65: LGTM!The function
blob_countcorrectly returns the blob count of the block template.
Line range hint
73-79: LGTM!The function
remove_transaction_at_indexcorrectly removes a transaction at a specified index and updates the state diff.
Line range hint
83-109: LGTM!The function
retaincorrectly retains transactions that do not conflict with the given account state.
Line range hint
133-137: LGTM!The function
get_diffcorrectly returns the nonce and balance diff for a given address.
Line range hint
127-131: LGTM!The struct
StateDiffcorrectly tracks intermediate changes to the state according to the block template.
Line range hint
19-28: LGTM!The struct
BlockTemplatecorrectly serves as a fallback block template and keeps intermediary state for new commitment requests.bolt-sidecar/src/primitives/commitment.rs (6)
47-53: LGTM!The function
deserialize_tx_signedcorrectly deserializes a signed transaction.
56-63: LGTM!The function
serialize_tx_signedcorrectly serializes a signed transaction.
Line range hint
66-71: LGTM!The function
deserialize_from_strcorrectly deserializes a value from a string.
Line range hint
74-82: LGTM!The function
signature_as_strcorrectly serializes a signature as a string.
88-91: LGTM!The function
digestcorrectly returns the digest of an inclusion request.
Line range hint
106-144: LGTM!The tests correctly cover deserialization of inclusion and commitment requests.
bolt-sidecar/src/builder/signature.rs (6)
17-31: LGTM!The function
sign_builder_messagecorrectly signs a builder message using a BLS secret key.
35-48: LGTM!The function
verify_signed_builder_messagecorrectly verifies a signed builder message using a BLS public key.
51-63: LGTM!The function
verify_signaturecorrectly verifies a BLS signature for a given message and public key.
69-72: LGTM!The function
sign_messagecorrectly signs arbitrary bytes with a BLS secret key.
83-89: LGTM!The function
compute_signing_rootcorrectly computes the signing root for a given object root and signing domain.
96-114: LGTM!The function
compute_builder_domaincorrectly computes the Application Builder domain for signing arbitrary builder-api messages.bolt-sidecar/src/client/mevboost.rs (2)
126-126: LGTM!The code changes are approved.
126-126: LGTM!The code changes are approved.
bolt-sidecar/src/state/fetcher.rs (5)
28-28: LGTM!The code changes are approved.
51-51: LGTM!The code changes are approved.
78-83: LGTM!The code changes are approved.
105-111: LGTM!The code changes are approved.
146-146: LGTM!The code changes are approved.
bolt-sidecar/src/api/spec.rs (2)
67-68: LGTM!The code changes are approved.
110-112: LGTM!The code changes are approved.
bolt-sidecar/src/builder/mod.rs (3)
73-81: LGTM!The code changes are approved.
86-91: LGTM!The code changes are approved.
96-148: LGTM!The code changes are approved.
bolt-sidecar/bin/sidecar.rs (8)
3-3: LGTM!The code changes are approved.
25-25: LGTM!The code changes are approved.
31-58: LGTM!The code changes are approved.
68-120: LGTM!The code changes are approved.
121-132: LGTM!The code changes are approved.
133-144: LGTM!The code changes are approved.
145-159: LGTM!The code changes are approved.
160-164: LGTM!The code changes are approved.
bolt-sidecar/src/primitives/mod.rs (3)
Line range hint
10-17: Update import paths fordenebmodule.The import paths have been updated to reflect the new module structure. Ensure that all references to the
denebmodule are correctly updated across the codebase.Verification successful
The search results show multiple references to the
denebmodule across various files. Here are the locations:
bolt-sidecar/src/builder/compat.rsbolt-sidecar/src/builder/signature.rsbolt-sidecar/src/builder/mod.rsbolt-sidecar/src/state/consensus.rsbolt-sidecar/src/crypto/bls.rsbolt-sidecar/src/primitives/mod.rsbolt-sidecar/src/api/spec.rsbolt-sidecar/src/api/builder.rsbolt-sidecar/src/client/mevboost.rsWe need to ensure that all these references are correctly updated and consistent with the new module structure.
All references to the
denebmodule are correctly updated.The references to the
denebmodule across the codebase are consistent and correctly reflect the new module structure. No issues were found.
bolt-sidecar/src/crypto/bls.rsbolt-sidecar/src/state/consensus.rsbolt-sidecar/src/builder/signature.rsbolt-sidecar/src/builder/mod.rsbolt-sidecar/src/builder/compat.rsbolt-sidecar/src/api/spec.rsbolt-sidecar/src/api/builder.rsbolt-sidecar/src/primitives/mod.rsbolt-sidecar/src/client/mevboost.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the `deneb` module are correctly updated. # Test: Search for references to `deneb`. Expect: Only updated references. rg --type rust 'deneb'Length of output: 1027
Script:
#!/bin/bash # Description: Check the context around each reference to the `deneb` module in the listed files to ensure correctness. # Extract 5 lines before and after each reference to `deneb` for context. for file in $(rg --files-with-matches 'deneb' --type rust); do echo "File: $file" rg -C 5 'deneb' $file echo "----------------------------------------" doneLength of output: 6279
90-90: Update field name inFetchPayloadRequest.The field name
responsehas been updated toresponse_txfor clarity and consistency.Verification successful
Update field name in
FetchPayloadRequest.The field name
responsehas been correctly updated toresponse_txfor clarity and consistency.
bolt-sidecar/src/primitives/mod.rs: Line 90Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `response_tx` are correctly updated. # Test: Search for references to `response_tx`. Expect: Only updated references. rg --type rust 'response_tx'Length of output: 1278
Script:
#!/bin/bash # Description: Verify the definition of `response_tx` in `json_rpc/api.rs`. # Test: Search for the definition of `response_tx` to confirm its context. rg --type rust 'response_tx' --context 5 json_rpc/api.rsLength of output: 112
Script:
#!/bin/bash # Description: Verify the definition of `response_tx` in existing files. # Test: Search for the definition of `response_tx` to confirm its context. rg --type rust 'response_tx' --context 5Length of output: 7457
148-148: Update field inPayloadAndBlobs.The
blobs_bundlefield is no longer optional, ensuring it is always present.bolt-sidecar/src/state/consensus.rs (5)
11-11: IntroduceCommitmentDeadlinestruct.The
CommitmentDeadlinestruct is introduced to handle commitment deadlines in theConsensusStatestruct.
87-90: Updatevalidate_requestmethod to check commitment deadline.The
validate_requestmethod now includes logic to check the commitment deadline, ensuring that requests are validated against the deadline.
101-104: Updateupdate_headmethod to reset commitment deadline.The
update_headmethod now includes logic to reset the commitment deadline when the head is updated.
38-49: UpdateConsensusStatestruct to include new fields.The
ConsensusStatestruct now includeslatest_slot_timestamp,latest_slot,commitment_deadline, andcommitment_deadline_durationto enhance slot and deadline tracking.
26-26: UpdateEpochstruct to includeproposer_duties.The
Epochstruct now includesproposer_dutiesto track proposer duties.Verification successful
Update
Epochstruct to includeproposer_duties.The
Epochstruct now includesproposer_dutiesto track proposer duties. All references toproposer_dutiesare correctly updated within thebolt-sidecar/src/state/consensus.rsfile, and there are no references outside this file that need updating.
bolt-sidecar/src/state/consensus.rs: All references toproposer_dutiesare correctly updated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `proposer_duties` are correctly updated. # Test: Search for references to `proposer_duties`. Expect: Only updated references. rg --type rust 'proposer_duties'Length of output: 732
Script:
#!/bin/bash # Description: Verify all references to `proposer_duties` outside `bolt-sidecar/src/state/consensus.rs`. # Test: Search for references to `proposer_duties` in the entire codebase except `bolt-sidecar/src/state/consensus.rs`. rg --type rust 'proposer_duties' --glob '!bolt-sidecar/src/state/consensus.rs'Length of output: 78
bolt-sidecar/src/json_rpc/api.rs (2)
10-10: IntroduceCommitmentRequeststruct.The
CommitmentRequeststruct is introduced to handle commitment requests in theApiEventstruct.
55-58: Update field name inApiEvent.The field name
responsehas been updated toresponse_txfor clarity and consistency.Verification successful
Update field name in
ApiEvent.The field name
responsehas been successfully updated toresponse_txfor clarity and consistency. No remaining references to the oldresponsefield were found.
bolt-sidecar/bin/sidecar.rsbolt-sidecar/src/json_rpc/api.rsbolt-sidecar/src/primitives/mod.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `response_tx` are correctly updated. # Test: Search for references to `response_tx`. Expect: Only updated references. rg --type rust 'response_tx'Length of output: 1278
Script:
#!/bin/bash # Description: Verify no remaining references to the old `response` field. # Test: Search for references to `response`. Expect: No references to the old field. rg --type rust '\bresponse\b'Length of output: 4730
bolt-sidecar/src/builder/compat.rs (3)
8-8: IntroduceTransactionSignedtype.The
TransactionSignedtype is introduced to ensure compatibility with the new transaction structure.
Line range hint
146-163: Updateto_consensus_execution_payloadfunction to useTransactionSigned.The
to_consensus_execution_payloadfunction now uses theTransactionSignedtype to ensure compatibility with the new transaction structure.
180-188: Updateto_consensus_withdrawalfunction to useExecutionAddress.The
to_consensus_withdrawalfunction now uses theExecutionAddresstype to ensure compatibility with the new address structure.bolt-sidecar/src/state/mod.rs (3)
4-8: IntroduceCommitmentDeadlinestruct.The
CommitmentDeadlinestruct is introduced to handle commitment deadlines.
27-32: UpdateCommitmentDeadlinestruct to includesleepfield.The
CommitmentDeadlinestruct now includes asleepfield to handle sleep durations.
41-46: Updatewaitmethod to handlesleepfield.The
waitmethod now includes logic to handle thesleepfield, ensuring that the method correctly handles sleep durations.bolt-sidecar/src/api/builder.rs (3)
Line range hint
35-43: LGTM!The
BuilderProxyServerstruct is well-defined and the new fields are appropriately added.
Line range hint
249-266: LGTM!The
start_builder_proxy_serverfunction is well-structured and the changes are appropriate.
274-276: LGTM!The
indexfunction is straightforward and appropriate.bolt-sidecar/src/state/execution.rs (7)
Line range hint
14-42: LGTM!The
ValidationErrorenum is well-defined and appropriately documents each error.
Line range hint
63-80: LGTM!The
ExecutionStatestruct is well-defined and the fields are appropriately documented.
84-91: LGTM!The
newfunction is well-structured and the changes are appropriate.
Line range hint
176-185: LGTM!The
commit_transactionfunction is well-structured and the changes are appropriate.
187-197: LGTM!The
update_headfunction is well-structured and the changes are appropriate.
Line range hint
200-207: LGTM!The
apply_state_updatefunction is well-structured and the changes are appropriate.
Line range hint
209-237: LGTM!The
refresh_templatesfunction is well-structured and the changes are appropriate.bolt-sidecar/src/config.rs (2)
Line range hint
1-70: LGTM!The constants and command-line options are well-defined and documented.
Line range hint
71-306: LGTM!The
Configstruct and related implementations are well-defined and appropriately handle the configuration options.bolt-sidecar/src/builder/payload_builder.rs (7)
41-47: LGTM!The
FallbackPayloadBuilderstruct is well-defined and appropriately documents its purpose and fields.
52-65: LGTM!The
newfunction is well-structured and the changes are appropriate.
Line range hint
263-313: LGTM!The
fetch_next_payload_hintfunction is well-structured and the changes are appropriate.
Line range hint
316-324: LGTM!The
parse_geth_responsefunction is well-structured and the changes are appropriate.
Line range hint
327-377: LGTM!The
build_header_with_hints_and_contextfunction is well-structured and the changes are appropriate.
396-424: LGTM!The
test_build_fallback_payloadfunction is well-structured and the changes are appropriate.
427-433: LGTM!The
test_empty_el_withdrawals_rootfunction is well-structured and the changes are appropriate.
feat: refactor config and add ChainConfig
TxEnvelopein favour ofreth_primitives::TransactionSignedWith this PR, we will be able to build blocks locally including the preconfirmed transactions.
Summary by CodeRabbit
New Features
CommitmentDeadlinestruct for managing commitment deadlines.LocalBuilderfor local builder management.Bug Fixes
CommitmentsRpcandExecutionStatemethods.ConsensusStateto manage commitments based on timing.Refactor
Tests