feat(sidecar): commit-boost integration #203
Conversation
|
@coderabbitai review |
WalkthroughAhoy there! The changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CommitBoostSigner
participant SignerClient
Client->>CommitBoostSigner: Request to sign data
CommitBoostSigner->>SignerClient: Forward signing request
SignerClient-->>CommitBoostSigner: Return signed data
CommitBoostSigner-->>Client: Send signed data
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
bolt-sidecar/src/driver.rs
Outdated
| // Constraints are signed with a BLS private key or Commit-Boost | ||
| let constraint_signer = if let Some(private_key) = cfg.private_key.clone() { | ||
| BlsSignerType::PrivateKey(BlsSigner::new(private_key)) | ||
| } else { | ||
| let commit_boost_client = CommitBoostClient::new( | ||
| cfg.commit_boost_address.clone().expect("CommitBoost URL must be provided"), | ||
| &cfg.commit_boost_jwt_hex.clone().expect("CommitBoost JWT must be provided"), | ||
| ) | ||
| .await?; | ||
| BlsSignerType::CommitBoost(commit_boost_client) | ||
| }; |
There was a problem hiding this comment.
It would be nice to refactor the signing config (SigningOpts) to avoid unwrapping CLI options here.
can be done in a separate PR.
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Outside diff range comments (3)
bolt-sidecar/src/primitives/constraint.rs (1)
Line range hint
66-78: Ahoy, me hearty! This be a fine piece of code ye've crafted!Shiver me timbers! Ye've made the
digestmethod as sleek as a pirate's cutlass. Returnin' a fixed-size array instead of a vector be a smart move, savvy? It'll save us from the curse of unnecessary memory allocation.But wait, ye scurvy dog! I spy with me eye a chance to make this treasure even shinier. How 'bout we combine the data into a single vector, eh? It'll save us from walkin' the plank of multiple allocations. Feast yer eyes on this:
fn digest(&self) -> [u8; 32] { - let mut data = Vec::new(); - data.extend_from_slice(&self.validator_index.to_le_bytes()); - data.extend_from_slice(&self.slot.to_le_bytes()); - - let mut constraint_bytes = Vec::new(); - for constraint in &self.constraints { - constraint_bytes.extend_from_slice(&constraint.as_bytes()); - } - data.extend_from_slice(&constraint_bytes); + let mut data = Vec::with_capacity(16 + self.constraints.iter().map(|c| c.as_bytes().len()).sum::<usize>()); + data.extend_from_slice(&self.validator_index.to_le_bytes()); + data.extend_from_slice(&self.slot.to_le_bytes()); + self.constraints.iter().for_each(|constraint| data.extend_from_slice(&constraint.as_bytes())); // Compute the Keccak-256 hash and return the 32-byte array directly keccak256(data).0 }This be reducin' the number of times we need to swab the deck (reallocate memory), makin' our code run faster than a ship with the wind at its back!
bolt-sidecar/src/primitives/commitment.rs (1)
Line range hint
1-329: Avast ye, matey! Our ship be sailing true, but there be room for more treasure!While ye've done a fine job with the changes, I spy with me eye some opportunities for future plunder:
- Consider making other methods async where it makes sense, to keep pace with our new async ways.
- Mayhaps we could add some more error context using
eyre::WrapErrin places where we're returning errors.- Think about adding some doc comments to explain the
SignerECDSAAsynctrait for any landlubbers who might join our crew later.What say ye to these ideas for our next voyage, eh? They'd make our code shine brighter than buried treasure!
bolt-sidecar/src/driver.rs (1)
Line range hint
1-320: Avast ye, we've reached the end of our code voyage!Ye've navigated these treacherous waters with the skill of a seasoned captain. The changes ye've made be integrating smoother than a well-oiled cannon, and the rest of the code be standin' strong like the mast of the Black Pearl.
Now, before we weigh anchor, I've got one last piece of advice from me treasure trove of wisdom. Consider addin' some error logging in the
handle_fetch_payload_requestmethod. It'd be mighty helpful for debuggin' if the seas get rough. Something like this:fn handle_fetch_payload_request(&mut self, request: FetchPayloadRequest) { info!(slot = request.slot, "Received local payload request"); let Some(payload_and_bid) = self.local_builder.get_cached_payload() else { warn!(slot = request.slot, "No local payload found"); if let Err(e) = request.response_tx.send(None) { error!(err = ?e, "Failed to send None response for missing payload"); } return; }; // ... rest of the method ... }What say ye to this final touch to our map of code?
|
Just working on a mock test |
partially solves #201
Note - This is not e2e tested with commit-boost
Summary by CodeRabbit
New Features
CommitBoostSigner, to the public interface.Improvements
SidecarDriverto streamline the signing process and support asynchronous operations.Bug Fixes
commit_and_signmethod.Chores