Skip to content

Comments

fix: separate Base Sepolia and OP Sepolia BASE_FEE_PARAMS#8789

Merged
mattsse merged 3 commits intoparadigmxyz:mainfrom
ibremseth:op-sepolia-config
Jun 12, 2024
Merged

fix: separate Base Sepolia and OP Sepolia BASE_FEE_PARAMS#8789
mattsse merged 3 commits intoparadigmxyz:mainfrom
ibremseth:op-sepolia-config

Conversation

@ibremseth
Copy link
Contributor

Op-reth was not syncing for OP Sepolia and finally tracked down the issue to a bad setting when trying to calculate the next expected base fee. This PR is my attempt to fix it, but happy to implement another way!

From op-geth:

@ibremseth ibremseth changed the title Separate Base Sepolia and OP Sepolia BASE_FEE_PARAMS fix: separate Base Sepolia and OP Sepolia BASE_FEE_PARAMS Jun 12, 2024
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good, looks like it needs a cargo +nightly fmt

Comment on lines +114 to +141
fn calculate_base_sepolia_base_fee_success() {
let base_fee = [
1000000000, 1000000000, 1000000000, 1072671875, 1059263476, 1049238967, 1049238967, 0,
1, 2,
];
let gas_used = [
10000000, 10000000, 10000000, 9000000, 10001000, 0, 10000000, 10000000, 10000000,
10000000,
];
let gas_limit = [
10000000, 12000000, 14000000, 10000000, 14000000, 2000000, 18000000, 18000000,
18000000, 18000000,
];
let next_base_fee = [
1180000000, 1146666666, 1122857142, 1244299375, 1189416692, 1028254188, 1144836295, 1,
2, 3,
];

for i in 0..base_fee.len() {
assert_eq!(
next_base_fee[i],
calc_next_block_base_fee(
gas_used[i] as u128,
gas_limit[i] as u128,
base_fee[i] as u128,
BASE_SEPOLIA_BASE_FEE_PARAMS,
) as u64
);
Copy link
Member

@Rjected Rjected Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these from an op-geth test? or just adjusted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just adjusted from before! This was a copy of the original calculate_optimism_sepolia_base_fee_success test above (since it already was setup for using an elasticity multiplier of 10)

And then I changed calculate_optimism_sepolia_base_fee_success to expect the correct values when using an elasticity multiplier of 6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, makes sense

@mattsse mattsse added this pull request to the merge queue Jun 12, 2024
Merged via the queue into paradigmxyz:main with commit a80f012 Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants