Skip to content

Commit

Permalink
fix(api): Accept integer block count in eth_feeHistory (#3077)
Browse files Browse the repository at this point in the history
## What ❔

...besides hexadecimal count accepted previously.

## Why ❔

To be more compatible with Web3 tooling.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk_supervisor fmt` and `zk_supervisor
lint`.
  • Loading branch information
slowli authored Oct 14, 2024
1 parent bb5d147 commit 4d527d4
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 46 deletions.
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]);

// 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

0 comments on commit 4d527d4

Please sign in to comment.