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

Light client improvements #6156

Merged
merged 5 commits into from Jul 27, 2017
Merged

Light client improvements #6156

merged 5 commits into from Jul 27, 2017

Conversation

rphmeier
Copy link
Contributor

  • a few bug fixes in on_demand and parity_rpc
  • blockchain import command for light clients.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jul 26, 2017
@rphmeier
Copy link
Contributor Author

rphmeier commented Jul 26, 2017

@arkpar e029248 and be7dd85 may be worth backporting before 1.7

@arkpar
Copy link
Collaborator

arkpar commented Jul 26, 2017

Contains commits from #6148, probably needs a rebase

caps.serve_headers == c.serve_headers &&
caps.serve_chain_since >= c.serve_chain_since &&
caps.serve_state_since >= c.serve_chain_since
caps.serve_headers >= c.serve_headers &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

serve_headers is a bool as far as I can see. Why >= ?

Copy link
Contributor Author

@rphmeier rphmeier Jul 27, 2017

Choose a reason for hiding this comment

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

the purpose of this function is to check whether the capabilities c are encompassed by the capabilities caps. If c.serve_headers is false, caps.serve_headers can be false or true. If c.serve_headers is true, caps.serve_headers can only be true.

caps.serve_state_since >= c.serve_chain_since
caps.serve_headers >= c.serve_headers &&
(c.serve_chain_since.is_none() || caps.serve_chain_since <= c.serve_chain_since) &&
(c.serve_state_since.is_none() || caps.serve_state_since <= c.serve_state_since)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does is_none allow a request?
Shouldn't chain and state numbers be strictly lower?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the request doesn't require any state or chain serving, then it doesn't matter what the peer capabilities are. If the request does require chain or state serving, then the block number this peer supports beyond should be <= the request required block number. but this should also guard against caps.serve_since.is_none(). I will rewrite for clarity

@arkpar arkpar added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 27, 2017
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 27, 2017
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 27, 2017
@arkpar arkpar merged commit 70ef33f into master Jul 27, 2017
@arkpar arkpar deleted the lightclient-upgrade branch July 27, 2017 11:50
arkpar pushed a commit that referenced this pull request Jul 27, 2017
* no seal checking

* import command and --no-seal-check for light client

* fix eth_call

* tweak registry dapps lookup

* ignore failed requests to non-server peers
arkpar added a commit that referenced this pull request Jul 27, 2017
* Light client improvements (#6156)

* no seal checking

* import command and --no-seal-check for light client

* fix eth_call

* tweak registry dapps lookup

* ignore failed requests to non-server peers

* Fix connecting to wildcard addresses. (#6167)

* Don't display an overlay in case the time sync check fails. (#6164)

* Small improvements to time estimation.

* Temporarily disable NTP time check by default.
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.

None yet

2 participants