BM-3002: Broker Backend Service#2004
Conversation
github-actions
Bot
changed the title
jonastheis
left a comment
jonastheis
left a comment
There was a problem hiding this comment.
The direction looks great. However I have a bunch of questions that I marked inline.
High-level I think what is unclear to me are mostly the following:
Provertrait and usage of it (it still contains risc0 types)- how does
Backendrelate toOrderPricingContextandRequestEvaluator? - why does
OrderPricernot useBackendRouterbut instead implementsOrderPricingContext? - risc0 types are not yet contained to a single crate
I haven't really looked at the submitter and batcher yet.
| } | ||
| } | ||
|
|
||
| macro_rules! string_id { |
There was a problem hiding this comment.
why can't we use this for BackendId?
| Failed(OrderFulfillmentFailure), | ||
| } | ||
|
|
||
| pub struct SubmissionPlan { |
There was a problem hiding this comment.
Can you explain how this translates to a JointVerifier or the OnchainAssessor?
| async fn estimate_batch_size(&self, cmd: BatchSizeEstimateRequest) | ||
| -> Result<BatchSizeEstimate>; | ||
|
|
||
| async fn update_batch(&self, cmd: UpdateBatch) -> Result<BatchUpdate>; | ||
|
|
||
| async fn close_batch(&self, cmd: CloseBatch) -> Result<BatchClose, BackendError>; |
There was a problem hiding this comment.
I think either pub trait Backend: BatchProcessor + Send + Sync {
or better
fn batch_processor(&self) -> Result<BatchProcessorObj>;
as this keeps the Backend trait cleaner in case a backend doesn't support batching
There was a problem hiding this comment.
Why is this in the general crate? shouldn't this have its own crate and be split in a few files maybe?
| } | ||
|
|
||
| fn supported_selectors(&self) -> Vec<FixedBytes<4>> { | ||
| SupportedSelectors::default().selectors.keys().copied().collect() |
There was a problem hiding this comment.
I think we should name the selectors explicitly here to avoid any accidental issues
| fn build_provers(&self, config: &ConfigLock) -> Result<(ProverObj, ProverObj)> { | ||
| let prover: ProverObj; | ||
| let aggregation_prover: ProverObj; | ||
| let batch_prover: ProverObj; |
There was a problem hiding this comment.
Why is the Prover trait from crates/boundless-market/src/prover_utils/prover.rs still used? It still contains risc0 specific types
| chain_span.clone(), | ||
| ); | ||
|
|
||
| let risc0_backend_id = BackendId::new("risc0_v3")?; |
There was a problem hiding this comment.
I think the backend impl should name its own BackendId
| let risc0_backend = Arc::new( | ||
| Risc0Backend::new( | ||
| risc0_backend_id.clone(), | ||
| prover.clone(), |
There was a problem hiding this comment.
Shouldn't the backend own the prover and expose it only if necessary?
There was a problem hiding this comment.
I'm not sure I understand why OrderPricer still implements OrderPricingContext carries a prover and supported_selectors? Shouldn't this all go through the backend?
| } | ||
| } | ||
|
|
||
| impl<P> Risc0RequestEvaluatorContext for OrderPricer<P> |
There was a problem hiding this comment.
Here we have again some risc0 types?
capossele
force-pushed
the
angelo/backend-service-mvp
branch
from
6b35ec1 to
97b965d
Compare
Keep submission preparation tolerant of per-order failures so submittable orders can still be fulfilled. Backend fulfillment construction now reports per-order artifact failures back to the submitter, which marks those orders failed while continuing with the artifacts that were built.
2 participants
Introduces a broker-side backend abstraction and router so order processing, cancellation, batching, and fulfillment artifact construction are dispatched by verifier selector/backend id instead of being hardwired directly into the broker services.