Merge pull request #1103 from ethereumjs/fix-goerli-clique-difficulty-bug

holgerd77 · web-flow · commit 761bef0454ec · 2021-02-14T09:34:57.000+01:00
Client, Blockchain: Fix Goerli Clique Difficulty Bug
diff --git a/packages/blockchain/README.md b/packages/blockchain/README.md
@@ -46,6 +46,24 @@ blockchain.iterator('i', (block) => {
 
 [Documentation](./docs/README.md)
 
+# DEVELOPER
+
+For debugging blockchain control flows the [debug](https://github.com/visionmedia/debug) library is used and can be activated on the CL with `DEBUG=[Logger Selection] node [Your Script to Run].js`.
+
+The following initial logger is currently available:
+
+| Logger | Description |
+| - | - |
+| `blockchain:clique` | Clique operations like updating the vote and/or signer list  |
+
+The following is an example for a logger run:
+
+Run with the clique logger:
+
+```shell
+DEBUG=blockchain:clique ts-node test.ts
+```
+
 # EthereumJS
 
 See our organizational [documentation](https://ethereumjs.readthedocs.io) for an introduction to `EthereumJS` as well as information on current standards and best practices.
diff --git a/packages/blockchain/package.json b/packages/blockchain/package.json
@@ -39,6 +39,7 @@
     "@ethereumjs/block": "^3.0.0",
     "@ethereumjs/common": "^2.0.0",
     "@ethereumjs/ethash": "^1.0.0",
+    "debug": "^2.2.0",
     "ethereumjs-util": "^7.0.8",
     "level-mem": "^5.0.1",
     "lru-cache": "^5.1.1",
diff --git a/packages/blockchain/src/index.ts b/packages/blockchain/src/index.ts
@@ -1,3 +1,4 @@
+import { debug as createDebugLogger } from 'debug'
 import Semaphore from 'semaphore-async-await'
 import { Address, BN, rlp } from 'ethereumjs-util'
 import { Block, BlockData, BlockHeader } from '@ethereumjs/block'
@@ -17,6 +18,8 @@ import {
   CLIQUE_NONCE_DROP,
 } from './clique'
 
+const debug = createDebugLogger('blockchain:clique')
+
 import type { LevelUp } from 'levelup'
 const level = require('level-mem')
 
@@ -445,6 +448,7 @@ export default class Blockchain implements BlockchainInterface {
       genesisBlock.header.cliqueEpochTransitionSigners(),
     ]
     await this.cliqueUpdateSignerStates(genesisSignerState)
+    debug(`[Block 0] Genesis block -> update signer states`)
     await this.cliqueUpdateVotes()
   }
 
@@ -483,6 +487,12 @@ export default class Blockchain implements BlockchainInterface {
     dbOps.push(DBOp.set(DBTarget.CliqueSignerStates, rlp.encode(formatted)))
 
     await this.dbManager.batch(dbOps)
+    // Output active signers for debugging purposes
+    let i = 0
+    for (const signer of this.cliqueActiveSigners()) {
+      debug(`Clique signer [${i}]: ${signer}`)
+      i++
+    }
   }
 
   /**
@@ -498,71 +508,115 @@ export default class Blockchain implements BlockchainInterface {
       const nonce = header.nonce
       const latestVote: CliqueVote = [header.number, [signer, beneficiary, nonce]]
 
-      const alreadyVoted = this._cliqueLatestVotes.find((vote) => {
-        return (
-          vote[1][0].equals(signer) && vote[1][1].equals(beneficiary) && vote[1][2].equals(nonce)
-        )
-      })
-        ? true
-        : false
-
-      // Always add the latest vote to the history no matter if already voted
-      // the same vote or not
-      this._cliqueLatestVotes.push(latestVote)
-
-      // remove any opposite votes for [signer, beneficiary]
-      const oppositeNonce = nonce.equals(CLIQUE_NONCE_AUTH) ? CLIQUE_NONCE_DROP : CLIQUE_NONCE_AUTH
-      this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
-        (vote) =>
-          !(
-            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) {
+      // Do two rounds here, one to execute on a potential previously reached consensus
+      // on the newly touched beneficiary, one with the added new vote
+      for (let round = 1; round <= 2; round++) {
+        // See if there is a new majority consensus to update the signer list
         const lastEpochBlockNumber = header.number.sub(
           header.number.mod(new BN(this._common.consensusConfig().epoch))
         )
-        const beneficiaryVotesAuth = this._cliqueLatestVotes.filter(
-          (vote) =>
+        const limit = this.cliqueSignerLimit()
+        let activeSigners = this.cliqueActiveSigners()
+        let consensus = false
+
+        // AUTH vote analysis
+        let votes = this._cliqueLatestVotes.filter((vote) => {
+          return (
             vote[0].gte(lastEpochBlockNumber) &&
+            !vote[1][0].equals(signer) &&
             vote[1][1].equals(beneficiary) &&
             vote[1][2].equals(CLIQUE_NONCE_AUTH)
-        )
-        const beneficiaryVotesDrop = this._cliqueLatestVotes.filter(
-          (vote) =>
+          )
+        })
+        const beneficiaryVotesAUTH: Address[] = []
+        for (const vote of votes) {
+          const num = beneficiaryVotesAUTH.filter((voteCMP) => {
+            return voteCMP.equals(vote[1][0])
+          }).length
+          if (num === 0) {
+            beneficiaryVotesAUTH.push(vote[1][0])
+          }
+        }
+        let numBeneficiaryVotesAUTH = beneficiaryVotesAUTH.length
+        if (round === 2 && nonce.equals(CLIQUE_NONCE_AUTH)) {
+          numBeneficiaryVotesAUTH += 1
+        }
+        // Majority consensus
+        if (numBeneficiaryVotesAUTH >= limit) {
+          consensus = true
+          // Authorize new signer
+          activeSigners.push(beneficiary)
+          activeSigners.sort((a, b) => {
+            // Sort by buffer size
+            return a.toBuffer().compare(b.toBuffer())
+          })
+          // Discard votes for added signer
+          this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
+            (vote) => !vote[1][1].equals(beneficiary)
+          )
+          debug(
+            `[Block ${header.number.toNumber()}] Clique majority consensus (AUTH ${beneficiary})`
+          )
+        }
+        // DROP vote
+        votes = this._cliqueLatestVotes.filter((vote) => {
+          return (
             vote[0].gte(lastEpochBlockNumber) &&
+            !vote[1][0].equals(signer) &&
             vote[1][1].equals(beneficiary) &&
             vote[1][2].equals(CLIQUE_NONCE_DROP)
-        )
-        const limit = this.cliqueSignerLimit()
-        const consensus =
-          beneficiaryVotesAuth.length >= limit || beneficiaryVotesDrop.length >= limit
-        const auth = beneficiaryVotesAuth.length >= limit
+          )
+        })
+        const beneficiaryVotesDROP: Address[] = []
+        for (const vote of votes) {
+          const num = beneficiaryVotesDROP.filter((voteCMP) => {
+            return voteCMP.equals(vote[1][0])
+          }).length
+          if (num === 0) {
+            beneficiaryVotesDROP.push(vote[1][0])
+          }
+        }
+        let numBeneficiaryVotesDROP = beneficiaryVotesDROP.length
+
+        if (round === 2 && nonce.equals(CLIQUE_NONCE_DROP)) {
+          numBeneficiaryVotesDROP += 1
+        }
         // Majority consensus
+        if (numBeneficiaryVotesDROP >= limit) {
+          consensus = true
+          // Drop signer
+          activeSigners = activeSigners.filter((signer) => !signer.equals(beneficiary))
+          this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
+            // Discard votes from removed signer and for removed signer
+            (vote) => !vote[1][0].equals(beneficiary) && !vote[1][1].equals(beneficiary)
+          )
+          debug(
+            `[Block ${header.number.toNumber()}] Clique majority consensus (DROP ${beneficiary})`
+          )
+        }
+        if (round === 1) {
+          // Always add the latest vote to the history no matter if already voted
+          // the same vote or not
+          this._cliqueLatestVotes.push(latestVote)
+          debug(
+            `[Block ${header.number.toNumber()}] New clique vote: ${signer} -> ${beneficiary} ${
+              nonce.equals(CLIQUE_NONCE_AUTH) ? 'AUTH' : 'DROP'
+            }`
+          )
+        }
         if (consensus) {
-          let activeSigners = this.cliqueActiveSigners()
-          if (auth) {
-            // Authorize new signer
-            activeSigners.push(beneficiary)
-            // Discard votes for added signer
-            this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
-              (vote) => !vote[1][1].equals(beneficiary)
+          if (round === 1) {
+            debug(
+              `[Block ${header.number.toNumber()}] Clique majority consensus on existing votes -> update signer states`
             )
           } else {
-            // Drop signer
-            activeSigners = activeSigners.filter((signer) => !signer.equals(beneficiary))
-            // Discard votes from removed signer
-            this._cliqueLatestVotes = this._cliqueLatestVotes.filter(
-              (vote) => !vote[1][0].equals(beneficiary)
+            debug(
+              `[Block ${header.number.toNumber()}] Clique majority consensus on new vote -> update signer states`
             )
           }
           const newSignerState: CliqueSignerState = [header.number, activeSigners]
           await this.cliqueUpdateSignerStates(newSignerState)
+          return
         }
       }
     }
@@ -823,11 +877,7 @@ export default class Blockchain implements BlockchainInterface {
 
       if (this._validateBlocks && !isGenesis) {
         // this calls into `getBlock`, which is why we cannot lock yet
-        if (item instanceof BlockHeader) {
-          await block.header.validate(this)
-        } else {
-          await block.validate(this)
-        }
+        await block.validate(this)
       }
 
       if (this._validateConsensus) {
diff --git a/packages/blockchain/test/clique.spec.ts b/packages/blockchain/test/clique.spec.ts
@@ -44,61 +44,61 @@ tape('Clique: Initialization', (t) => {
   }
 
   const B: Signer = {
-    address: new Address(Buffer.from('dc7bc81ddf67d037d7439f8e6ff12f3d2a100f71', 'hex')),
+    address: new Address(Buffer.from('6f62d8382bf2587361db73ceca28be91b2acb6df', 'hex')),
     privateKey: Buffer.from(
-      '86b0ff7b6cf70786f29f297c57562905ab0b6c32d69e177a46491e56da9e486e',
+      '2a6e9ad5a6a8e4f17149b8bc7128bf090566a11dbd63c30e5a0ee9f161309cd6',
       'hex'
     ),
     publicKey: Buffer.from(
-      'd3e3d2b722e325bfc085ff5638a112b4e7e88ff13f92fc7f6cfc14b5a25e8d1545a2f27d8537b96e8919949d5f8c139ae7fc81aea7cf7fe5d43d7faaa038e35b',
+      'ca0a55f6e81cb897aee6a1c390aa83435c41048faa0564b226cfc9f3df48b73e846377fb0fd606df073addc7bd851f22547afbbdd5c3b028c91399df802083a2',
       'hex'
     ),
   }
 
   const C: Signer = {
-    address: new Address(Buffer.from('8458f408106c4875c96679f3f556a511beabe138', 'hex')),
+    address: new Address(Buffer.from('83c30730d1972baa09765a1ac72a43db27fedce5', 'hex')),
     privateKey: Buffer.from(
-      '159e95d07a6c64ddbafa6036cdb7b8114e6e8cdc449ca4b0468a6d0c955f991b',
+      'f216ddcf276079043c52b5dd144aa073e6b272ad4bfeaf4fbbc044aa478d1927',
       'hex'
     ),
     publicKey: Buffer.from(
-      'f02724341e2df54cf53515f079b1354fa8d437e79c5b091b8d8cc7cbcca00fd8ad854cb3b3a85b06c44ecb7269404a67be88b561f2224c94d133e5fc21be915c',
+      '555b19a5cbe6dd082a4a1e1e0520dd52a82ba24fd5598ea31f0f31666c40905ed319314c5fb06d887b760229e1c0e616294e7b1cb5dfefb71507c9112132ce56',
       'hex'
     ),
   }
 
   const D: Signer = {
-    address: new Address(Buffer.from('83c30730d1972baa09765a1ac72a43db27fedce5', 'hex')),
+    address: new Address(Buffer.from('8458f408106c4875c96679f3f556a511beabe138', 'hex')),
     privateKey: Buffer.from(
-      'f216ddcf276079043c52b5dd144aa073e6b272ad4bfeaf4fbbc044aa478d1927',
+      '159e95d07a6c64ddbafa6036cdb7b8114e6e8cdc449ca4b0468a6d0c955f991b',
       'hex'
     ),
     publicKey: Buffer.from(
-      '555b19a5cbe6dd082a4a1e1e0520dd52a82ba24fd5598ea31f0f31666c40905ed319314c5fb06d887b760229e1c0e616294e7b1cb5dfefb71507c9112132ce56',
+      'f02724341e2df54cf53515f079b1354fa8d437e79c5b091b8d8cc7cbcca00fd8ad854cb3b3a85b06c44ecb7269404a67be88b561f2224c94d133e5fc21be915c',
       'hex'
     ),
   }
 
   const E: Signer = {
-    address: new Address(Buffer.from('6f62d8382bf2587361db73ceca28be91b2acb6df', 'hex')),
+    address: new Address(Buffer.from('ab80a948c661aa32d09952d2a6c4ad77a4c947be', 'hex')),
     privateKey: Buffer.from(
-      '2a6e9ad5a6a8e4f17149b8bc7128bf090566a11dbd63c30e5a0ee9f161309cd6',
+      '48ec5a6c4a7fc67b10a9d4c8a8f594a81ae42e41ed061fa5218d96abb6012344',
       'hex'
     ),
     publicKey: Buffer.from(
-      'ca0a55f6e81cb897aee6a1c390aa83435c41048faa0564b226cfc9f3df48b73e846377fb0fd606df073addc7bd851f22547afbbdd5c3b028c91399df802083a2',
+      'adefb82b9f54e80aa3532263e4478739de16fcca6828f4ae842f8a07941c347fa59d2da1300569237009f0f122dc1fd6abb0db8fcb534280aa94948a5cc95f94',
       'hex'
     ),
   }
 
   const F: Signer = {
-    address: new Address(Buffer.from('ab80a948c661aa32d09952d2a6c4ad77a4c947be', 'hex')),
+    address: new Address(Buffer.from('dc7bc81ddf67d037d7439f8e6ff12f3d2a100f71', 'hex')),
     privateKey: Buffer.from(
-      '48ec5a6c4a7fc67b10a9d4c8a8f594a81ae42e41ed061fa5218d96abb6012344',
+      '86b0ff7b6cf70786f29f297c57562905ab0b6c32d69e177a46491e56da9e486e',
       'hex'
     ),
     publicKey: Buffer.from(
-      'adefb82b9f54e80aa3532263e4478739de16fcca6828f4ae842f8a07941c347fa59d2da1300569237009f0f122dc1fd6abb0db8fcb534280aa94948a5cc95f94',
+      'd3e3d2b722e325bfc085ff5638a112b4e7e88ff13f92fc7f6cfc14b5a25e8d1545a2f27d8537b96e8919949d5f8c139ae7fc81aea7cf7fe5d43d7faaa038e35b',
       'hex'
     ),
   }
@@ -384,7 +384,7 @@ tape('Clique: Initialization', (t) => {
     await addNextBlock(blockchain, blocks, A)
     await addNextBlock(blockchain, blocks, B, [C, true])
 
-    st.deepEqual(blockchain.cliqueActiveSigners(), [A.address, B.address, D.address, C.address])
+    st.deepEqual(blockchain.cliqueActiveSigners(), [A.address, B.address, C.address, D.address])
     st.end()
   })
 
diff --git a/packages/blockchain/test/reorg.spec.ts b/packages/blockchain/test/reorg.spec.ts
@@ -177,7 +177,9 @@ tape('reorg tests', (t) => {
 
       let signerStates = (blockchain as any)._cliqueLatestSignerStates
       t.ok(
-        !signerStates.find((s: any) => s[0].eqn(2) && s[1][1].equals(beneficiary1)),
+        !signerStates.find(
+          (s: any) => s[0].eqn(2) && s[1].find((a: Address) => a.equals(beneficiary1))
+        ),
         'should not find reorged signer state'
       )
 
@@ -203,7 +205,9 @@ tape('reorg tests', (t) => {
 
       signerStates = (blockchain as any)._cliqueLatestSignerStates
       t.ok(
-        !!signerStates.find((s: any) => s[0].eqn(2) && s[1][1].equals(beneficiary2)),
+        !!signerStates.find(
+          (s: any) => s[0].eqn(2) && s[1].find((a: Address) => a.equals(beneficiary2))
+        ),
         'should find reorged signer state'
       )
 
diff --git a/packages/client/lib/service/ethereumservice.ts b/packages/client/lib/service/ethereumservice.ts
@@ -17,7 +17,7 @@ export interface EthereumServiceOptions extends ServiceOptions {
   /* Sync retry interval in ms (default: 8000) */
   interval?: number
 
-  /* Protocol timeout in ms (default: 4000) */
+  /* Protocol timeout in ms (default: 6000) */
   timeout?: number
 }
 
@@ -42,7 +42,7 @@ export class EthereumService extends Service {
     this.flow = new FlowControl()
     this.chain = options.chain ?? new Chain(options)
     this.interval = options.interval ?? 8000
-    this.timeout = options.timeout ?? 4000
+    this.timeout = options.timeout ?? 6000
   }
 
   /**
diff --git a/packages/client/lib/sync/execution/vmexecution.ts b/packages/client/lib/sync/execution/vmexecution.ts
@@ -115,7 +115,10 @@ export class VMExecution extends Execution {
               this.hardfork = this.config.execCommon.setHardforkByBlockNumber(blockNumber)
               this.vm._updateOpcodes()
             }
-            await this.vm.runBlock({ block, root: parentState })
+            // Block validation is redundant here and leads to consistency problems
+            // on PoA clique along blockchain-including validation checks
+            // (signer states might have moved on when sync is ahead)
+            await this.vm.runBlock({ block, root: parentState, skipBlockValidation: true })
             txCounter += block.transactions.length
             // set as new head block
             headBlock = block
diff --git a/packages/vm/README.md b/packages/vm/README.md
@@ -177,7 +177,7 @@ The following loggers are currently available:
 
 | Logger | Description |
 | - | - |
-| `vm:block` | Block operations (run txs, generating receipts, block rewards,...)
+| `vm:block` | Block operations (run txs, generating receipts, block rewards,...) |
 | `vm:tx` | Transaction operations (account updates, checkpointing,...) |
 | `vm:tx:gas` | Transaction gas logger |
 | `vm:evm` | EVM control flow, CALL or CREATE message execution |

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ export interface EthereumServiceOptions extends ServiceOptions {`
`17`	`17`	`/* Sync retry interval in ms (default: 8000) */`
`18`	`18`	`interval?: number`
`19`	`19`
`20`		`- /* Protocol timeout in ms (default: 4000) */`
	`20`	`+ /* Protocol timeout in ms (default: 6000) */`
`21`	`21`	`timeout?: number`
`22`	`22`	`}`
`23`	`23`
`@@ -42,7 +42,7 @@ export class EthereumService extends Service {`
`42`	`42`	`this.flow = new FlowControl()`
`43`	`43`	`this.chain = options.chain ?? new Chain(options)`
`44`	`44`	`this.interval = options.interval ?? 8000`
`45`		`- this.timeout = options.timeout ?? 4000`
	`45`	`+ this.timeout = options.timeout ?? 6000`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`/**`