Skip to content

Commit 5daf7f9

Browse files
committed
Improve testing around SSZ/validation bypassing. Fix issue where bypassing ignored mid_bid despite having the bid value available
1 parent a949c83 commit 5daf7f9

4 files changed

Lines changed: 647 additions & 29 deletions

File tree

crates/common/src/utils.rs

Lines changed: 220 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -886,18 +886,22 @@ where
886886
#[cfg(test)]
887887
mod test {
888888
use alloy::primitives::keccak256;
889-
use axum::http::{HeaderMap, HeaderValue};
890-
use reqwest::header::ACCEPT;
889+
use axum::http::{HeaderMap, HeaderName, HeaderValue};
890+
use bytes::Bytes;
891+
use reqwest::header::{ACCEPT, CONTENT_TYPE};
891892

892893
use super::{
893-
create_admin_jwt, create_jwt, decode_admin_jwt, decode_jwt, random_jwt_secret,
894-
validate_admin_jwt, validate_jwt,
894+
BodyDeserializeError, CONSENSUS_VERSION_HEADER, create_admin_jwt, create_jwt,
895+
decode_admin_jwt, decode_jwt, deserialize_body, get_consensus_version_header,
896+
get_content_type, random_jwt_secret, validate_admin_jwt, validate_jwt,
895897
};
896898
use crate::{
897899
constants::SIGNER_JWT_EXPIRATION,
900+
pbs::error::SszValueError,
898901
types::{Jwt, JwtAdminClaims, ModuleId},
899902
utils::{
900-
APPLICATION_JSON, APPLICATION_OCTET_STREAM, EncodingType, WILDCARD, get_accept_types,
903+
APPLICATION_JSON, APPLICATION_OCTET_STREAM, EncodingType, ForkName, WILDCARD,
904+
get_accept_types, get_bid_value_from_signed_builder_bid_ssz,
901905
},
902906
};
903907

@@ -1203,4 +1207,215 @@ mod test {
12031207
// Two calls should produce distinct values with overwhelming probability.
12041208
assert_ne!(secret, random_jwt_secret());
12051209
}
1210+
1211+
// ── get_content_type ─────────────────────────────────────────────────────
1212+
1213+
#[test]
1214+
fn test_content_type_missing_defaults_to_json() {
1215+
let headers = HeaderMap::new();
1216+
assert_eq!(get_content_type(&headers), EncodingType::Json);
1217+
}
1218+
1219+
#[test]
1220+
fn test_content_type_json() {
1221+
let mut headers = HeaderMap::new();
1222+
headers.insert(CONTENT_TYPE, HeaderValue::from_str(APPLICATION_JSON).unwrap());
1223+
assert_eq!(get_content_type(&headers), EncodingType::Json);
1224+
}
1225+
1226+
#[test]
1227+
fn test_content_type_ssz() {
1228+
let mut headers = HeaderMap::new();
1229+
headers.insert(CONTENT_TYPE, HeaderValue::from_str(APPLICATION_OCTET_STREAM).unwrap());
1230+
assert_eq!(get_content_type(&headers), EncodingType::Ssz);
1231+
}
1232+
1233+
#[test]
1234+
fn test_content_type_unknown_defaults_to_json() {
1235+
let mut headers = HeaderMap::new();
1236+
headers.insert(CONTENT_TYPE, HeaderValue::from_str("application/xml").unwrap());
1237+
assert_eq!(get_content_type(&headers), EncodingType::Json);
1238+
}
1239+
1240+
// ── get_consensus_version_header ─────────────────────────────────────────
1241+
1242+
#[test]
1243+
fn test_consensus_version_header_electra() {
1244+
let mut headers = HeaderMap::new();
1245+
let name = HeaderName::try_from(CONSENSUS_VERSION_HEADER).unwrap();
1246+
headers.insert(name, HeaderValue::from_str("electra").unwrap());
1247+
assert_eq!(get_consensus_version_header(&headers), Some(ForkName::Electra));
1248+
}
1249+
1250+
#[test]
1251+
fn test_consensus_version_header_missing() {
1252+
let headers = HeaderMap::new();
1253+
assert_eq!(get_consensus_version_header(&headers), None);
1254+
}
1255+
1256+
#[test]
1257+
fn test_consensus_version_header_invalid() {
1258+
let mut headers = HeaderMap::new();
1259+
let name = HeaderName::try_from(CONSENSUS_VERSION_HEADER).unwrap();
1260+
headers.insert(name, HeaderValue::from_str("not_a_fork").unwrap());
1261+
assert_eq!(get_consensus_version_header(&headers), None);
1262+
}
1263+
1264+
// ── EncodingType ─────────────────────────────────────────────────────────
1265+
1266+
#[test]
1267+
fn test_encoding_type_from_str_variants() {
1268+
use std::str::FromStr;
1269+
assert_eq!(EncodingType::from_str(APPLICATION_JSON).unwrap(), EncodingType::Json);
1270+
assert_eq!(EncodingType::from_str(APPLICATION_OCTET_STREAM).unwrap(), EncodingType::Ssz);
1271+
// empty string defaults to JSON per the impl
1272+
assert_eq!(EncodingType::from_str("").unwrap(), EncodingType::Json);
1273+
assert!(EncodingType::from_str("application/xml").is_err());
1274+
}
1275+
1276+
#[test]
1277+
fn test_encoding_type_display() {
1278+
assert_eq!(EncodingType::Json.to_string(), APPLICATION_JSON);
1279+
assert_eq!(EncodingType::Ssz.to_string(), APPLICATION_OCTET_STREAM);
1280+
}
1281+
1282+
// ── get_bid_value_from_signed_builder_bid_ssz ────────────────────────────
1283+
1284+
#[test]
1285+
fn test_ssz_value_extraction_unsupported_fork() {
1286+
let dummy_bytes = vec![0u8; 1000];
1287+
let err =
1288+
get_bid_value_from_signed_builder_bid_ssz(&dummy_bytes, ForkName::Altair).unwrap_err();
1289+
assert!(matches!(err, SszValueError::UnsupportedFork { .. }));
1290+
}
1291+
1292+
#[test]
1293+
fn test_ssz_value_extraction_truncated_payload() {
1294+
// A payload that is far too short for any supported fork's value offset
1295+
let tiny_bytes = vec![0u8; 4];
1296+
let err =
1297+
get_bid_value_from_signed_builder_bid_ssz(&tiny_bytes, ForkName::Electra).unwrap_err();
1298+
assert!(matches!(err, SszValueError::InvalidPayloadLength { .. }));
1299+
}
1300+
1301+
/// Per-fork positive tests: construct a `SignedBuilderBid` with a known
1302+
/// value for each supported fork, SSZ-encode it, and verify
1303+
/// `get_bid_value_from_signed_builder_bid_ssz` round-trips correctly.
1304+
#[test]
1305+
fn test_ssz_value_extraction_with_known_bid() {
1306+
use alloy::primitives::U256;
1307+
use ssz::Encode;
1308+
1309+
use crate::{
1310+
pbs::{
1311+
BuilderBid, BuilderBidBellatrix, BuilderBidCapella, BuilderBidDeneb,
1312+
BuilderBidElectra, BuilderBidFulu, ExecutionPayloadHeaderBellatrix,
1313+
ExecutionPayloadHeaderCapella, ExecutionPayloadHeaderDeneb,
1314+
ExecutionPayloadHeaderElectra, ExecutionPayloadHeaderFulu, ExecutionRequests,
1315+
SignedBuilderBid,
1316+
},
1317+
types::{BlsPublicKeyBytes, BlsSignature},
1318+
utils::TestRandomSeed,
1319+
};
1320+
1321+
// Distinctive value — large enough that endianness bugs produce a
1322+
// different number and zero-matches are impossible.
1323+
let known_value = U256::from(0x0102_0304_0506_0708_u64);
1324+
let pubkey = BlsPublicKeyBytes::test_random();
1325+
let sig = BlsSignature::test_random();
1326+
1327+
// ── Bellatrix ────────────────────────────────────────────────────────
1328+
{
1329+
let message = BuilderBid::Bellatrix(BuilderBidBellatrix {
1330+
header: ExecutionPayloadHeaderBellatrix::test_random(),
1331+
value: known_value,
1332+
pubkey,
1333+
});
1334+
let bid = SignedBuilderBid { message, signature: sig.clone() };
1335+
let got =
1336+
get_bid_value_from_signed_builder_bid_ssz(&bid.as_ssz_bytes(), ForkName::Bellatrix)
1337+
.expect("Bellatrix extraction failed");
1338+
assert_eq!(got, known_value, "Bellatrix: value mismatch");
1339+
}
1340+
1341+
// ── Capella ──────────────────────────────────────────────────────────
1342+
{
1343+
let message = BuilderBid::Capella(BuilderBidCapella {
1344+
header: ExecutionPayloadHeaderCapella::test_random(),
1345+
value: known_value,
1346+
pubkey,
1347+
});
1348+
let bid = SignedBuilderBid { message, signature: sig.clone() };
1349+
let got =
1350+
get_bid_value_from_signed_builder_bid_ssz(&bid.as_ssz_bytes(), ForkName::Capella)
1351+
.expect("Capella extraction failed");
1352+
assert_eq!(got, known_value, "Capella: value mismatch");
1353+
}
1354+
1355+
// ── Deneb ────────────────────────────────────────────────────────────
1356+
{
1357+
let message = BuilderBid::Deneb(BuilderBidDeneb {
1358+
header: ExecutionPayloadHeaderDeneb::test_random(),
1359+
blob_kzg_commitments: Default::default(),
1360+
value: known_value,
1361+
pubkey,
1362+
});
1363+
let bid = SignedBuilderBid { message, signature: sig.clone() };
1364+
let got =
1365+
get_bid_value_from_signed_builder_bid_ssz(&bid.as_ssz_bytes(), ForkName::Deneb)
1366+
.expect("Deneb extraction failed");
1367+
assert_eq!(got, known_value, "Deneb: value mismatch");
1368+
}
1369+
1370+
// ── Electra ──────────────────────────────────────────────────────────
1371+
{
1372+
let message = BuilderBid::Electra(BuilderBidElectra {
1373+
header: ExecutionPayloadHeaderElectra::test_random(),
1374+
blob_kzg_commitments: Default::default(),
1375+
execution_requests: ExecutionRequests::default(),
1376+
value: known_value,
1377+
pubkey,
1378+
});
1379+
let bid = SignedBuilderBid { message, signature: sig.clone() };
1380+
let got =
1381+
get_bid_value_from_signed_builder_bid_ssz(&bid.as_ssz_bytes(), ForkName::Electra)
1382+
.expect("Electra extraction failed");
1383+
assert_eq!(got, known_value, "Electra: value mismatch");
1384+
}
1385+
1386+
// ── Fulu ─────────────────────────────────────────────────────────────
1387+
{
1388+
let message = BuilderBid::Fulu(BuilderBidFulu {
1389+
header: ExecutionPayloadHeaderFulu::test_random(),
1390+
blob_kzg_commitments: Default::default(),
1391+
execution_requests: ExecutionRequests::default(),
1392+
value: known_value,
1393+
pubkey,
1394+
});
1395+
let bid = SignedBuilderBid { message, signature: sig };
1396+
let got =
1397+
get_bid_value_from_signed_builder_bid_ssz(&bid.as_ssz_bytes(), ForkName::Fulu)
1398+
.expect("Fulu extraction failed");
1399+
assert_eq!(got, known_value, "Fulu: value mismatch");
1400+
}
1401+
}
1402+
1403+
// ── deserialize_body error paths ─────────────────────────────────────────
1404+
1405+
#[tokio::test]
1406+
async fn test_deserialize_body_missing_content_type() {
1407+
let headers = HeaderMap::new();
1408+
let body = Bytes::from_static(b"{}");
1409+
let err = deserialize_body(&headers, body).await.unwrap_err();
1410+
assert!(matches!(err, BodyDeserializeError::UnsupportedMediaType));
1411+
}
1412+
1413+
#[tokio::test]
1414+
async fn test_deserialize_body_ssz_missing_version_header() {
1415+
let mut headers = HeaderMap::new();
1416+
headers.insert(CONTENT_TYPE, HeaderValue::from_str(APPLICATION_OCTET_STREAM).unwrap());
1417+
let body = Bytes::from_static(b"\x00\x01\x02\x03");
1418+
let err = deserialize_body(&headers, body).await.unwrap_err();
1419+
assert!(matches!(err, BodyDeserializeError::MissingVersionHeader));
1420+
}
12061421
}

crates/pbs/src/mev_boost/get_header.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,11 @@ pub async fn get_header<S: BuilderApiState>(
180180
[EncodingType::Ssz.content_type(), EncodingType::Json.content_type()].join(",")
181181
}
182182
};
183-
send_headers.insert(ACCEPT, HeaderValue::from_str(&accept_types).unwrap());
183+
send_headers.insert(
184+
ACCEPT,
185+
HeaderValue::from_str(&accept_types)
186+
.map_err(|e| PbsError::GeneralRequest(format!("invalid accept header value: {e}")))?,
187+
);
184188

185189
// Send requests to all relays concurrently
186190
let slot = params.slot as i64;
@@ -379,7 +383,9 @@ async fn send_one_get_header(
379383
) -> Result<(u64, Option<CompoundGetHeaderResponse>), PbsError> {
380384
match request_info.validation.mode {
381385
HeaderValidationMode::None => {
382-
// No validation so do some light processing and forward the response directly
386+
// Minimal processing: extract fork and value, forward response bytes directly.
387+
// Expensive crypto/structural validation is skipped (sigverify, parent hash,
388+
// timestamp), but the min_bid check is applied.
383389
let (start_request_time, get_header_response) = send_get_header_light(
384390
&relay,
385391
url,
@@ -393,6 +399,14 @@ async fn send_one_get_header(
393399
match get_header_response {
394400
None => Ok((start_request_time, None)),
395401
Some(res) => {
402+
let min_bid = request_info.validation.min_bid_wei;
403+
if res.value < min_bid {
404+
return Err(PbsError::Validation(ValidationError::BidTooLow {
405+
min: min_bid,
406+
got: res.value,
407+
}));
408+
}
409+
396410
// Make sure the response is encoded in one of the accepted
397411
// types since we're passing the raw response directly to the client
398412
if !request_info.accepted_types.contains(&res.encoding_type) {
@@ -540,8 +554,10 @@ async fn send_get_header_full(
540554
Ok((start_request_time, Some(get_header_response)))
541555
}
542556

543-
/// Send and decode a light get_header response, only extracting the fork and
544-
/// value from the builder bid.
557+
/// Send a get_header request and decode only the fork and bid value from the
558+
/// response, leaving the raw bytes intact for direct forwarding to the caller.
559+
/// Used in `HeaderValidationMode::None` where expensive crypto/structural
560+
/// checks are skipped.
545561
async fn send_get_header_light(
546562
relay: &RelayClient,
547563
url: Arc<Url>,
@@ -857,7 +873,7 @@ mod tests {
857873
use cb_common::{
858874
pbs::*,
859875
signature::sign_builder_message,
860-
types::{BlsSecretKey, Chain},
876+
types::{BlsPublicKeyBytes, BlsSecretKey, BlsSignature, Chain},
861877
utils::{TestRandomSeed, timestamp_of_slot_start_sec},
862878
};
863879
use ssz::Encode;

0 commit comments

Comments
 (0)