Skip to content

Conversation

einar-polygon
Copy link
Contributor

@einar-polygon einar-polygon commented Oct 17, 2024

This is a quick attempt to fix the bug align the unexpected behavior in block 19794433 tx 0xb487d37c4f13407af55797add295c127f19ea6c823b8ba4e53fe1a3457d902c9 where JUMPs get performed and their destination added to the JUMPDEST table despite of an Out-of-Gas condition.

I am going to add a regression test as part of this PR.

Offending instruction from RPC structlog:

{
 "step": 1198,
 "curr_depth": 2,
 "tx_hash": "0xb487d37c4f13407af55797add295c127f19ea6c823b8ba4e53fe1a3457d902c9",
 "code_hash": "0xb9c1c929064cd21734c102a698e68bf617feefcfa5a9f62407c45401546736bf",
 "ctx": 2,
 "pc": 563,
 "pc_hex": "00000233",
 "gas": 2,
 "gas_cost": 10,
 "op": "JUMPI",
 "entry": "StructLog { pc: 563, op: \"JUMPI\", gas: 2, gas_cost: 10, depth: 2, error: None, stack: Some([599290589, 10000, 46305234306404416513646796018318532483072908513802138792889746402715613072618, 131758842981497199554169577046225233044076042219, 294, 131758842981497199554169577046225233044076042219, 128, 10000, 0, 0, 288, 0, 1, 567]), return_data: None, memory: None, memory_size: None, storage: None, refund_counter: None }"
}

@einar-polygon einar-polygon added the bug Something isn't working label Oct 17, 2024
@einar-polygon einar-polygon self-assigned this Oct 17, 2024
@github-actions github-actions bot added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label Oct 17, 2024
@Nashtare
Copy link
Collaborator

Nashtare commented Oct 17, 2024

Just to clarify, is this "bug" affecting develop too or is it purely related to the structlogs interaction in the JD branch?
I'd tend to believe it's the latter (in which case we may want to target your opened PR instead of develop, but I haven't tested it)

@einar-polygon
Copy link
Contributor Author

I am arguing for the former as follows:

develop does:

perform_op
check_gas

where this PR does:

check_gas
perform_op

The consequence is, that the jump is performed before the OOG is detected. The endpoint does not perform the jump.
This difference results in the extra JD offset in the simulation.

0000022d: DUP3
0000022e: DUP1
0000022f: ISZERO
00000230: PUSH2 0x237
00000233: JUMPI        <---- OOG
00000234: DUP3
00000235: DUP3
00000236: RETURN
00000237: JUMPDEST     <---- shouldn't be in the reached set.

@Nashtare
Copy link
Collaborator

Hmm, the current develop doesn't run into any issues with the failed JUMP though (I just tested it, as OOG exceptions are properly caught), this ordering logic seems specific to the way structlogs report the ops.

As long as the switch is harmless with simulations, it's fine to merge against develop.

@einar-polygon einar-polygon removed the bug Something isn't working label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crate: evm_arithmetization Anything related to the evm_arithmetization crate.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants