Structural Preparations for PoA Support #937
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Some tests are failing, so relabelling this as |
holgerd77
force-pushed
the
poa-preparations
branch
from
281f2ce to
29821d2
Compare
|
Ok, this was an easy fix 😄 , ready again for review. |
|
Added some questions and some nitpicks. But looks great in general 😄 |
holgerd77
force-pushed
the
poa-preparations
branch
from
57724b6 to
60ecd32
Compare
|
Ok, this is now ready for re-review, for an overview on the changes pushed see 60ecd32, thanks @jochem-brouwer and @rumkin for all the valuable suggestions. I took a lot of time on giving the validation methods a better documentation, I think this should make this a lot clearer on first-time read, I also learned a lot myself here. 😄 I also made the |
|
OK this is great! I also love that you added a lot of documentation here. I think the way to proceed (see this comment) is to (in a new PR) remove most of this consensus logic from blockchain/block and use this strategy pattern so we can choose which strategy we use for validating the block/block header. So these concrete instances would be a PoW consensus and a Clique consensus. These would allow us to validate the consensus (PoW/Clique) and also validate the block/block header (note that in Clique there are no uncles, for instance, so the uncle hash is always the empty list hash). |
|
@holgerd77 I suggest we merge this one after my block/blockchain PRs are merged so you can rebase on top of my changes (seems to be less work than the other way around). |
|
Currently blocked by #942 |
|
Will take the next hour or so to update (hopefully rebase) with the changes from #942 and previous block/blockchain touching PRs. |
…ation and access methods
…n methods which include difficulty validation
|
Oh, that's a tough one. 😂 |
…on renaming, throw on unsupported chain consensus types (PoA)
…tion methods, allow blockchain and block validate methods to operate on non-PoW chains
holgerd77
force-pushed
the
poa-preparations
branch
from
60ecd32 to
8bb93e9
Compare
holgerd77
force-pushed
the
poa-preparations
branch
from
445fa57 to
f81afa3
Compare
|
Ok, this is now ready for re-review. 😄 //cc @jochem-brouwer |
This PR provides some ground for a future PoA support, see #689. I am doing this now since I hope that this will allow us for a future PoA feature addition without the need for another major release respectively introducing breaking changes. I have done some reading along on the Clique PoA EIP-225 and I think this should get a solid enough structural setup with this PR which should allow for non-breaking integration (by adding signer validation on PoA chains along an activated
Blockchain.validateConsensussetting), this might benefit from some validation though.The PR introduces code parts to now throw on
BlockorBlockchainvalidation triggers on setups which we just don't support yet (validate blocks on 'goerli') or which doesn't make sense (run 'validateDifficulty()' on goerli) and therefore reduces undefined behavior within the libraries.@jochem-brouwer if this PR is supported and reviewed, we can merge on top of your blockchain refactoring changes with me rebasing or the the other way around, as you prefer.