Skip to content

Commit

Permalink
set deprecated flags to no op, revert should_override_builder
Browse files Browse the repository at this point in the history
  • Loading branch information
eserilev committed Jan 5, 2024
1 parent e1bff37 commit 7a6cb6b
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 25 deletions.
7 changes: 2 additions & 5 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,13 @@ where
}

pub fn mock_execution_layer(self) -> Self {
self.mock_execution_layer_with_config(None)
self.mock_execution_layer_with_config()
}

pub fn mock_execution_layer_with_config(mut self, builder_threshold: Option<u128>) -> Self {
pub fn mock_execution_layer_with_config(mut self) -> Self {
let mock = mock_execution_layer_from_parts::<E>(
self.spec.as_ref().expect("cannot build without spec"),
self.runtime.task_executor.clone(),
builder_threshold,
);
self.execution_layer = Some(mock.el.clone());
self.mock_execution_layer = Some(mock);
Expand Down Expand Up @@ -574,7 +573,6 @@ where
pub fn mock_execution_layer_from_parts<T: EthSpec>(
spec: &ChainSpec,
task_executor: TaskExecutor,
builder_threshold: Option<u128>,
) -> MockExecutionLayer<T> {
let shanghai_time = spec.capella_fork_epoch.map(|epoch| {
HARNESS_GENESIS_TIME + spec.seconds_per_slot * T::slots_per_epoch() * epoch.as_u64()
Expand All @@ -593,7 +591,6 @@ pub fn mock_execution_layer_from_parts<T: EthSpec>(
DEFAULT_TERMINAL_BLOCK,
shanghai_time,
cancun_time,
builder_threshold,
Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()),
spec.clone(),
Some(kzg),
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/execution_layer/src/engine_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ pub struct GetPayloadResponse<T: EthSpec> {
pub block_value: Uint256,
#[superstruct(only(Deneb))]
pub blobs_bundle: BlobsBundle<T>,
#[superstruct(only(Deneb), partial_getter(copy))]
pub should_override_builder: bool,
}

impl<E: EthSpec> GetPayloadResponse<E> {
Expand Down
3 changes: 3 additions & 0 deletions beacon_node/execution_layer/src/engine_api/json_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ pub struct JsonGetPayloadResponse<T: EthSpec> {
pub block_value: Uint256,
#[superstruct(only(V3))]
pub blobs_bundle: JsonBlobsBundleV1<T>,
#[superstruct(only(V3))]
pub should_override_builder: bool,
}

impl<T: EthSpec> From<JsonGetPayloadResponse<T>> for GetPayloadResponse<T> {
Expand All @@ -320,6 +322,7 @@ impl<T: EthSpec> From<JsonGetPayloadResponse<T>> for GetPayloadResponse<T> {
execution_payload: response.execution_payload.into(),
block_value: response.block_value,
blobs_bundle: response.blobs_bundle.into(),
should_override_builder: response.should_override_builder,
})
}
}
Expand Down
12 changes: 12 additions & 0 deletions beacon_node/execution_layer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,18 @@ impl<T: EthSpec> ExecutionLayer<T> {
)));
}

if local.should_override_builder().unwrap_or(false) {
info!(
self.log(),
"Using local payload because execution engine suggested we ignore builder payload";
"local_block_value" => %local_value,
"relay_value" => %relay_value
);
return Ok(ProvenancedPayload::Local(BlockProposalContentsType::Full(
local.try_into()?,
)));
}

info!(
self.log(),
"Relay block is more profitable than local block";
Expand Down
1 change: 1 addition & 0 deletions beacon_node/execution_layer/src/test_utils/handle_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ pub async fn handle_rpc<T: EthSpec>(
GENERIC_ERROR_CODE,
))?
.into(),
should_override_builder: false,
})
.unwrap()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{
test_utils::{
MockServer, DEFAULT_BUILDER_THRESHOLD_WEI, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK,
DEFAULT_TERMINAL_DIFFICULTY,
MockServer, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK, DEFAULT_TERMINAL_DIFFICULTY,
},
Config, *,
};
Expand Down Expand Up @@ -30,7 +29,6 @@ impl<T: EthSpec> MockExecutionLayer<T> {
DEFAULT_TERMINAL_BLOCK,
None,
None,
None,
Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()),
spec,
None,
Expand All @@ -43,7 +41,6 @@ impl<T: EthSpec> MockExecutionLayer<T> {
terminal_block: u64,
shanghai_time: Option<u64>,
cancun_time: Option<u64>,
builder_threshold: Option<u128>,
jwt_key: Option<JwtKey>,
spec: ChainSpec,
kzg: Option<Kzg>,
Expand Down
1 change: 0 additions & 1 deletion beacon_node/execution_layer/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ pub use mock_execution_layer::MockExecutionLayer;
pub const DEFAULT_TERMINAL_DIFFICULTY: u64 = 6400;
pub const DEFAULT_TERMINAL_BLOCK: u64 = 64;
pub const DEFAULT_JWT_SECRET: [u8; 32] = [42; 32];
pub const DEFAULT_BUILDER_THRESHOLD_WEI: u128 = 1_000_000_000_000_000_000;
pub const DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI: u128 = 10_000_000_000_000_000;
pub const DEFAULT_BUILDER_PAYLOAD_VALUE_WEI: u128 = 20_000_000_000_000_000;
pub const DEFAULT_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilities {
Expand Down
22 changes: 7 additions & 15 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use eth2::{
BeaconNodeHttpClient, Error, StatusCode, Timeouts,
};
use execution_layer::test_utils::{
MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_BUILDER_THRESHOLD_WEI,
MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI,
DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI,
};
use futures::stream::{Stream, StreamExt};
Expand Down Expand Up @@ -79,8 +79,7 @@ struct ApiTester {

struct ApiTesterConfig {
spec: ChainSpec,
retain_historic_states: bool,
builder_threshold: Option<u128>,
retain_historic_states: bool
}

impl Default for ApiTesterConfig {
Expand All @@ -90,7 +89,6 @@ impl Default for ApiTesterConfig {
Self {
spec,
retain_historic_states: false,
builder_threshold: None,
}
}
}
Expand Down Expand Up @@ -132,7 +130,7 @@ impl ApiTester {
.logger(logging::test_logger())
.deterministic_keypairs(VALIDATOR_COUNT)
.fresh_ephemeral_store()
.mock_execution_layer_with_config(config.builder_threshold)
.mock_execution_layer_with_config()
.build();

harness
Expand Down Expand Up @@ -394,16 +392,12 @@ impl ApiTester {
tester
.mock_builder
.as_ref()
.unwrap()
.add_operation(Operation::Value(Uint256::from(
DEFAULT_BUILDER_THRESHOLD_WEI,
)));
.unwrap();
tester
}

pub async fn new_mev_tester_no_builder_threshold() -> Self {
pub async fn new_mev_tester_default_payload_value() -> Self {
let mut config = ApiTesterConfig {
builder_threshold: Some(0),
retain_historic_states: false,
spec: E::default_spec(),
};
Expand Down Expand Up @@ -6238,7 +6232,7 @@ async fn builder_chain_health_optimistic_head_v3() {

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn builder_payload_chosen_by_profit() {
ApiTester::new_mev_tester_no_builder_threshold()
ApiTester::new_mev_tester_default_payload_value()
.await
.test_builder_payload_chosen_when_more_profitable()
.await
Expand All @@ -6250,7 +6244,7 @@ async fn builder_payload_chosen_by_profit() {

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn builder_payload_chosen_by_profit_v3() {
ApiTester::new_mev_tester_no_builder_threshold()
ApiTester::new_mev_tester_default_payload_value()
.await
.test_builder_payload_v3_chosen_when_more_profitable()
.await
Expand All @@ -6263,7 +6257,6 @@ async fn builder_payload_chosen_by_profit_v3() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn builder_works_post_capella() {
let mut config = ApiTesterConfig {
builder_threshold: Some(0),
retain_historic_states: false,
spec: E::default_spec(),
};
Expand All @@ -6284,7 +6277,6 @@ async fn builder_works_post_capella() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn builder_works_post_deneb() {
let mut config = ApiTesterConfig {
builder_threshold: Some(0),
retain_historic_states: false,
spec: E::default_spec(),
};
Expand Down
29 changes: 29 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,24 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
conditions.")
.takes_value(false)
)
.arg(
Arg::with_name("builder-profit-threshold")
.long("builder-profit-threshold")
.value_name("WEI_VALUE")
.help("This flag is deprecated and has no effect.")
.hidden(true)
.default_value("0")
.takes_value(true)
)
.arg(
Arg::with_name("ignore-builder-override-suggestion-threshold")
.long("ignore-builder-override-suggestion-threshold")
.value_name("PERCENTAGE")
.help("This flag is deprecated and has no effect.")
.hidden(true)
.default_value("10.0")
.takes_value(true)
)
.arg(
Arg::with_name("builder-user-agent")
.long("builder-user-agent")
Expand Down Expand Up @@ -1173,6 +1191,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
This enables --http and --validator-monitor-auto and enables SSE logging.")
.takes_value(false)
)
.arg(
Arg::with_name("always-prefer-builder-payload")
.long("always-prefer-builder-payload")
.hidden(true)
.help("This flag is deprecated and has no effect.")
// The builder profit threshold flag is used to provide preference
// to local payloads, therefore it fundamentally conflicts with
// always using the builder.
.conflicts_with("builder-profit-threshold")
.conflicts_with("ignore-builder-override-suggestion-threshold")
)
.arg(
Arg::with_name("invalid-gossip-verified-blocks-path")
.long("invalid-gossip-verified-blocks-path")
Expand Down

0 comments on commit 7a6cb6b

Please sign in to comment.