Skip to content

Commit

Permalink
VM, Blockchain, Client: fix storing unsettled promises (develop) (#1811)
Browse files Browse the repository at this point in the history 
* blockchain: remove storing unsettled promise

* vm: remove storing unsettled promises. make copy() async

* client: update to new async changes

* block: doc nit

* nits

* fix vm benchmarks, add _init to mockchain

* Make blockchain and vm constructors protected

* lint

* vm: update new tests to await

Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com>
  • Loading branch information
@ryanio @acolytec3 @holgerd77
2 people authored and holgerd77 committed Apr 5, 2022
1 parent f3aff4a commit 5b370f6
Show file tree
Hide file tree
Showing 67 changed files with 351 additions and 358 deletions.
2 changes: 1 addition & 1 deletion packages/block/src/header.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class BlockHeader {
/**
* This constructor takes the values, validates them, assigns them and freezes the object.
*
* @deprecated - Use the public static factory methods to assist in creating a Header object from
* @deprecated Use the public static factory methods to assist in creating a Header object from
* varying data types. For a default empty header, use {@link BlockHeader.fromHeaderData}.
*
*/
Expand Down
91 changes: 39 additions & 52 deletions packages/blockchain/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@ import {
CLIQUE_NONCE_AUTH,
CLIQUE_NONCE_DROP,
} from './clique'

const debug = createDebugLogger('blockchain:clique')

// eslint-disable-next-line implicit-dependencies/no-implicit
import type { LevelUp } from 'levelup'
const level = require('level-mem')

const debug = createDebugLogger('blockchain:clique')

type OnBlock = (block: Block, reorg: boolean) => Promise<void> | void

export interface BlockchainInterface {
Expand Down Expand Up @@ -112,7 +111,7 @@ export interface BlockchainOptions {
validateBlocks?: boolean

/**
* The blockchain only initializes succesfully if it has a genesis block. If
* The blockchain only initializes successfully if it has a genesis block. If
* there is no block available in the DB and a `genesisBlock` is provided,
* then the provided `genesisBlock` will be used as genesis. If no block is
* present in the DB and no block is provided, then the genesis block as
Expand All @@ -130,18 +129,25 @@ export default class Blockchain implements BlockchainInterface {

private _genesis?: Buffer // the genesis hash of this blockchain

// The following two heads and the heads stored within the `_heads` always point
// to a hash in the canonical chain and never to a stale hash.
// With the exception of `_headHeaderHash` this does not necessarily need to be
// the hash with the highest total difficulty.
private _headBlockHash?: Buffer // the hash of the current head block
private _headHeaderHash?: Buffer // the hash of the current head header
// A Map which stores the head of each key (for instance the "vm" key) which is
// updated along a {@link Blockchain.iterator} method run and can be used to (re)run
// non-verified blocks (for instance in the VM).
/**
* The following two heads and the heads stored within the `_heads` always point
* to a hash in the canonical chain and never to a stale hash.
* With the exception of `_headHeaderHash` this does not necessarily need to be
* the hash with the highest total difficulty.
*/
/** The hash of the current head block */
private _headBlockHash?: Buffer
/** The hash of the current head header */
private _headHeaderHash?: Buffer

/**
* A Map which stores the head of each key (for instance the "vm" key) which is
* updated along a {@link Blockchain.iterator} method run and can be used to (re)run
* non-verified blocks (for instance in the VM).
*/
private _heads: { [key: string]: Buffer }

public initPromise: Promise<void>
protected _isInitialized = false
private _lock: Semaphore

private _common: Common
Expand Down Expand Up @@ -207,9 +213,7 @@ export default class Blockchain implements BlockchainInterface {

public static async create(opts: BlockchainOptions = {}) {
const blockchain = new Blockchain(opts)
await blockchain.initPromise!.catch((e) => {
throw e
})
await blockchain._init(opts.genesisBlock)
return blockchain
}

Expand All @@ -233,16 +237,16 @@ export default class Blockchain implements BlockchainInterface {
}

/**
* Creates new Blockchain object
* Creates new Blockchain object.
*
* @deprecated - The direct usage of this constructor is discouraged since
* @deprecated The direct usage of this constructor is discouraged since
* non-finalized async initialization might lead to side effects. Please
* use the async {@link Blockchain.create} constructor instead (same API).
*
* @param opts - An object with the options that this constructor takes. See
* @param opts An object with the options that this constructor takes. See
* {@link BlockchainOptions}.
*/
constructor(opts: BlockchainOptions = {}) {
protected constructor(opts: BlockchainOptions = {}) {
// Throw on chain or hardfork options removed in latest major release to
// prevent implicit chain setup on a wrong chain
if ('chain' in opts || 'hardfork' in opts) {
Expand Down Expand Up @@ -291,9 +295,6 @@ export default class Blockchain implements BlockchainInterface {
if (opts.genesisBlock && !opts.genesisBlock.isGenesis()) {
throw 'supplied block is not a genesis block'
}

// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.initPromise = this._init(opts.genesisBlock)
}

/**
Expand Down Expand Up @@ -329,13 +330,14 @@ export default class Blockchain implements BlockchainInterface {
}

/**
* This method is called in the constructor and either sets up the DB or reads
* This method is called in {@link Blockchain.create} and either sets up the DB or reads
* values from the DB and makes these available to the consumers of
* Blockchain.
*
* @hidden
*/
private async _init(genesisBlock?: Block): Promise<void> {
if (this._isInitialized) return
let dbGenesisBlock
try {
const genesisHash = await this.dbManager.numberToHash(BigInt(0))
Expand Down Expand Up @@ -421,23 +423,14 @@ export default class Blockchain implements BlockchainInterface {
}
this._headBlockHash = genesisHash
}

if (this._hardforkByHeadBlockNumber) {
const latestHeader = await this._getHeader(this._headHeaderHash)
const td = await this.getTotalDifficulty(this._headHeaderHash)
this._common.setHardforkByBlockNumber(latestHeader.number, td)
}
}

/**
* Perform the `action` function after we have initialized this module and
* have acquired a lock
* @param action - the action function to run after initializing and acquiring
* a lock
* @hidden
*/
private async initAndLock<T>(action: () => Promise<T>): Promise<T> {
await this.initPromise
return await this.runWithLock(action)
this._isInitialized = true
}

/**
Expand Down Expand Up @@ -758,7 +751,7 @@ export default class Blockchain implements BlockchainInterface {
* @param name - Optional name of the iterator head (default: 'vm')
*/
async getIteratorHead(name = 'vm'): Promise<Block> {
return await this.initAndLock<Block>(async () => {
return await this.runWithLock<Block>(async () => {
// if the head is not found return the genesis hash
const hash = this._heads[name] || this._genesis
if (!hash) {
Expand All @@ -782,7 +775,7 @@ export default class Blockchain implements BlockchainInterface {
* on a first run)
*/
async getHead(name = 'vm'): Promise<Block> {
return await this.initAndLock<Block>(async () => {
return await this.runWithLock<Block>(async () => {
// if the head is not found return the headHeader
const hash = this._heads[name] || this._headBlockHash
if (!hash) {
Expand All @@ -798,7 +791,7 @@ export default class Blockchain implements BlockchainInterface {
* Returns the latest header in the canonical chain.
*/
async getLatestHeader(): Promise<BlockHeader> {
return await this.initAndLock<BlockHeader>(async () => {
return await this.runWithLock<BlockHeader>(async () => {
if (!this._headHeaderHash) {
throw new Error('No head header set')
}
Expand All @@ -811,7 +804,7 @@ export default class Blockchain implements BlockchainInterface {
* Returns the latest full block in the canonical chain.
*/
async getLatestBlock(): Promise<Block> {
return this.initAndLock<Block>(async () => {
return this.runWithLock<Block>(async () => {
if (!this._headBlockHash) {
throw new Error('No head block set')
}
Expand All @@ -831,7 +824,6 @@ export default class Blockchain implements BlockchainInterface {
* @param blocks - The blocks to be added to the blockchain
*/
async putBlocks(blocks: Block[]) {
await this.initPromise
for (let i = 0; i < blocks.length; i++) {
await this.putBlock(blocks[i])
}
Expand All @@ -846,7 +838,6 @@ export default class Blockchain implements BlockchainInterface {
* @param block - The block to be added to the blockchain
*/
async putBlock(block: Block) {
await this.initPromise
await this._putBlockOrHeader(block)
}

Expand All @@ -860,7 +851,6 @@ export default class Blockchain implements BlockchainInterface {
* @param headers - The headers to be added to the blockchain
*/
async putHeaders(headers: Array<any>) {
await this.initPromise
for (let i = 0; i < headers.length; i++) {
await this.putHeader(headers[i])
}
Expand All @@ -875,7 +865,6 @@ export default class Blockchain implements BlockchainInterface {
* @param header - The header to be added to the blockchain
*/
async putHeader(header: BlockHeader) {
await this.initPromise
await this._putBlockOrHeader(header)
}

Expand Down Expand Up @@ -1056,7 +1045,6 @@ export default class Blockchain implements BlockchainInterface {
// 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)
await this.initPromise
return await this._getBlock(blockId)
}

Expand Down Expand Up @@ -1091,7 +1079,7 @@ export default class Blockchain implements BlockchainInterface {
skip: number,
reverse: boolean
): Promise<Block[]> {
return await this.initAndLock<Block[]>(async () => {
return await this.runWithLock<Block[]>(async () => {
const blocks: Block[] = []
let i = -1

Expand Down Expand Up @@ -1128,7 +1116,7 @@ export default class Blockchain implements BlockchainInterface {
* @param hashes - Ordered array of hashes (ordered on `number`).
*/
async selectNeededHashes(hashes: Array<Buffer>): Promise<Buffer[]> {
return await this.initAndLock<Buffer[]>(async () => {
return await this.runWithLock<Buffer[]>(async () => {
let max: number
let mid: number
let min: number
Expand Down Expand Up @@ -1173,7 +1161,6 @@ export default class Blockchain implements BlockchainInterface {
// But is this the way to go? If we know this is called from the
// iterator/runBlockchain we are safe, but if this is called from anywhere
// else then this might lead to a concurrency problem?
await this.initPromise
await this._delBlock(blockHash)
}

Expand Down Expand Up @@ -1273,7 +1260,7 @@ export default class Blockchain implements BlockchainInterface {
* @hidden
*/
private async _iterator(name: string, onBlock: OnBlock, maxBlocks?: number): Promise<number> {
return await this.initAndLock<number>(async (): Promise<number> => {
return await this.runWithLock<number>(async (): Promise<number> => {
const headHash = this._heads[name] || this._genesis
let lastBlock: Block | undefined

Expand Down Expand Up @@ -1314,7 +1301,7 @@ export default class Blockchain implements BlockchainInterface {

/**
* Set header hash of a certain `tag`.
* When calling the iterator, the iterator will start running the first child block after the header hash currenntly stored.
* When calling the iterator, the iterator will start running the first child block after the header hash currently stored.
* @param tag - The tag to save the headHash to
* @param headHash - The head hash to save
*/
Expand All @@ -1324,14 +1311,14 @@ export default class Blockchain implements BlockchainInterface {

/**
* Set header hash of a certain `tag`.
* When calling the iterator, the iterator will start running the first child block after the header hash currenntly stored.
* When calling the iterator, the iterator will start running the first child block after the header hash currently stored.
* @param tag - The tag to save the headHash to
* @param headHash - The head hash to save
*
* @deprecated use {@link Blockchain.setIteratorHead()} instead
*/
async setHead(tag: string, headHash: Buffer) {
await this.initAndLock<void>(async () => {
await this.runWithLock<void>(async () => {
this._heads[tag] = headHash
await this._saveHeads()
})
Expand Down
2 changes: 1 addition & 1 deletion packages/blockchain/test/clique.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { CLIQUE_NONCE_AUTH, CLIQUE_NONCE_DROP } from '../src/clique'
tape('Clique: Initialization', (t) => {
t.test('should initialize a clique blockchain', async (st) => {
const common = new Common({ chain: Chain.Rinkeby, hardfork: Hardfork.Chainstart })
const blockchain = new Blockchain({ common })
const blockchain = await Blockchain.create({ common })

const head = await blockchain.getHead()
st.equal(head.hash().toString('hex'), common.genesis().hash.slice(2), 'correct genesis hash')
Expand Down
Loading

0 comments on commit 5b370f6

Please sign in to comment.