Skip to content

Commit

Permalink
add and handle further modifications and flags to handle skeleton fun…
Browse files Browse the repository at this point in the history
…ctionality
  • Loading branch information
g11tech committed Aug 17, 2024
1 parent ee7b200 commit b0456c3
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 105 deletions.
32 changes: 18 additions & 14 deletions packages/blockchain/src/blockchain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { DBManager } from './db/manager.js'
import { DBTarget } from './db/operation.js'

import type { OptimisticOpts } from './db/operation.js'
import type {
BlockchainEvents,
BlockchainInterface,
Expand Down Expand Up @@ -272,8 +273,8 @@ export class Blockchain implements BlockchainInterface {
* heads/hashes are overwritten.
* @param block - The block to be added to the blockchain
*/
async putBlock(block: Block, optimistic: boolean = false) {
await this._putBlockOrHeader(block, optimistic)
async putBlock(block: Block, opts?: OptimisticOpts) {
await this._putBlockOrHeader(block, opts)
}

/**
Expand Down Expand Up @@ -344,7 +345,7 @@ export class Blockchain implements BlockchainInterface {
* header using the iterator method.
* @hidden
*/
private async _putBlockOrHeader(item: Block | BlockHeader, optimistic: boolean = false) {
private async _putBlockOrHeader(item: Block | BlockHeader, optimisticOpts?: OptimisticOpts) {
await this.runWithLock<void>(async () => {
// Save the current sane state incase _putBlockOrHeader midway with some
// dirty changes in head trackers
Expand All @@ -362,13 +363,13 @@ export class Blockchain implements BlockchainInterface {
if (isGenesis) {
if (equalsBytes(this.genesisBlock.hash(), block.hash())) {
// Try to re-put the existing genesis block, accept this
optimistic = false
// genesis block is not optimistic
optimisticOpts = undefined
return
}
throw new Error(
'Cannot put a different genesis block than current blockchain genesis: create a new Blockchain',
)
// genesis block is not optimistic
}

if (block.common.chainId() !== this.common.chainId()) {
Expand All @@ -377,12 +378,12 @@ export class Blockchain implements BlockchainInterface {
)
}

if (this._validateBlocks && !isGenesis && item instanceof Block) {
if (this._validateBlocks && !isGenesis && item instanceof Block && optimisticOpts === undefined) {
// this calls into `getBlock`, which is why we cannot lock yet
await this.validateBlock(block)
}

if (this._validateConsensus) {
if (this._validateConsensus && optimisticOpts === undefined) {
await this.consensus!.validateConsensus(block)
}

Expand All @@ -397,20 +398,20 @@ export class Blockchain implements BlockchainInterface {
if (!block.isGenesis()) {
td += parentTd
}
// since its linked its no more optimistic
optimistic = false
} catch (e) {
// opimistic insertion does care about td
if (!optimistic) {
if (optimisticOpts === undefined) {
throw e
}
}

let dbOps: DBOp[] = []
if (optimistic) {
if (optimisticOpts !== undefined) {
dbOps = dbOps.concat(DBSetBlockOrHeader(item))
dbOps.push(DBSetHashToNumber(blockHash, blockNumber))
dbOps.push(DBOp.set(DBTarget.OptimisticNumberToHash, blockHash, { blockNumber }))
if (optimisticOpts.fcUed) {
dbOps.push(DBOp.set(DBTarget.OptimisticNumberToHash, blockHash, { blockNumber }))
}
await this.dbManager.batch(dbOps)
} else {
const currentTd = { header: BIGINT_0, block: BIGINT_0 }
Expand Down Expand Up @@ -676,13 +677,16 @@ export class Blockchain implements BlockchainInterface {
* this will be immediately looked up, otherwise it will wait until we have
* unlocked the DB
*/
async getBlock(blockId: Uint8Array | number | bigint): Promise<Block> {
async getBlock(
blockId: Uint8Array | number | bigint,
optimisticOpts?: OptimisticOpts,
): Promise<Block> {
// cannot wait for a lock here: it is used both in `validate` of `Block`
// (calls `getBlock` to get `parentHash`) it is also called from `runBlock`
// in the `VM` if we encounter a `BLOCKHASH` opcode: then a bigint is used we
// need to then read the block from the canonical chain Q: is this safe? We
// know it is OK if we call it from the iterator... (runBlock)
const block = await this.dbManager.getBlock(blockId)
const block = await this.dbManager.getBlock(blockId, optimisticOpts)

if (block === undefined) {
if (typeof blockId === 'object') {
Expand Down
30 changes: 24 additions & 6 deletions packages/blockchain/src/db/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { Cache } from './cache.js'
import { DBOp, DBTarget } from './operation.js'

import type { DatabaseKey } from './operation.js'
import type { DatabaseKey, OptimisticOpts } from './operation.js'
import type { Block, BlockBodyBytes, BlockBytes, BlockOptions } from '@ethereumjs/block'
import type { Common } from '@ethereumjs/common'
import type { BatchDBOp, DB, DBObject, DelBatch, PutBatch } from '@ethereumjs/util'
Expand Down Expand Up @@ -47,6 +47,7 @@ export class DBManager {
body: new Cache({ max: 256 }),
numberToHash: new Cache({ max: 2048 }),
hashToNumber: new Cache({ max: 2048 }),
optimisticNumberToHash: new Cache({ max: 2048 }),
}
}

Expand Down Expand Up @@ -86,7 +87,7 @@ export class DBManager {
*/
async getBlock(
blockId: Uint8Array | bigint | number,
optimistic: boolean = false,
optimisticOpts?: OptimisticOpts,
): Promise<Block | undefined> {
if (typeof blockId === 'number' && Number.isInteger(blockId)) {
blockId = BigInt(blockId)
Expand All @@ -98,13 +99,30 @@ export class DBManager {
if (blockId instanceof Uint8Array) {
hash = blockId
number = await this.hashToNumber(blockId)
if (number === undefined) {
return undefined
}

if (optimisticOpts?.fcUed === true) {
let optimisticHash = await this.optimisticNumberToHash(number)
if (optimisticHash === undefined && optimisticOpts.linked === true) {
optimisticHash = await this.numberToHash(number)
}
if (optimisticHash === undefined || !equalsBytes(optimisticHash, hash)) {
return undefined
}
}
} else if (typeof blockId === 'bigint') {
number = blockId
if (optimistic) {
if (optimisticOpts !== undefined) {
if (!optimisticOpts.fcUed) {
throw Error(`Invalid fcUed optimistic block by number lookup`)
}
hash = await this.optimisticNumberToHash(blockId)
}
// hash will be undefined if it no optimistic lookup was done or if that was not successful
if (hash === undefined) {
if (hash === undefined && optimisticOpts.linked === true) {
hash = await this.numberToHash(blockId)
}
} else {
hash = await this.numberToHash(blockId)
}
} else {
Expand Down
2 changes: 2 additions & 0 deletions packages/blockchain/src/db/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {

import type { CacheMap } from './manager.js'

export type OptimisticOpts = { fcUed: boolean; linked?: boolean }

export enum DBTarget {
Heads,
HeadHeader,
Expand Down
5 changes: 3 additions & 2 deletions packages/blockchain/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { OptimisticOpts } from './db/operation.js'
import type { Blockchain } from './index.js'
import type { Block, BlockHeader } from '@ethereumjs/block'
import type { Common, ConsensusAlgorithm } from '@ethereumjs/common'
Expand All @@ -16,7 +17,7 @@ export interface BlockchainInterface {
*
* @param block - The block to be added to the blockchain.
*/
putBlock(block: Block): Promise<void>
putBlock(block: Block, optimisticOpts?: OptimisticOpts): Promise<void>

/**
* Deletes a block from the blockchain. All child blocks in the chain are
Expand All @@ -29,7 +30,7 @@ export interface BlockchainInterface {
/**
* Returns a block by its hash or number.
*/
getBlock(blockId: Uint8Array | number | bigint): Promise<Block>
getBlock(blockId: Uint8Array | number | bigint, optimisticOpts?: OptimisticOpts): Promise<Block>

/**
* Iterates through blocks starting at the specified iterator head and calls
Expand Down
94 changes: 11 additions & 83 deletions packages/client/src/service/skeleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1362,21 +1362,7 @@ export class Skeleton extends MetaDBManager {
*/
private async putBlock(block: Block, onlyUnfinalized: boolean = false): Promise<boolean> {
// Serialize the block with its hardfork so that its easy to load the block latter
const rlp = this.serialize({ hardfork: block.common.hardfork(), blockRLP: block.serialize() })
await this.put(DBKey.SkeletonUnfinalizedBlockByHash, block.hash(), rlp)

if (!onlyUnfinalized) {
await this.put(DBKey.SkeletonBlock, bigIntToBytes(block.header.number), rlp)
// this is duplication of the unfinalized blocks but for now an easy reference
// will be pruned on finalization changes. this could be simplified and deduped
// but will anyway will move into blockchain class and db on upcoming skeleton refactor
await this.put(
DBKey.SkeletonBlockHashToNumber,
block.hash(),
bigIntToBytes(block.header.number),
)
}

await this.chain.blockchain.putBlock(block, { fcUed: !onlyUnfinalized })
return true
}

Expand All @@ -1395,22 +1381,10 @@ export class Skeleton extends MetaDBManager {
* Gets a block from the skeleton or canonical db by number.
*/
async getBlock(number: bigint, onlyCanonical = false): Promise<Block | undefined> {
try {
const skeletonBlockRlp = await this.get(DBKey.SkeletonBlock, bigIntToBytes(number))
if (skeletonBlockRlp === null) {
throw Error(`SkeletonBlock rlp lookup failed for ${number} onlyCanonical=${onlyCanonical}`)
}
return this.skeletonBlockRlpToBlock(skeletonBlockRlp)
} catch (error: any) {
// If skeleton is linked, it probably has deleted the block and put it into the chain
if (onlyCanonical && !this.status.linked) return undefined
// As a fallback, try to get the block from the canonical chain in case it is available there
try {
return await this.chain.getBlock(number)
} catch (error) {
return undefined
}
}
return this.chain.blockchain.dbManager.getBlock(number, {

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

Unhandled error

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.fillCanonicalChain src/service/skeleton.ts:1205:32 This error originated in "test/sync/skeleton.spec.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running. The latest test that might've caused the error is "should fill the canonical chain after being linked to genesis". It might mean one of the following: - The error was thrown, while Vitest was running this test. - If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

Unhandled error

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.fillCanonicalChain src/service/skeleton.ts:1205:32 This error originated in "test/sync/skeleton.spec.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running. The latest test that might've caused the error is "should fill the canonical chain after being linked to a canonical block past genesis". It might mean one of the following: - The error was thrown, while Vitest was running this test. - If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

Unhandled error

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.fillCanonicalChain src/service/skeleton.ts:1205:32 This error originated in "test/sync/skeleton.spec.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running. The latest test that might've caused the error is "should abort filling the canonical chain if the terminal block is invalid". It might mean one of the following: - The error was thrown, while Vitest was running this test. - If the error occurred after the test had been completed, this was the last documented test before it was thrown.

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

test/sync/skeleton.spec.ts > [Skeleton] / initSync > A single leftover subchain is present and the new head is extending

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.processNewHead src/service/skeleton.ts:362:31 ❯ src/service/skeleton.ts:441:32 ❯ Skeleton.runWithLock src/service/skeleton.ts:161:27 ❯ test/sync/skeleton.spec.ts:247:7

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

test/sync/skeleton.spec.ts > [Skeleton] / initSync > Duplicate announcement should not modify subchain

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.processNewHead src/service/skeleton.ts:319:40 ❯ src/service/skeleton.ts:441:32 ❯ Skeleton.runWithLock src/service/skeleton.ts:161:27 ❯ test/sync/skeleton.spec.ts:247:7

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

test/sync/skeleton.spec.ts > [Skeleton] / initSync > A new alternate head is announced in the middle should truncate subchain

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.processNewHead src/service/skeleton.ts:319:40 ❯ src/service/skeleton.ts:441:32 ❯ Skeleton.runWithLock src/service/skeleton.ts:161:27 ❯ test/sync/skeleton.spec.ts:247:7

Check failure on line 1384 in packages/client/src/service/skeleton.ts

View workflow job for this annotation

GitHub Actions / test-client (18)

test/sync/skeleton.spec.ts > [Skeleton] / initSync > The old subchains to be truncated/cleared and a new chain be created for the dangling head

Error: Invalid fcUed optimistic block by number lookup ❯ DBManager.getBlock ../blockchain/src/db/manager.ts:119:17 ❯ Skeleton.getBlock src/service/skeleton.ts:1384:44 ❯ Skeleton.processNewHead src/service/skeleton.ts:319:40 ❯ src/service/skeleton.ts:441:32 ❯ Skeleton.runWithLock src/service/skeleton.ts:161:27 ❯ test/sync/skeleton.spec.ts:247:7
fcUed: onlyCanonical,
linked: this.status.linked,
})
}

/**
Expand All @@ -1420,63 +1394,17 @@ export class Skeleton extends MetaDBManager {
hash: Uint8Array,
onlyCanonical: boolean = false,
): Promise<Block | undefined> {
const number = await this.get(DBKey.SkeletonBlockHashToNumber, hash)
if (number) {
const block = await this.getBlock(bytesToBigInt(number), onlyCanonical)
if (block !== undefined && equalsBytes(block.hash(), hash)) {
return block
}
}

if (onlyCanonical === true && !this.status.linked) {
return undefined
}

let block = onlyCanonical === false ? await this.getUnfinalizedBlock(hash) : undefined
if (block === undefined && (onlyCanonical === false || this.status.linked)) {
block = await this.chain.getBlock(hash).catch((_e) => undefined)
}

if (onlyCanonical === false) {
return block
} else {
if (this.status.linked && block !== undefined) {
const canBlock = await this.chain.getBlock(block.header.number).catch((_e) => undefined)
if (canBlock !== undefined && equalsBytes(canBlock.hash(), block.hash())) {
// block is canonical
return block
}
}

// no canonical block found or the block was not canonical
return undefined
}
}

async getUnfinalizedBlock(hash: Uint8Array): Promise<Block | undefined> {
try {
const skeletonBlockRlp = await this.get(DBKey.SkeletonUnfinalizedBlockByHash, hash)
if (skeletonBlockRlp === null) {
throw Error(`SkeletonUnfinalizedBlockByHash rlp lookup failed for hash=${short(hash)}`)
}
return this.skeletonBlockRlpToBlock(skeletonBlockRlp)
} catch (_e) {
return undefined
}
return this.chain.blockchain.dbManager.getBlock(hash, {
fcUed: !onlyCanonical,
linked: this.status.linked,
})
}

/**
* Deletes a skeleton block from the db by number
*/
async deleteBlock(block: Block): Promise<boolean> {
try {
await this.delete(DBKey.SkeletonBlock, bigIntToBytes(block.header.number))
await this.delete(DBKey.SkeletonBlockHashToNumber, block.hash())
await this.delete(DBKey.SkeletonUnfinalizedBlockByHash, block.hash())
return true
} catch (error: any) {
return false
}
async deleteBlock(_block: Block): Promise<boolean> {
return true
}

/**
Expand Down

0 comments on commit b0456c3

Please sign in to comment.