Skip to content

Commit

Permalink
Revert "Eip 7709 implement gas costs (hyperledger#7983)"
Browse files Browse the repository at this point in the history
This reverts commit 5f1e6d1.
  • Loading branch information
lu-pinto committed Jan 22, 2025
1 parent 02f7a2e commit b9b55ac
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 94 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.hyperledger.besu.ethereum.mainnet.ClearEmptyAccountStrategy.NotClearEmptyAccount;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecBuilder.BlockValidatorBuilder;
import org.hyperledger.besu.ethereum.mainnet.blockhash.CancunBlockHashProcessor;
import org.hyperledger.besu.ethereum.mainnet.blockhash.Eip7709BlockHashProcessor;
import org.hyperledger.besu.ethereum.mainnet.blockhash.FrontierBlockHashProcessor;
import org.hyperledger.besu.ethereum.mainnet.blockhash.PragueBlockHashProcessor;
import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
Expand Down Expand Up @@ -992,7 +991,7 @@ static ProtocolSpecBuilder verkleDefinition(
CoinbaseFeePriceCalculator.eip1559()))
.withdrawalsProcessor(new WithdrawalsProcessor(clearEmptyAccountStrategy))
.executionWitnessValidator(new ExecutionWitnessValidator.AllowedExecutionWitness())
.blockHashProcessor(new Eip7709BlockHashProcessor())
.blockHashProcessor(new PragueBlockHashProcessor())
.blockHeaderFunctions(new VerkleDevnetBlockHeaderFunctions())
.name("Verkle");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
public class Eip7709BlockHashLookup implements BlockHashLookup {
private static final Logger LOG = LoggerFactory.getLogger(Eip7709BlockHashLookup.class);
private static final long BLOCKHASH_SERVE_WINDOW = 256L;
private static final long WARM_STORAGE_READ_COST = 100L;

private final Address contractAddress;
private final long historyServeWindow;
Expand Down Expand Up @@ -82,12 +81,6 @@ public Hash apply(final MessageFrame frame, final Long blockNumber) {
return ZERO;
}

final long cost = cost(frame, blockNumber);
frame.decrementRemainingGas(cost);
if (frame.getRemainingGas() < cost) {
return null;
}

final Hash cachedHash = hashByNumber.get(blockNumber);
if (cachedHash != null) {
return cachedHash;
Expand All @@ -112,19 +105,4 @@ public Hash apply(final MessageFrame frame, final Long blockNumber) {
hashByNumber.put(blockNumber, blockHash);
return blockHash;
}

private long cost(final MessageFrame frame, final long blockNumber) {
final UInt256 key = UInt256.valueOf(blockNumber % BLOCKHASH_SERVE_WINDOW);
long gas = frame.getAccessWitness().touchAndChargeStorageLoad(contractAddress, key);

if (gas == 0) {
return getWarmStorageReadCost();
}

return gas;
}

protected long getWarmStorageReadCost() {
return WARM_STORAGE_READ_COST;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package org.hyperledger.besu.ethereum.vm;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
Expand All @@ -25,7 +24,6 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.datatypes.AccessWitness;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Wei;
Expand All @@ -37,7 +35,6 @@
import org.hyperledger.besu.evm.fluent.SimpleWorld;
import org.hyperledger.besu.evm.frame.BlockValues;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.stateless.NoopAccessWitness;
import org.hyperledger.besu.evm.worldstate.WorldUpdater;

import java.util.ArrayList;
Expand All @@ -57,13 +54,11 @@ public class Eip7709BlockHashLookupTest {
private List<BlockHeader> headers;
private BlockHashLookup lookup;
private MessageFrame frame;
private WorldUpdater worldUpdater;

@BeforeEach
void setUp() {
headers = new ArrayList<>();
worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, Long.MAX_VALUE);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, createWorldUpdater(0, CURRENT_BLOCK_NUMBER));
lookup =
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
}
Expand All @@ -84,14 +79,12 @@ private WorldUpdater createWorldUpdater(final int fromBlockNumber, final int toB
}

private MessageFrame createMessageFrame(
final long currentBlockNumber, final WorldUpdater worldUpdater, final long remainingGas) {
final long currentBlockNumber, final WorldUpdater worldUpdater) {
final MessageFrame messageFrame = mock(MessageFrame.class);
final BlockValues blockValues = mock(BlockValues.class);
when(blockValues.getNumber()).thenReturn(currentBlockNumber);
when(messageFrame.getBlockValues()).thenReturn(blockValues);
when(messageFrame.getWorldUpdater()).thenReturn(worldUpdater);
when(messageFrame.getRemainingGas()).thenReturn(remainingGas);
when(messageFrame.getAccessWitness()).thenReturn(NoopAccessWitness.get());
return messageFrame;
}

Expand All @@ -106,52 +99,32 @@ void shouldGetHashOfMaxBlocksBehind() {
}

@Test
void shouldReturnZeroHashWhenSystemContractNotExists() {
worldUpdater = new SimpleWorld();
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, Long.MAX_VALUE);
assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.ZERO);
void shouldReturnEmptyHashWhenRequestedBlockHigherThanHead() {
assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
}

@Test
@SuppressWarnings("ReturnValueIgnored")
void shouldDecrementRemainingGasFromFrame() {
AccessWitness accessWitness = mock(AccessWitness.class);
when(accessWitness.touchAndChargeStorageLoad(any(), any())).thenReturn(100L);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, 200L);
when(frame.getAccessWitness()).thenReturn(accessWitness);
lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L);
verify(frame).decrementRemainingGas(eq(100L));
verify(frame).getBlockValues();
verify(frame).getAccessWitness();
verify(frame).getRemainingGas();
verify(frame).getWorldUpdater();
verifyNoMoreInteractions(frame);
}

@Test
void insufficientGasReturnsNullBlockHash() {
worldUpdater = new SimpleWorld();
AccessWitness accessWitness = mock(AccessWitness.class);
when(accessWitness.touchAndChargeStorageLoad(any(), any())).thenReturn(100L);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, 1L);
when(frame.getAccessWitness()).thenReturn(accessWitness);
final Hash blockHash = lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L);
assertThat(blockHash).isNull();
void shouldReturnEmptyHashWhenSystemContractNotExists() {
final WorldUpdater worldUpdater = new SimpleWorld();
when(frame.getWorldUpdater()).thenReturn(worldUpdater);
assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.ZERO);
}

@Test
void shouldReturnZeroHashWhenParentBlockNotInContract() {
worldUpdater = createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, Long.MAX_VALUE);
void shouldReturnEmptyHashWhenParentBlockNotInContract() {
frame =
createMessageFrame(
CURRENT_BLOCK_NUMBER,
createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER));
lookup =
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER - 20, Hash.ZERO);
}

@Test
void shouldCacheBlockHashes() {
worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, worldUpdater, Long.MAX_VALUE);
final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
when(frame.getWorldUpdater()).thenReturn(worldUpdater);
final Account account = worldUpdater.get(STORAGE_ADDRESS);
clearInvocations(account);

Expand All @@ -176,8 +149,7 @@ void shouldCacheBlockHashes() {

@Test
void shouldGetHashWhenParentIsGenesis() {
worldUpdater = createWorldUpdater(0, 1);
frame = createMessageFrame(1, worldUpdater, Long.MAX_VALUE);
frame = createMessageFrame(1, createWorldUpdater(0, 1));
lookup =
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
assertHashForBlockNumber(0);
Expand All @@ -191,14 +163,13 @@ void shouldReturnZeroWhenRequestedBlockEqualToImportingBlock() {
@Test
void shouldReturnZeroWhenRequestedBlockAheadOfCurrent() {
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER + 1, Hash.ZERO);
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER + 20, Hash.ZERO);
}

@Test
void shouldReturnZeroWhenRequestedBlockTooFarBehindCurrent() {
assertHashForBlockNumber(
Math.toIntExact(CURRENT_BLOCK_NUMBER - BLOCKHASH_SERVE_WINDOW - 1), Hash.ZERO);
assertHashForBlockNumber(2, Hash.ZERO);
assertHashForBlockNumber(10, Hash.ZERO);
}

private void assertHashForBlockNumber(final int blockNumber) {
Expand Down
2 changes: 1 addition & 1 deletion ethereum/referencetests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ dependencies {
}
verkleRefTestImplemention dependencies.create('ethereum:execution-spec-tests') {
version {
strictly '[email protected].9-alpha-1'
strictly '[email protected].8'
}
artifact {
name = 'fixtures'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,11 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
return new OperationResult(cost, null);
}

final long remainingGas = frame.getRemainingGas();
final BlockHashLookup blockHashLookup = frame.getBlockHashLookup();
final Hash blockHash = blockHashLookup.apply(frame, blockArg.toLong());
final long lookupCost = remainingGas - frame.getRemainingGas();
// give lookupCost back as it will be taken after
frame.incrementRemainingGas(lookupCost);
if (blockHash == null) {
return new OperationResult(cost + lookupCost, ExceptionalHaltReason.INSUFFICIENT_GAS);
}
frame.pushStackItem(blockHash);

return new OperationResult(cost + lookupCost, null);
return new OperationResult(cost, null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,9 @@ void shouldFailWithInsufficientGas() {
ExceptionalHaltReason.INSUFFICIENT_GAS,
200,
(__, ___) -> Hash.hash(Bytes.fromHexString("0x1293487297")),
1,
1);
}

@Test
void shouldFailWithInsufficientGasBlockHashLookup() {
assertFailure(
Bytes32.fromHexString("0x64"),
ExceptionalHaltReason.INSUFFICIENT_GAS,
200,
(__, ___) -> null,
21L,
0);
}

private void assertBlockHash(
final long requestedBlock,
final Bytes32 expectedOutput,
Expand Down Expand Up @@ -116,8 +104,7 @@ private void assertFailure(
final ExceptionalHaltReason haltReason,
final long currentBlockNumber,
final BlockHashLookup blockHashLookup,
final long initialGas,
final int stackSize) {
final long initialGas) {
final MessageFrame frame =
new TestMessageFrameBuilder()
.blockHashLookup(blockHashLookup)
Expand All @@ -127,6 +114,6 @@ private void assertFailure(
.build();
Operation.OperationResult operationResult = blockHashOperation.execute(frame, null);
assertThat(operationResult.getHaltReason()).isEqualTo(haltReason);
assertThat(frame.stackSize()).isEqualTo(stackSize);
assertThat(frame.stackSize()).isOne();
}
}

0 comments on commit b9b55ac

Please sign in to comment.