Skip to content
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.

Sign up for GitHub

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

Clique: get signers by block num #2610

Merged
merged 11 commits into from
Apr 10, 2023
Merged

Clique: get signers by block num #2610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
48cb53c
blockchain/clique: add support for clique signers by block num
jochem-brouwer Mar 30, 2023
d344d90
blockchain: clique sort clique latest signer states
jochem-brouwer Mar 30, 2023
43457f9
client: fix build
jochem-brouwer Mar 30, 2023
efab9ee
blockchain/clique: fix tests and add test
jochem-brouwer Mar 30, 2023
0e28a79
blockchain/clique: fix clique bugs
jochem-brouwer Mar 30, 2023
604f264
blockchain/tests/clique: fix tests
jochem-brouwer Mar 30, 2023
2a064e2
blockchain: clique: add reorg test
jochem-brouwer Apr 5, 2023
a98c80b
blockchain/clique: add reorg over epoch test (fails) [no ci]
jochem-brouwer Apr 5, 2023
4bd488d
replace equalsBytes with deepEquals
acolytec3 Apr 10, 2023
8c96d06
blockchain/clique: address comments
jochem-brouwer Apr 10, 2023
924a888
blockchain/clique: ensure tests are ran
jochem-brouwer Apr 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 43 additions & 18 deletions packages/blockchain/src/consensus/clique.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ type CliqueLatestBlockSigners = CliqueBlockSigner[]

/**
* This class encapsulates Clique-related consensus functionality when used with the Blockchain class.
* Note: reorgs which happen between epoch transitions, which change the internal voting state over the reorg
* will result in failure and is currently not supported.
* The hotfix for this could be: re-load the latest epoch block (this has the clique state in the extraData of the header)
* Now replay all blocks on top of it. This should validate the chain up to the new/reorged tip which previously threw.
*/
export class CliqueConsensus implements Consensus {
blockchain: Blockchain | undefined
Expand All @@ -58,7 +62,7 @@ export class CliqueConsensus implements Consensus {
*
* This defines a limit for reorgs on PoA clique chains.
*/
private CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT = 100
private CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT = 200

/**
* List with the latest signer states checkpointed on blocks where
Expand Down Expand Up @@ -112,6 +116,7 @@ export class CliqueConsensus implements Consensus {
async setup({ blockchain }: ConsensusOptions): Promise<void> {
this.blockchain = blockchain
this._cliqueLatestSignerStates = await this.getCliqueLatestSignerStates()
this._cliqueLatestSignerStates.sort((a, b) => (a[0] > b[0] ? 1 : -1))
this._cliqueLatestVotes = await this.getCliqueLatestVotes()
this._cliqueLatestBlockSigners = await this.getCliqueLatestBlockSigners()
}
Expand All @@ -126,7 +131,7 @@ export class CliqueConsensus implements Consensus {
}

const { header } = block
const valid = header.cliqueVerifySignature(this.cliqueActiveSigners())
const valid = header.cliqueVerifySignature(this.cliqueActiveSigners(header.number))
if (!valid) {
throw new Error('invalid PoA block signature (clique)')
}
Expand All @@ -140,7 +145,7 @@ export class CliqueConsensus implements Consensus {
// only active (non-stale) votes will counted (if vote.blockNumber >= lastEpochBlockNumber

const checkpointSigners = header.cliqueEpochTransitionSigners()
const activeSigners = this.cliqueActiveSigners()
const activeSigners = this.cliqueActiveSigners(header.number)
for (const [i, cSigner] of checkpointSigners.entries()) {
if (activeSigners[i]?.equals(cSigner) !== true) {
throw new Error(
Expand All @@ -161,7 +166,7 @@ export class CliqueConsensus implements Consensus {
throw new Error(`${msg} ${header.errorStr()}`)
}

const signers = this.cliqueActiveSigners()
const signers = this.cliqueActiveSigners(header.number)
if (signers.length === 0) {
// abort if signers are unavailable
const msg = 'no signers available'
Expand Down Expand Up @@ -215,7 +220,17 @@ export class CliqueConsensus implements Consensus {
*/
private async cliqueUpdateSignerStates(signerState?: CliqueSignerState) {
if (signerState) {
const blockNumber = signerState[0]
const known = this._cliqueLatestSignerStates.find((value) => {
if (value[0] === blockNumber) {
return true
}
})
if (known !== undefined) {
return
}
this._cliqueLatestSignerStates.push(signerState)
this._cliqueLatestSignerStates.sort((a, b) => (a[0] > b[0] ? 1 : -1))
}

// trim to CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
Expand All @@ -240,10 +255,15 @@ export class CliqueConsensus implements Consensus {
])
await this.blockchain!.db.put(CLIQUE_SIGNERS_KEY, RLP.encode(formatted), DB_OPTS)
// Output active signers for debugging purposes
let i = 0
for (const signer of this.cliqueActiveSigners()) {
debug(`Clique signer [${i}]: ${signer}`)
i++
if (signerState !== undefined) {
let i = 0
try {
for (const signer of this.cliqueActiveSigners(signerState[0])) {
debug(`Clique signer [${i}]: ${signer} (block: ${signerState[0]})`)
i++
}
// eslint-disable-next-line no-empty
} catch (e) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the error that could occur here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have commented because now I don't remember. I think it was something when I tried to sync Goerli, and then for some reason signerState[0] was undefined, which throws the method. (If you want I can re-investigate)

}
}

Expand All @@ -268,8 +288,8 @@ export class CliqueConsensus implements Consensus {
header.number -
(header.number %
BigInt((this.blockchain!._common.consensusConfig() as CliqueConfig).epoch))
const limit = this.cliqueSignerLimit()
let activeSigners = this.cliqueActiveSigners()
const limit = this.cliqueSignerLimit(header.number)
let activeSigners = [...this.cliqueActiveSigners(header.number)]
let consensus = false

// AUTH vote analysis
Expand Down Expand Up @@ -400,12 +420,17 @@ export class CliqueConsensus implements Consensus {
/**
* Returns a list with the current block signers
*/
cliqueActiveSigners(): Address[] {
cliqueActiveSigners(blockNum: bigint): Address[] {
const signers = this._cliqueLatestSignerStates
if (signers.length === 0) {
return []
}
return [...signers[signers.length - 1][1]]
for (let i = signers.length - 1; i >= 0; i--) {
if (signers[i][0] < blockNum) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just clarify my understanding here, we're just returning the most recent signer state, (which might be from a block older than the current chain head if no signer state change occurs on the current block, right?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you got the idea here, but just for clarity:

  • This solves the reorg problem, because before it always returned whatever active signers there were at current chain head. (On client this could result in execution vs block import being out of sync which thus require different active signers if you verify them)
  • Note that _cliqueLatestSignerStates is sorted
  • We return the first candidate which has active signers < requested block num (so the state we want)
  • Note that signers always has 1 member

return signers[i][1]
}
}
throw new Error(`Could not load signers for block ${blockNum}`)
}

/**
Expand All @@ -415,8 +440,8 @@ export class CliqueConsensus implements Consensus {
* 1 -> 1, 2 -> 2, 3 -> 2, 4 -> 2, 5 -> 3, ...
* @hidden
*/
private cliqueSignerLimit() {
return Math.floor(this.cliqueActiveSigners().length / 2) + 1
private cliqueSignerLimit(blockNum: bigint) {
return Math.floor(this.cliqueActiveSigners(blockNum).length / 2) + 1
}

/**
Expand All @@ -430,7 +455,7 @@ export class CliqueConsensus implements Consensus {
// skip genesis, first block
return false
}
const limit = this.cliqueSignerLimit()
const limit = this.cliqueSignerLimit(header.number)
// construct recent block signers list with this block
let signers = this._cliqueLatestBlockSigners
signers = signers.slice(signers.length < limit ? 0 : 1)
Expand Down Expand Up @@ -484,7 +509,7 @@ export class CliqueConsensus implements Consensus {

// trim length to `this.cliqueSignerLimit()`
const length = this._cliqueLatestBlockSigners.length
const limit = this.cliqueSignerLimit()
const limit = this.cliqueSignerLimit(header.number)
if (length > limit) {
this._cliqueLatestBlockSigners = this._cliqueLatestBlockSigners.slice(
length - limit,
Expand Down Expand Up @@ -591,8 +616,8 @@ export class CliqueConsensus implements Consensus {
* Helper to determine if a signer is in or out of turn for the next block.
* @param signer The signer address
*/
async cliqueSignerInTurn(signer: Address): Promise<boolean> {
const signers = this.cliqueActiveSigners()
async cliqueSignerInTurn(signer: Address, blockNum: bigint): Promise<boolean> {
const signers = this.cliqueActiveSigners(blockNum)
const signerIndex = signers.findIndex((address) => address.equals(signer))
if (signerIndex === -1) {
throw new Error('Signer not found')
Expand Down
Loading