Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Allow download of old blocks from peers with equal or lower difficulty #9226

Merged
merged 1 commit into from
Jul 27, 2018

Conversation

ascjones
Copy link
Contributor

Fixes #9225

Currently we only allow downloading of old blocks if the peer difficulty was greater than our syncing difficulty (see #9225).

Following a suggestion from @ngotchac: this change allows downloading of blocks from peers where the difficulty is greater than the last downloaded old block. This means that we will still request old blocks from nodes that are behind us but still might have the blocks we want.

Previously we only allow downloading of old blocks if the peer
difficulty was greater than our syncing difficulty. This change allows
downloading of blocks from peers where the difficulty is greater then
the last downloaded old block.
@ascjones ascjones requested a review from ngotchac July 26, 2018 15:17
@ascjones ascjones added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jul 26, 2018
@5chdn 5chdn added this to the 2.1 milestone Jul 26, 2018
This was referenced Jul 26, 2018
io.chain().block_total_difficulty(BlockId::Number(d.last_imported_block_number()))
});

if force || last_imported_old_block_difficulty.map_or(true, |ld| peer_difficulty.map_or(true, |pd| pd > ld)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should request old blocks if the peer's difficulty is unknown, or our last imported old block diff. is unknown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case of the peer's difficulty being unknown, I was copying the existing behaviour: https://github.com/paritytech/parity-ethereum/blob/82a7ed2f912f22df78ee2417964f82d5a2ecfc43/ethcore/sync/src/chain/mod.rs#L731. So I guess we're being optimistic and sending the request anyway, we're assuming that most of the time we know the peer difficulty.

Not so sure about the case where the imported block diff is not known. That probably would mean (assuming old_blocks is Some) that last_imported_block_number has not been imported for whatever reason. So what to do in that case? Better to send the request anyway or not? Might be okay since it should almost always be Some, and in case it is None it might be okay to ask for it again.

@ascjones ascjones merged commit fb503f5 into master Jul 27, 2018
@ascjones ascjones deleted the aj/ancient-blocks branch July 27, 2018 09:13
JohnnySheffield added a commit to JohnnySheffield/parity-ethereum that referenced this pull request Jul 27, 2018
* master:
  add changelog for 1.11.8 stable and 2.0.1 beta (openethereum#9230)
  fix typo (openethereum#9232)
  Fix potential as_usize overflow when casting from U256 in miner (openethereum#9221)
  Allow old blocks from peers with lower difficulty (openethereum#9226)
  Removes duplicate libudev-dev from Dockerfile (openethereum#9220)
  snap: remove ssl dependencies from snapcraft definition (openethereum#9222)
  remove ssl from dockerfiles, closes openethereum#8880 (openethereum#9195)
ordian added a commit to ordian/parity that referenced this pull request Jul 31, 2018
* 'master' of https://github.com/paritytech/parity:
  removed client error (openethereum#9253)
  Implement EIP-1052 (EXTCODEHASH) and fix several issues in state account cache (openethereum#9234)
  Improve Tracer documentation (openethereum#9237)
  Update Dockerfile (openethereum#9242)
  block cleanup (openethereum#9117)
  Increase the number of sessions. (openethereum#9203)
  add changelog for 1.11.8 stable and 2.0.1 beta (openethereum#9230)
  fix typo (openethereum#9232)
  Fix potential as_usize overflow when casting from U256 in miner (openethereum#9221)
  Allow old blocks from peers with lower difficulty (openethereum#9226)
  Removes duplicate libudev-dev from Dockerfile (openethereum#9220)
  snap: remove ssl dependencies from snapcraft definition (openethereum#9222)
  remove ssl from dockerfiles, closes openethereum#8880 (openethereum#9195)
  Insert PROOF messages for some cases in blockchain (openethereum#9141)
  [Chain] Add more bootnodes (openethereum#9174)
  ethcore: update bn version (openethereum#9217)
  deserialize block only once during verification (openethereum#9161)
  Simple build instruction fix (openethereum#9215)
  Added --tx-queue-no-early-reject flag to disable early tx queue rejects (openethereum#9143)
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 23, 2018
5chdn pushed a commit that referenced this pull request Aug 23, 2018
Previously we only allow downloading of old blocks if the peer
difficulty was greater than our syncing difficulty. This change allows
downloading of blocks from peers where the difficulty is greater then
the last downloaded old block.
5chdn pushed a commit that referenced this pull request Aug 24, 2018
Previously we only allow downloading of old blocks if the peer
difficulty was greater than our syncing difficulty. This change allows
downloading of blocks from peers where the difficulty is greater then
the last downloaded old block.
5chdn added a commit that referenced this pull request Aug 30, 2018
* parity-version: bump stable to 1.11.9

* Fix compilation error on nightly rust (#8707)

On nightly rust passing `public_url` works but that breaks on stable. This works for both.

* parity-version: bump stable to 1.11.10

* Check if synced when using eth_getWork (#9193) (#9210)

* Check if synced when using eth_getWork (#9193)

* Don't use fn syncing

* Fix identation

* Fix typo

* Don't check for warping

* rpc: avoid calling queue_info twice on eth_getWork

* Fix potential as_usize overflow when casting from U256 in miner (#9221)

* Allow old blocks from peers with lower difficulty (#9226)

Previously we only allow downloading of old blocks if the peer
difficulty was greater than our syncing difficulty. This change allows
downloading of blocks from peers where the difficulty is greater then
the last downloaded old block.

* Update Dockerfile (#9242)

* Update Dockerfile

fix Docker build

* fix dockerfile paths: parity -> parity-ethereum (#9248)

* Update tobalaba.json (#9313)

* Light client `Provide default nonce in transactions when it´s missing` (#9370)

* Provide `default_nonce` in tx`s when it´s missing

When `nonce` is missing in a `EthTransaction` will cause it to fall in
these cases provide `default_nonce` value instead!

* Changed http:// to https:// on Yasm link (#9369)

Changed http:// to https:// on Yasm link in README.md

* Provide `default_nonce` in tx`s when it´s missing

When `nonce` is missing in a `EthTransaction` will cause it to fall in
these cases provide `default_nonce` value instead!

* Address grumbles

* ethcore: kovan: delay activation of strict score validation (#9406)

* Use impl Future in the light client RPC helpers (#8628)

* Better support for eth_getLogs in light mode (#9186)

* Light client on-demand request for headers range.

* Cache headers in HeaderWithAncestors response.

Also fulfills request locally if all headers are in cache.

* LightFetch::logs fetches missing headers on demand.

* LightFetch::logs limit the number of headers requested at a time.

* LightFetch::logs refactor header fetching logic.

* Enforce limit on header range length in light client logs request.

* Fix light request tests after struct change.

* Respond to review comments.

* Propagate transactions for next 4 blocks. (#9265)

Closes #9255

This PR also removes the limit of max 64 transactions per packet, currently we only attempt to prevent the packet size to go over 8MB. This will only be the case for super-large transactions or high-block-gas-limit chains.

Patching this is important only for chains that have blocks that can fit more than 4k transactions (over 86M block gas limit)

For mainnet, we should actually see a tiny bit faster propagation since instead of computing 4k pending set, we only need `4 * 8M / 21k = 1523` transactions.

Running some tests on `dekompile` node right now, to check how it performs in the wild.

* ethcore: fix pow difficulty validation (#9328)

* ethcore: fix pow difficulty validation

* ethcore: validate difficulty is not zero

* ethcore: add issue link to regression test

* ethcore: fix tests

* ethcore: move difficulty_to_boundary to ethash crate

* ethcore: reuse difficulty_to_boundary and boundary_to_difficulty

* ethcore: fix grumbles in difficulty_to_boundary_aux
5chdn added a commit that referenced this pull request Aug 31, 2018
* parity-version: bump beta to 2.0.2

* remove ssl from dockerfiles, closes #8880 (#9195)

* snap: remove ssl dependencies from snapcraft definition (#9222)

* parity-version: bump beta to 2.0.3

* Remove all dapp permissions related settings (#9120)

* Completely remove all dapps struct from rpc

* Remove unused pub use

* Remove dapp policy/permission func in ethcore

* Remove all dapps settings from rpc

* Fix rpc tests

* Use both origin and user_agent

* Address grumbles

* Address grumbles

* Fix tests

* Check if synced when using eth_getWork (#9193) (#9210)

* Check if synced when using eth_getWork (#9193)

* Don't use fn syncing

* Fix identation

* Fix typo

* Don't check for warping

* rpc: avoid calling queue_info twice on eth_getWork

* Fix potential as_usize overflow when casting from U256 in miner (#9221)

* Allow old blocks from peers with lower difficulty (#9226)

Previously we only allow downloading of old blocks if the peer
difficulty was greater than our syncing difficulty. This change allows
downloading of blocks from peers where the difficulty is greater then
the last downloaded old block.

* Update Dockerfile (#9242)

* Update Dockerfile

fix Docker build

* fix dockerfile paths: parity -> parity-ethereum (#9248)

* Propagate transactions for next 4 blocks. (#9265)

Closes #9255 

This PR also removes the limit of max 64 transactions per packet, currently we only attempt to prevent the packet size to go over 8MB. This will only be the case for super-large transactions or high-block-gas-limit chains.

Patching this is important only for chains that have blocks that can fit more than 4k transactions (over 86M block gas limit)

For mainnet, we should actually see a tiny bit faster propagation since instead of computing 4k pending set, we only need `4 * 8M / 21k = 1523` transactions.

Running some tests on `dekompile` node right now, to check how it performs in the wild.

* Update tobalaba.json (#9313)

* Fix load share (#9321)

* fix(light_sync): calculate `load_share` properly

* refactor(api.rs): extract `light_params` fn, add test

* style(api.rs): add trailing commas

* ethcore: fix pow difficulty validation (#9328)

* ethcore: fix pow difficulty validation

* ethcore: validate difficulty is not zero

* ethcore: add issue link to regression test

* ethcore: fix tests

* ethcore: move difficulty_to_boundary to ethash crate

* ethcore: reuse difficulty_to_boundary and boundary_to_difficulty

* ethcore: fix grumbles in difficulty_to_boundary_aux

* Light client `Provide default nonce in transactions when it´s missing` (#9370)

* Provide `default_nonce` in tx`s when it´s missing

When `nonce` is missing in a `EthTransaction` will cause it to fall in
these cases provide `default_nonce` value instead!

* Changed http:// to https:// on Yasm link (#9369)

Changed http:// to https:// on Yasm link in README.md

* Provide `default_nonce` in tx`s when it´s missing

When `nonce` is missing in a `EthTransaction` will cause it to fall in
these cases provide `default_nonce` value instead!

* Address grumbles

* ethcore: kovan: delay activation of strict score validation (#9406)

* Better support for eth_getLogs in light mode (#9186)

* Light client on-demand request for headers range.

* Cache headers in HeaderWithAncestors response.

Also fulfills request locally if all headers are in cache.

* LightFetch::logs fetches missing headers on demand.

* LightFetch::logs limit the number of headers requested at a time.

* LightFetch::logs refactor header fetching logic.

* Enforce limit on header range length in light client logs request.

* Fix light request tests after struct change.

* Respond to review comments.

* Add update docs script to CI (#9219)

* Add update docs script to CI

Added a script to CI that will use the jsonrpc tool to update rpc
documentation then commit and push those to the wiki repo.

* fix gitlab ci lint

* Only apply jsonrpc docs update on tags

* Update gitlab-rpc-docs.sh

* Copy correct parity repo to jsonrpc folder

Copy correct parity repo to jsonrpc folder before attempting to build docs since the CI runner clones the repo as parity and not parity-ethereum.

* Fix JSONRPC docs CI job

Update remote config in wiki repo before pushing changes using a github
token for authentication. Add message to wiki tag when pushing changes.
Use project directory to correctly copy parity code base into the
jsonrpc repo for doc generation.

* Fix set_remote_wiki function call in CI

* Prevent blockchain & miner racing when accessing pending block. (#9310)

* Prevent blockchain & miner racing when accessing pending block.

* Fix unavailability of pending block during reseal.

* Prevent sync restart if import queue full (#9381)

* Add POA Networks: Core and Sokol (#9413)

* ethcore: add poa network and sokol chainspecs

* rpc: simplify chain spec docs

* cli: rearrange networks by main/test and size/range

* parity: don't blacklist 0x00a328 on sokol testnet

* parity: add sokol and poanet to params and clean up a bit, add tests

* ethcore: add the poa networks and clean up a bit

* ethcore: fix path to poacore chain spec

* parity: rename poa networks to poacore and poasokol

* parity: fix configuration tests

* parity: fix parameter tests

* ethcore: rename POA Core and POA Sokol

* Update tobalaba.json (#9419)

* Update hardcoded sync (#9421)

- Update foundation hardcoded header to block 6219777 
- Update ropsten hardcoded header to block 3917825 
- Update kovan hardcoded header to block 8511489
ascjones added a commit that referenced this pull request Sep 17, 2018
#9226 might be too
permissive and causing the behaviour of the retraction soon after the
fork block. With this change the peer difficulty has to be greater than
or euqal to our syncing difficulty, so should still fix
#9225
5chdn pushed a commit that referenced this pull request Oct 9, 2018
* Log block set in block_sync for easier debugging

* logging macros

* Match no args in sync logging macros

* Add QueueFull error

* Only allow importing headers if the first matches requested

* WIP

* Test for chain head gaps and log

* Calc distance even with 2 heads

* Revert previous commits, preparing simple fix

This reverts commit 5f38aa8.

* Reject headers with no gaps when ChainHead

* Reset block sync download when queue full

* Simplify check for subchain heads

* Add comment to explain subchain heads filter

* Fix is_subchain_heads check and comment

* Prevent premature round completion after restart

This is a problem on mainnet where multiple stale peer requests will
force many rounds to complete quickly, forcing the retraction.

* Reset stale old blocks request after queue full

* Revert "Reject headers with no gaps when ChainHead"

This reverts commit 0eb8655.

* Add BlockSet to BlockDownloader logging

Currently it is difficult to debug this because there are two instances,
one for OldBlocks and one for NewBlocks. This adds the BlockSet to all
log messages for easy log filtering.

* Reset OldBlocks download from last enqueued

Previously when the ancient block queue was full it would restart the
download from the last imported block, so the ones still in the queue would be
redownloaded. Keeping the existing downloader instance and just
resetting it will start again from the last enqueued block.:wq

* Ignore expired Body and Receipt requests

* Log when ancient block download being restarted

* Only request old blocks from peers with >= difficulty

#9226 might be too
permissive and causing the behaviour of the retraction soon after the
fork block. With this change the peer difficulty has to be greater than
or euqal to our syncing difficulty, so should still fix
#9225

* Some logging and clear stalled blocks head

* Revert "Some logging and clear stalled blocks head"

This reverts commit 757641d.

* Reset stalled header if useless more than once

* Store useless headers in HashSet

* Add sync target to logging macro

* Don't disable useless peer and fix log macro

* Clear useless headers on reset and comments

* Use custom error for collecting blocks

Previously we resued BlockImportError, however only the Invalid case and
this made little sense with the QueueFull error.

* Remove blank line

* Test for reset sync after consecutive useless headers

* Don't reset after consecutive headers when chain head

* Delete commented out imports

* Return DownloadAction from collect_blocks instead of error

* Don't reset after round complete, was causing test hangs

* Add comment explaining reset after useless

* Replace HashSet with counter for useless headers

* Refactor sync reset on bad block/queue full

* Add missing target for log message

* Fix compiler errors and test after merge

* ethcore: revert ethereum tests submodule update
andresilva pushed a commit that referenced this pull request Oct 10, 2018
* Log block set in block_sync for easier debugging

* logging macros

* Match no args in sync logging macros

* Add QueueFull error

* Only allow importing headers if the first matches requested

* WIP

* Test for chain head gaps and log

* Calc distance even with 2 heads

* Revert previous commits, preparing simple fix

This reverts commit 5f38aa8.

* Reject headers with no gaps when ChainHead

* Reset block sync download when queue full

* Simplify check for subchain heads

* Add comment to explain subchain heads filter

* Fix is_subchain_heads check and comment

* Prevent premature round completion after restart

This is a problem on mainnet where multiple stale peer requests will
force many rounds to complete quickly, forcing the retraction.

* Reset stale old blocks request after queue full

* Revert "Reject headers with no gaps when ChainHead"

This reverts commit 0eb8655.

* Add BlockSet to BlockDownloader logging

Currently it is difficult to debug this because there are two instances,
one for OldBlocks and one for NewBlocks. This adds the BlockSet to all
log messages for easy log filtering.

* Reset OldBlocks download from last enqueued

Previously when the ancient block queue was full it would restart the
download from the last imported block, so the ones still in the queue would be
redownloaded. Keeping the existing downloader instance and just
resetting it will start again from the last enqueued block.:wq

* Ignore expired Body and Receipt requests

* Log when ancient block download being restarted

* Only request old blocks from peers with >= difficulty

#9226 might be too
permissive and causing the behaviour of the retraction soon after the
fork block. With this change the peer difficulty has to be greater than
or euqal to our syncing difficulty, so should still fix
#9225

* Some logging and clear stalled blocks head

* Revert "Some logging and clear stalled blocks head"

This reverts commit 757641d.

* Reset stalled header if useless more than once

* Store useless headers in HashSet

* Add sync target to logging macro

* Don't disable useless peer and fix log macro

* Clear useless headers on reset and comments

* Use custom error for collecting blocks

Previously we resued BlockImportError, however only the Invalid case and
this made little sense with the QueueFull error.

* Remove blank line

* Test for reset sync after consecutive useless headers

* Don't reset after consecutive headers when chain head

* Delete commented out imports

* Return DownloadAction from collect_blocks instead of error

* Don't reset after round complete, was causing test hangs

* Add comment explaining reset after useless

* Replace HashSet with counter for useless headers

* Refactor sync reset on bad block/queue full

* Add missing target for log message

* Fix compiler errors and test after merge

* ethcore: revert ethereum tests submodule update
andresilva pushed a commit that referenced this pull request Oct 10, 2018
* Log block set in block_sync for easier debugging

* logging macros

* Match no args in sync logging macros

* Add QueueFull error

* Only allow importing headers if the first matches requested

* WIP

* Test for chain head gaps and log

* Calc distance even with 2 heads

* Revert previous commits, preparing simple fix

This reverts commit 5f38aa8.

* Reject headers with no gaps when ChainHead

* Reset block sync download when queue full

* Simplify check for subchain heads

* Add comment to explain subchain heads filter

* Fix is_subchain_heads check and comment

* Prevent premature round completion after restart

This is a problem on mainnet where multiple stale peer requests will
force many rounds to complete quickly, forcing the retraction.

* Reset stale old blocks request after queue full

* Revert "Reject headers with no gaps when ChainHead"

This reverts commit 0eb8655.

* Add BlockSet to BlockDownloader logging

Currently it is difficult to debug this because there are two instances,
one for OldBlocks and one for NewBlocks. This adds the BlockSet to all
log messages for easy log filtering.

* Reset OldBlocks download from last enqueued

Previously when the ancient block queue was full it would restart the
download from the last imported block, so the ones still in the queue would be
redownloaded. Keeping the existing downloader instance and just
resetting it will start again from the last enqueued block.:wq

* Ignore expired Body and Receipt requests

* Log when ancient block download being restarted

* Only request old blocks from peers with >= difficulty

#9226 might be too
permissive and causing the behaviour of the retraction soon after the
fork block. With this change the peer difficulty has to be greater than
or euqal to our syncing difficulty, so should still fix
#9225

* Some logging and clear stalled blocks head

* Revert "Some logging and clear stalled blocks head"

This reverts commit 757641d.

* Reset stalled header if useless more than once

* Store useless headers in HashSet

* Add sync target to logging macro

* Don't disable useless peer and fix log macro

* Clear useless headers on reset and comments

* Use custom error for collecting blocks

Previously we resued BlockImportError, however only the Invalid case and
this made little sense with the QueueFull error.

* Remove blank line

* Test for reset sync after consecutive useless headers

* Don't reset after consecutive headers when chain head

* Delete commented out imports

* Return DownloadAction from collect_blocks instead of error

* Don't reset after round complete, was causing test hangs

* Add comment explaining reset after useless

* Replace HashSet with counter for useless headers

* Refactor sync reset on bad block/queue full

* Add missing target for log message

* Fix compiler errors and test after merge

* ethcore: revert ethereum tests submodule update
5chdn pushed a commit that referenced this pull request Oct 10, 2018
* produce portable binaries (#9725)

* HF in POA Core (2018-10-22) (#9724)

poanetwork/poa-chain-spec#87

* Use static call and apparent value transfer for block reward contract code (#9603)

* Verify block syncing responses against requests (#9670)

* sync: Validate received BlockHeaders packets against stored request.

* sync: Validate received BlockBodies and BlockReceipts.

* sync: Fix broken tests.

* sync: Unit tests for BlockDownloader::import_headers.

* sync: Unit tests for import_{bodies,receipts}.

* tests: Add missing method doc.

* Fix ancient blocks sync (#9531)

* Log block set in block_sync for easier debugging

* logging macros

* Match no args in sync logging macros

* Add QueueFull error

* Only allow importing headers if the first matches requested

* WIP

* Test for chain head gaps and log

* Calc distance even with 2 heads

* Revert previous commits, preparing simple fix

This reverts commit 5f38aa8.

* Reject headers with no gaps when ChainHead

* Reset block sync download when queue full

* Simplify check for subchain heads

* Add comment to explain subchain heads filter

* Fix is_subchain_heads check and comment

* Prevent premature round completion after restart

This is a problem on mainnet where multiple stale peer requests will
force many rounds to complete quickly, forcing the retraction.

* Reset stale old blocks request after queue full

* Revert "Reject headers with no gaps when ChainHead"

This reverts commit 0eb8655.

* Add BlockSet to BlockDownloader logging

Currently it is difficult to debug this because there are two instances,
one for OldBlocks and one for NewBlocks. This adds the BlockSet to all
log messages for easy log filtering.

* Reset OldBlocks download from last enqueued

Previously when the ancient block queue was full it would restart the
download from the last imported block, so the ones still in the queue would be
redownloaded. Keeping the existing downloader instance and just
resetting it will start again from the last enqueued block.:wq

* Ignore expired Body and Receipt requests

* Log when ancient block download being restarted

* Only request old blocks from peers with >= difficulty

#9226 might be too
permissive and causing the behaviour of the retraction soon after the
fork block. With this change the peer difficulty has to be greater than
or euqal to our syncing difficulty, so should still fix
#9225

* Some logging and clear stalled blocks head

* Revert "Some logging and clear stalled blocks head"

This reverts commit 757641d.

* Reset stalled header if useless more than once

* Store useless headers in HashSet

* Add sync target to logging macro

* Don't disable useless peer and fix log macro

* Clear useless headers on reset and comments

* Use custom error for collecting blocks

Previously we resued BlockImportError, however only the Invalid case and
this made little sense with the QueueFull error.

* Remove blank line

* Test for reset sync after consecutive useless headers

* Don't reset after consecutive headers when chain head

* Delete commented out imports

* Return DownloadAction from collect_blocks instead of error

* Don't reset after round complete, was causing test hangs

* Add comment explaining reset after useless

* Replace HashSet with counter for useless headers

* Refactor sync reset on bad block/queue full

* Add missing target for log message

* Fix compiler errors and test after merge

* ethcore: revert ethereum tests submodule update

* Add hardcoded headers (#9730)

* add foundation hardcoded header #6486017

* add ropsten hardcoded headers #4202497

* add kovan hardcoded headers #9023489

* gitlab ci: releasable_branches: change variables condition to schedule (#9729)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants