client: some skeleton improvements from observations on devnet syncs

packages/block/src/index.ts

@@ -1,4 +1,5 @@
 export { Block } from './block.js'
+export { executionPayloadFromBeaconPayload } from './from-beacon-payload.js'
 export { BlockHeader } from './header.js'
 export { getDifficulty, valuesArrayToHeaderData } from './helpers.js'
 export * from './types.js'

packages/client/package.json

@@ -98,6 +98,7 @@
     "@types/yargs": "^17.0.24",
     "constants-browserify": "^1.0.0",
     "crypto-browserify": "^3.12.0",
+    "eventsource": "^2.0.2",
     "file-replace-loader": "^1.2.0",
     "it-pair": "^1.0.0",
     "it-pushable": "^1.4.2",

packages/client/src/sync/skeleton.ts

@@ -237,7 +237,7 @@ export class Skeleton extends MetaDBManager {
       const mayBeDupBlock = await this.getBlock(number)
       if (mayBeDupBlock !== undefined && equalsBytes(mayBeDupBlock.header.hash(), head.hash())) {
         this.config.logger.debug(
-          `Skeleton duplicate announcement tail=${lastchain.tail} head=${
+          `Skeleton duplicate ${force ? 'setHead' : 'announcement'} tail=${lastchain.tail} head=${
             lastchain.head
           } number=${number} hash=${short(head.hash())}`
         )
@@ -322,15 +322,22 @@ export class Skeleton extends MetaDBManager {
           head.hash()
         )} force=${force}`
       )
+
+      const subchain0Head = this.status.progress.subchains[0]?.head ?? BigInt(0)
       const reorg = await this.processNewHead(head, force)
+
       if (force && reorg) {
-        // It could just be a reorg at this head with previous tail preserved
+        // It could just be a reorg at this head with previous tail preserved unless
+        //   1. parent is not present in skeleton (it could be in chain for whatever reason) or
+        //   2. the parent < current skeleton tail
+        // then we need to reset the subchain itself
         const subchain = this.status.progress.subchains[0]
-        const parent = await this.getBlock(head.header.number - BigInt(1))
+        const parent = await this.getBlock(head.header.number - BigInt(1), true)
         if (
           subchain === undefined ||
           parent === undefined ||
-          !equalsBytes(parent.hash(), head.header.parentHash)
+          !equalsBytes(parent.hash(), head.header.parentHash) ||
+          parent.header.number < subchain.tail
         ) {
           const s = {
             head: head.header.number,
@@ -359,7 +366,7 @@ export class Skeleton extends MetaDBManager {
       if (force || init) {
         await this.writeSyncStatus()
       }
-      if (force && this.status.linked) {
+      if (force && this.status.linked && head.header.number > subchain0Head) {
         void this.fillCanonicalChain()
       }
       // Earlier we were throwing on reorg, essentially for the purposes for killing the reverse fetcher
@@ -418,18 +425,33 @@ export class Skeleton extends MetaDBManager {
         continue
       } else {
         // Partially overwritten, trim the head to the overwritten size
+        this.status.progress.subchains[1].head = this.status.progress.subchains[0].tail - BigInt(1)
         this.config.logger.debug(
-          `Previous subchain partially overwritten head=${head} tail=${tail} next=${short(next)}`
+          `Previous subchain partially overwritten head=${head} tail=${tail} next=${short(
+            next
+          )} with newHead=${this.status.progress.subchains[1].head}`
         )
-        this.status.progress.subchains[1].head = this.status.progress.subchains[0].tail - BigInt(1)
         edited = true
       }
       // If the old subchain is an extension of the new one, merge the two
       // and let the skeleton syncer restart (to clean internal state)
 
-      const subChain1Head = await this.getBlock(this.status.progress.subchains[1].head)
+      // subchains are useful if subChain1Head is in skeleton only and its tail correct
+      const subChain1Head = await this.getBlock(this.status.progress.subchains[1].head, true)
+      // tail lookup can be from skeleton or chain
+      const subChain1Tail = await this.getBlock(this.status.progress.subchains[1].tail)
       if (
-        subChain1Head !== undefined &&
+        subChain1Head === undefined ||
+        subChain1Tail === undefined ||
+        !equalsBytes(subChain1Tail.header.parentHash, this.status.progress.subchains[1].next)
+      ) {
+        // if subChain1Head is not in the skeleton then all previous subchains are not useful
+        // and better to junk
+        this.config.logger.debug(
+          `Removing all previous subchains as skeleton missing block at previous subchain head=${this.status.progress.subchains[1].head} or its tail=${this.status.progress.subchains[1].tail}`
+        )
+        this.status.progress.subchains.splice(1, this.status.progress.subchains.length - 1)
+      } else if (
         equalsBytes(subChain1Head.hash(), this.status.progress.subchains[0].next) === true
       ) {
         // only merge is we can integrate a big progress, as each merge leads
@@ -565,13 +587,14 @@ export class Skeleton extends MetaDBManager {
     })
   }
 
-  private async backStep(): Promise<bigint | null> {
+  private async backStep(fromBlock: bigint): Promise<bigint | null> {
     try {
       if (this.config.skeletonFillCanonicalBackStep <= 0) return null
       const { head, tail } = this.bounds()
+      // by default we try back stepping from tail or fromBlock whichever is bigger
+      let newTail: bigint | null = tail < fromBlock ? fromBlock : tail
 
       let tailBlock
-      let newTail: bigint | null = tail
       do {
         newTail = newTail + BigInt(this.config.skeletonFillCanonicalBackStep)
         tailBlock = await this.getBlock(newTail, true)
@@ -597,6 +620,7 @@ export class Skeleton extends MetaDBManager {
         return null
       }
     } finally {
+      this.status.canonicalHeadReset = true
       this.status.linked = await this.checkLinked()
     }
   }
@@ -646,7 +670,8 @@ export class Skeleton extends MetaDBManager {
           `fillCanonicalChain block number=${number} not found, backStepping`
         )
         await this.runWithLock<void>(async () => {
-          await this.backStep()
+          // backstep the subchain from the block that was not found
+          await this.backStep(number)
         })
         break
       }
@@ -661,6 +686,7 @@ export class Skeleton extends MetaDBManager {
           await this.chain.putBlocks([oldHead], true)
         }
       }
+
       if (numBlocksInserted !== 1) {
         this.config.logger.error(
           `Failed to put block number=${number} fork=${block.common.hardfork()} hash=${short(
@@ -694,7 +720,7 @@ export class Skeleton extends MetaDBManager {
           )
         }
         await this.runWithLock<void>(async () => {
-          await this.backStep()
+          await this.backStep(number)
         })
         break
       }

packages/client/test/sim/beaconsync.md

@@ -0,0 +1,31 @@
+### Beaconsync sim setup
+
+## Prerequisites
+
+1. ZSH terminal
+2. Docker (without sudo)
+3. `jq` & `curl` installed
+4. `ethereumjs-monorepo` codebase build via `npm i` (for e.g. at `/usr/app/ethereumjs`)
+
+You may pre-download docker images for lodestar (`docker pull chainsafe/lodestar:latest`) and geth (`docker pull ethereum/client-go:v1.12.2`) to avoid any test timeout issues.
+
+Note: All commands should be run from the `client` package directory root (so something like `/usr/app/ethereumjs/packages/client`)
+
+## How to run
+
+1. Cleanup some datadirs (if you have had previous runs)
+
+```bash
+rm -rf ./datadir
+``` 
+
+2. Run the sim
+
+```bash
+BEACON_SYNC=true  NETWORK=mainnet NETWORKID=1337903 ELCLIENT=geth npx vitest run test/sim/beaconsync.spec.ts
+```
+
+or just
+```bash
+rm -rf ./datadir; DEBUG=ethjs,client:* BEACON_SYNC=true  NETWORK=mainnet NETWORKID=1337903 ELCLIENT=geth npx vitest run test/sim/beaconsync.spec.ts
+```