Skip to content

Commit

Permalink
fixup! eip-7709 implement BLOCKHASH opcode from system contract state
Browse files Browse the repository at this point in the history 
 address review comments

Signed-off-by: Luis Pinto <[email protected]>
  • Loading branch information
@lu-pinto
lu-pinto committed Dec 17, 2024
1 parent 2bc7fc6 commit 71e2965
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 45 deletions.
4 changes: 2 additions & 2 deletions .../main/java/org/hyperledger/besu/ethereum/mainnet/blockhash/Eip7709BlockHashProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

import org.hyperledger.besu.ethereum.chain.Blockchain;
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
import org.hyperledger.besu.ethereum.vm.ContractBasedBlockHashLookup;
import org.hyperledger.besu.ethereum.vm.Eip7709BlockHashLookup;
import org.hyperledger.besu.evm.blockhash.BlockHashLookup;

public class Eip7709BlockHashProcessor extends PragueBlockHashProcessor {

@Override
public BlockHashLookup createBlockHashLookup(
final Blockchain blockchain, final ProcessableBlockHeader blockHeader) {
return new ContractBasedBlockHashLookup(blockHeader, historyStorageAddress, historyServeWindow);
return new Eip7709BlockHashLookup(historyStorageAddress, historyServeWindow);
}
}
29 changes: 10 additions & 19 deletions ...reum/vm/ContractBasedBlockHashLookup.java → ...u/ethereum/vm/Eip7709BlockHashLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
import org.hyperledger.besu.evm.account.Account;
import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
import org.hyperledger.besu.evm.frame.MessageFrame;
Expand All @@ -35,58 +34,50 @@
* Retrieves block hashes from system contract storage and caches hashes by number, used by
* BLOCKHASH operation.
*/
public class ContractBasedBlockHashLookup implements BlockHashLookup {
private static final Logger LOG = LoggerFactory.getLogger(ContractBasedBlockHashLookup.class);
public class Eip7709BlockHashLookup implements BlockHashLookup {
private static final Logger LOG = LoggerFactory.getLogger(Eip7709BlockHashLookup.class);
private static final long BLOCKHASH_SERVE_WINDOW = 256L;

private final ProcessableBlockHeader blockHeader;
private final Address contractAddress;
private final long historyServeWindow;
private final long blockHashServeWindow;
private final HashMap<Long, Hash> hashByNumber = new HashMap<>();

/**
* Constructs a ContractBasedBlockHashLookup.
* Constructs a Eip7709BlockHashLookup.
*
* @param currentBlock current block header being processed.
* @param contractAddress the address of the contract storing the history.
* @param historyServeWindow the number of blocks for which history should be saved.
*/
public ContractBasedBlockHashLookup(
final ProcessableBlockHeader currentBlock,
final Address contractAddress,
final long historyServeWindow) {
this(currentBlock, contractAddress, historyServeWindow, BLOCKHASH_SERVE_WINDOW);
public Eip7709BlockHashLookup(final Address contractAddress, final long historyServeWindow) {
this(contractAddress, historyServeWindow, BLOCKHASH_SERVE_WINDOW);
}

/**
* Constructs a ContractBasedBlockHashLookup with a specified blockHashServeWindow. This
* constructor is only used for testing.
* Constructs a Eip7709BlockHashLookup with a specified blockHashServeWindow. This constructor is
* only used for testing.
*
* @param currentBlock current block header being processed.
* @param contractAddress the address of the contract storing the history.
* @param historyServeWindow the number of blocks for which history should be saved.
* @param blockHashServeWindow the number of block for which contract can serve the BLOCKHASH
* opcode.
*/
@VisibleForTesting
ContractBasedBlockHashLookup(
final ProcessableBlockHeader currentBlock,
Eip7709BlockHashLookup(
final Address contractAddress,
final long historyServeWindow,
final long blockHashServeWindow) {
this.blockHeader = currentBlock;
this.contractAddress = contractAddress;
this.historyServeWindow = historyServeWindow;
this.blockHashServeWindow = blockHashServeWindow;
}

@Override
public Hash apply(final MessageFrame frame, final Long blockNumber) {
final long currentBlockNumber = blockHeader.getNumber();
final long currentBlockNumber = frame.getBlockValues().getNumber();
final long minBlockServe = Math.max(0, currentBlockNumber - blockHashServeWindow);
if (blockNumber >= currentBlockNumber || blockNumber < minBlockServe) {
LOG.debug("failed to read hash from system account for block {}", blockNumber);
LOG.trace("failed to read hash from system account for block {}", blockNumber);
return ZERO;
}

Expand Down
45 changes: 21 additions & 24 deletions .../vm/ContractBasedBlockHashLookupTest.java → ...hereum/vm/Eip7709BlockHashLookupTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
import org.hyperledger.besu.evm.fluent.SimpleAccount;
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.worldstate.WorldUpdater;

Expand All @@ -44,7 +45,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class ContractBasedBlockHashLookupTest {
public class Eip7709BlockHashLookupTest {
private static final long BLOCKHASH_SERVE_WINDOW = 160;
private static final Address STORAGE_ADDRESS = Address.fromHexString("0x0");
private static final long HISTORY_SERVE_WINDOW = 200L;
Expand All @@ -57,15 +58,9 @@ public class ContractBasedBlockHashLookupTest {
@BeforeEach
void setUp() {
headers = new ArrayList<>();
frame = mock(MessageFrame.class);
final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
when(frame.getWorldUpdater()).thenReturn(worldUpdater);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, createWorldUpdater(0, CURRENT_BLOCK_NUMBER));
lookup =
new ContractBasedBlockHashLookup(
createHeader(CURRENT_BLOCK_NUMBER, headers.getLast()),
STORAGE_ADDRESS,
HISTORY_SERVE_WINDOW,
BLOCKHASH_SERVE_WINDOW);
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
}

private WorldUpdater createWorldUpdater(final int fromBlockNumber, final int toBlockNumber) {
Expand All @@ -83,6 +78,16 @@ private WorldUpdater createWorldUpdater(final int fromBlockNumber, final int toB
return worldUpdaterMock;
}

private MessageFrame createMessageFrame(
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);
return messageFrame;
}

@Test
void shouldGetHashOfImmediateParent() {
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER - 1);
Expand All @@ -107,15 +112,12 @@ void shouldReturnEmptyHashWhenSystemContractNotExists() {

@Test
void shouldReturnEmptyHashWhenParentBlockNotInContract() {
final WorldUpdater worldUpdater =
createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
when(frame.getWorldUpdater()).thenReturn(worldUpdater);
frame =
createMessageFrame(
CURRENT_BLOCK_NUMBER,
createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER));
lookup =
new ContractBasedBlockHashLookup(
new BlockHeaderTestFixture().number(CURRENT_BLOCK_NUMBER).buildHeader(),
STORAGE_ADDRESS,
HISTORY_SERVE_WINDOW,
BLOCKHASH_SERVE_WINDOW);
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER - 20, Hash.ZERO);
}

Expand Down Expand Up @@ -147,14 +149,9 @@ void shouldCacheBlockHashes() {

@Test
void shouldGetHashWhenParentIsGenesis() {
final WorldUpdater worldUpdater = createWorldUpdater(0, 1);
when(frame.getWorldUpdater()).thenReturn(worldUpdater);
frame = createMessageFrame(1, createWorldUpdater(0, 1));
lookup =
new ContractBasedBlockHashLookup(
createHeader(1, headers.getFirst()),
STORAGE_ADDRESS,
HISTORY_SERVE_WINDOW,
BLOCKHASH_SERVE_WINDOW);
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
assertHashForBlockNumber(0);
}

Expand Down

0 comments on commit 71e2965

Please sign in to comment.