Skip to content

Commit

Permalink
address grumbles
Browse files Browse the repository at this point in the history
  • Loading branch information
rphmeier committed May 12, 2017
1 parent 0fd3e36 commit 2d87f56
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 33 deletions.
41 changes: 25 additions & 16 deletions ethcore/light/src/on_demand/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,23 +354,32 @@ impl net_request::CheckedRequest for CheckedRequest {
fn check_response(&self, cache: &Mutex<::cache::Cache>, response: &Self::Response) -> Result<Response, Error> {
use ::request::Response as NetResponse;

// helper for expecting a specific response for a given request.
macro_rules! expect {
($res: pat => $e: expr) => {
match *response {
$res => $e,
_ => Err(Error::WrongKind),
}
}
}

// check response against contained prover.
match (self, response) {
(&CheckedRequest::HeaderProof(ref prover, _), &NetResponse::HeaderProof(ref res)) =>
prover.check_response(cache, &res.proof).map(Response::HeaderProof),
(&CheckedRequest::HeaderByHash(ref prover, _), &NetResponse::Headers(ref res)) =>
prover.check_response(cache, &res.headers).map(Response::HeaderByHash),
(&CheckedRequest::Receipts(ref prover, _), &NetResponse::Receipts(ref res)) =>
prover.check_response(cache, &res.receipts).map(Response::Receipts),
(&CheckedRequest::Body(ref prover, _), &NetResponse::Body(ref res)) =>
prover.check_response(cache, &res.body).map(Response::Body),
(&CheckedRequest::Account(ref prover, _), &NetResponse::Account(ref res)) =>
prover.check_response(cache, &res.proof).map(Response::Account),
(&CheckedRequest::Code(ref prover, _), &NetResponse::Code(ref res)) =>
prover.check_response(cache, &res.code).map(Response::Code),
(&CheckedRequest::Execution(ref prover, _), &NetResponse::Execution(ref res)) =>
prover.check_response(cache, &res.items).map(Response::Execution),
_ => Err(Error::WrongKind),
match *self {
CheckedRequest::HeaderProof(ref prover, _) => expect!(NetResponse::HeaderProof(ref res) =>
prover.check_response(cache, &res.proof).map(Response::HeaderProof)),
CheckedRequest::HeaderByHash(ref prover, _) => expect!(NetResponse::Headers(ref res) =>
prover.check_response(cache, &res.headers).map(Response::HeaderByHash)),
CheckedRequest::Receipts(ref prover, _) => expect!(NetResponse::Receipts(ref res) =>
prover.check_response(cache, &res.receipts).map(Response::Receipts)),
CheckedRequest::Body(ref prover, _) => expect!(NetResponse::Body(ref res) =>
prover.check_response(cache, &res.body).map(Response::Body)),
CheckedRequest::Account(ref prover, _) => expect!(NetResponse::Account(ref res) =>
prover.check_response(cache, &res.proof).map(Response::Account)),
CheckedRequest::Code(ref prover, _) => expect!(NetResponse::Code(ref res) =>
prover.check_response(cache, &res.code).map(Response::Code)),
CheckedRequest::Execution(ref prover, _) => expect!(NetResponse::Execution(ref res) =>
prover.check_response(cache, &res.items).map(Response::Execution)),
}
}
}
Expand Down
35 changes: 19 additions & 16 deletions ethcore/light/src/types/request/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ use request::{
/// Build chained requests. Push them onto the series with `push`,
/// and produce a `Requests` object with `build`. Outputs are checked for consistency.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RequestBuilder<T: IncompleteRequest> {
pub struct RequestBuilder<T> {
output_kinds: HashMap<(usize, usize), OutputKind>,
requests: Vec<T>,
}

impl<T: IncompleteRequest> Default for RequestBuilder<T> {
impl<T> Default for RequestBuilder<T> {
fn default() -> Self {
RequestBuilder {
output_kinds: HashMap::new(),
Expand Down Expand Up @@ -73,13 +73,13 @@ impl<T: IncompleteRequest> RequestBuilder<T> {

/// Requests pending responses.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Requests<T: IncompleteRequest> {
pub struct Requests<T> {
outputs: HashMap<(usize, usize), Output>,
requests: Vec<T>,
answered: usize,
}

impl<T: IncompleteRequest + Clone> Requests<T> {
impl<T> Requests<T> {
/// Get access to the underlying slice of requests.
// TODO: unimplemented -> Vec<Request>, // do we _have to_ allocate?
pub fn requests(&self) -> &[T] { &self.requests }
Expand All @@ -92,17 +92,6 @@ impl<T: IncompleteRequest + Clone> Requests<T> {
self.answered == self.requests.len()
}

/// Get the next request as a filled request. Returns `None` when all requests answered.
pub fn next_complete(&self) -> Option<T::Complete> {
if self.is_complete() {
None
} else {
Some(self.requests[self.answered].clone()
.complete()
.expect("All outputs checked as invariant of `Requests` object; qed"))
}
}

/// Map requests from one type into another.
pub fn map_requests<F, U>(self, f: F) -> Requests<U>
where F: FnMut(T) -> U, U: IncompleteRequest
Expand All @@ -115,6 +104,19 @@ impl<T: IncompleteRequest + Clone> Requests<T> {
}
}

impl<T: IncompleteRequest + Clone> Requests<T> {
/// Get the next request as a filled request. Returns `None` when all requests answered.
pub fn next_complete(&self) -> Option<T::Complete> {
if self.is_complete() {
None
} else {
Some(self.requests[self.answered].clone()
.complete()
.expect("All outputs checked as invariant of `Requests` object; qed"))
}
}
}

impl<T: super::CheckedRequest> Requests<T> {
/// Supply a response for the next request.
/// Fails on: wrong request kind, all requests answered already.
Expand All @@ -124,7 +126,8 @@ impl<T: super::CheckedRequest> Requests<T> {
let idx = self.answered;

// check validity.
if idx == self.requests.len() { return Err(ResponseError::Unexpected) }
if self.is_complete() { return Err(ResponseError::Unexpected) }

let extracted = self.requests[idx]
.check_response(env, response).map_err(ResponseError::Validity)?;

Expand Down
2 changes: 1 addition & 1 deletion rpc/src/v1/impls/light/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl EthClient {
// three possible outcomes:
// - network is down.
// - we get a score, but our hash is non-canonical.
// - we get ascore, and our hash is canonical.
// - we get a score, and our hash is canonical.
let maybe_fut = sync.with_context(move |ctx| on_demand.hash_and_score_by_number(ctx, req));
match maybe_fut {
Some(fut) => fut.map(move |(hash, score)| {
Expand Down

0 comments on commit 2d87f56

Please sign in to comment.