-
Notifications
You must be signed in to change notification settings - Fork 784
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
Clique small fixes / follow-up work #1074
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
…` as number of blocks, not number of array entries * don't reset votes on epoch blocks to keep in case of reorgs, but only count in `cliqueUpdateVotes()` if blockNumber >= last epoch block
* in trimming signer votes, keep votes until `lastEpochBlockNumber - CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT` (as a safe number for potential recounts on reorgs)
…er.validateCliqueDifficulty()`
67fe4af
to
9db72c2
Compare
Can this PR get an update? Is this generally ready for review? |
@holgerd77 yes now ready for review, thanks! |
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.
Looks all good, thanks Ryan! 👍
// If cliqueSigner is provided, seal block with provided privateKey. | ||
if (options.cliqueSigner) { | ||
this.extraData = this.cliqueSealBlock(options.cliqueSigner) | ||
} |
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.
Nice, also like that the private key is not residing in the library, was a bit worried that we introduce some new potential security risks here.
const signerFound = signerList.find((signer) => { | ||
return signer.toBuffer().equals(signerAddress) | ||
return signer.equals(signerAddress) |
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.
Ah, yes, that's really a much nicer API! 😄
This PR addresses some further work from review in #1032:
cliqueSigner?: Buffer
CLIQUE_SIGNER_HISTORY_BLOCK_LIMIT
as number of blocks, not number of last statescliqueUpdateVotes()
by checking if the vote's blockNumber >= last epoch block.blockchain.cliqueActiveSigners()
inBlockHeader.validateCliqueDifficulty()
to not add to block's minimum blockchain interface (discussion)Address.equals(address)
checks with ethereumjs-util v7.0.8