Clique small fixes / follow-up work

packages/block/package.json

@@ -43,7 +43,7 @@
     "@ethereumjs/trie": "^4.0.0",
     "@ethereumjs/tx": "^3.0.0",
     "@types/bn.js": "^4.11.6",
-    "ethereumjs-util": "^7.0.7"
+    "ethereumjs-util": "^7.0.8"
   },
   "devDependencies": {
     "@ethereumjs/config-coverage": "^2.0.0",

packages/block/src/header.ts

@@ -4,6 +4,8 @@ import {
   BN,
   bnToHex,
   ecrecover,
+  ecsign,
+  intToBuffer,
   KECCAK256_RLP_ARRAY,
   KECCAK256_RLP,
   rlp,
@@ -238,6 +240,11 @@ export class BlockHeader {
       this.difficulty = this.canonicalDifficulty(options.calcDifficultyFromHeader)
     }
 
+    // If cliqueSigner is provided, seal block with provided privateKey.
+    if (options.cliqueSigner) {
+      this.extraData = this.cliqueSealBlock(options.cliqueSigner)
+    }
+
     const freeze = options?.freeze ?? true
     if (freeze) {
       Object.freeze(this)
@@ -371,21 +378,25 @@ export class BlockHeader {
 
   /**
    * For poa, validates `difficulty` is correctly identified as INTURN or NOTURN.
+   * Returns false if invalid.
    */
   validateCliqueDifficulty(blockchain: Blockchain): boolean {
     if (!this.difficulty.eq(CLIQUE_DIFF_INTURN) && !this.difficulty.eq(CLIQUE_DIFF_NOTURN)) {
       throw new Error(
         `difficulty for clique block must be INTURN (2) or NOTURN (1), received: ${this.difficulty.toString()}`
       )
     }
-    const signers = blockchain.cliqueActiveSigners()
+    if ('cliqueActiveSigners' in blockchain === false) {
+      throw new Error(
+        'PoA blockchain requires method blockchain.cliqueActiveSigners() to validate clique difficulty'
+      )
+    }
+    const signers = (blockchain as any).cliqueActiveSigners()
     if (signers.length === 0) {
       // abort if signers are unavailable
       return true
     }
-    const signerIndex = signers.findIndex((address: Address) =>
-      address.toBuffer().equals(this.cliqueSigner().toBuffer())
-    )
+    const signerIndex = signers.findIndex((address: Address) => address.equals(this.cliqueSigner()))
     const inTurn = this.number.modn(signers.length) === signerIndex
     if (
       (inTurn && this.difficulty.eq(CLIQUE_DIFF_INTURN)) ||
@@ -426,10 +437,13 @@ export class BlockHeader {
    * - The `parentHash` is part of the blockchain (it is a valid header)
    * - Current block number is parent block number + 1
    * - Current block has a strictly higher timestamp
-   * - Additional PoA -> Clique check: Current block has a timestamp diff greater or equal to PERIOD
-   * - Current block has valid difficulty (only PoW, otherwise pass) and gas limit
-   * - In case that the header is an uncle header, it should not be too old or young in the chain.
-   * @param blockchain - validate against a @ethereumjs/blockchain
+   * - Additional PoW checks ->
+   *   - Current block has valid difficulty and gas limit
+   *   - In case that the header is an uncle header, it should not be too old or young in the chain.
+   * - Additional PoA clique checks ->
+   *   - Current block has a timestamp diff greater or equal to PERIOD
+   *   - Current block has difficulty correctly marked as INTURN or NOTURN
+   * @param blockchain - validate against an @ethereumjs/blockchain
    * @param height - If this is an uncle header, this is the height of the block that is including it
    */
   async validate(blockchain: Blockchain, height?: BN): Promise<void> {
@@ -602,6 +616,27 @@ export class BlockHeader {
     return this.extraData.slice(-CLIQUE_EXTRA_SEAL)
   }
 
+  /**
+   * Seal block with the provided signer.
+   * Returns the final extraData field to be assigned to `this.extraData`.
+   * @hidden
+   */
+  private cliqueSealBlock(privateKey: Buffer) {
+    this._requireClique('cliqueSealBlock')
+    const signature = ecsign(this.hash(), privateKey)
+    const signatureB = Buffer.concat([signature.r, signature.s, intToBuffer(signature.v - 27)])
+
+    let extraDataWithoutSeal = this.extraData.slice(0, this.extraData.length - CLIQUE_EXTRA_SEAL)
+    // ensure extraDataWithoutSeal is at least 32 bytes (CLIQUE_EXTRA_VANITY)
+    if (extraDataWithoutSeal.length < CLIQUE_EXTRA_VANITY) {
+      const remainingLength = Buffer.alloc(CLIQUE_EXTRA_VANITY - extraDataWithoutSeal.length)
+      extraDataWithoutSeal = Buffer.concat([extraDataWithoutSeal, remainingLength])
+    }
+
+    const extraData = Buffer.concat([extraDataWithoutSeal, signatureB])
+    return extraData
+  }
+
   /**
    * Returns a list of signers
    * (only clique PoA, throws otherwise)
@@ -636,9 +671,9 @@ export class BlockHeader {
    */
   cliqueVerifySignature(signerList: Address[]): boolean {
     this._requireClique('cliqueVerifySignature')
-    const signerAddress = this.cliqueSigner().toBuffer()
+    const signerAddress = this.cliqueSigner()
     const signerFound = signerList.find((signer) => {
-      return signer.toBuffer().equals(signerAddress)
+      return signer.equals(signerAddress)
     })
     return !!signerFound
   }

packages/block/src/types.ts

@@ -1,4 +1,4 @@
-import { Address, AddressLike, BNLike, BufferLike } from 'ethereumjs-util'
+import { AddressLike, BNLike, BufferLike } from 'ethereumjs-util'
 import Common from '@ethereumjs/common'
 import { TxData, JsonTx } from '@ethereumjs/tx'
 import { Block } from './block'
@@ -54,6 +54,11 @@ export interface BlockOptions {
    * Default: true
    */
   freeze?: boolean
+  /**
+   * Provide a clique signer's privateKey to seal this block.
+   * Will throw if provided on a non-PoA chain.
+   */
+  cliqueSigner?: Buffer
 }
 
 /**
@@ -130,5 +135,4 @@ export interface JsonHeader {
 
 export interface Blockchain {
   getBlock(hash: Buffer): Promise<Block>
-  cliqueActiveSigners(): Address[]
 }

packages/block/test/clique.spec.ts

@@ -1,7 +1,7 @@
 import tape from 'tape'
 import Common from '@ethereumjs/common'
 import { BlockHeader } from '../src/header'
-import { Address, ecsign, intToBuffer } from 'ethereumjs-util'
+import { Address } from 'ethereumjs-util'
 
 tape('[Header]: Clique PoA Functionality', function (t) {
   const common = new Common({ chain: 'rinkeby', hardfork: 'chainstart' })
@@ -78,22 +78,16 @@ tape('[Header]: Clique PoA Functionality', function (t) {
   }
 
   t.test('Signing', function (st) {
-    let header = BlockHeader.fromHeaderData(
+    const cliqueSigner = A.privateKey
+
+    const header = BlockHeader.fromHeaderData(
       { number: 1, extraData: Buffer.alloc(97) },
-      { common, freeze: false }
+      { common, freeze: false, cliqueSigner }
     )
-    const signature = ecsign(header.hash(), A.privateKey)
-    const signatureB = Buffer.concat([signature.r, signature.s, intToBuffer(signature.v - 27)])
-    const extraData = Buffer.concat([header.cliqueExtraVanity(), signatureB])
-
-    header = BlockHeader.fromHeaderData({ number: 1, extraData }, { common, freeze: false })
 
     st.equal(header.extraData.length, 97)
     st.ok(header.cliqueVerifySignature([A.address]), 'should verify signature')
-    st.ok(
-      header.cliqueSigner().toBuffer().equals(A.address.toBuffer()),
-      'should recover the correct signer address'
-    )
+    st.ok(header.cliqueSigner().equals(A.address), 'should recover the correct signer address')
 
     st.end()
   })

packages/block/test/header.spec.ts

@@ -11,7 +11,7 @@ tape('[Block]: Header functions', function (t) {
     function compareDefaultHeader(st: tape.Test, header: BlockHeader) {
       st.ok(header.parentHash.equals(zeros(32)))
       st.ok(header.uncleHash.equals(KECCAK256_RLP_ARRAY))
-      st.ok(header.coinbase.buf.equals(Address.zero().buf))
+      st.ok(header.coinbase.equals(Address.zero()))
       st.ok(header.stateRoot.equals(zeros(32)))
       st.ok(header.transactionsTrie.equals(KECCAK256_RLP))
       st.ok(header.receiptTrie.equals(KECCAK256_RLP))

packages/blockchain/package.json

@@ -39,7 +39,7 @@
     "@ethereumjs/block": "^3.0.0",
     "@ethereumjs/common": "^2.0.0",
     "@ethereumjs/ethash": "^1.0.0",
-    "ethereumjs-util": "^7.0.7",
+    "ethereumjs-util": "^7.0.8",
     "level-mem": "^5.0.1",
     "lru-cache": "^5.1.1",
     "rlp": "^2.2.3",

packages/blockchain/src/index.ts

@@ -131,10 +131,10 @@ export default class Blockchain implements BlockchainInterface {
   _ethash?: Ethash
 
   /**
-   * Keep signer history data (signer states and votes) for all
-   * block numbers >= HEAD_BLOCK - CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
+   * Keep signer history data (signer states and votes)
+   * for all block numbers >= HEAD_BLOCK - CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
    *
-   * This defines a limit for reorgs on PoA clique chains
+   * This defines a limit for reorgs on PoA clique chains.
    */
   private CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT = 100
 
@@ -148,12 +148,12 @@ export default class Blockchain implements BlockchainInterface {
    * The top element from the array represents the list of current signers.
    * On reorgs elements from the array are removed until BLOCK_NUMBER > REORG_BLOCK.
    *
-   * Always keep at least one item on the stack
+   * Always keep at least one item on the stack.
    */
   private _cliqueLatestSignerStates: CliqueLatestSignerStates = []
 
   /**
-   * List with the latest signer votes
+   * List with the latest signer votes.
    *
    * Format:
    * [ [BLOCK_NUMBER_1, [SIGNER, BENEFICIARY, AUTH]], [BLOCK_NUMBER_1, [SIGNER, BENEFICIARY, AUTH]] ]
@@ -431,8 +431,7 @@ export default class Blockchain implements BlockchainInterface {
     let signers = this._cliqueLatestBlockSigners
     signers = signers.slice(signers.length < limit ? 0 : 1)
     signers.push([header.number, header.cliqueSigner()])
-    const seen = signers.filter((s) => s[1].toBuffer().equals(header.cliqueSigner().toBuffer()))
-      .length
+    const seen = signers.filter((s) => s[1].equals(header.cliqueSigner())).length
     return seen > 1
   }
 
@@ -460,15 +459,20 @@ export default class Blockchain implements BlockchainInterface {
 
     if (signerState) {
       this._cliqueLatestSignerStates.push(signerState)
+    }
 
-      // trim length to CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
-      const length = this._cliqueLatestSignerStates.length
-      const limit = this.CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
-      if (length > limit) {
-        this._cliqueLatestSignerStates = this._cliqueLatestSignerStates.slice(
-          length - limit,
-          length
-        )
+    // trim to CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
+    const limit = this.CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
+    const blockSigners = this._cliqueLatestBlockSigners
+    const lastBlockNumber = blockSigners[blockSigners.length - 1]?.[0]
+    if (lastBlockNumber) {
+      const blockLimit = lastBlockNumber.subn(limit)
+      const states = this._cliqueLatestSignerStates
+      const lastItem = states[states.length - 1]
+      this._cliqueLatestSignerStates = states.filter((state) => state[0].gte(blockLimit))
+      if (this._cliqueLatestSignerStates.length === 0) {
+        // always keep at least one item on the stack
+        this._cliqueLatestSignerStates.push(lastItem)
       }
     }
 
@@ -497,9 +501,7 @@ export default class Blockchain implements BlockchainInterface {
 
       const alreadyVoted = this._cliqueLatestVotes.find((vote) => {
         return (
-          vote[1][0].toBuffer().equals(signer.toBuffer()) &&
-          vote[1][1].toBuffer().equals(beneficiary.toBuffer()) &&
-          vote[1][2].equals(nonce)
+          vote[1][0].equals(signer) && vote[1][1].equals(beneficiary) && vote[1][2].equals(nonce)
         )
       })
         ? true
@@ -514,23 +516,28 @@ export default class Blockchain implements BlockchainInterface {
       this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
         (vote) =>
           !(
-            vote[1][0].toBuffer().equals(signer.toBuffer()) &&
-            vote[1][1].toBuffer().equals(beneficiary.toBuffer()) &&
+            vote[1][0].equals(signer) &&
+            vote[1][1].equals(beneficiary) &&
             vote[1][2].equals(oppositeNonce)
           )
       )
 
       // If same vote not already in history see if there is a new majority consensus
       // to update the signer list
       if (!alreadyVoted) {
+        const lastEpochBlockNumber = header.number.sub(
+          header.number.mod(new BN(this._common.consensusConfig().epoch))
+        )
         const beneficiaryVotesAuth = this._cliqueLatestVotes.filter(
           (vote) =>
-            vote[1][1].toBuffer().equals(beneficiary.toBuffer()) &&
+            vote[0].gte(lastEpochBlockNumber) &&
+            vote[1][1].equals(beneficiary) &&
             vote[1][2].equals(CLIQUE_NONCE_AUTH)
         )
         const beneficiaryVotesDrop = this._cliqueLatestVotes.filter(
           (vote) =>
-            vote[1][1].toBuffer().equals(beneficiary.toBuffer()) &&
+            vote[0].gte(lastEpochBlockNumber) &&
+            vote[1][1].equals(beneficiary) &&
             vote[1][2].equals(CLIQUE_NONCE_DROP)
         )
         const limit = this.cliqueSignerLimit()
@@ -545,16 +552,14 @@ export default class Blockchain implements BlockchainInterface {
             activeSigners.push(beneficiary)
             // Discard votes for added signer
             this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
-              (vote) => !vote[1][1].toBuffer().equals(beneficiary.toBuffer())
+              (vote) => !vote[1][1].equals(beneficiary)
             )
           } else {
             // Drop signer
-            activeSigners = activeSigners.filter(
-              (signer) => !signer.toBuffer().equals(beneficiary.toBuffer())
-            )
+            activeSigners = activeSigners.filter((signer) => !signer.equals(beneficiary))
             // Discard votes from removed signer
             this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
-              (vote) => !vote[1][0].toBuffer().equals(beneficiary.toBuffer())
+              (vote) => !vote[1][0].equals(beneficiary)
             )
           }
           const newSignerState: CliqueSignerState = [header.number, activeSigners]
@@ -563,11 +568,16 @@ export default class Blockchain implements BlockchainInterface {
       }
     }
 
-    // trim latest votes length to CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
-    const length = this._cliqueLatestVotes.length
+    // trim to lastEpochBlockNumber - CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
     const limit = this.CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
-    if (length > limit) {
-      this._cliqueLatestVotes = this._cliqueLatestVotes.slice(length - limit, length)
+    const blockSigners = this._cliqueLatestBlockSigners
+    const lastBlockNumber = blockSigners[blockSigners.length - 1]?.[0]
+    if (lastBlockNumber) {
+      const lastEpochBlockNumber = lastBlockNumber.sub(
+        lastBlockNumber.mod(new BN(this._common.consensusConfig().epoch))
+      )
+      const blockLimit = lastEpochBlockNumber.subn(limit)
+      this._cliqueLatestVotes = this._cliqueLatestVotes.filter((state) => state[0].gte(blockLimit))
     }
 
     // save votes to db
@@ -583,7 +593,8 @@ export default class Blockchain implements BlockchainInterface {
 
   /**
    * Update snapshot of latest clique block signers.
-   * Length trimmed to `this.cliqueSignerLimit()`
+   * Used for checking for 'recently signed' error.
+   * Length trimmed to `this.cliqueSignerLimit()`.
    * @param header BlockHeader
    * @hidden
    */
@@ -868,18 +879,17 @@ export default class Blockchain implements BlockchainInterface {
 
         // Clique: update signer votes and state
         if (this._common.consensusAlgorithm() === 'clique') {
-          // Reset all votes on epoch transition blocks
           if (header.cliqueIsEpochTransition()) {
-            this._cliqueLatestVotes = []
-            await this.cliqueUpdateVotes()
+            // note: keep votes on epoch transition blocks in case of reorgs.
+            // only active (non-stale) votes will counted (if vote.blockNumber >= lastEpochBlockNumber)
 
             // validate checkpoint signers towards active signers
             const checkpointSigners = header.cliqueEpochTransitionSigners()
             const activeSigners = this.cliqueActiveSigners()
-            for (const cSigner of checkpointSigners) {
-              if (!activeSigners.find((aSigner) => aSigner.toBuffer().equals(cSigner.toBuffer()))) {
+            for (const [i, cSigner] of checkpointSigners.entries()) {
+              if (!activeSigners[i] || !activeSigners[i].equals(cSigner)) {
                 throw new Error(
-                  'checkpoint signer not found in active signers list: ' + cSigner.toString()
+                  `checkpoint signer not found in active signers list at index ${i}: ${cSigner.toString()}`
                 )
               }
             }