Skip to content

Conversation

@buck54321
Copy link
Member

re: #2104 (comment)

We don't use reorg. No need for complex tip change handling.

Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Less RPCs. An hour of eth_getBlockByNumber at 20 req/min seemed to be about ~7MiB (according to provider dashboard), so this is good to have.

}
if bhdr.Hash() == best.hash {
if bn == eth.bestHeight {
// Same hash, nothing to do.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Same hash"

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://www.paradigm.xyz/2021/07/ethereum-reorgs-after-the-merge#post-merge-ethereum-with-proof-of-stake

Finally, long reorgs are not possible because all blocks that are deeper than 2 epochs in the past are considered "finalized", i.e. it is impossible to revert past them. If an attacker caused two conflicting blocks to be finalized (e.g by controlling 67% of the stake), the system would need to fall back to social intervention to recover.

So, I think, if confirms are 3+ we are fine.

@chappjc
Copy link
Member

chappjc commented Feb 6, 2023

Good find @JoeGruffins. I wonder what "social intervention to recover" is supposed to mean, panic mode and politicking I guess.

// reorganization.
update(true, false)
send(true, nil)
eth.log.Debugf("Tip change from %d to %d.", eth.bestHeight, bn)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final note that tip change log no longer includes the block hashes, and I'm fine with that.
If we really need the reorg bool and the best block hashes back, we can keep polling using eth_blockNumber, and put back the eth.node.headerByHeight call if the height has changed. At least the frequency will be more like 12 seconds instead of 1 sec.

@chappjc chappjc merged commit f9c55ec into decred:master Feb 6, 2023
@chappjc chappjc added this to the 0.6 milestone Feb 6, 2023
@chappjc chappjc added the ETH label Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants