-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
EIP-3534: Restricted Chain Context Type Transactions (Draft) #3534
EIP-3534: Restricted Chain Context Type Transactions (Draft) #3534
Conversation
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
|
||
##### Encoding | ||
|
||
Encoding should follow the same rules as the optional `accessList` value. |
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.
the access list is a nested structure, while this is just a simple array. This needs to be more explicitly specified. Consider just RLP encoding or SSZ encoding the list for compatibility with existing tools.
|
||
##### Encoding | ||
|
||
Encoding should follow the same rules as other transaction integer values, eg. `nonce`, `gasPrice`, `gasLimit`, etc. |
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 doesn't specify where the timestamp should go. Also, be explicit about encoding mechanism rather than referring to other fields.
- `ANNOTATION_PREFIX` `7` combines `segmentId` and `eligibleMinerList` and `expiry`. | ||
|
||
|
||
Annotation values are provided in an array. |
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.
How is the array encoded into bytes?
How is the array affixed to the prefix?
How is array item order determined?
ethereum#3534 (review) Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
@MicahZoltu Thank you for the thoughtful and valuable feedback. I'll address the remaining review comments ASAP, and will notify you when I've pushed for a second pass. |
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) I've chosen to use 'ancestorId' instead of the review-proposed 'ancestorHash' because the value is a synthesis of block number and hash values intended to uniquely identifiy a block. Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
ethereum#3534 (comment) Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Hey again @MicahZoltu. As promised, I've made and now pushed quite a few changes in response to your excellent initial feedback. I've used 👍 on your review comments as a personal way for me to track what I've tackled, and I think I've got them all covered (for now 😂). Please forgive the ugly history; I'm assuming a merge will be a squash, and there isn't much use in trying to make sense of history at this point in the EIP's young life. Thanks again for your time and attention. |
…t one Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
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.
A few more minor things, but once those are addressed we should be able to merge this as a draft.
EIPS/eip-3534.md
Outdated
- `ELIGIBLE_MINER_LIST` `[address...]`. A list of addresses. | ||
- `MAX_ELEMENTS` `3`. The maximum number of addresses that can be provided. | ||
|
||
The `ELIGIBLE_MINER_LIST` value is an array of valid [address hashes](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L321) referencing an exclusive, unique set of allowed [`beneficiary`](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L336) values. |
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.
Avoid external links, including the Ethereum Yellow paper. It can be assumed that readers of this will know what an Ethereum address is. Also, you shouldn't call it an address hash, because an address
is already a hash/truncation of a public key and address hash
would be a hash of the address which I'm pretty sure is not what you want. Same with beneficiary
, it can be assumed that readers of this will know what a block beneficiary
is, so just call it that (note the addition of the word block
).
EIPS/eip-3534.md
Outdated
- `INELIGIBLE_MINER_LIST` `[address...]`. A list of addresses. | ||
- `MAX_ELEMENTS` `3`. The maximum number of addresses that can be provided. | ||
|
||
The `INELIGIBLE_MINER_LIST` value is an array of valid [address hashes](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L321) referencing an unique set of disallowed [`beneficiary`](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L336) values. |
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.
Same comment as above, no external links and these two aren't necessary.
EIPS/eip-3534.md
Outdated
- `ANNOTATION_PREFIX` `8`. | ||
- `EXPIRY` `integer`. A positive, unsigned scalar. | ||
|
||
The `EXPIRY` value is a scalar equal to the maximum valid [`timestamp`](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L345) of a block header for a block including this transaction. |
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.
Same comment as above, no external links and this one is unnecessary.
EIPS/eip-3534.md
Outdated
The goal of the `ancestorId` is to disambiguate one chain segment from another, and in doing so, enable a transaction to define with adequate precision which chain it needs to be on. | ||
When a transaction's `ancestorId` references a block, we want to be pretty sure that that reference won't get confused with a different block than the one the author of the transaction had in mind. | ||
|
||
Block hashes are effectively unique (at least ethereum/go-ethereum assumes so in their database schema), and are [cited as approximations for random numbers in the yellow paper](https://github.com/ethereum/yellowpaper/blob/41c1837f7b1ddcd65f135763c8e965146cd0ab70/Paper.tex#L1261). |
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.
No external links. Also, this particular citation doesn't seem to add value to this discussion so I recommend just removing it. We can assume the readers of this document understand the uniqueness and collision resistant properties of the keccak256 hashing algorithm and the block hash selection process.
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Signed-off-by: ia <isaac.ardis@gmail.com>
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):
|
Signed-off-by: ia <isaac.ardis@gmail.com>
Assume the reader understands how hashes work.
Signed-off-by: ia <isaac.ardis@gmail.com>
@MicahZoltu As always and again, thank you. I've just pushed changes addressing all of your review comments. |
Signed-off-by: ia <isaac.ardis@gmail.com>
Signed-off-by: ia <isaac.ardis@gmail.com>
…m#3534) Defines a new transaction type with constraints on ancestor block hash, block author, and/or block timestamp.
This is submitting a
Draft
statusStandards > Core
proposal for consideration.Simple Summary
Add a transaction type which contains a Chain Context value restricting a transaction's validity
to a chain meeting certain contextual requirements.