VM, Blockchain, Client: fix storing unsettled promises (develop) (#1811)

* 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>

Author: 2 people

packages/block/src/header.ts

@@ -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}.
    *
    */

packages/blockchain/src/index.ts

@@ -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 {
@@ -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
@@ -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
@@ -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
   }
 
@@ -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) {
@@ -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)
   }
 
   /**
@@ -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))
@@ -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
   }
 
   /**
@@ -763,7 +756,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) {
@@ -787,7 +780,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) {
@@ -803,7 +796,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')
       }
@@ -816,7 +809,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')
       }
@@ -836,7 +829,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])
     }
@@ -851,7 +843,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)
   }
 
@@ -865,7 +856,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])
     }
@@ -880,7 +870,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)
   }
 
@@ -1061,7 +1050,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)
   }
 
@@ -1096,7 +1084,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
 
@@ -1133,7 +1121,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
@@ -1178,7 +1166,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)
   }
 
@@ -1278,7 +1265,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
 
@@ -1319,7 +1306,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
    */
@@ -1329,14 +1316,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()
     })

packages/blockchain/test/clique.spec.ts

@@ -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')