-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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-2124: Fork identifier for chain compatibility checks #2125
Comments
Hi @karalabe , very interesting proposal. I had a look after it was mentioned in the allcoredevs call. I have a question:
I assume that you intended the first case. Then, it seems to me that the edge case of Istanbul-Ropsten case is not properly covered. At block B-1 the situation would be: Updated nodes would have: Stale Nodes would have:
I see two possible solutions X and Y: X: Considering the fork for the FORK_HASH on the block before. Y: Instead of sending FORK_NEXT, the current block number (BLOCK_NUM) is sent. As FORK_NEXT is used to tell "whether a remote node is out of sync or whether its software is stale.", this seems easier to do with a block number. New decision tree: FORK_HASH matches local past forks:
I can also transform it into a table if you prefer that. There are a few cases that are different. See you in Osaka |
Hi @karalabe, as discussed in Osaka BLOCK_NUM might not be ideal. So disregard my proposal from above. Quick QuestionHence, I still have the following question:
I assume that you intended the first case. Then, it seems to me that the edge case of Istanbul-Ropsten case is not properly covered. At block B-1 the situation would be: Updated nodes would have: Stale Nodes would have:
Of course, as soon as the updated nodes find a single block (and manage to distribute this block), the issue is resolved. Is this sufficient from your point of view? Comments on the TableI found the following entries missing inside the table, even though they are probably clear.
Here are the cases that are not clear to me, they are taken from your table:
|
@ritzdorf This is a very nice corner case indeed, thank you! In your expanded example with Ropsten Istanbul, getting stuck on Wrt the second two cases you brought up, I think both are valid corner cases. If we would have the first special case of your table handled correctly, that would also solve the Ropsten Istanbul splitting correctly, because non-updaters would have split off from not-yet-forked Istanbul nodes, even though the Istanbul ones would not detect the issue. This case however cannot be solved by the more complex FORK_HASH modification. Perhaps a better alternative solution would be to extend the validation code a bit. The fork ID fields would remain the same as is now, but the validator would also take into consideration the local chain head. Then:
Does this make sense to you? (still need to figure out Clique, might need that initial solution too after all) |
Thanks for your feedback @karalabe.
I agree that it is odd. It was more like a brainstormed suggestion.
Yes. Thank you. That is roughly what I had in mind. Sorry for not writing it more clearly.
With Clique do you mean a set of non-forking nodes that are isolated from the non-forking miners?
|
Can you explain a bit more why this "seems odd"? Taking a slice of bytes from a cryptographic hash function is a safe and reasonable way to get well-distributed bytes on some domain. Agreed |
This ship sailed long ago :) That said, I wrote that it seems odd because we're discarding part of the checksum vs. crc32 would keep all the entropy. |
No worries! Just chiming in while going over the specs |
Watch an overview of the 'fork identifier', its purpose, and usage for the Ethereum chain in syncing nodes by @fjl |
|
It's implemented by all clients.
…On Fri, May 14, 2021, 22:53 William Entriken ***@***.***> wrote:
- Can this please be implemented in two or more competing
implementations before this is published as final status?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2125 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA7UGMMKJGCWPCMZMFEB6TTNV5STANCNFSM4HZI3O2Q>
.
|
Thank you. Can specific clients (at least some) please be noted in the EIP text? |
EIPs should not be linking out to client implementations. See the template at https://raw.githubusercontent.com/ethereum/EIPs/master/eip-template.md and note that there is no longer an Implementations section, there is now only a reference implementation section which is intended to contain an inline implementation and it is optional. |
It is okay that EIP reviews (which are unpaid, I'm assuming) do not accept a responsibility of verifying technical aspects of EIPs. But if EIPs are to be anything other than a pastebin, then any interop specifications (i.e. all EIPs) should include multiple reference implementation. And they should be verified by the community (e.g. the Last Call process). |
Specifications almost never say how to implement something, only what to
implement. Reference code is a courtesy in the spec world, not a
requirement. Would you expect the yellow paper to include multiple
implementations of the entire Ethereum EVM?
…On Sun, May 16, 2021, 05:57 William Entriken ***@***.***> wrote:
It is okay that EIP reviews (which are unpaid, I'm assuming) do not accept
a responsibility of verifying technical aspects of EIPs.
But if EIPs are to be anything other than a pastebin, then any interop
specifications (i.e. all EIPs) should include multiple reference
implementation. And they should be verified by the community (e.g. the Last
Call process).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2125 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA7UGJI4EWSYMKBMQZMI5TTN4YCJANCNFSM4HZI3O2Q>
.
|
Closing this for housekeeping purposes. Feel free to continue using this issue for discussion about EIP-2124. |
Discussion thread for #2124.
Abstract
There are many public and private Ethereum networks, but the discovery protocol doesn't differentiate between them. The only way to check if a peer is good or bad (same chain or not), is to establish a TCP/IP connection, wrap it with RLPx cryptography, then execute an
eth
handshake. This is an extreme cost to bear if it turns out that the remote peer is on a different network and it's not even precise enough to differentiate Ethereum and Ethereum Classic. This cost is magnified for small networks, where a lot more trial and errors are needed to find good nodes.Even if the peer is on the same chain, during non-controversial consensus upgrades, not everybody updates their nodes in time (developer nodes, leftovers, etc). These stale nodes put a meaningless burden on the peer-to-peer network, since they just latch on to good nodes, but don't accept upgraded blocks. This causes valuable peer slots and bandwidth to be lost until the stale nodes finally update. This is a serious issue for test networks, where leftovers can linger for months.
This EIP proposes a new identity scheme to both precisely and concisely summarize the chain's current status (genesis and all applied forks). The conciseness is particularly important to make the identity useful across datagram protocols too. The EIP solves a number of issues:
This EIP does not attempt to solve the clean separation of 3-way-forks! If at the same future block number, the network splits into three (non-fork, fork-A and fork-B), separating the forkers from each another will need case-by-case special handling. Not handling this keeps the proposal pragmatic, simple and also avoids making it too easy to fork off mainnet.
To keep the scope limited, this EIP only defines the identity scheme and validation rules. The same scheme and algorithm can be embedded into various networking protocols, allowing both the
eth/6x
handshake to be more precise (Ethereum vs. Ethereum Classic); as well as the discovery to be more useful (reject surely peers without ever connecting).The text was updated successfully, but these errors were encountered: