From 6d64162c847c7cd145202ad7c0af276a286138cc Mon Sep 17 00:00:00 2001 From: g11tech Date: Fri, 3 Nov 2023 18:21:24 +0530 Subject: [PATCH] client: Patch fcu skeleton blockfill process to avoid chain reset (#3137) --- packages/client/src/rpc/modules/engine.ts | 6 - packages/client/src/service/skeleton.ts | 139 ++++++++++++---------- 2 files changed, 78 insertions(+), 67 deletions(-) diff --git a/packages/client/src/rpc/modules/engine.ts b/packages/client/src/rpc/modules/engine.ts index ba7aff7918..ea56d80d10 100644 --- a/packages/client/src/rpc/modules/engine.ts +++ b/packages/client/src/rpc/modules/engine.ts @@ -1231,12 +1231,6 @@ export class Engine { } } this.service.txPool.removeNewBlockTxs(blocks) - - const isPrevSynced = this.chain.config.synchronized - this.config.updateSynchronizedState(headBlock.header) - if (!isPrevSynced && this.chain.config.synchronized) { - this.service.txPool.checkRunState() - } } else if (!headBlock.isGenesis()) { // even if the vmHead is same still validations need to be done regarding the correctness // of the sequence and canonical-ity diff --git a/packages/client/src/service/skeleton.ts b/packages/client/src/service/skeleton.ts index 9531932c2c..4e4fbdbbdf 100644 --- a/packages/client/src/service/skeleton.ts +++ b/packages/client/src/service/skeleton.ts @@ -110,8 +110,6 @@ export class Skeleton extends MetaDBManager { private lastFcuTime = 0 private lastsyncedAt = 0 - private fillLogIndex = 0 - private STATUS_LOG_INTERVAL = 8000 /** How often to log sync status (in ms) */ /** @@ -644,7 +642,8 @@ export class Skeleton extends MetaDBManager { const prevLinked = this.status.linked const reorged = await this.setHead(headBlock, true) if (reorged && prevLinked && !this.status.linked) { - await this.blockingTailBackfillWithCutoff(this.chain.config.engineNewpayloadMaxExecute).catch( + // blocking fill with engineParentLookupMaxDepth as fcU tries to put max engineParentLookupMaxDepth + await this.blockingTailBackfillWithCutoff(this.chain.config.engineParentLookupMaxDepth).catch( (e) => { this.config.logger.debug(`blockingTailBackfillWithCutoff exited with error=${e}`) } @@ -782,7 +781,13 @@ export class Skeleton extends MetaDBManager { this.finalizedBlock = finalizedBlock ?? this.finalizedBlock }) - await this.blockingFillWithCutoff(this.chain.config.engineNewpayloadMaxExecute) + // blocking fill with engineParentLookupMaxDepth as fcU tries to put max engineParentLookupMaxDepth + // blocks if there are executed blocks to fill with. This blocking causes it to not interfere + // with the setHead mechanism. This is however a hack and a better solution needs to be devised + // to handle it blockchain level as because of async nature of new payloads and fcUs and the skeleton + // there is always a chance for uncordinated put blocks unless they are all cordinated through skeleton + // which might also be a valid + await this.blockingFillWithCutoff(this.chain.config.engineParentLookupMaxDepth) return { reorged, safeBlock: this.safeBlock, finalizedBlock: this.finalizedBlock } } @@ -1173,6 +1178,8 @@ export class Skeleton extends MetaDBManager { // run till it has not been determined that tail reset is required by concurrent setHead calls // filling is switched on and off by fillCanonicalChain only so no need to monitor that + let fillLogIndex = 0 + let skippedLogIndex = 0 while (!this.status.canonicalHeadReset && canonicalHead < subchain.head) { // Get next block const number = canonicalHead + BIGINT_1 @@ -1201,72 +1208,82 @@ export class Skeleton extends MetaDBManager { // Insert into chain let numBlocksInserted = 0 - try { - numBlocksInserted = await this.chain.putBlocks([block], true) - } catch (e) { - this.config.logger.error(`fillCanonicalChain putBlock error=${(e as Error).message}`) - if (oldHead !== null && oldHead.header.number >= block.header.number) { - // Put original canonical head block back if reorg fails - // UPDATE - // not sure we can put oldHead because the oldHead chain might have been partially overwritten - // skipping for now, leaving code here for future cleanup/debugging - // - // await this.chain.putBlocks([oldHead], true) - } - } - - // handle insertion failures - if (numBlocksInserted !== 1) { - this.config.logger.error( - `Failed to put block number=${number} fork=${block.common.hardfork()} hash=${short( - block.hash() - )} parentHash=${short(block.header.parentHash)}from skeleton chain to canonical` - ) - // Lets log some parent by number and parent by hash, that may help to understand whats going on - let parent = null + let numBlocksSkipped = 0 + // chain height has to be <= block number as we will skip putting this block as it might currently + // cause chain reset. This can happen if any other async process added a batch of blocks like + // execution's setHead. If that caused this chain to be not canonical anymore than the next + // putblocks should fail causing the fill to exit with skeleton stepback + if (this.chain.blocks.height <= block.header.number) { try { - parent = await this.chain.getBlock(number - BIGINT_1) - this.config.logger.info( - `ParentByNumber number=${parent?.header.number}, hash=${short( - parent?.hash() ?? 'undefined' - )} hf=${parent?.common.hardfork()}` - ) + numBlocksInserted = await this.chain.putBlocks([block], true) } catch (e) { - this.config.logger.error(`Failed to fetch parent of number=${number}`) + this.config.logger.error(`fillCanonicalChain putBlock error=${(e as Error).message}`) + if (oldHead !== null && oldHead.header.number >= block.header.number) { + // Put original canonical head block back if reorg fails + // UPDATE + // not sure we can put oldHead because the oldHead chain might have been partially overwritten + // skipping for now, leaving code here for future cleanup/debugging + // + // await this.chain.putBlocks([oldHead], true) + } } - let parentWithHash = null - try { - parentWithHash = await this.chain.getBlock(block.header.parentHash) - this.config.logger.info( - `parentByHash number=${parentWithHash?.header.number}, hash=${short( - parentWithHash?.hash() ?? 'undefined' - )} hf=${parentWithHash?.common.hardfork()} ` - ) - } catch (e) { + // handle insertion failures + if (numBlocksInserted !== 1) { this.config.logger.error( - `Failed to fetch parent with parentWithHash=${short(block.header.parentHash)}` + `Failed to put block number=${number} fork=${block.common.hardfork()} hash=${short( + block.hash() + )} parentHash=${short(block.header.parentHash)}from skeleton chain to canonical` ) - } + // Lets log some parent by number and parent by hash, that may help to understand whats going on + let parent = null + try { + parent = await this.chain.getBlock(number - BIGINT_1) + this.config.logger.info( + `ParentByNumber number=${parent?.header.number}, hash=${short( + parent?.hash() ?? 'undefined' + )} hf=${parent?.common.hardfork()}` + ) + } catch (e) { + this.config.logger.error(`Failed to fetch parent of number=${number}`) + } - // see if backstepping is required ot this is just canonicalHeadReset - await this.runWithLock(async () => { - if (!this.status.canonicalHeadReset) { - this.config.logger.debug( - `fillCanonicalChain canonicalHeadReset=${this.status.canonicalHeadReset}, backStepping...` + let parentWithHash = null + try { + parentWithHash = await this.chain.getBlock(block.header.parentHash) + this.config.logger.info( + `parentByHash number=${parentWithHash?.header.number}, hash=${short( + parentWithHash?.hash() ?? 'undefined' + )} hf=${parentWithHash?.common.hardfork()} ` ) - await this.backStep(number) - } else { - this.config.logger.debug( - `fillCanonicalChain canonicalHeadReset=${this.status.canonicalHeadReset}, breaking out...` + } catch (e) { + this.config.logger.error( + `Failed to fetch parent with parentWithHash=${short(block.header.parentHash)}` ) } - }) - break + + // see if backstepping is required ot this is just canonicalHeadReset + await this.runWithLock(async () => { + if (!this.status.canonicalHeadReset) { + this.config.logger.debug( + `fillCanonicalChain canonicalHeadReset=${this.status.canonicalHeadReset}, backStepping...` + ) + await this.backStep(number) + } else { + this.config.logger.debug( + `fillCanonicalChain canonicalHeadReset=${this.status.canonicalHeadReset}, breaking out...` + ) + } + }) + break + } + } else { + numBlocksSkipped = 1 } - canonicalHead += BigInt(numBlocksInserted) - this.fillLogIndex += numBlocksInserted + canonicalHead += BigInt(numBlocksInserted + numBlocksSkipped) + fillLogIndex += numBlocksInserted + skippedLogIndex += numBlocksSkipped // Delete skeleton block to clean up as we go, if block is fetched and chain is linked // it will be fetched from the chain without any issues // @@ -1283,16 +1300,16 @@ export class Skeleton extends MetaDBManager { await this.deleteBlock(block) } }) - if (this.fillLogIndex >= this.config.numBlocksPerIteration) { + if (fillLogIndex >= this.config.numBlocksPerIteration) { this.config.logger.debug( `Skeleton canonical chain fill status: canonicalHead=${canonicalHead} chainHead=${this.chain.blocks.height} subchainHead=${subchain.head}` ) - this.fillLogIndex = 0 + fillLogIndex = 0 } } this.filling = false this.config.logger.debug( - `Successfully put blocks start=${start} end=${canonicalHead} skeletonHead=${subchain.head} from skeleton chain to canonical syncTargetHeight=${this.config.syncTargetHeight}` + `Successfully put=${fillLogIndex} skipped (because already inserted)=${skippedLogIndex} blocks start=${start} end=${canonicalHead} skeletonHead=${subchain.head} from skeleton chain to canonical syncTargetHeight=${this.config.syncTargetHeight}` ) }