[CGKIELE-154] stabilize sync for iele testnet #442
Conversation
| broadcastNewBlockHash(newBlock, peersWithoutBlock) | ||
|
|
||
| if (syncConfig.broadcastNewBlockHashes) { | ||
| // the usefulness of this message is debatable, especially in private networks |
There was a problem hiding this comment.
Since we may need to make a decision in the future, can we start the comment as // NOTE or // Note, so that it is easily grep-able?
There was a problem hiding this comment.
What decision do you mean? The message is part of ETH protocol, and other clients support it, so I think we also should retain it.
That said, I really don't know why it was introduced in the protocol or what are its practical applications. Maybe @KonradStaniec can shed light upon it?
There was a problem hiding this comment.
Isn't it used to inform other peers about the new blocks we have? Why do you think it's not needed?
There was a problem hiding this comment.
(I think what Christos means is adding a "Note" prefix to comments)
There was a problem hiding this comment.
Yes, adding a Note prefix to comments. But no stopper here.
There was a problem hiding this comment.
@rtkaczyk newBlocks message is only sent to a subset of peers (if I remember correctly sqrt(n) peers, where n is the number of current peers), newBlockHashes is sent to all peers.
And main reason for both of them to exist is that newBlockHashes is much more lightweight and not load peer network that much (newBlockHashes is only a hash, and then peer needs to download block directly, newBlocks is the whole new block )
There was a problem hiding this comment.
I've added the "NOTE" just in case, but I still don't think there's anything to decide here
There was a problem hiding this comment.
The main problems with NewBlockHashes:
- it's sent to the same peers that receive
NewBlock, which is redundant - it's handled differently than
NewBlock, where orphaned block is ignored, which is just weird NewBlockshould be completely sufficient, because nodes that received the message should forward it to the ones that didn't
rtkaczyk
force-pushed
the
fix/stabilizeSyncForIeleTestnet
branch
from
278f509 to
36c0368
Compare
No description provided.