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

LES Part 1 #3322

Merged
merged 27 commits into from
Nov 10, 2016
Merged

LES Part 1 #3322

merged 27 commits into from
Nov 10, 2016

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Nov 9, 2016

Protocol handler, framework for responding to and making requests.
Introduces the ethcore-light crate which is currently dead code and unlinked.

The vision is that T: BlockChainClient + ?Sized will implement Provider.
Then a LightSync service will handle a LightClient, a LightProtocol, and a NetworkManager with listeners for key events.

RPC Requests will access the shared LightProtocol object and make requests with callbacks.

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

rphmeier commented Nov 9, 2016

}

// Handle a request for block bodies.
fn get_block_bodies(&self, peer: &PeerId, io: &NetworkContext, data: UntrustedRlp) -> Result<(), Error> {
Copy link
Contributor Author

@rphmeier rphmeier Nov 9, 2016

Choose a reason for hiding this comment

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

lots of duplicated code here from get_block_headers -- I stopped implementing request handlers here for the moment as I realized there is probably a decent opportunity for abstraction.

These parts are common to basically every request handler:

  - check if peer exists
  - deserialize request UntrustedRlp -> (req_id, T)
  - check cost: &T -> U256
  - request from provider: T -> U
  - actual cost: &U -> U256
  - response: (req_id, U) -> (packet, Vec<u8>)

So it ought to be possible to build some kind of request-handler builder with a fluent API that handles all of the common parts. This will be included in the next PR, which should encompass

  • All request handlers
  • Event listeners
  • provider implementation for client
  • ...and maybe a bit more if it'll fit.

@rphmeier
Copy link
Contributor Author

rphmeier commented Nov 9, 2016

No idea why travis is failing -- I didn't fiddle with the subproject commit or anything like that

impl Client {
/// Import a header as rlp-encoded bytes.
pub fn import_header(&self, bytes: Bytes) -> Result<H256, BlockImportError> {
let header = ::rlp::decode(&bytes);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the RLP guaranteed to be valid here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it will be addressed when the light Client struct is fully fleshed out.

};

let amount: U256 = amount.into();
cost.0 + (amount * cost.1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it guaranteed to not overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, I suppose not. but with any reasonable cost table I think you would have a very hard time downloading a packet containing enough requests to overflow a 256-bit int

Copy link
Contributor Author

Choose a reason for hiding this comment

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

helper functions for determining the maximum number of a given type of request which can be made will definitely be useful in the future, particularly for ranking peers.

/// Network ID structure.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[repr(u32)]
pub enum NetworkId {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this used for? What about other values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used in the handshake, only the two values are valid per the specification but more could be added in the future. I figured an enum would convey the semantics of a limited value space properly.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 10, 2016
@gavofyork gavofyork merged commit 3854b8a into master Nov 10, 2016
@gavofyork gavofyork deleted the les-impl branch November 10, 2016 17:30
@gavofyork gavofyork added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Nov 11, 2016
@gavofyork gavofyork removed the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Dec 16, 2016
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.

3 participants