add builder server for epbs #855
Conversation
|
I would be great to normalize to use either get_bid or get_execution_payload_bid (I like this most since it matched the protocol) |
|
eyre::Result should only be used at top level eg: reading the cfg before starting), I commented some but there might be more. |
|
|
||
| /// Fetch genesis data from the beacon chain. | ||
| /// Returns the genesis time, genesis validators root, and genesis fork version. | ||
| pub async fn get_genesis(&self) -> eyre::Result<GenesisData> { |
| async fn generate_bid( | ||
| &self, | ||
| params: &GetBidParams, | ||
| ) -> eyre::Result<Option<SignedExecutionPayloadBid>>; |
| /// The `validator_id` can be either: | ||
| /// - A hex encoded BLS public key | ||
| /// - A validator index as a string | ||
| pub async fn get_validator(&self, validator_id: &str) -> eyre::Result<ValidatorData> { |
There was a problem hiding this comment.
Don't make this pub. Make 2 other funcs (calling this one), one taking BlsPublicKey and the other taking int.
| /// | ||
| /// Implementations can use these notifications to: | ||
| /// - Generate EPBS bids | ||
| pub trait BlockObserver: Send + Sync + std::fmt::Debug { |
There was a problem hiding this comment.
we have to think if we can unify with BlockBuildingSink.
| let relay_set = relay_set.clone(); | ||
| let last_finalize_command = last_finalize_command.clone(); | ||
| let block_sink = block_sink.clone(); | ||
| let block_observer = self.block_observer.clone(); |
There was a problem hiding this comment.
we bid to serveral relays with specific bids so hooking the block_observer makes no sense since in ePBS there is no relay concept.
The whole bidding core will have to change.
| let state_clone = self.state.clone(); | ||
| let cancel_clone = cancel.clone(); | ||
| tokio::spawn(async move { | ||
| let mut interval = tokio::time::interval(Duration::from_secs(60)); |
There was a problem hiding this comment.
Move to a constant and justify/explain the value in a comment.
| if should_update { | ||
| info!( | ||
| slot, | ||
| ?parent_hash, | ||
| block_hash = ?block.sealed_block.hash(), | ||
| bid_value = %block.trace.bid_value, | ||
| cached_blocks = best_blocks.len() + 1, | ||
| "EPBS: Cached new best block for slot" | ||
| ); | ||
| best_blocks.insert(key, cached); | ||
| } |
There was a problem hiding this comment.
Can you put this in the match above?
| let should_update = match best_blocks.get(&key) { | ||
| Some(existing) => block.trace.bid_value > existing.block.trace.bid_value, | ||
| None => true, | ||
| }; | ||
|
|
||
| if should_update { | ||
| info!( | ||
| slot, | ||
| ?parent_hash, | ||
| block_hash = ?block.sealed_block.hash(), | ||
| bid_value = %block.trace.bid_value, | ||
| cached_blocks = best_blocks.len() + 1, | ||
| "EPBS: Cached new best block for slot" | ||
| ); | ||
| best_blocks.insert(key, cached); | ||
| } |
There was a problem hiding this comment.
You should always use the last bid, not the best since bids can go down because of a cancellation and we should honor that.
| /// EPBS Builder API server port. | ||
| #[serde(default = "default_epbs_server_port")] | ||
| pub epbs_server_port: u16, | ||
| /// Secret key for the builder's validator (for signing EPBS bids). |
There was a problem hiding this comment.
Using the relay key as default makes no sense. This should be mandatory.
| /// GET /eth/v1/builder/execution_payload_bid/{slot}/{parent_hash}/{parent_root}/{proposer_index} | ||
| /// | ||
| /// returns a SignedExecutionPayloadBid for the given slot. | ||
| pub async fn get_bid_handler( |
There was a problem hiding this comment.
Shouldn't we check SignedBidRequestAuth in the body?
2 participants
📝 Summary
get_bidsendpoint for retrieving bids for the proposer💡 Motivation and Context
✅ I have completed the following steps:
make lintmake test