Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
tomg10 committed Oct 7, 2024
1 parent f53b00e commit fd506c2
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 32 deletions.
10 changes: 5 additions & 5 deletions core/lib/config/src/configs/eth_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl EthConfig {
pubdata_sending_mode: PubdataSendingMode::Calldata,
tx_aggregation_paused: false,
tx_aggregation_only_prove_and_execute: false,
time_in_mempool_cap: 1800,
time_in_mempool_in_l1_blocks_cap: 1800,
}),
gas_adjuster: Some(GasAdjusterConfig {
default_priority_fee_per_gas: 1000000000,
Expand Down Expand Up @@ -130,8 +130,8 @@ pub struct SenderConfig {
pub tx_aggregation_only_prove_and_execute: bool,

/// Cap of time in mempool for price calculations
#[serde(default = "SenderConfig::default_time_in_mempool_cap")]
pub time_in_mempool_cap: u32,
#[serde(default = "SenderConfig::default_time_in_mempool_in_l1_blocks_cap")]
pub time_in_mempool_in_l1_blocks_cap: u32,
}

impl SenderConfig {
Expand Down Expand Up @@ -174,9 +174,9 @@ impl SenderConfig {
false
}

pub const fn default_time_in_mempool_cap() -> u32 {
pub const fn default_time_in_mempool_in_l1_blocks_cap() -> u32 {
let blocks_per_hour = 3600 / 12;
// cap it at 6h to not allow nearly infinite values when a tx is stuck for a long time
// we cap it at 6h to not allow nearly infinite values when a tx is stuck for a long time
// 1,001 ^ 1800 ~= 6, so by default we cap exponential price formula at roughly median * 6
blocks_per_hour * 6
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/config/src/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ impl Distribution<configs::eth_sender::SenderConfig> for EncodeDist {
pubdata_sending_mode: PubdataSendingMode::Calldata,
tx_aggregation_paused: false,
tx_aggregation_only_prove_and_execute: false,
time_in_mempool_cap: self.sample(rng),
time_in_mempool_in_l1_blocks_cap: self.sample(rng),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/lib/env_config/src/eth_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod tests {
pubdata_sending_mode: PubdataSendingMode::Calldata,
tx_aggregation_only_prove_and_execute: false,
tx_aggregation_paused: false,
time_in_mempool_cap: 2000,
time_in_mempool_in_l1_blocks_cap: 2000,
}),
gas_adjuster: Some(GasAdjusterConfig {
default_priority_fee_per_gas: 20000000000,
Expand Down Expand Up @@ -132,7 +132,7 @@ mod tests {
ETH_SENDER_SENDER_TIMESTAMP_CRITERIA_MAX_ALLOWED_LAG="30"
ETH_SENDER_SENDER_MAX_AGGREGATED_TX_GAS="4000000"
ETH_SENDER_SENDER_MAX_ETH_TX_DATA_SIZE="120000"
ETH_SENDER_SENDER_TIME_IN_MEMPOOL_CAP="2000"
ETH_SENDER_SENDER_TIME_IN_MEMPOOL_IN_L1_BLOCKS_CAP="2000"
ETH_SENDER_SENDER_L1_BATCH_MIN_AGE_BEFORE_EXECUTE_SECONDS="1000"
ETH_SENDER_SENDER_MAX_ACCEPTABLE_PRIORITY_FEE_IN_GWEI="100000000000"
ETH_SENDER_SENDER_PUBDATA_SENDING_MODE="Calldata"
Expand Down
8 changes: 4 additions & 4 deletions core/lib/protobuf_config/src/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ impl ProtoRepr for proto::Sender {
.parse(),
tx_aggregation_only_prove_and_execute: self.tx_aggregation_paused.unwrap_or(false),
tx_aggregation_paused: self.tx_aggregation_only_prove_and_execute.unwrap_or(false),
time_in_mempool_cap: self
.time_in_mempool_cap
.unwrap_or(Self::Type::default_time_in_mempool_cap()),
time_in_mempool_in_l1_blocks_cap: self
.time_in_mempool_in_l1_blocks_cap
.unwrap_or(Self::Type::default_time_in_mempool_in_l1_blocks_cap()),
})
}

Expand Down Expand Up @@ -150,7 +150,7 @@ impl ProtoRepr for proto::Sender {
),
tx_aggregation_only_prove_and_execute: Some(this.tx_aggregation_only_prove_and_execute),
tx_aggregation_paused: Some(this.tx_aggregation_paused),
time_in_mempool_cap: Some(this.time_in_mempool_cap),
time_in_mempool_in_l1_blocks_cap: Some(this.time_in_mempool_in_l1_blocks_cap),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/lib/protobuf_config/src/proto/config/eth_sender.proto
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ message Sender {
reserved 19; reserved "proof_loading_mode";
optional bool tx_aggregation_paused = 20; // required
optional bool tx_aggregation_only_prove_and_execute = 21; // required
optional uint32 time_in_mempool_cap = 22; // optional
optional uint32 time_in_mempool_in_l1_blocks_cap = 22; // optional
}

message GasAdjuster {
Expand Down
21 changes: 13 additions & 8 deletions core/node/eth_sender/src/eth_fees_oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) trait EthFeesOracle: 'static + Sync + Send + fmt::Debug {
fn calculate_fees(
&self,
previous_sent_tx: &Option<TxHistory>,
time_in_mempool: u32,
time_in_mempool_in_l1_blocks: u32,
operator_type: OperatorType,
) -> Result<EthFees, EthSenderError>;
}
Expand All @@ -32,7 +32,7 @@ pub(crate) trait EthFeesOracle: 'static + Sync + Send + fmt::Debug {
pub(crate) struct GasAdjusterFeesOracle {
pub gas_adjuster: Arc<dyn TxParamsProvider>,
pub max_acceptable_priority_fee_in_gwei: u64,
pub time_in_mempool_cap: u32,
pub time_in_mempool_in_l1_blocks_cap: u32,
}

impl GasAdjusterFeesOracle {
Expand Down Expand Up @@ -81,11 +81,16 @@ impl GasAdjusterFeesOracle {
fn calculate_fees_no_blob_sidecar(
&self,
previous_sent_tx: &Option<TxHistory>,
time_in_mempool: u32,
time_in_mempool_in_l1_blocks: u32,
) -> Result<EthFees, EthSenderError> {
// cap it at 6h to not allow nearly infinite values when a tx is stuck for a long time
let capped_time_in_mempool = min(time_in_mempool, self.time_in_mempool_cap);
let mut base_fee_per_gas = self.gas_adjuster.get_base_fee(capped_time_in_mempool);
// we cap it to not allow nearly infinite values when a tx is stuck for a long time
let capped_time_in_mempool_in_l1_blocks = min(
time_in_mempool_in_l1_blocks,
self.time_in_mempool_in_l1_blocks_cap,
);
let mut base_fee_per_gas = self
.gas_adjuster
.get_base_fee(capped_time_in_mempool_in_l1_blocks);
self.assert_fee_is_not_zero(base_fee_per_gas, "base");
if let Some(previous_sent_tx) = previous_sent_tx {
self.verify_base_fee_not_too_low_on_resend(
Expand Down Expand Up @@ -163,14 +168,14 @@ impl EthFeesOracle for GasAdjusterFeesOracle {
fn calculate_fees(
&self,
previous_sent_tx: &Option<TxHistory>,
time_in_mempool: u32,
time_in_mempool_in_l1_blocks: u32,
operator_type: OperatorType,
) -> Result<EthFees, EthSenderError> {
let has_blob_sidecar = operator_type == OperatorType::Blob;
if has_blob_sidecar {
self.calculate_fees_with_blob_sidecar(previous_sent_tx)
} else {
self.calculate_fees_no_blob_sidecar(previous_sent_tx, time_in_mempool)
self.calculate_fees_no_blob_sidecar(previous_sent_tx, time_in_mempool_in_l1_blocks)
}
}
}
15 changes: 10 additions & 5 deletions core/node/eth_sender/src/eth_tx_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl EthTxManager {
let fees_oracle = GasAdjusterFeesOracle {
gas_adjuster,
max_acceptable_priority_fee_in_gwei: config.max_acceptable_priority_fee_in_gwei,
time_in_mempool_cap: config.time_in_mempool_cap,
time_in_mempool_in_l1_blocks_cap: config.time_in_mempool_in_l1_blocks_cap,
};
let l1_interface = Box::new(RealL1Interface {
ethereum_gateway,
Expand Down Expand Up @@ -112,7 +112,7 @@ impl EthTxManager {
&mut self,
storage: &mut Connection<'_, Core>,
tx: &EthTx,
time_in_mempool: u32,
time_in_mempool_in_l1_blocks: u32,
current_block: L1BlockNumber,
) -> Result<H256, EthSenderError> {
let previous_sent_tx = storage
Expand All @@ -128,7 +128,7 @@ impl EthTxManager {
pubdata_price: _,
} = self.fees_oracle.calculate_fees(
&previous_sent_tx,
time_in_mempool,
time_in_mempool_in_l1_blocks,
self.operator_type(tx),
)?;

Expand Down Expand Up @@ -602,13 +602,18 @@ impl EthTxManager {
.await?
{
// New gas price depends on the time this tx spent in mempool.
let time_in_mempool = l1_block_numbers.latest.0 - sent_at_block;
let time_in_mempool_in_l1_blocks = l1_block_numbers.latest.0 - sent_at_block;

// We don't want to return early in case resend does not succeed -
// the error is logged anyway, but early returns will prevent
// sending new operations.
let _ = self
.send_eth_tx(storage, &tx, time_in_mempool, l1_block_numbers.latest)
.send_eth_tx(
storage,
&tx,
time_in_mempool_in_l1_blocks,
l1_block_numbers.latest,
)
.await?;
}
Ok(())
Expand Down
6 changes: 3 additions & 3 deletions core/node/fee_model/src/l1_gas_price/gas_adjuster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,14 @@ impl TxParamsProvider for GasAdjuster {
// smooth out base_fee increases in general.
// In other words, in order to pay less fees, we are ready to wait longer.
// But the longer we wait, the more we are ready to pay.
fn get_base_fee(&self, time_in_mempool: u32) -> u64 {
fn get_base_fee(&self, time_in_mempool_in_l1_blocks: u32) -> u64 {
let a = self.config.pricing_formula_parameter_a;
let b = self.config.pricing_formula_parameter_b;

// Currently we use an exponential formula.
// The alternative is a linear one:
// `let scale_factor = a + b * time_in_mempool as f64;`
let scale_factor = a * b.powf(time_in_mempool as f64);
// `let scale_factor = a + b * time_in_mempool_in_l1_blocks as f64;`
let scale_factor = a * b.powf(time_in_mempool_in_l1_blocks as f64);
let median = self.base_fee_statistics.median();
METRICS.median_base_fee_per_gas.set(median);
let new_fee = median as f64 * scale_factor;
Expand Down
2 changes: 1 addition & 1 deletion core/node/fee_model/src/l1_gas_price/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod main_node_fetcher;
/// This trait, as a bound, should only be used in components that actually sign and send transactions.
pub trait TxParamsProvider: fmt::Debug + 'static + Send + Sync {
/// Returns the recommended `max_fee_per_gas` value (EIP1559).
fn get_base_fee(&self, time_in_mempool: u32) -> u64;
fn get_base_fee(&self, time_in_mempool_in_l1_blocks: u32) -> u64;

/// Returns the recommended `max_priority_fee_per_gas` value (EIP1559).
fn get_priority_fee(&self) -> u64;
Expand Down
4 changes: 2 additions & 2 deletions etc/env/base/eth_sender.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ default_priority_fee_per_gas = 1_000_000_000
max_base_fee_samples = 10_000
# These two are parameters of the base_fee_per_gas formula in GasAdjuster.
# The possible formulas are:
# 1. base_fee_median * (A + B * time_in_mempool)
# 2. base_fee_median * A * B ^ time_in_mempool
# 1. base_fee_median * (A + B * time_in_mempool_in_l1_blocks)
# 2. base_fee_median * A * B ^ time_in_mempool_in_l1_blocks
# Currently the second is used.
# To confirm, see core/bin/zksync_core/src/eth_sender/gas_adjuster/mod.rs
pricing_formula_parameter_a = 1.5
Expand Down

0 comments on commit fd506c2

Please sign in to comment.