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

Fix light requests handler error handling and logging target #8046

Closed
wants to merge 5 commits into from
Closed

Fix light requests handler error handling and logging target #8046

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c629cd
Fix light requests handler error handling and logging target
tomaka Feb 4, 2021
d869cde
Revert service.rs
tomaka Feb 4, 2021
2614d43
Fix unused imports
tomaka Feb 4, 2021
8b85cb3
Some test fix
tomaka Feb 5, 2021
79a846b
Merge remote-tracking branch 'upstream/master' into light-handler-fixes
tomaka Feb 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions client/network/src/light_client_requests/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl<B: Block> LightClientRequestHandler<B> {
request: &schema::v1::light::RemoteCallRequest,
) -> Result<schema::v1::light::Response, HandleRequestError> {
log::trace!(
target: LOG_TARGET,
"Remote call request from {} ({} at {:?}).",
peer, request.method, request.block,
);
Expand All @@ -172,10 +173,11 @@ impl<B: Block> LightClientRequestHandler<B> {
Ok((_, proof)) => proof,
Err(e) => {
log::trace!(
target: LOG_TARGET,
"remote call request from {} ({} at {:?}) failed with: {}",
peer, request.method, request.block, e,
);
StorageProof::empty()
return Err(HandleRequestError::NotFulfillable);
}
};

Expand All @@ -193,11 +195,12 @@ impl<B: Block> LightClientRequestHandler<B> {
request: &schema::v1::light::RemoteReadRequest,
) -> Result<schema::v1::light::Response, HandleRequestError> {
if request.keys.is_empty() {
log::debug!("Invalid remote read request sent by {}.", peer);
log::debug!(target: LOG_TARGET, "Invalid remote read request sent by {}.", peer);
return Err(HandleRequestError::BadRequest("Remote read request without keys."))
}

log::trace!(
target: LOG_TARGET,
"Remote read request from {} ({} at {:?}).",
peer, fmt_keys(request.keys.first(), request.keys.last()), request.block,
);
Expand All @@ -211,10 +214,11 @@ impl<B: Block> LightClientRequestHandler<B> {
Ok(proof) => proof,
Err(error) => {
log::trace!(
target: LOG_TARGET,
"remote read request from {} ({} at {:?}) failed with: {}",
peer, fmt_keys(request.keys.first(), request.keys.last()), request.block, error,
);
StorageProof::empty()
return Err(HandleRequestError::NotFulfillable);
}
};

Expand All @@ -232,11 +236,12 @@ impl<B: Block> LightClientRequestHandler<B> {
request: &schema::v1::light::RemoteReadChildRequest,
) -> Result<schema::v1::light::Response, HandleRequestError> {
if request.keys.is_empty() {
log::debug!("Invalid remote child read request sent by {}.", peer);
log::debug!(target: LOG_TARGET, "Invalid remote child read request sent by {}.", peer);
return Err(HandleRequestError::BadRequest("Remove read child request without keys."))
}

log::trace!(
target: LOG_TARGET,
"Remote read child request from {} ({} {} at {:?}).",
peer,
HexDisplay::from(&request.storage_key),
Expand All @@ -259,14 +264,15 @@ impl<B: Block> LightClientRequestHandler<B> {
Ok(proof) => proof,
Err(error) => {
log::trace!(
target: LOG_TARGET,
"remote read child request from {} ({} {} at {:?}) failed with: {}",
peer,
HexDisplay::from(&request.storage_key),
fmt_keys(request.keys.first(), request.keys.last()),
request.block,
error,
);
StorageProof::empty()
return Err(HandleRequestError::NotFulfillable);
}
};

Expand All @@ -283,17 +289,18 @@ impl<B: Block> LightClientRequestHandler<B> {
peer: &PeerId,
request: &schema::v1::light::RemoteHeaderRequest,
) -> Result<schema::v1::light::Response, HandleRequestError> {
log::trace!("Remote header proof request from {} ({:?}).", peer, request.block);
log::trace!(target: LOG_TARGET, "Remote header proof request from {} ({:?}).", peer, request.block);

let block = Decode::decode(&mut request.block.as_ref())?;
let (header, proof) = match self.client.header_proof(&BlockId::Number(block)) {
Ok((header, proof)) => (header.encode(), proof),
Err(error) => {
log::trace!(
target: LOG_TARGET,
"Remote header proof request from {} ({:?}) failed with: {}.",
peer, request.block, error
);
(Default::default(), StorageProof::empty())
return Err(HandleRequestError::NotFulfillable);
}
};

Expand All @@ -311,6 +318,7 @@ impl<B: Block> LightClientRequestHandler<B> {
request: &schema::v1::light::RemoteChangesRequest,
) -> Result<schema::v1::light::Response, HandleRequestError> {
log::trace!(
target: LOG_TARGET,
"Remote changes proof request from {} for key {} ({:?}..{:?}).",
peer,
if !request.storage_key.is_empty() {
Expand All @@ -337,6 +345,7 @@ impl<B: Block> LightClientRequestHandler<B> {
Ok(proof) => proof,
Err(error) => {
log::trace!(
target: LOG_TARGET,
"Remote changes proof request from {} for key {} ({:?}..{:?}) failed with: {}.",
peer,
format!("{} : {}", HexDisplay::from(&request.storage_key), HexDisplay::from(&key.0)),
Expand All @@ -345,12 +354,7 @@ impl<B: Block> LightClientRequestHandler<B> {
error,
);

light::ChangesProof::<B::Header> {
max_block: Zero::zero(),
proof: Vec::new(),
roots: BTreeMap::new(),
roots_proof: StorageProof::empty(),
}
return Err(HandleRequestError::NotFulfillable);
}
};

Expand Down Expand Up @@ -381,6 +385,9 @@ enum HandleRequestError {
/// A bad request has been received.
#[display(fmt = "bad request: {}", _0)]
BadRequest(&'static str),
/// Received request concerns blocks that aren't available, or a similar client-side problem.
#[display(fmt = "Request cannot be fulfilled")]
NotFulfillable,
/// Encoding or decoding of some data failed.
#[display(fmt = "codec error: {}", _0)]
Codec(codec::Error),
Expand Down
3 changes: 3 additions & 0 deletions client/network/src/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,7 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
}
{
let mut peers_notifications_sinks = this.peers_notifications_sinks.lock();
println!("peers_notifications_sinks.insert({:?}, {:?})", remote, protocol);
Copy link
Member

Choose a reason for hiding this comment

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

Pierre :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah fuck

peers_notifications_sinks.insert((remote.clone(), protocol.clone()), notifications_sink);
}
this.event_streams.send(Event::NotificationStreamOpened {
Expand All @@ -1540,6 +1541,7 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
remote, protocol, notifications_sink
})) => {
let mut peers_notifications_sinks = this.peers_notifications_sinks.lock();
println!("peers_notifications_sinks.replace({:?}, {:?})", remote, protocol);
if let Some(s) = peers_notifications_sinks.get_mut(&(remote, protocol)) {
*s = notifications_sink;
} else {
Expand Down Expand Up @@ -1581,6 +1583,7 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
});
{
let mut peers_notifications_sinks = this.peers_notifications_sinks.lock();
println!("peers_notifications_sinks.remove({:?}, {:?})", remote, protocol);
peers_notifications_sinks.remove(&(remote.clone(), protocol));
}
},
Expand Down