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

ethcore-sync: fix connection to peers behind chain fork block #8710

Merged
merged 1 commit into from Jun 2, 2018

Conversation

andresilva
Copy link
Contributor

We're currently not connecting to any peers that are behind the chain's fork block (#1920000 for foundation). This is a regression introduced recently in #8543.

This PR reverts to the previous behavior, where we still accept connections from/to an unconfirmed peer. This behavior is guarded against by PeerInfo#can_sync() where we'll make sure that the given peer has confirmed the fork header before we try to sync from him.

@andresilva andresilva added F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. labels May 24, 2018
@folsen
Copy link
Contributor

folsen commented May 28, 2018

This is an important fix to get in asap, @ngotchac and @tomusdrw could you please prioritise reviewing?

Copy link
Contributor

@ngotchac ngotchac left a comment

Choose a reason for hiding this comment

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

LGTM

@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed F2-bug 🐞 The client fails to follow expected behavior. labels May 31, 2018
@5chdn 5chdn added this to the 1.12 milestone May 31, 2018
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 1, 2018
@5chdn 5chdn added the B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. label Jun 2, 2018
@5chdn 5chdn merged commit 2060ea5 into master Jun 2, 2018
@5chdn 5chdn deleted the andre/fix-sync-fork-header branch June 2, 2018 08:29
@5chdn 5chdn mentioned this pull request Jun 2, 2018
57 tasks
dvdplm added a commit that referenced this pull request Jun 4, 2018
* master:
  Remove HostTrait altogether (#8681)
  ethcore-sync: fix connection to peers behind chain fork block (#8710)
  Remove public node settings from cli (#8758)
  Custom Error Messages on ENFILE and EMFILE IO Errors (#8744)
  CI: Fixes for Android Pipeline (#8745)
  Remove NetworkService::config() (#8653)
  Fix XOR distance calculation in discovery Kademlia impl (#8589)
  Print warnings when fetching pending blocks (#8711)
@andresilva andresilva mentioned this pull request Jun 4, 2018
13 tasks
ordian added a commit to ordian/parity that referenced this pull request Jun 4, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity:
  Remove Result wrapper from AccountProvider in RPC impls (openethereum#8763)
  Update `license header` and `scripts` (openethereum#8666)
  Remove HostTrait altogether (openethereum#8681)
  ethcore-sync: fix connection to peers behind chain fork block (openethereum#8710)
  Remove public node settings from cli (openethereum#8758)
5chdn pushed a commit that referenced this pull request Jun 4, 2018
* Fix light sync with initial validator-set contract (#8528)

* Fix #8468

* Use U256::max_value() instead

* Fix again

* Also change initial transaction gas

* Resumable warp-sync / Seed downloaded snapshots (#8544)

* Start dividing sync chain : first supplier method

* WIP - updated chain sync supplier

* Finish refactoring the Chain Sync Supplier

* Create Chain Sync Requester

* Add Propagator for Chain Sync

* Add the Chain Sync Handler

* Move tests from mod -> handler

* Move tests to propagator

* Refactor SyncRequester arguments

* Refactoring peer fork header handler

* Fix wrong highest block number in snapshot sync

* Small refactor...

* Resume warp-sync downloaded chunks

* Add comments

* Refactoring the previous chunks import

* Fix tests

* Address PR grumbles

* Fix not seeding current snapshot

* Address PR Grumbles

* Address PR grumble

* Retry failed CI job

* Update SnapshotService readiness check
Fix restoration locking issue for previous chunks restoration

* Fix tests

* Fix tests

* Fix test

* Early abort importing previous chunks

* PR Grumbles

* Update Gitlab CI config

* SyncState back to Waiting when Manifest peers disconnect

* Move fix

* Better fix

* Revert GitLab CI changes

* Fix Warning

* Refactor resuming snapshots

* Fix string construction

* Revert "Refactor resuming snapshots"

This reverts commit 75fd4b5.

* Update informant log

* Fix string construction

* Refactor resuming snapshots

* Fix informant

* PR Grumbles

* Update informant message : show chunks done

* PR Grumbles

* Fix

* Fix Warning

* PR Grumbles

* Don't open Browser post-install on Mac (#8641)

Since we start parity with the UI disabled per default now, opening the browser post installation will show an annoying error message, confusing the user. This patch removes opening the browser to prevent that annoyance.

fixes #8194

* Fix not downloading old blocks (#8642)

* Fix PoW blockchains sealing notifications in chain_new_blocks (#8656)

* Shutdown the Snapshot Service early (#8658)

* Shutdown the Snapshot Service when shutting down the runner

* Rename `service` to `client_service`

* Fix tests

* Fix cli signer (#8682)

* Update ethereum-types so `{:#x}` applies 0x prefix

* Set the request index to that of the current request (#8683)

* Set the request index to that of the current request

When setting up the chain of (two) requests to look up a block by hash, the second need to refer to the first. This fixes an issue where the back ref was set to the subsequent request, not the current one. When the requests are executed we loop through them in order and ensure the requests that should produce headers all match up. We do this by index so they better be right.

In other words: off by one.

* network-devp2p: handle UselessPeer disconnect (#8686)

* Fix local transactions policy. (#8691)

* CI: Fixes for Android Pipeline (#8745)

* ci: Remove check for shared libraries in gitlab script

* ci: allow android arm build to fail

* Custom Error Messages on ENFILE and EMFILE IO Errors (#8744)

* Custom Error Messages on ENFILE and EMFILE IO Errors

Add custom mapping of ENFILE and EMFILE IO Errors (Failure because of missing system resource) right when chaining ioError into ::util::Network::Error to improve Error Messages given to user

Note: Adds libc as a dependency to util/network

* Use assert-matches for more readable tests

* Fix Wording and consistency

* ethcore-sync: fix connection to peers behind chain fork block (#8710)
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. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants