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

Client: ensure safe/finalized blocks are part of the canonical chain on forkchoiceUpdated #2577

Merged
merged 6 commits into from
Mar 10, 2023
25 changes: 18 additions & 7 deletions packages/client/lib/rpc/modules/engine.ts
Original file line number Diff line number Diff line change
@@ -158,7 +158,7 @@ const payloadAttributesFieldValidatorsV2 = {
/**
* Formats a block to {@link ExecutionPayloadV1}.
*/
const blockToExecutionPayload = (block: Block, value: bigint) => {
export const blockToExecutionPayload = (block: Block, value: bigint) => {
const blockJson = block.toJSON()
const header = blockJson.header!
const transactions = block.transactions.map((tx) => bufferToHex(tx.serialize())) ?? []
@@ -832,10 +832,15 @@ export class Engine {
const zeroBlockHash = zeros(32)
const safe = toBuffer(safeBlockHash)
if (!safe.equals(headBlock.hash()) && !safe.equals(zeroBlockHash)) {
const msg = 'Safe block not in canonical chain'
try {
await this.chain.getBlock(safe)
} catch (error) {
const message = 'safe block not available'
const safeBlock = await this.chain.getBlock(safe)
const canonical = await this.chain.getBlock(safeBlock.header.number)
if (!canonical.hash().equals(safe)) {
throw new Error(msg)
}
} catch (error: any) {
const message = error.message === msg ? msg : 'safe block not available'
throw {
code: INVALID_PARAMS,
message,
@@ -844,11 +849,17 @@ export class Engine {
}
const finalized = toBuffer(finalizedBlockHash)
if (!finalized.equals(zeroBlockHash)) {
const msg = 'Finalized block not in canonical chain'
try {
await this.chain.getBlock(finalized)
} catch (error) {
const finalizedBlock = await this.chain.getBlock(finalized)
const canonical = await this.chain.getBlock(finalizedBlock.header.number)
if (!canonical.hash().equals(finalized)) {
throw new Error(msg)
}
} catch (error: any) {
const message = error.message === msg ? msg : 'finalized block not available'
throw {
message: 'finalized block not available',
message,
code: INVALID_PARAMS,
}
}
112 changes: 111 additions & 1 deletion packages/client/test/rpc/engine/forkchoiceUpdatedV1.spec.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import { Block, BlockHeader } from '@ethereumjs/block'
import { Chain, Common, Hardfork } from '@ethereumjs/common'
import { bufferToHex, zeros } from '@ethereumjs/util'
import * as tape from 'tape'
import * as td from 'testdouble'

import { INVALID_PARAMS } from '../../../lib/rpc/error-code'
import { blockToExecutionPayload } from '../../../lib/rpc/modules'
import blocks = require('../../testdata/blocks/beacon.json')
import genesisJSON = require('../../testdata/geth-genesis/post-merge.json')
import { baseRequest, baseSetup, params, setupChain } from '../helpers'
import { checkError } from '../util'

import { batchBlocks } from './newPayloadV1.spec'

const crypto = require('crypto')

const method = 'engine_forkchoiceUpdatedV1'

const originalValidate = BlockHeader.prototype._consensusFormatValidation
@@ -27,6 +31,26 @@ const validPayloadAttributes = {
suggestedFeeRecipient: '0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b',
}

const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Merge })

function createBlock(parentBlock: Block) {
const prevRandao = crypto.randomBytes(32)
const block = Block.fromBlockData(
{
header: {
parentHash: parentBlock.hash(),
mixHash: prevRandao,
number: parentBlock.header.number + BigInt(1),
stateRoot: parentBlock.header.stateRoot,
timestamp: parentBlock.header.timestamp + BigInt(1),
gasLimit: parentBlock.header.gasLimit,
},
},
{ common }
)
return block
}

export const validPayload = [validForkChoiceState, validPayloadAttributes]

tape(`${method}: call with invalid head block hash without 0x`, async (t) => {
@@ -271,7 +295,93 @@ tape(`${method}: latest block after reorg`, async (t) => {
])

expectRes = (res: any) => {
t.equal(res.body.result.payloadStatus.latestValidHash, blocks[1].blockHash)
t.equal(res.body.error.code, -32602)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note; this FCU payload is indeed invalid, because headBlock has block number 1, while safeBlock has block number 2 (and finalBlock also)

}
await baseRequest(t, server, req, 200, expectRes)
})

tape(`${method}: validate safeBlockHash is part of canonical chain`, async (t) => {
const { server, chain } = await setupChain(genesisJSON, 'post-merge', { engine: true })

const genesis = await chain.getBlock(BigInt(0))

// Build the payload for the canonical chain
const canonical = [genesis]

for (let i = 0; i < 2; i++) {
canonical.push(createBlock(canonical[canonical.length - 1]))
}

// Build an alternative payload
const reorg = [genesis]
for (let i = 0; i < 2; i++) {
reorg.push(createBlock(reorg[reorg.length - 1]))
}

const canonicalPayload = canonical.map(
(e) => blockToExecutionPayload(e, BigInt(0)).executionPayload
)
const reorgPayload = reorg.map((e) => blockToExecutionPayload(e, BigInt(0)).executionPayload)

await batchBlocks(t, server, canonicalPayload.slice(1))
await batchBlocks(t, server, reorgPayload.slice(1))

// Safe block hash is not in the canonical chain
const req = params(method, [
{
headBlockHash: reorgPayload[2].blockHash,
safeBlockHash: canonicalPayload[1].blockHash,
finalizedBlockHash: reorgPayload[1].blockHash,
},
])

const expectRes = (res: any) => {
t.equal(res.body.error.code, -32602)
t.ok(res.body.error.message.includes('Safe'))
t.ok(res.body.error.message.includes('canonical'))
}
await baseRequest(t, server, req, 200, expectRes)
})

tape(`${method}: validate finalizedBlockHash is part of canonical chain`, async (t) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: had to copy the previous test, because if baseRequest encounters an error, it ends the test suite thus you cannot test anything else afterwards.

const { server, chain } = await setupChain(genesisJSON, 'post-merge', { engine: true })

const genesis = await chain.getBlock(BigInt(0))

// Build the payload for the canonical chain
const canonical = [genesis]

for (let i = 0; i < 2; i++) {
canonical.push(createBlock(canonical[canonical.length - 1]))
}

// Build an alternative payload
const reorg = [genesis]
for (let i = 0; i < 2; i++) {
reorg.push(createBlock(reorg[reorg.length - 1]))
}

const canonicalPayload = canonical.map(
(e) => blockToExecutionPayload(e, BigInt(0)).executionPayload
)
const reorgPayload = reorg.map((e) => blockToExecutionPayload(e, BigInt(0)).executionPayload)

await batchBlocks(t, server, canonicalPayload.slice(1))
await batchBlocks(t, server, reorgPayload.slice(1))

// Finalized block hash is not in the canonical chain
const req = params(method, [
{
headBlockHash: reorgPayload[2].blockHash,
safeBlockHash: reorgPayload[1].blockHash,
finalizedBlockHash: canonicalPayload[1].blockHash,
},
])

const expectRes = (res: any) => {
t.equal(res.body.error.code, -32602)
t.ok(res.body.error.message.includes('Finalized'))
t.ok(res.body.error.message.includes('canonical'))
}
await baseRequest(t, server, req, 200, expectRes)
})
12 changes: 9 additions & 3 deletions packages/client/test/rpc/engine/newPayloadV1.spec.ts
Original file line number Diff line number Diff line change
@@ -19,9 +19,15 @@ const [blockData] = blocks

const originalValidate = BlockHeader.prototype._consensusFormatValidation

export const batchBlocks = async (t: Test, server: HttpServer) => {
for (let i = 0; i < 3; i++) {
const req = params(method, [blocks[i]])
/**
*
* @param t Test suite
* @param server HttpServer
* @param inputBlocks Array of valid ExecutionPayloadV1 data
*/
export const batchBlocks = async (t: Test, server: HttpServer, inputBlocks: any[] = blocks) => {
for (let i = 0; i < inputBlocks.length; i++) {
const req = params('engine_newPayloadV1', [inputBlocks[i]])
const expectRes = (res: any) => {
t.equal(res.body.result.status, 'VALID')
}