Skip to content

Commit 3e2a2a7

Browse files
Handle Inconsistent Response in Miner Api (flashbots/rollup-boost#221)
* Handle Inconsistent Response in Miner Api * Fix log message * retry layer * just handle miner_setMaxDASize * rename attempts -> retries * remove http retry layer * fix tests * don't update on retry * switch to join handle abort * typo * cleanup match in send_to_l2 * fix tests * cleanup, fix tests * cleanup * comment * remove stray comment * add tests * Extra test * add more logs * cleanup and fix test * Add additional test case * add additional test case * remove stray comment --------- Co-authored-by: Ferran Borreguero <ferran.borreguero@gmail.com>
1 parent 91ae37d commit 3e2a2a7

17 files changed

Lines changed: 570 additions & 111 deletions

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jsonrpsee = { version = "0.24", features = ["server", "http-client", "macros"] }
1818
moka = { version = "0.12.10", features = ["future"] }
1919
http = "1.1.0"
2020
dotenvy = "0.15.7"
21-
tower = "0.4.13"
21+
tower = { version = "0.4.13", features = ["timeout"] }
2222
tower-http = { version = "0.5.2", features = [
2323
"decompression-full",
2424
"sensitive-headers",

src/cli.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
use std::{net::SocketAddr, path::PathBuf};
1+
use std::{net::SocketAddr, path::PathBuf, sync::Arc};
22

33
use alloy_rpc_types_engine::JwtSecret;
44
use clap::{Parser, Subcommand};
55
use eyre::bail;
66
use jsonrpsee::{RpcModule, server::Server};
7+
use parking_lot::Mutex;
78
use tokio::signal::unix::{SignalKind, signal as unix_signal};
89
use tracing::{Level, info};
910

@@ -159,12 +160,13 @@ impl Args {
159160

160161
let (probe_layer, probes) = ProbeLayer::new();
161162

163+
let execution_mode = Arc::new(Mutex::new(self.execution_mode));
162164
let rollup_boost = RollupBoostServer::new(
163165
l2_client,
164166
builder_client,
165-
self.execution_mode,
167+
execution_mode.clone(),
166168
self.block_selection_policy,
167-
probes,
169+
probes.clone(),
168170
self.health_check_interval,
169171
self.max_unsafe_interval,
170172
);
@@ -185,6 +187,8 @@ impl Args {
185187
l2_auth_jwt,
186188
builder_args.builder_url,
187189
builder_auth_jwt,
190+
probes,
191+
execution_mode,
188192
));
189193

190194
let server = Server::builder()

src/client/http.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,30 @@
1+
use std::time::Duration;
2+
13
use crate::client::auth::AuthLayer;
24
use crate::payload::PayloadSource;
5+
use alloy_primitives::bytes::Bytes;
36
use alloy_rpc_types_engine::JwtSecret;
47
use http::Uri;
5-
use http_body_util::BodyExt;
8+
use http_body_util::{BodyExt, Full};
9+
use hyper::body::Body;
610
use hyper_rustls::HttpsConnector;
711
use hyper_util::client::legacy::Client;
812
use hyper_util::client::legacy::connect::HttpConnector;
913
use hyper_util::rt::TokioExecutor;
1014
use jsonrpsee::core::BoxError;
11-
use jsonrpsee::http_client::HttpBody;
15+
use jsonrpsee::server::HttpBody;
1216
use opentelemetry::trace::SpanKind;
13-
use tower::{Service as _, ServiceBuilder, ServiceExt};
17+
use tower::{
18+
Service as _, ServiceBuilder, ServiceExt,
19+
timeout::{Timeout, TimeoutLayer},
20+
};
1421
use tower_http::decompression::{Decompression, DecompressionLayer};
1522
use tracing::{debug, error, instrument};
1623

1724
use super::auth::Auth;
1825

19-
pub type HttpClientService = Decompression<Auth<Client<HttpsConnector<HttpConnector>, HttpBody>>>;
26+
pub type HttpClientService =
27+
Timeout<Decompression<Auth<Client<HttpsConnector<HttpConnector>, HttpBody>>>>;
2028

2129
#[derive(Clone, Debug)]
2230
pub struct HttpClient {
@@ -38,6 +46,7 @@ impl HttpClient {
3846
let client = Client::builder(TokioExecutor::new()).build(connector);
3947

4048
let client = ServiceBuilder::new()
49+
.layer(TimeoutLayer::new(Duration::from_secs(1)))
4150
.layer(DecompressionLayer::new())
4251
.layer(AuthLayer::new(secret))
4352
.service(client);
@@ -52,36 +61,41 @@ impl HttpClient {
5261
/// Forwards an HTTP request to the `authrpc`, attaching the provided JWT authorization.
5362
#[instrument(
5463
skip(self, req),
55-
fields(otel.kind = ?SpanKind::Client,
56-
url = %self.url,
57-
method,
58-
code,
64+
fields(
65+
otel.kind = ?SpanKind::Client,
66+
url = %self.url,
67+
method,
68+
code,
5969
),
6070
err(Debug)
6171
)]
62-
pub async fn forward(
72+
pub async fn forward<B>(
6373
&mut self,
64-
mut req: http::Request<HttpBody>,
74+
mut req: http::Request<B>,
6575
method: String,
66-
) -> Result<http::Response<HttpBody>, BoxError> {
76+
) -> Result<http::Response<Full<Bytes>>, BoxError>
77+
where
78+
B: Body<Data = Bytes, Error: Into<Box<dyn std::error::Error + Send + Sync>>>
79+
+ Send
80+
+ 'static,
81+
{
6782
debug!("forwarding {} to {}", method, self.target);
6883
tracing::Span::current().record("method", method);
6984
*req.uri_mut() = self.url.clone();
7085

86+
let req = req.map(HttpBody::new);
87+
7188
let res = self.client.ready().await?.call(req).await?;
7289

7390
let (parts, body) = res.into_parts();
74-
let body_bytes = body.collect().await?.to_bytes().to_vec();
91+
let body_bytes = body.collect().await?.to_bytes();
7592

7693
if let Some(code) = parse_response_code(&body_bytes)? {
7794
error!(%code, "error in forwarded response");
7895
tracing::Span::current().record("code", code);
7996
}
8097

81-
Ok(http::Response::from_parts(
82-
parts,
83-
HttpBody::from(body_bytes),
84-
))
98+
Ok(http::Response::from_parts(parts, Full::from(body_bytes)))
8599
}
86100
}
87101

src/consistent_request.rs

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
//! consistent_request.rs
2+
use std::{
3+
sync::{
4+
Arc,
5+
atomic::{AtomicBool, Ordering},
6+
},
7+
time::Duration,
8+
};
9+
10+
use eyre::{Result as EyreResult, bail};
11+
use jsonrpsee::{core::BoxError, http_client::HttpBody};
12+
use parking_lot::Mutex;
13+
use tokio::{sync::watch, task::JoinHandle};
14+
use tracing::{error, info, warn};
15+
16+
use crate::{
17+
BufferedRequest, BufferedResponse, ExecutionMode, Health, HttpClient, Probes, Response,
18+
};
19+
20+
/// A request manager that ensures requests are sent consistently
21+
/// across both the builder and l2 client.
22+
#[derive(Clone)]
23+
pub struct ConsistentRequest {
24+
method: String,
25+
l2_client: HttpClient,
26+
builder_client: HttpClient,
27+
has_disabled_execution_mode: Arc<AtomicBool>,
28+
req_tx: watch::Sender<Option<BufferedRequest>>,
29+
res_rx: watch::Receiver<Option<Result<BufferedResponse, BoxError>>>,
30+
probes: Arc<Probes>,
31+
execution_mode: Arc<Mutex<ExecutionMode>>,
32+
}
33+
34+
impl ConsistentRequest {
35+
pub fn new(
36+
method: String,
37+
l2_client: HttpClient,
38+
builder_client: HttpClient,
39+
probes: Arc<Probes>,
40+
execution_mode: Arc<Mutex<ExecutionMode>>,
41+
) -> Self {
42+
let (req_tx, mut req_rx) = watch::channel(None);
43+
let (res_tx, mut res_rx) = watch::channel(None);
44+
req_rx.mark_unchanged();
45+
res_rx.mark_unchanged();
46+
47+
let has_disabled_execution_mode = Arc::new(AtomicBool::new(false));
48+
49+
let manager = Self {
50+
method,
51+
l2_client,
52+
builder_client,
53+
has_disabled_execution_mode,
54+
req_tx,
55+
res_rx,
56+
probes,
57+
execution_mode,
58+
};
59+
60+
let clone = manager.clone();
61+
tokio::spawn(async move {
62+
let mut attempt: Option<JoinHandle<_>> = None;
63+
loop {
64+
req_rx
65+
.changed()
66+
.await
67+
.expect("channel should always be open");
68+
69+
if let Some(attempt) = attempt {
70+
if !attempt.is_finished() {
71+
error!(
72+
target: "proxy::call",
73+
method = clone.method,
74+
"request cancelled"
75+
);
76+
attempt.abort();
77+
}
78+
}
79+
80+
let req = req_rx
81+
.borrow_and_update()
82+
.as_ref()
83+
.expect("value should always be Some")
84+
.clone();
85+
86+
let mut clone = clone.clone();
87+
let res_tx_clone = res_tx.clone();
88+
attempt = Some(tokio::spawn(async move {
89+
clone.send_with_retry_cancel_safe(req, res_tx_clone).await
90+
}));
91+
}
92+
});
93+
94+
manager
95+
}
96+
97+
/// This function may be cancelled at any time from an incoming request.
98+
async fn send_with_retry_cancel_safe(
99+
&mut self,
100+
req: BufferedRequest,
101+
res_tx: watch::Sender<Option<Result<BufferedResponse, BoxError>>>,
102+
) -> EyreResult<()> {
103+
// We send the l2 request first, because we need to avoid the situation where the
104+
// l2 fails and the builder succeeds. If this were to happen, it would be too dangerous to
105+
// return the l2 error response back to the caller, since the builder would now have an
106+
// invalid state. We can return early if the l2 request fails. Note we're specifically
107+
// spawning new tasks here to avoid any issues with cancellation. We should ensure we're
108+
// in a valid state at all await points.
109+
let mut manager = self.clone();
110+
let res_tx_clone = res_tx.clone();
111+
let req_clone = req.clone();
112+
tokio::spawn(async move { manager.send_to_l2(req_clone, res_tx_clone).await }).await??;
113+
114+
loop {
115+
let mut manager_clone = self.clone();
116+
let req_clone = req.clone();
117+
118+
match tokio::spawn(async move { manager_clone.send_to_builder(req_clone).await })
119+
.await?
120+
{
121+
Ok(_) => return Ok(()),
122+
Err(_) => tokio::time::sleep(Duration::from_millis(200)).await,
123+
}
124+
}
125+
}
126+
127+
async fn send_to_l2(
128+
&mut self,
129+
req: BufferedRequest,
130+
res_tx: watch::Sender<Option<Result<BufferedResponse, BoxError>>>,
131+
) -> EyreResult<()> {
132+
let l2_res = self.l2_client.forward(req, self.method.clone()).await;
133+
match l2_res {
134+
Ok(_) => {
135+
res_tx.send(Some(l2_res))?;
136+
Ok(())
137+
}
138+
Err(_) => {
139+
res_tx.send(Some(l2_res))?;
140+
Err(eyre::eyre!("failed to send request to L2 client"))
141+
}
142+
}
143+
}
144+
145+
async fn send_to_builder(&mut self, req: BufferedRequest) -> EyreResult<()> {
146+
match self.builder_client.forward(req, self.method.clone()).await {
147+
Ok(_) => {
148+
if self.has_disabled_execution_mode.load(Ordering::SeqCst) {
149+
let mut mode = self.execution_mode.lock();
150+
*mode = ExecutionMode::Enabled;
151+
drop(mode);
152+
self.has_disabled_execution_mode
153+
.store(false, Ordering::SeqCst);
154+
info!(target: "proxy::call", message = "setting execution mode to Enabled");
155+
}
156+
Ok(())
157+
}
158+
Err(e) => {
159+
// l2 request succeeded, but builder request failed
160+
// This state can only be recovered from if either the builder is restarted
161+
// or if a retry eventually goes through.
162+
error!(target: "proxy::call", method = self.method, "inconsistent responses from builder and L2");
163+
let mut mode = self.execution_mode.lock();
164+
if *mode == ExecutionMode::Enabled {
165+
*mode = ExecutionMode::Disabled;
166+
// Drop before aquiring health lock
167+
drop(mode);
168+
self.has_disabled_execution_mode
169+
.store(true, Ordering::SeqCst);
170+
warn!(target: "proxy::call", "setting execution mode to Disabled");
171+
// This health status will likely be later set back to healthy
172+
// but this should be enough to trigger a new leader election.
173+
self.probes.set_health(Health::PartialContent);
174+
}
175+
176+
bail!("failed to send request to builder: {e}");
177+
}
178+
}
179+
}
180+
181+
// Send a request, ensuring consistent responses from both the builder and l2 client.
182+
pub async fn send(&mut self, req: BufferedRequest) -> Result<Response, BoxError> {
183+
self.req_tx.send(Some(req))?;
184+
self.res_rx.changed().await?;
185+
match self
186+
.res_rx
187+
.borrow_and_update()
188+
.as_ref()
189+
.expect("value should always be Some")
190+
{
191+
Ok(v) => Ok(v.clone().map(HttpBody::new)),
192+
Err(e) => Err(format!("error sending consistent request: {e}").into()),
193+
}
194+
}
195+
}

src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(clippy::complexity)]
2+
13
mod client;
24
pub use client::{auth::*, http::*, rpc::*};
35

@@ -30,3 +32,5 @@ pub use payload::*;
3032

3133
mod selection;
3234
pub use selection::*;
35+
36+
mod consistent_request;

0 commit comments

Comments
 (0)