Skip to content

Commit

Permalink
[EASY] Bump response size limit (#3080)
Browse files Browse the repository at this point in the history
# Description
Since October 6th we see alerts like `Failed to buffer the request body:
length limit exceeded` indicating that the response returned by multiple
solvers (`oneinch`, `zeroex`, `paraswap`, `balancer`) is too big for the
driver.
It's unclear why this suddenly started happening on a Sunday where
nothing gets released or why it only seems to affect dex solvers but
making the max response size configurable seems like a reasonable thing
to do regardless.

# Changes
* Made drivers response size limit configurable and increased the
default value by 3x.
* Added trace log to print responses that exceed the limit to easily
debug this the next time this happens (disabled by default but be
temporarily enabled without restarting the pod)

## How to test
e2e test covers parsing of the new config parameter
  • Loading branch information
MartinquaXD authored Oct 23, 2024
1 parent 388486d commit 11df525
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 6 deletions.
1 change: 1 addition & 0 deletions crates/driver/example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ absolute-slippage = "40000000000000000" # Denominated in wei, optional
relative-slippage = "0.1" # Percentage in the [0, 1] range
account = "0x0000000000000000000000000000000000000000000000000000000000000001" # The private key of the solver
merge-solutions = true # Multiple solutions proposed by the solver may be combined into one by the driver
response-size-limit-max-bytes = 30000000

[solver.request-headers]
fake-header-one = "FAKE-HEADER-VALUE" # For instance an authorization token which must be provided on each request
Expand Down
1 change: 1 addition & 0 deletions crates/driver/src/infra/config/file/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub async fn load(chain: eth::ChainId, path: &Path) -> infra::Config {
s3: config.s3.map(Into::into),
solver_native_token: config.manage_native_token.to_domain(),
quote_tx_origin: config.quote_tx_origin.map(eth::Address),
response_size_limit_max_bytes: config.response_size_limit_max_bytes,
}
}))
.await,
Expand Down
8 changes: 8 additions & 0 deletions crates/driver/src/infra/config/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ struct SolverConfig {
/// Which `tx.origin` is required to make a quote simulation pass.
#[serde(default)]
quote_tx_origin: Option<eth::H160>,

/// Maximum HTTP response size the driver will accept in bytes.
#[serde(default = "default_response_size_limit_max_bytes")]
response_size_limit_max_bytes: usize,
}

#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize)]
Expand Down Expand Up @@ -590,6 +594,10 @@ fn default_http_timeout() -> Duration {
Duration::from_secs(10)
}

fn default_response_size_limit_max_bytes() -> usize {
30_000_000
}

#[derive(Clone, Debug, Deserialize, Default)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum GasEstimatorType {
Expand Down
8 changes: 4 additions & 4 deletions crates/driver/src/infra/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ use {

pub mod dto;

const SOLVER_RESPONSE_MAX_BYTES: usize = 10_000_000;

// TODO At some point I should be checking that the names are unique, I don't
// think I'm doing that.
/// The solver name. The user can configure this to be anything that they like.
Expand Down Expand Up @@ -124,6 +122,7 @@ pub struct Config {
pub solver_native_token: ManageNativeToken,
/// Which `tx.origin` is required to make quote verification pass.
pub quote_tx_origin: Option<eth::Address>,
pub response_size_limit_max_bytes: usize,
}

impl Solver {
Expand Down Expand Up @@ -234,7 +233,7 @@ impl Solver {
if let Some(id) = observe::request_id::get_task_local_storage() {
req = req.header("X-REQUEST-ID", id);
}
let res = util::http::send(SOLVER_RESPONSE_MAX_BYTES, req).await;
let res = util::http::send(self.config.response_size_limit_max_bytes, req).await;
super::observe::solver_response(&url, res.as_deref());
let res = res?;
let res: dto::Solutions = serde_json::from_str(&res)
Expand All @@ -260,8 +259,9 @@ impl Solver {
if let Some(id) = observe::request_id::get_task_local_storage() {
req = req.header("X-REQUEST-ID", id);
}
let response_size = self.config.response_size_limit_max_bytes;
let future = async move {
if let Err(error) = util::http::send(SOLVER_RESPONSE_MAX_BYTES, req).await {
if let Err(error) = util::http::send(response_size, req).await {
tracing::warn!(?error, "failed to notify solver");
}
};
Expand Down
8 changes: 6 additions & 2 deletions crates/driver/src/util/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ pub async fn send(limit_bytes: usize, req: reqwest::RequestBuilder) -> Result<St
let mut res = req.send().await?;
let mut data = Vec::new();
while let Some(chunk) = res.chunk().await? {
if data.len() + chunk.len() > limit_bytes {
data.extend_from_slice(&chunk);
if data.len() > limit_bytes {
tracing::trace!(
response = String::from_utf8_lossy(&data).as_ref(),
"response size exceeded"
);
return Err(Error::ResponseTooLarge { limit_bytes });
}
data.extend_from_slice(&chunk);
}
let body = String::from_utf8(data).map_err(Error::NotUtf8)?;
if res.status().is_success() {
Expand Down

0 comments on commit 11df525

Please sign in to comment.