search

ethereum / ethereumj

DEPRECATED! Java implementation of the Ethereum yellowpaper. For JSON-RPC and other client features check Ethereum Harmony
GNU Lesser General Public License v3.0
2.18k stars 1.1k forks source link

Validate header chain in downloader #1222

Closed mkalinin closed 5 years ago

mkalinin commented 5 years ago

What's done

Fixes a problem that has been discovered on Ropsten net during the mess happened around Constantinople HF. The fix is covered with unit tests and ready for merge. Testing on Ropsten's data went well.

The problem

BlockDownloader didn't run header validations that depends on parent block (like difficulty check). These validations used to be run in BlockchainImpl before block started to be imported. Thus, if there is a peer with invalid chain in a list of active peers, BlockDownloader could stick to that chain and never do a re-branch and the sync gets stuck as a result.

The fix

Parent header checks have been moved to BlockDownloader and now all checks are run there. Hence, preventing BlockDownloader from going down the wrong chain.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.03%) to 56.764% when pulling 1166301be1a1d9e74ef9308f8dd338553bf2e448 on fix/validate-header-chain into 561582b965648f8d359b50d4c40c0984e5a4d48e on develop.

mkalinin commented 5 years ago

@Nashatyrev please, take a look again

Nashatyrev commented 5 years ago

I like this variant much more. Just added minor change to avoid a == null || a.isEmpty() pattern