-
Notifications
You must be signed in to change notification settings - Fork 760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
blockchain: allow optimistic block insertion in blockchain #3584
base: master
Are you sure you want to change the base?
Changes from 3 commits
3b24f64
a69d63f
e640849
bbb140b
23fd13f
c7f2ae2
d9e3297
7bd20c2
e4253d6
3bbed08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,11 +8,14 @@ import { | |
hashToNumberKey, | ||
headerKey, | ||
numberToHashKey, | ||
optimisticNumberToHashKey, | ||
tdKey, | ||
} from './constants.js' | ||
|
||
import type { CacheMap } from './manager.js' | ||
|
||
export type OptimisticOpts = { fcUed: boolean; linked?: boolean } | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These options are not really "understandable" in a blockchain context respectively bring in concepts from the client (FcU), and a blockchain "should not need to know what an FcU is". 😛 Can we rename this more to the point, so with what this is doing here from the blockchain perspective (I honestly also cannot read this out of the name and bring the two things together)? Also These options should - if we keep - also move to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (maybe then directly give this some code docs) |
||
export enum DBTarget { | ||
Heads, | ||
HeadHeader, | ||
|
@@ -25,6 +28,7 @@ export enum DBTarget { | |
CliqueSignerStates, | ||
CliqueVotes, | ||
CliqueBlockSigners, | ||
OptimisticNumberToHash, | ||
} | ||
|
||
/** | ||
|
@@ -88,6 +92,11 @@ export class DBOp { | |
this.cacheString = 'numberToHash' | ||
break | ||
} | ||
case DBTarget.OptimisticNumberToHash: { | ||
this.baseDBOp.key = optimisticNumberToHashKey(key!.blockNumber!) | ||
this.cacheString = 'optimisticNumberToHash' | ||
break | ||
} | ||
case DBTarget.TotalDifficulty: { | ||
this.baseDBOp.key = tdKey(key!.blockNumber!, key!.blockHash!) | ||
this.cacheString = 'td' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this diff has primarily shifted into the else part of the optimistic condition on L414 with some basic initializations still out side the if {} else {} block