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.

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

vm: remove backfill of block hashes on 2935 activation #3478

Merged
merged 1 commit into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 0 additions & 24 deletions packages/vm/src/runBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,6 @@ export async function accumulateParentBlockHash(
)
const historyServeWindow = this.common.param('vm', 'historyServeWindow')

const forkTime = this.common.eipTimestamp(2935)

// getAccount with historyAddress will throw error as witnesses are not bundeled
// but we need to put account so as to query later for slot
try {
Expand Down Expand Up @@ -524,28 +522,6 @@ export async function accumulateParentBlockHash(
}
await putBlockHash(this, parentHash, currentBlockNumber - BIGINT_1)
Copy link
Member

Choose a reason for hiding this comment

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

I think we can now remove this function, since it is only called once.


// in stateless execution parentBlock is not in blockchain but in chain's blockCache
// need to move the blockCache to the blockchain, in any case we can ignore forkblock
// which is where we need this code segment
try {
const parentBlock = await this.blockchain.getBlock(parentHash)

// If on the fork block, store the old block hashes as well
if (forkTime !== null && parentBlock.header.timestamp < forkTime) {
// forkTime could be null in test fixtures
let ancestor = parentBlock
for (let i = 0; i < Number(historyServeWindow) - 1; i++) {
if (ancestor.header.number === BIGINT_0) {
break
}

ancestor = await this.blockchain.getBlock(ancestor.header.parentHash)
await putBlockHash(this, ancestor.hash(), ancestor.header.number)
}
}
// eslint-disable-next-line no-empty
} catch (_e) {}

// do cleanup if the code was not deployed
await this.evm.journal.cleanup()
}
Expand Down
34 changes: 22 additions & 12 deletions packages/vm/test/api/EIPs/eip-2935-historical-block-hashes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ describe('EIP 2935: historical block hashes', () => {
})
it('should ensure blocks older than 256 blocks can be retrieved from the history contract', async () => {
// Test: build a chain with 256+ blocks and then retrieve BLOCKHASH of the genesis block and block 1
const blocksActivation = 256 // This ensures that block 0 - 255 all get stored into the hash contract
const blocksActivation = 256 // This ensures that only block 255 gets stored into the hash contract
// More than blocks activation to build, so we can ensure that we can also retrieve block 0 or block 1 hash at block 300
const blocksToBuild = 300
const blocksToBuild = 500
Copy link
Member

Choose a reason for hiding this comment

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

Do we now need 500 blocks, or can we also "just" create a few blocks (say 5)? We only need to test that the last blockhash is put into the contract at fork activation time.

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 the test tests various scenarios of blockhash as well as the contract code windows as well as activation time

const commonGetHistoryServeWindow = eip2935ActiveAtCommon(0)
commonGetHistoryServeWindow.setEIPs([2935])
const common = eip2935ActiveAtCommon(blocksActivation)
Expand All @@ -225,23 +225,23 @@ describe('EIP 2935: historical block hashes', () => {
validateConsensus: false,
})
const vm = await VM.create({ common, blockchain })
let parentBlock = await vm.blockchain.getBlock(0)
let lastBlock = await vm.blockchain.getBlock(0)
for (let i = 1; i <= blocksToBuild; i++) {
parentBlock = await (
lastBlock = await (
await vm.buildBlock({
parentBlock,
parentBlock: lastBlock,
blockOpts: {
putBlockIntoBlockchain: false,
setHardfork: true,
},
headerData: {
timestamp: parentBlock.header.number + BIGINT_1,
timestamp: lastBlock.header.number + BIGINT_1,
},
})
).build()
await vm.blockchain.putBlock(parentBlock)
await vm.blockchain.putBlock(lastBlock)
await vm.runBlock({
block: parentBlock,
block: lastBlock,
generate: true,
skipHeaderValidation: true,
setHardfork: true,
Expand All @@ -263,14 +263,24 @@ describe('EIP 2935: historical block hashes', () => {
historyAddress,
setLengthLeft(bigIntToBytes(BigInt(i) % historyServeWindow), 32)
)

// we will evaluate on lastBlock where 7709 is active and BLOCKHASH
// will look from the state if within 256 window
const ret = await vm.evm.runCall({
// Code: RETURN the BLOCKHASH of block i
// PUSH(i) BLOCKHASH PUSH(32) MSTORE PUSH(64) PUSH(0) RETURN
// Note: need to return a contract with starting zero bytes to avoid non-deployable contracts by EIP 3540
data: hexToBytes('0x61' + i.toString(16).padStart(4, '0') + '4060205260406000F3'),
block: parentBlock,
block: lastBlock,
})
if (i <= blocksToBuild - 1 && i >= blocksToBuild - Number(historyServeWindow)) {

// contract will only have hashes between blocksActivation -1 and blocksToBuild -1 thresholded by
// historyServeWindow window
if (
i >= blocksActivation - 1 &&
i <= blocksToBuild - 1 &&
i >= blocksToBuild - Number(historyServeWindow)
) {
assert.ok(equalsBytes(setLengthLeft(storage, 32), block.hash()))
if (i >= blocksToBuild - 256) {
assert.ok(equalsBytes(ret.execResult.returnValue, setLengthLeft(block.hash(), 64)))
Expand All @@ -294,8 +304,8 @@ describe('EIP 2935: historical block hashes', () => {
{ common }
)

// should be able to resolve blockhash via contract code
for (const i of [0, 1, blocksActivation, blocksToBuild - 1]) {
// should be able to resolve blockhash via contract code but from the blocksActivation -1 onwards
for (const i of [blocksActivation - 1, blocksActivation, blocksToBuild - 1]) {
const blockHashi = await testBlockhashContract(vm, block, BigInt(i))
const blocki = await blockchain.getBlock(i)
assert.ok(equalsBytes(blockHashi, blocki.hash()))
Expand Down
Loading