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

Implement PIP messages, request builder, and handlers #4945

Merged
merged 57 commits into from
Mar 23, 2017
Merged

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Mar 17, 2017

OnDemand (and by extension RPC) still uses these messages inefficiently. A more sophisticated dispatch mechanism will come in a follow-up PR.

Should be merged after #4934

Information for reviewers:

  • Previously, ethcore-light implemented a bastardized version of LES.
  • This is the first step towards migrating to PIP by moving to heterogeneous back-referencing requests
  • The bulk of the code changed here is just request definitions coupled with encoding boilerplate.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Mar 17, 2017
return usize::MAX;
pub fn compute_cost(&self, request: &Request) -> U256 {
match *request {
Request::Headers(ref req) => self.costs.headers * req.max.into(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably would be better to use saturating_ops here?

/// Compute the cost of a set of requests.
/// This is the base cost plus the cost of each individual request.
pub fn compute_cost_multi(&self, requests: &[Request]) -> U256 {
requests.iter().fold(self.costs.base, |cost, req| cost + self.compute_cost(req))
Copy link
Collaborator

Choose a reason for hiding this comment

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

saturating_add?

@debris
Copy link
Collaborator

debris commented Mar 17, 2017

merged with master to trigger the CI build

@arkpar
Copy link
Collaborator

arkpar commented Mar 22, 2017

Looks good overall. To huge too look into details.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 22, 2017
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.

5 participants