Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): Accept integer block count in eth_feeHistory #3077

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions core/lib/basic_types/src/web3/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,35 @@ mod tests;

pub type Index = U64;

/// Number that can be either hex-encoded or decimal.
#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
#[serde(untagged)]
pub enum U64Number {
Hex(U64),
Number(u64),
}

impl From<U64Number> for u64 {
fn from(value: U64Number) -> Self {
match value {
U64Number::Hex(number) => number.as_u64(),
U64Number::Number(number) => number,
}
}
}

impl From<u64> for U64Number {
fn from(value: u64) -> Self {
Self::Number(value)
}
}

impl From<U64> for U64Number {
fn from(value: U64) -> Self {
Self::Hex(value)
}
}

// `Signature`, `keccak256`: from `web3::signing`

/// A struct that represents the components of a secp256k1 signature.
Expand Down
10 changes: 10 additions & 0 deletions core/lib/basic_types/src/web3/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,13 @@ fn test_bytes_serde_json() {
let decoded: Bytes = serde_json::from_str(&encoded).unwrap();
assert_eq!(original, decoded);
}

#[test]
fn deserializing_u64_number() {
let number: U64Number = serde_json::from_value(serde_json::json!(123)).unwrap();
assert_eq!(u64::from(number), 123);
let number: U64Number = serde_json::from_value(serde_json::json!("0x123")).unwrap();
assert_eq!(u64::from(number), 0x123);
let number: U64Number = serde_json::from_value(serde_json::json!("123")).unwrap();
assert_eq!(u64::from(number), 0x123);
}
16 changes: 6 additions & 10 deletions core/lib/eth_client/src/clients/http/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,16 +396,12 @@ where
let chunk_end = (chunk_start + FEE_HISTORY_MAX_REQUEST_CHUNK).min(upto_block);
let chunk_size = chunk_end - chunk_start;

let fee_history = EthNamespaceClient::fee_history(
client,
U64::from(chunk_size),
zksync_types::api::BlockNumber::from(chunk_end),
vec![],
)
.rpc_context("fee_history")
.with_arg("chunk_size", &chunk_size)
.with_arg("block", &chunk_end)
.await?;
let fee_history = client
.fee_history(U64::from(chunk_size).into(), chunk_end.into(), vec![])
.rpc_context("fee_history")
.with_arg("chunk_size", &chunk_size)
.with_arg("block", &chunk_end)
.await?;

// Check that the lengths are the same.
if fee_history.inner.base_fee_per_gas.len() != fee_history.l2_pubdata_price.len() {
Expand Down
5 changes: 3 additions & 2 deletions core/lib/web3_decl/src/namespaces/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use zksync_types::{
use crate::{
client::{ForWeb3Network, L2},
types::{
Block, Bytes, Filter, FilterChanges, Index, Log, SyncState, TransactionReceipt, U256, U64,
Block, Bytes, Filter, FilterChanges, Index, Log, SyncState, TransactionReceipt, U64Number,
U256, U64,
},
};

Expand Down Expand Up @@ -180,7 +181,7 @@ pub trait EthNamespace {
#[method(name = "feeHistory")]
async fn fee_history(
&self,
block_count: U64,
block_count: U64Number,
newest_block: BlockNumber,
reward_percentiles: Vec<f32>,
) -> RpcResult<FeeHistory>;
Expand Down
4 changes: 3 additions & 1 deletion core/lib/web3_decl/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
pub use zksync_types::{
api::{Block, BlockNumber, Log, TransactionReceipt, TransactionRequest},
ethabi,
web3::{BlockHeader, Bytes, CallRequest, FeeHistory, Index, SyncState, TraceFilter, Work},
web3::{
BlockHeader, Bytes, CallRequest, FeeHistory, Index, SyncState, TraceFilter, U64Number, Work,
},
Address, Transaction, H160, H256, H64, U256, U64,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use zksync_types::{
Log, Transaction, TransactionId, TransactionReceipt, TransactionVariant,
},
transaction_request::CallRequest,
web3::{Bytes, Index, SyncState},
web3::{Bytes, Index, SyncState, U64Number},
Address, H256, U256, U64,
};
use zksync_web3_decl::{
Expand Down Expand Up @@ -260,11 +260,11 @@ impl EthNamespaceServer for EthNamespace {

async fn fee_history(
&self,
block_count: U64,
block_count: U64Number,
newest_block: BlockNumber,
reward_percentiles: Vec<f32>,
) -> RpcResult<FeeHistory> {
self.fee_history_impl(block_count, newest_block, reward_percentiles)
self.fee_history_impl(block_count.into(), newest_block, reward_percentiles)
.await
.map_err(|err| self.current_method().map_err(err))
}
Expand Down
7 changes: 2 additions & 5 deletions core/node/api_server/src/web3/namespaces/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,18 +683,15 @@ impl EthNamespace {

pub async fn fee_history_impl(
&self,
block_count: U64,
block_count: u64,
newest_block: BlockNumber,
reward_percentiles: Vec<f32>,
) -> Result<FeeHistory, Web3Error> {
self.current_method()
.set_block_id(BlockId::Number(newest_block));

// Limit `block_count`.
let block_count = block_count
.as_u64()
.min(self.state.api_config.fee_history_limit)
.max(1);
let block_count = block_count.clamp(1, self.state.api_config.fee_history_limit);

let mut connection = self.state.acquire_connection().await?;
let newest_l2_block = self
Expand Down
123 changes: 122 additions & 1 deletion core/node/api_server/src/web3/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use zksync_multivm::interface::{
TransactionExecutionMetrics, TransactionExecutionResult, TxExecutionStatus, VmEvent,
VmExecutionMetrics,
};
use zksync_node_fee_model::BatchFeeModelInputProvider;
use zksync_node_genesis::{insert_genesis_batch, mock_genesis_config, GenesisParams};
use zksync_node_test_utils::{
create_l1_batch, create_l1_batch_metadata, create_l2_block, create_l2_transaction,
Expand All @@ -32,6 +33,7 @@ use zksync_system_constants::{
use zksync_types::{
api,
block::{pack_block_info, L2BlockHasher, L2BlockHeader},
fee_model::{BatchFeeInput, FeeParams},
get_nonce_key,
l2::L2Tx,
storage::get_code_key,
Expand All @@ -54,7 +56,7 @@ use zksync_web3_decl::{
http_client::HttpClient,
rpc_params,
types::{
error::{ErrorCode, OVERSIZED_RESPONSE_CODE},
error::{ErrorCode, INVALID_PARAMS_CODE, OVERSIZED_RESPONSE_CODE},
ErrorObjectOwned,
},
},
Expand Down Expand Up @@ -435,6 +437,14 @@ async fn store_events(
Ok((tx_location, events))
}

fn scaled_sensible_fee_input(scale: f64) -> BatchFeeInput {
<dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
scale,
scale,
)
}

#[derive(Debug)]
struct HttpServerBasicsTest;

Expand Down Expand Up @@ -1228,3 +1238,114 @@ impl HttpTest for GetBytecodeTest {
async fn getting_bytecodes() {
test_http_server(GetBytecodeTest).await;
}

#[derive(Debug)]
struct FeeHistoryTest;

#[async_trait]
impl HttpTest for FeeHistoryTest {
async fn test(
&self,
client: &DynClient<L2>,
pool: &ConnectionPool<Core>,
) -> anyhow::Result<()> {
let mut connection = pool.connection().await?;
let block1 = L2BlockHeader {
batch_fee_input: scaled_sensible_fee_input(1.0),
base_fee_per_gas: 100,
..create_l2_block(1)
};
store_custom_l2_block(&mut connection, &block1, &[]).await?;
let block2 = L2BlockHeader {
batch_fee_input: scaled_sensible_fee_input(2.0),
base_fee_per_gas: 200,
..create_l2_block(2)
};
store_custom_l2_block(&mut connection, &block2, &[]).await?;

let all_pubdata_prices = [
0,
block1.batch_fee_input.fair_pubdata_price(),
block2.batch_fee_input.fair_pubdata_price(),
]
.map(U256::from);

let history = client
.fee_history(1_000.into(), api::BlockNumber::Latest, vec![])
.await?;
assert_eq!(history.inner.oldest_block, 0.into());
assert_eq!(
history.inner.base_fee_per_gas,
[0, 100, 200, 200].map(U256::from) // The latest value is duplicated
);
assert_eq!(history.l2_pubdata_price, all_pubdata_prices);
// Values below are not filled.
assert_eq!(history.inner.gas_used_ratio, [0.0; 3]);
assert_eq!(history.inner.base_fee_per_blob_gas, [U256::zero(); 4]);
assert_eq!(history.inner.blob_gas_used_ratio, [0.0; 3]);
slowli marked this conversation as resolved.
Show resolved Hide resolved

// Check supplying hexadecimal block count
let hex_history: api::FeeHistory = client
.request(
"eth_feeHistory",
rpc_params!["0xaa", "latest", [] as [f64; 0]],
)
.await?;
assert_eq!(hex_history, history);

// ...and explicitly decimal count (which should've been supplied in the first call) for exhaustiveness
let dec_history: api::FeeHistory = client
.request(
"eth_feeHistory",
rpc_params![1_000, "latest", [] as [f64; 0]],
)
.await?;
assert_eq!(dec_history, history);

// Check partial histories: blocks 0..=1
let history = client
.fee_history(1_000.into(), api::BlockNumber::Number(1.into()), vec![])
.await?;
assert_eq!(history.inner.oldest_block, 0.into());
assert_eq!(
history.inner.base_fee_per_gas,
[0, 100, 100].map(U256::from)
);
assert_eq!(history.l2_pubdata_price, all_pubdata_prices[..2]);

// Blocks 1..=2
let history = client
.fee_history(2.into(), api::BlockNumber::Latest, vec![])
.await?;
assert_eq!(history.inner.oldest_block, 1.into());
assert_eq!(
history.inner.base_fee_per_gas,
[100, 200, 200].map(U256::from)
);
assert_eq!(history.l2_pubdata_price, all_pubdata_prices[1..]);

// Blocks 1..=1
let history = client
.fee_history(1.into(), api::BlockNumber::Number(1.into()), vec![])
.await?;
assert_eq!(history.inner.oldest_block, 1.into());
assert_eq!(history.inner.base_fee_per_gas, [100, 100].map(U256::from));
assert_eq!(history.l2_pubdata_price, all_pubdata_prices[1..2]);

// Non-existing newest block.
let err = client
.fee_history(1000.into(), api::BlockNumber::Number(100.into()), vec![])
.await
.unwrap_err();
assert_matches!(
err,
ClientError::Call(err) if err.code() == INVALID_PARAMS_CODE
);
Ok(())
}
}

#[tokio::test]
async fn getting_fee_history() {
test_http_server(FeeHistoryTest).await;
}
30 changes: 6 additions & 24 deletions core/node/api_server/src/web3/tests/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@ use api::state_override::{OverrideAccount, StateOverride};
use zksync_multivm::interface::{
ExecutionResult, VmExecutionLogs, VmExecutionResultAndLogs, VmRevertReason,
};
use zksync_node_fee_model::BatchFeeModelInputProvider;
use zksync_types::{
api::ApiStorageLog,
fee_model::{BatchFeeInput, FeeParams},
get_intrinsic_constants,
transaction_request::CallRequest,
K256PrivateKey, L2ChainId, PackedEthSignature, StorageLogKind, StorageLogWithPreviousValue,
U256,
api::ApiStorageLog, fee_model::BatchFeeInput, get_intrinsic_constants,
transaction_request::CallRequest, K256PrivateKey, L2ChainId, PackedEthSignature,
StorageLogKind, StorageLogWithPreviousValue, U256,
};
use zksync_utils::u256_to_h256;
use zksync_vm_executor::oneshot::MockOneshotExecutor;
Expand All @@ -42,11 +38,7 @@ impl ExpectedFeeInput {
fn expect_for_block(&self, number: api::BlockNumber, scale: f64) {
*self.0.lock().unwrap() = match number {
api::BlockNumber::Number(number) => create_l2_block(number.as_u32()).batch_fee_input,
_ => <dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
scale,
scale,
),
_ => scaled_sensible_fee_input(scale),
};
}

Expand Down Expand Up @@ -165,12 +157,7 @@ impl HttpTest for CallTest {
// Check that the method handler fetches fee inputs for recent blocks. To do that, we create a new block
// with a large fee input; it should be loaded by `ApiFeeInputProvider` and override the input provided by the wrapped mock provider.
let mut block_header = create_l2_block(2);
block_header.batch_fee_input =
<dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
2.5,
2.5,
);
block_header.batch_fee_input = scaled_sensible_fee_input(2.5);
store_custom_l2_block(&mut connection, &block_header, &[]).await?;
// Fee input is not scaled further as per `ApiFeeInputProvider` implementation
self.fee_input.expect_custom(block_header.batch_fee_input);
Expand Down Expand Up @@ -607,12 +594,7 @@ impl HttpTest for TraceCallTest {
// Check that the method handler fetches fee inputs for recent blocks. To do that, we create a new block
// with a large fee input; it should be loaded by `ApiFeeInputProvider` and override the input provided by the wrapped mock provider.
let mut block_header = create_l2_block(2);
block_header.batch_fee_input =
<dyn BatchFeeModelInputProvider>::default_batch_fee_input_scaled(
FeeParams::sensible_v1_default(),
3.0,
3.0,
);
block_header.batch_fee_input = scaled_sensible_fee_input(3.0);
store_custom_l2_block(&mut connection, &block_header, &[]).await?;
// Fee input is not scaled further as per `ApiFeeInputProvider` implementation
self.fee_input.expect_custom(block_header.batch_fee_input);
Expand Down
Loading