Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused verification code #330

Merged
merged 4 commits into from
Apr 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions core/src/client/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::verification::{QueueConfig, VerifierType};
use crate::verification::QueueConfig;
use kvdb_rocksdb::CompactionProfile;
use std::path::Path;
use std::str::FromStr;
Expand Down Expand Up @@ -71,8 +71,6 @@ pub struct ClientConfig {
pub db_compaction: DatabaseCompactionProfile,
/// State db cache-size.
pub state_cache_size: usize,
/// Type of block verifier used by client.
pub verifier_type: VerifierType,
}

impl Default for ClientConfig {
Expand All @@ -84,7 +82,6 @@ impl Default for ClientConfig {
db_cache_size: Default::default(),
db_compaction: Default::default(),
state_cache_size: DEFAULT_STATE_CACHE_SIZE as usize * mb,
verifier_type: Default::default(),
}
}
}
16 changes: 5 additions & 11 deletions core/src/client/importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::miner::{Miner, MinerService};
use crate::service::ClientIoMessage;
use crate::types::BlockId;
use crate::verification::queue::{BlockQueue, HeaderQueue};
use crate::verification::{self, PreverifiedBlock, Verifier};
use crate::verification::{PreverifiedBlock, Verifier};
use crate::views::{BlockView, HeaderView};
use cio::IoChannel;
use ctypes::header::{Header, Seal};
Expand All @@ -42,7 +42,7 @@ pub struct Importer {
pub import_lock: Mutex<()>, // FIXME Maybe wrap the whole `Importer` instead?

/// Used to verify blocks
pub verifier: Box<dyn Verifier>,
pub verifier: Verifier,

/// Queue containing pending blocks
pub block_queue: BlockQueue,
Expand All @@ -64,19 +64,13 @@ impl Importer {
message_channel: IoChannel<ClientIoMessage>,
miner: Arc<Miner>,
) -> Result<Importer, Error> {
let block_queue = BlockQueue::new(
&config.queue,
engine.clone(),
message_channel.clone(),
config.verifier_type.verifying_seal(),
);
let block_queue = BlockQueue::new(&config.queue, engine.clone(), message_channel.clone());

let header_queue =
HeaderQueue::new(&config.queue, engine.clone(), message_channel, config.verifier_type.verifying_seal());
let header_queue = HeaderQueue::new(&config.queue, engine.clone(), message_channel);

Ok(Importer {
import_lock: Mutex::new(()),
verifier: verification::new(config.verifier_type),
verifier: Verifier,
block_queue,
header_queue,
miner,
Expand Down
5 changes: 0 additions & 5 deletions core/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,6 @@ pub trait ConsensusEngine: Sync + Send {
Ok(())
}

/// Phase 2 verification. Perform costly checks such as transaction signatures. Returns either a null `Ok` or a general error detailing the problem with import.
fn verify_block_seal(&self, _header: &Header) -> Result<(), Error> {
Ok(())
}

/// Phase 3 verification. Check block information against parent. Returns either a null `Ok` or a general error detailing the problem with import.
/// The verification must be conducted only with the two headers' information because it does not guarantee whether the two corresponding bodies have been imported.
fn verify_block_family(&self, _header: &Header, _parent: &Header) -> Result<(), Error> {
Expand Down
7 changes: 1 addition & 6 deletions core/src/consensus/solo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,12 @@ impl ConsensusEngine for Solo {
mod tests {
use crate::scheme::Scheme;
use ctypes::Header;
use primitives::H520;

#[test]
fn fail_to_verify() {
let engine = Scheme::new_test_solo().engine;
let mut header: Header = Header::default();
let header: Header = Header::default();

assert!(engine.verify_header_basic(&header).is_ok());

header.set_seal(vec![::rlp::encode(&H520::default())]);

assert!(engine.verify_block_seal(&header).is_ok());
}
}
45 changes: 0 additions & 45 deletions core/src/verification/canon_verifier.rs

This file was deleted.

40 changes: 0 additions & 40 deletions core/src/verification/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,11 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

mod canon_verifier;
mod noop_verifier;
pub mod queue;
#[cfg_attr(feature = "cargo-clippy", allow(clippy::module_inception))]
mod verification;
mod verifier;

pub use self::canon_verifier::CanonVerifier;
pub use self::noop_verifier::NoopVerifier;
pub use self::queue::{BlockQueue, Config as QueueConfig};
pub use self::verification::*;
pub use self::verifier::Verifier;

/// Verifier type.
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum VerifierType {
/// Verifies block normally.
Canon,
/// Verifies block normally, but skips seal verification.
CanonNoSeal,
/// Does not verify block at all.
/// Used in tests.
Noop,
}

impl VerifierType {
/// Check if seal verification is enabled for this verifier type.
pub fn verifying_seal(self) -> bool {
match self {
VerifierType::Canon => true,
VerifierType::Noop | VerifierType::CanonNoSeal => false,
}
}
}

impl Default for VerifierType {
fn default() -> Self {
VerifierType::Canon
}
}

/// Create a new verifier based on type.
pub fn new(v: VerifierType) -> Box<dyn Verifier> {
match v {
VerifierType::Canon | VerifierType::CanonNoSeal => Box::new(CanonVerifier),
VerifierType::Noop => Box::new(NoopVerifier),
}
}
44 changes: 0 additions & 44 deletions core/src/verification/noop_verifier.rs

This file was deleted.

26 changes: 5 additions & 21 deletions core/src/verification/queue/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ pub trait Kind: 'static + Sized + Send + Sync {
fn create(input: Self::Input, engine: &dyn CodeChainEngine) -> Result<Self::Unverified, Error>;

/// Attempt to verify the `Unverified` item using the given engine.
fn verify(
unverified: Self::Unverified,
engine: &dyn CodeChainEngine,
check_seal: bool,
) -> Result<Self::Verified, Error>;
fn verify(unverified: Self::Unverified) -> Result<Self::Verified, Error>;

fn signal() -> ClientIoMessage;
}
Expand Down Expand Up @@ -117,16 +113,8 @@ pub mod headers {
Ok(input)
}

fn verify(
un: Self::Unverified,
engine: &dyn CodeChainEngine,
check_seal: bool,
) -> Result<Self::Verified, Error> {
if check_seal {
engine.verify_block_seal(&un).map(|_| un)
} else {
Ok(un)
}
fn verify(un: Self::Unverified) -> Result<Self::Verified, Error> {
Ok(un)
}

fn signal() -> ClientIoMessage {
Expand Down Expand Up @@ -172,13 +160,9 @@ pub mod blocks {
}
}

fn verify(
un: Self::Unverified,
engine: &dyn CodeChainEngine,
check_seal: bool,
) -> Result<Self::Verified, Error> {
fn verify(un: Self::Unverified) -> Result<Self::Verified, Error> {
let hash = un.hash();
match verify_block_seal(un.header, un.bytes, engine, check_seal) {
match verify_block_seal(un.header, un.bytes) {
Ok(verified) => Ok(verified),
Err(e) => {
cwarn!(CLIENT, "Stage 2 block verification failed for {}: {:?}", hash, e);
Expand Down
17 changes: 5 additions & 12 deletions core/src/verification/queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,7 @@ impl QueueSignal {
}

impl<K: Kind> VerificationQueue<K> {
pub fn new(
config: &Config,
engine: Arc<dyn CodeChainEngine>,
message_channel: IoChannel<ClientIoMessage>,
check_seal: bool,
) -> Self {
pub fn new(config: &Config, engine: Arc<dyn CodeChainEngine>, message_channel: IoChannel<ClientIoMessage>) -> Self {
let verification = Arc::new(Verification {
unverified: Mutex::new(VecDeque::new()),
verifying: Mutex::new(VecDeque::new()),
Expand All @@ -130,7 +125,6 @@ impl<K: Kind> VerificationQueue<K> {
verifying: AtomicUsize::new(0),
verified: AtomicUsize::new(0),
},
check_seal,
more_to_verify_mutex: SMutex::new(()),
});
let deleting = Arc::new(AtomicBool::new(false));
Expand Down Expand Up @@ -185,7 +179,7 @@ impl<K: Kind> VerificationQueue<K> {

fn verify(
verification: &Verification<K>,
engine: &dyn CodeChainEngine,
_engine: &dyn CodeChainEngine,
ready_signal: &QueueSignal,
empty: &SCondvar,
more_to_verify: &SCondvar,
Expand Down Expand Up @@ -232,7 +226,7 @@ impl<K: Kind> VerificationQueue<K> {
};

let hash = item.hash();
let is_ready = match K::verify(item, engine, verification.check_seal) {
let is_ready = match K::verify(item) {
Ok(verified) => {
let mut verifying = verification.verifying.lock();
let mut idx = None;
Expand Down Expand Up @@ -486,7 +480,6 @@ struct Verification<K: Kind> {
verified: Mutex<VecDeque<K::Verified>>,
bad: Mutex<HashSet<BlockHash>>,
sizes: Sizes,
check_seal: bool,
more_to_verify_mutex: SMutex<()>,
}

Expand All @@ -513,7 +506,7 @@ mod tests {
let engine = scheme.engine;

let config = Config::default();
BlockQueue::new(&config, engine, IoChannel::disconnected(), true)
BlockQueue::new(&config, engine, IoChannel::disconnected())
}

#[test]
Expand All @@ -523,7 +516,7 @@ mod tests {
let engine = scheme.engine;

let config = Config::default();
let _ = BlockQueue::new(&config, engine, IoChannel::disconnected(), true);
let _ = BlockQueue::new(&config, engine, IoChannel::disconnected());
}

#[test]
Expand Down
10 changes: 1 addition & 9 deletions core/src/verification/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,7 @@ fn verify_transactions_root(
/// Phase 2 verification. Perform costly checks such as transaction signatures and block nonce for ethash.
/// Still operates on a individual block
/// Returns a `PreverifiedBlock` structure populated with transactions
pub fn verify_block_seal(
header: Header,
bytes: Bytes,
engine: &dyn CodeChainEngine,
check_seal: bool,
) -> Result<PreverifiedBlock, Error> {
if check_seal {
engine.verify_block_seal(&header)?;
}
pub fn verify_block_seal(header: Header, bytes: Bytes) -> Result<PreverifiedBlock, Error> {
let transactions: Vec<_> =
BlockView::new(&bytes).transactions().into_iter().map(TryInto::try_into).collect::<Result<_, _>>()?;
Ok(PreverifiedBlock {
Expand Down
Loading