EIP-225 Clique Spec Improvements #3426
Conversation
…ield name for easier association
…description to avoid ascending-time-order confusion
EIPS/eip-225.md
Outdated
| * Should be filled with zeroes normally, modified only while voting. | ||
| * Arbitrary values are permitted nonetheless (even meaningless ones such as voting out non signers) to avoid extra complexity in implementations around voting mechanics. | ||
| * **Must** be filled with zeroes on checkpoint (i.e. epoch transition) blocks. | ||
| * Transaction execution **must** use the actual block signer (see `extraData`) for the `COINBASE` opcode. | ||
| * Transaction execution **must** use the actual block signer (see `extraData`) for the `COINBASE` opcode and transaction fees **must** be attributed to the signer account. | ||
| * Block execution **must** not distribute any block rewards. |
There was a problem hiding this comment.
This feels like an addition to the spec rather than a clarification like the other proposed edits are. Enterprise chains may want to add block rewards to encourage miners to stay online.
My opinion is that this should be removed from the edits or become a should instead of a must
There was a problem hiding this comment.
Thanks for having a look, definitely not my intention to change the spec in any way, have updated.
…iew) Co-authored-by: Micah Zoltu <micah@zoltu.net>
|
@karalabe Can you review this and make sure there are no normative changes (just clarification)? If so, please do a GitHub review/approve and I'll merge. |
|
Bumping again - @karalabe if you could take a look please. |
|
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s): |
|
Any chance that we get this merged? Sorry Peter, I know about these nasty side tasks which nobody asked for and which sneak in from the sideline 😋. I am also very willing to remove all things which have even the slightest controversial aspect. 😀 |
|
Someone may have to reach out to Peter via alternative means, since it doesn't appear that they are receiving pings here. |
|
Yeah, I just figured you'll auto-merge bike shedding fixes :P |
|
In this case, I don't know enough about the subject matter to be able to assert whether this is purely a non-normative or not, hence the need for author approval. |
|
@karalabe we actually lost ~3 development days with debugging on these "trivialities", so bear in mind the non-enlightened. 😄 Thanks for merging. |
|
(e.g. we were sorting Clique signers on a FIFO basis and were searching our butts of why the block hashes are not matching 😄) |
* eip-225: additionally label ethash header field with original miner field name for easier association * eip-225: added fille-with-zeros-on-genesis note to EXTRA_SEAL definition * eip-225: added ascending-byte-order note to extraData signer section description to avoid ascending-time-order confusion * eip-225: better differentiate between sighash and final block hash * eip-225: added note on transaction fee and block reward attribution * eip-225: added zero-based note to SIGNER_INDEX * EIP-225: changed block reward distribution must -> should after review * eip-225: removed block rewards note and corrected typo (from code review) Co-authored-by: Micah Zoltu <micah@zoltu.net> Co-authored-by: Micah Zoltu <micah@zoltu.net> Co-authored-by: Alita Moore <alita.moore805@gmail.com>
We have now finished our
TypeScriptClique implementation (ethereumjs/ethereumjs-monorepo#1032 as well as ethereumjs/ethereumjs-monorepo#1074 and ethereumjs/ethereumjs-monorepo#1088).Thanks for this wonderful spec, the clarity of the writing and the extensive test cases helped us a lot! 😄 We nevertheless stumbled upon a few things which caused us some significant extra debugging time. Just took the time to extract, think this should be useful for future implementations.
//cc @karalabe