Skip to content

Commit

Permalink
client: Patch fcu skeleton blockfill process to avoid chain reset (#3137
Browse files Browse the repository at this point in the history
)
  • Loading branch information
g11tech authored Nov 3, 2023
1 parent 3a00a32 commit 6d64162
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 67 deletions.
6 changes: 0 additions & 6 deletions packages/client/src/rpc/modules/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
139 changes: 78 additions & 61 deletions packages/client/src/service/skeleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) */

/**
Expand Down Expand Up @@ -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}`)
}
Expand Down Expand Up @@ -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 }
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<void>(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<void>(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
//
Expand All @@ -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}`
)
}

Expand Down

0 comments on commit 6d64162

Please sign in to comment.