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.

Sign up for GitHub

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

Jump to bottom

Fix eip2929 implementation #280

Merged
merged 9 commits into from
Jun 4, 2024
Merged

Fix eip2929 implementation #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1e31869
burn base fee after eip-1559 included in london fork
RomarQ Apr 19, 2024
654fb20
feat: add Cancun configuration
RomarQ May 8, 2024
80d2d3c
fix(eip-2929): some addresses were considered cold when they shouldn't
RomarQ May 8, 2024
4d29b50
Merge branch 'master' into rq/gas-costs
RomarQ Jun 4, 2024
6e9a7bd
Revert "feat: add Cancun configuration"
RomarQ Jun 4, 2024
7430679
Revert "burn base fee after eip-1559 included in london fork"
RomarQ Jun 4, 2024
b6f5bb4
Merge branch 'master' into rq/gas-costs
RomarQ Jun 4, 2024
caed3bc
add eip-2929 test
RomarQ Jun 4, 2024
c931676
remove duplicated test
RomarQ Jun 4, 2024
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
4 changes: 3 additions & 1 deletion .github/workflows/rust.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,6 @@ jobs:
jsontests/res/ethtests/GeneralStateTests/VMTests/vmBitwiseLogicOperation/ \
jsontests/res/ethtests/GeneralStateTests/VMTests/vmIOandFlowOperations/ \
jsontests/res/ethtests/GeneralStateTests/VMTests/vmLogTest/ \
jsontests/res/ethtests/GeneralStateTests/VMTests/vmTests/
jsontests/res/ethtests/GeneralStateTests/VMTests/vmTests/ \
jsontests/res/ethtests/GeneralStateTests/stEIP150singleCodeGasPrices/eip2929.json

7 changes: 0 additions & 7 deletions interpreter/src/eval/system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ pub fn balance<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, T
handler: &mut H,
) -> Control<Tr> {
pop!(machine, address);
handler.mark_hot(address.into(), None);
push_u256!(machine, handler.balance(address.into()));

Control::Continue
Expand Down Expand Up @@ -130,7 +129,6 @@ pub fn extcodesize<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
handler: &mut H,
) -> Control<Tr> {
pop!(machine, address);
handler.mark_hot(address.into(), None);
let code_size = handler.code_size(address.into());
push_u256!(machine, code_size);

Expand All @@ -142,7 +140,6 @@ pub fn extcodehash<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
handler: &mut H,
) -> Control<Tr> {
pop!(machine, address);
handler.mark_hot(address.into(), None);
let code_hash = handler.code_hash(address.into());
push!(machine, code_hash);

Expand All @@ -155,8 +152,6 @@ pub fn extcodecopy<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBacken
) -> Control<Tr> {
pop!(machine, address);
pop_u256!(machine, memory_offset, code_offset, len);

handler.mark_hot(address.into(), None);
try_or_fail!(machine.memory.resize_offset(memory_offset, len));

let code = handler.code(address.into());
Expand Down Expand Up @@ -266,7 +261,6 @@ pub fn sload<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, Tr>
handler: &mut H,
) -> Control<Tr> {
pop!(machine, index);
handler.mark_hot(machine.state.as_ref().context.address, Some(index));
let value = handler.storage(machine.state.as_ref().context.address, index);
push!(machine, value);

Expand All @@ -278,7 +272,6 @@ pub fn sstore<S: AsRef<RuntimeState>, H: RuntimeEnvironment + RuntimeBackend, Tr
handler: &mut H,
) -> Control<Tr> {
pop!(machine, index, value);
handler.mark_hot(machine.state.as_ref().context.address, Some(index));

match handler.set_storage(machine.state.as_ref().context.address, index, value) {
Ok(()) => Control::Continue,
Expand Down
8 changes: 8 additions & 0 deletions jsontests/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,11 @@ fn vm_tests() {
let tests_status = run::run_single(JSON_FILENAME, false).unwrap();
tests_status.print_total();
}

#[test]
fn sqt_eip_2930() {
const JSON_FILENAME: &str =
"res/ethtests/GeneralStateTests/stEIP150singleCodeGasPrices/eip2929.json";
let tests_status = run::run_single(JSON_FILENAME, false).unwrap();
tests_status.print_total();
}
87 changes: 67 additions & 20 deletions src/standard/gasometer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
stack: &Stack,
is_static: bool,
config: &Config,
handler: &H,
handler: &mut H,
) -> Result<(GasCost, Option<MemoryCost>), ExitError> {
let gas_cost = match opcode {
Opcode::RETURN => GasCost::Zero,
Expand All @@ -307,40 +307,59 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(

Opcode::EXTCODESIZE => {
let target = stack.peek(0)?.into();
GasCost::ExtCodeSize {
target_is_cold: handler.is_cold(target, None),
}

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder is this move (from interpreter to gasometer), related to the fix?

The reason why it was put in interpreter because the code is supposed to support custom gasometers. Otherwise with this change, any custom meters will not properly mark hot/cold.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in some circumstances, the addresses were not being considered as warm when they should. If the objective is to have the code as customisable as possible, probably having it in the gasometer is not that bad.


GasCost::ExtCodeSize { target_is_cold }
}
Opcode::BALANCE => {
let target = stack.peek(0)?.into();
GasCost::Balance {
target_is_cold: handler.is_cold(target, None),
}

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::Balance { target_is_cold }
}
Opcode::BLOCKHASH => GasCost::BlockHash,

Opcode::EXTCODEHASH if config.has_ext_code_hash => {
let target = stack.peek(0)?.into();
GasCost::ExtCodeHash {
target_is_cold: handler.is_cold(target, None),
}

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::ExtCodeHash { target_is_cold }
}
Opcode::EXTCODEHASH => GasCost::Invalid(opcode),

Opcode::CALLCODE => {
let target = stack.peek(1)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::CallCode {
value: U256::from_big_endian(&stack.peek(2)?[..]),
gas: U256::from_big_endian(&stack.peek(0)?[..]),
target_is_cold: handler.is_cold(target, None),
target_is_cold,
target_exists: { handler.exists(target) },
}
}
Opcode::STATICCALL => {
let target = stack.peek(1)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::StaticCall {
gas: U256::from_big_endian(&stack.peek(0)?[..]),
target_is_cold: handler.is_cold(target, None),
target_is_cold,
target_exists: { handler.exists(target) },
}
}
Expand All @@ -349,8 +368,13 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
},
Opcode::EXTCODECOPY => {
let target = stack.peek(0)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::ExtCodeCopy {
target_is_cold: handler.is_cold(target, None),
target_is_cold,
len: U256::from_big_endian(&stack.peek(3)?[..]),
}
}
Expand All @@ -365,17 +389,25 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
},
Opcode::SLOAD => {
let index = stack.peek(0)?;
GasCost::SLoad {
target_is_cold: handler.is_cold(address, Some(index)),
}

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(address, Some(index));
handler.mark_hot(address, Some(index));

GasCost::SLoad { target_is_cold }
}
Opcode::TLOAD if config.eip_1153_enabled => GasCost::TLoad,

Opcode::DELEGATECALL if config.has_delegate_call => {
let target = stack.peek(1)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::DelegateCall {
gas: U256::from_big_endian(&stack.peek(0)?[..]),
target_is_cold: handler.is_cold(target, None),
target_is_cold,
target_exists: { handler.exists(target) },
}
}
Expand All @@ -390,11 +422,16 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
Opcode::SSTORE if !is_static => {
let index = stack.peek(0)?;
let value = stack.peek(1)?;

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(address, Some(index));
handler.mark_hot(address, Some(index));

GasCost::SStore {
original: handler.original_storage(address, index),
current: handler.storage(address, index),
new: value,
target_is_cold: handler.is_cold(address, Some(index)),
target_is_cold,
}
}
Opcode::TSTORE if !is_static && config.eip_1153_enabled => GasCost::TStore,
Expand Down Expand Up @@ -424,9 +461,14 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
},
Opcode::SUICIDE if !is_static => {
let target = stack.peek(0)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::Suicide {
value: handler.balance(address),
target_is_cold: handler.is_cold(target, None),
target_is_cold,
target_exists: { handler.exists(target) },
already_removed: handler.deleted(address),
}
Expand All @@ -436,10 +478,15 @@ fn dynamic_opcode_cost<H: RuntimeBackend>(
|| (is_static && U256::from_big_endian(&stack.peek(2)?[..]) == U256::zero()) =>
{
let target = stack.peek(1)?.into();

// https://eips.ethereum.org/EIPS/eip-2929
let target_is_cold = handler.is_cold(target, None);
handler.mark_hot(target, None);

GasCost::Call {
value: U256::from_big_endian(&stack.peek(2)?[..]),
gas: U256::from_big_endian(&stack.peek(0)?[..]),
target_is_cold: handler.is_cold(target, None),
target_is_cold,
target_exists: { handler.exists(target) },
}
}
Expand Down