From 03c7f53db8b5bd7b8ecee07484a3abe4da19dee7 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Thu, 16 Apr 2020 15:52:26 +0900 Subject: [PATCH 1/2] Remove unnecessary serialize/deserialize while calling import_header --- core/src/client/client.rs | 9 +++------ core/src/client/mod.rs | 2 +- core/src/client/test_client.rs | 2 +- sync/src/block/extension.rs | 2 +- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 32905d37ce..58f2802f3f 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -416,12 +416,9 @@ impl ImportBlock for Client { Ok(self.importer.block_queue.import(unverified)?) } - fn import_header(&self, bytes: Bytes) -> Result { - let unverified = encoded::Header::new(bytes).decode(); - { - if self.block_chain().is_known_header(&unverified.hash()) { - return Err(BlockImportError::Import(ImportError::AlreadyInChain)) - } + fn import_header(&self, unverified: Header) -> Result { + if self.block_chain().is_known_header(&unverified.hash()) { + return Err(BlockImportError::Import(ImportError::AlreadyInChain)) } Ok(self.importer.header_queue.import(unverified)?) } diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index 66a6410147..9138021e7d 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -159,7 +159,7 @@ pub trait ImportBlock { fn import_block(&self, bytes: Bytes) -> Result; /// Import a header into the blockchain - fn import_header(&self, bytes: Bytes) -> Result; + fn import_header(&self, header: Header) -> Result; /// Import a trusted header into the blockchain /// Trusted header doesn't go through any verifications and doesn't update the best header diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 46d7378a72..18fe6f49ea 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -458,7 +458,7 @@ impl ImportBlock for TestBlockChainClient { Ok(h) } - fn import_header(&self, _bytes: Bytes) -> Result { + fn import_header(&self, _bytes: Header) -> Result { unimplemented!() } diff --git a/sync/src/block/extension.rs b/sync/src/block/extension.rs index b504606df6..4ecf8a3cfe 100644 --- a/sync/src/block/extension.rs +++ b/sync/src/block/extension.rs @@ -985,7 +985,7 @@ impl Extension { for header in completed { let hash = header.hash(); - match self.client.import_header(header.rlp_bytes()) { + match self.client.import_header(header) { Err(BlockImportError::Import(ImportError::AlreadyInChain)) => exists.push(hash), Err(BlockImportError::Import(ImportError::AlreadyQueued)) => queued.push(hash), // FIXME: handle import errors From eec2c7cd58666018ba603488a61600434710ae8d Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Thu, 16 Apr 2020 16:12:12 +0900 Subject: [PATCH 2/2] Make import_trusted_header receive moved Header instead of reference --- core/src/client/client.rs | 4 ++-- core/src/client/mod.rs | 2 +- core/src/client/test_client.rs | 2 +- sync/src/block/extension.rs | 33 +++++++++++++++++++-------------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/core/src/client/client.rs b/core/src/client/client.rs index 58f2802f3f..54ea12bfd5 100644 --- a/core/src/client/client.rs +++ b/core/src/client/client.rs @@ -423,12 +423,12 @@ impl ImportBlock for Client { Ok(self.importer.header_queue.import(unverified)?) } - fn import_trusted_header(&self, header: &Header) -> Result { + fn import_trusted_header(&self, header: Header) -> Result { if self.block_chain().is_known_header(&header.hash()) { return Err(BlockImportError::Import(ImportError::AlreadyInChain)) } let import_lock = self.importer.import_lock.lock(); - self.importer.import_trusted_header(header, self, &import_lock); + self.importer.import_trusted_header(&header, self, &import_lock); Ok(header.hash()) } diff --git a/core/src/client/mod.rs b/core/src/client/mod.rs index 9138021e7d..e4ce4e70f0 100644 --- a/core/src/client/mod.rs +++ b/core/src/client/mod.rs @@ -163,7 +163,7 @@ pub trait ImportBlock { /// Import a trusted header into the blockchain /// Trusted header doesn't go through any verifications and doesn't update the best header - fn import_trusted_header(&self, header: &Header) -> Result; + fn import_trusted_header(&self, header: Header) -> Result; /// Import a trusted block into the blockchain /// Trusted block doesn't go through any verifications and doesn't update the best block diff --git a/core/src/client/test_client.rs b/core/src/client/test_client.rs index 18fe6f49ea..f4e577d777 100644 --- a/core/src/client/test_client.rs +++ b/core/src/client/test_client.rs @@ -462,7 +462,7 @@ impl ImportBlock for TestBlockChainClient { unimplemented!() } - fn import_trusted_header(&self, _header: &Header) -> Result { + fn import_trusted_header(&self, _header: Header) -> Result { unimplemented!() } diff --git a/sync/src/block/extension.rs b/sync/src/block/extension.rs index 4ecf8a3cfe..9a6b213e8b 100644 --- a/sync/src/block/extension.rs +++ b/sync/src/block/extension.rs @@ -848,7 +848,7 @@ impl Extension { match response { ResponseMessage::Headers(headers) => { self.dismiss_request(from, id); - self.on_header_response(from, &headers) + self.on_header_response(from, headers) } ResponseMessage::Bodies(bodies) => { self.check_sync_variable(); @@ -938,17 +938,29 @@ impl Extension { } } - fn on_header_response(&mut self, from: &NodeId, headers: &[Header]) { + fn on_header_response(&mut self, from: &NodeId, mut headers: Vec
) { ctrace!(SYNC, "Received header response from({}) with length({})", from, headers.len()); match self.state { - State::SnapshotHeader(hash, _) => match headers { - [parent, header] if header.hash() == hash => { + State::SnapshotHeader(hash, _) => { + if headers.len() != 2 || headers[1].hash() != hash { + cdebug!( + SYNC, + "Peer {} responded with a invalid response. requested hash: {}, response length: {}", + from, + hash, + headers.len() + ) + } else { + let header = headers.pop().expect("headers.len() == 2"); + let header_hash = header.hash(); + let parent = headers.pop().expect("headers.len() == 1"); + let parent_hash = parent.hash(); match self.client.import_trusted_header(parent) { Ok(_) | Err(BlockImportError::Import(ImportError::AlreadyInChain)) | Err(BlockImportError::Import(ImportError::AlreadyQueued)) => {} Err(err) => { - cwarn!(SYNC, "Cannot import header({}): {:?}", parent.hash(), err); + cwarn!(SYNC, "Cannot import header({}): {:?}", parent_hash, err); return } } @@ -957,20 +969,13 @@ impl Extension { | Err(BlockImportError::Import(ImportError::AlreadyInChain)) | Err(BlockImportError::Import(ImportError::AlreadyQueued)) => {} Err(err) => { - cwarn!(SYNC, "Cannot import header({}): {:?}", header.hash(), err); + cwarn!(SYNC, "Cannot import header({}): {:?}", header_hash, err); return } } self.move_state(); } - _ => cdebug!( - SYNC, - "Peer {} responded with a invalid response. requested hash: {}, response length: {}", - from, - hash, - headers.len() - ), - }, + } State::Full => { let (mut completed, peer_is_caught_up) = if let Some(peer) = self.header_downloaders.get_mut(from) { peer.import_headers(&headers);