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

Make the Engine hold AccountProvider #3499

Closed
wants to merge 4 commits into from
Closed
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
22 changes: 14 additions & 8 deletions ethcore/src/engines/authority_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ pub struct AuthorityRound {
builtins: BTreeMap<Address, Builtin>,
transition_service: IoService<BlockArrived>,
message_channel: Mutex<Option<IoChannel<ClientIoMessage>>>,
account_provider: Mutex<Option<Arc<AccountProvider>>>,
step: AtomicUsize,
proposed: AtomicBool,
}
Expand Down Expand Up @@ -101,6 +102,7 @@ impl AuthorityRound {
builtins: builtins,
transition_service: try!(IoService::<BlockArrived>::start()),
message_channel: Mutex::new(None),
account_provider: Mutex::new(None),
step: AtomicUsize::new(initial_step),
proposed: AtomicBool::new(false)
});
Expand Down Expand Up @@ -219,12 +221,12 @@ impl Engine for AuthorityRound {
///
/// This operation is synchronous and may (quite reasonably) not be available, in which `false` will
/// be returned.
fn generate_seal(&self, block: &ExecutedBlock, accounts: Option<&AccountProvider>) -> Option<Vec<Bytes>> {
fn generate_seal(&self, block: &ExecutedBlock) -> Option<Vec<Bytes>> {
if self.proposed.load(AtomicOrdering::SeqCst) { return None; }
let header = block.header();
let step = self.step();
if self.is_step_proposer(step, header.author()) {
if let Some(ap) = accounts {
if let Some(ref ap) = *self.account_provider.lock() {
// Account should be permanently unlocked, otherwise sealing will fail.
if let Ok(signature) = ap.sign(*header.author(), None, header.bare_hash()) {
trace!(target: "poa", "generate_seal: Issuing a block for step {}.", step);
Expand Down Expand Up @@ -307,8 +309,11 @@ impl Engine for AuthorityRound {
}

fn register_message_channel(&self, message_channel: IoChannel<ClientIoMessage>) {
let mut guard = self.message_channel.lock();
*guard = Some(message_channel);
*self.message_channel.lock() = Some(message_channel);
}

fn register_account_provider(&self, account_provider: Arc<AccountProvider>) {
*self.account_provider.lock() = Some(account_provider);
}
}

Expand Down Expand Up @@ -382,6 +387,7 @@ mod tests {

let spec = Spec::new_test_round();
let engine = &*spec.engine;
engine.register_account_provider(Arc::new(tap));
let genesis_header = spec.genesis_header();
let mut db1 = get_temp_state_db().take();
spec.ensure_db_good(&mut db1).unwrap();
Expand All @@ -393,16 +399,16 @@ mod tests {
let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes, addr2, (3141562.into(), 31415620.into()), vec![]).unwrap();
let b2 = b2.close_and_lock();

if let Some(seal) = engine.generate_seal(b1.block(), Some(&tap)) {
if let Some(seal) = engine.generate_seal(b1.block()) {
assert!(b1.clone().try_seal(engine, seal).is_ok());
// Second proposal is forbidden.
assert!(engine.generate_seal(b1.block(), Some(&tap)).is_none());
assert!(engine.generate_seal(b1.block()).is_none());
}

if let Some(seal) = engine.generate_seal(b2.block(), Some(&tap)) {
if let Some(seal) = engine.generate_seal(b2.block()) {
assert!(b2.clone().try_seal(engine, seal).is_ok());
// Second proposal is forbidden.
assert!(engine.generate_seal(b2.block(), Some(&tap)).is_none());
assert!(engine.generate_seal(b2.block()).is_none());
}
}

Expand Down
13 changes: 10 additions & 3 deletions ethcore/src/engines/basic_authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub struct BasicAuthority {
params: CommonParams,
our_params: BasicAuthorityParams,
builtins: BTreeMap<Address, Builtin>,
account_provider: Mutex<Option<Arc<AccountProvider>>>
}

impl BasicAuthority {
Expand All @@ -67,6 +68,7 @@ impl BasicAuthority {
params: params,
our_params: our_params,
builtins: builtins,
account_provider: Mutex::new(None)
}
}
}
Expand Down Expand Up @@ -113,8 +115,8 @@ impl Engine for BasicAuthority {
///
/// This operation is synchronous and may (quite reasonably) not be available, in which `false` will
/// be returned.
fn generate_seal(&self, block: &ExecutedBlock, accounts: Option<&AccountProvider>) -> Option<Vec<Bytes>> {
if let Some(ap) = accounts {
fn generate_seal(&self, block: &ExecutedBlock) -> Option<Vec<Bytes>> {
if let Some(ref ap) = *self.account_provider.lock() {
let header = block.header();
let message = header.bare_hash();
// account should be pernamently unlocked, otherwise sealing will fail
Expand Down Expand Up @@ -179,6 +181,10 @@ impl Engine for BasicAuthority {
fn verify_transaction(&self, t: &SignedTransaction, _header: &Header) -> Result<(), Error> {
t.sender().map(|_|()) // Perform EC recovery and cache sender
}

fn register_account_provider(&self, ap: Arc<AccountProvider>) {
*self.account_provider.lock() = Some(ap);
}
}

#[cfg(test)]
Expand Down Expand Up @@ -253,14 +259,15 @@ mod tests {

let spec = new_test_authority();
let engine = &*spec.engine;
engine.register_account_provider(Arc::new(tap));
let genesis_header = spec.genesis_header();
let mut db_result = get_temp_state_db();
let mut db = db_result.take();
spec.ensure_db_good(&mut db).unwrap();
let last_hashes = Arc::new(vec![genesis_header.hash()]);
let b = OpenBlock::new(engine, Default::default(), false, db, &genesis_header, last_hashes, addr, (3141562.into(), 31415620.into()), vec![]).unwrap();
let b = b.close_and_lock();
let seal = engine.generate_seal(b.block(), Some(&tap)).unwrap();
let seal = engine.generate_seal(b.block()).unwrap();
assert!(b.try_seal(engine, seal).is_ok());
}

Expand Down
12 changes: 3 additions & 9 deletions ethcore/src/engines/instant_seal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use spec::CommonParams;
use evm::Schedule;
use block::ExecutedBlock;
use util::Bytes;
use account_provider::AccountProvider;

/// An engine which does not provide any consensus mechanism, just seals blocks internally.
pub struct InstantSeal {
Expand Down Expand Up @@ -60,7 +59,7 @@ impl Engine for InstantSeal {

fn is_sealer(&self, _author: &Address) -> Option<bool> { Some(true) }

fn generate_seal(&self, _block: &ExecutedBlock, _accounts: Option<&AccountProvider>) -> Option<Vec<Bytes>> {
fn generate_seal(&self, _block: &ExecutedBlock) -> Option<Vec<Bytes>> {
Some(Vec::new())
}
}
Expand All @@ -69,27 +68,22 @@ impl Engine for InstantSeal {
mod tests {
use util::*;
use tests::helpers::*;
use account_provider::AccountProvider;
use spec::Spec;
use header::Header;
use block::*;

#[test]
fn instant_can_seal() {
let tap = AccountProvider::transient_provider();
let addr = tap.insert_account("".sha3(), "").unwrap();

let spec = Spec::new_instant();
let engine = &*spec.engine;
let genesis_header = spec.genesis_header();
let mut db_result = get_temp_state_db();
let mut db = db_result.take();
spec.ensure_db_good(&mut db).unwrap();
let last_hashes = Arc::new(vec![genesis_header.hash()]);
let b = OpenBlock::new(engine, Default::default(), false, db, &genesis_header, last_hashes, addr, (3141562.into(), 31415620.into()), vec![]).unwrap();
let b = OpenBlock::new(engine, Default::default(), false, db, &genesis_header, last_hashes, Address::default(), (3141562.into(), 31415620.into()), vec![]).unwrap();
let b = b.close_and_lock();
// Seal with empty AccountProvider.
let seal = engine.generate_seal(b.block(), Some(&tap)).unwrap();
let seal = engine.generate_seal(b.block()).unwrap();
assert!(b.try_seal(engine, seal).is_ok());
}

Expand Down
5 changes: 3 additions & 2 deletions ethcore/src/engines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub trait Engine : Sync + Send {
///
/// This operation is synchronous and may (quite reasonably) not be available, in which None will
/// be returned.
fn generate_seal(&self, _block: &ExecutedBlock, _accounts: Option<&AccountProvider>) -> Option<Vec<Bytes>> { None }
fn generate_seal(&self, _block: &ExecutedBlock) -> Option<Vec<Bytes>> { None }

/// Phase 1 quick block verification. Only does checks that are cheap. `block` (the header's full block)
/// may be provided for additional checks. Returns either a null `Ok` or a general error detailing the problem with import.
Expand Down Expand Up @@ -146,5 +146,6 @@ pub trait Engine : Sync + Send {

/// Add a channel for communication with Client which can be used for sealing.
fn register_message_channel(&self, _message_channel: IoChannel<ClientIoMessage>) {}
// TODO: sealing stuff - though might want to leave this for later.
/// Add an account provider useful for Engines that sign stuff.
fn register_account_provider(&self, _account_provider: Arc<AccountProvider>) {}
}
24 changes: 6 additions & 18 deletions ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use std::time::{Instant, Duration};

use util::*;
use util::using_queue::{UsingQueue, GetAction};
use account_provider::AccountProvider;
use views::{BlockView, HeaderView};
use state::{State, CleanupMode};
use client::{MiningBlockChainClient, Executive, Executed, EnvInfo, TransactOptions, BlockID, CallAnalytics};
Expand Down Expand Up @@ -220,14 +219,13 @@ pub struct Miner {
extra_data: RwLock<Bytes>,
engine: Arc<Engine>,

accounts: Option<Arc<AccountProvider>>,
work_poster: Option<WorkPoster>,
gas_pricer: Mutex<GasPricer>,
}

impl Miner {
/// Creates new instance of miner.
fn new_raw(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec, accounts: Option<Arc<AccountProvider>>) -> Miner {
fn new_raw(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec) -> Miner {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The change is conflicting with #3491 - I'm actually using AccountProvider in miner to identify local transactions :/

let work_poster = match options.new_work_notify.is_empty() {
true => None,
false => Some(WorkPoster::new(&options.new_work_notify))
Expand Down Expand Up @@ -261,26 +259,20 @@ impl Miner {
author: RwLock::new(Address::default()),
extra_data: RwLock::new(Vec::new()),
options: options,
accounts: accounts,
engine: spec.engine.clone(),
work_poster: work_poster,
gas_pricer: Mutex::new(gas_pricer),
}
}

/// Creates new instance of miner with accounts and with given spec.
pub fn with_spec_and_accounts(spec: &Spec, accounts: Option<Arc<AccountProvider>>) -> Miner {
Miner::new_raw(Default::default(), GasPricer::new_fixed(20_000_000_000u64.into()), spec, accounts)
}

/// Creates new instance of miner without accounts, but with given spec.
/// Creates new instance of miner with given spec.
pub fn with_spec(spec: &Spec) -> Miner {
Miner::new_raw(Default::default(), GasPricer::new_fixed(20_000_000_000u64.into()), spec, None)
Miner::new_raw(Default::default(), GasPricer::new_fixed(20_000_000_000u64.into()), spec)
}

/// Creates new instance of a miner Arc.
pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec, accounts: Option<Arc<AccountProvider>>) -> Arc<Miner> {
Arc::new(Miner::new_raw(options, gas_pricer, spec, accounts))
pub fn new(options: MinerOptions, gas_pricer: GasPricer, spec: &Spec) -> Arc<Miner> {
Arc::new(Miner::new_raw(options, gas_pricer, spec))
}

fn forced_sealing(&self) -> bool {
Expand Down Expand Up @@ -461,10 +453,7 @@ impl Miner {
/// Err(Some(block)) returns for unsuccesful sealing while Err(None) indicates misspecified engine.
fn seal_block_internally(&self, block: ClosedBlock) -> Result<SealedBlock, Option<ClosedBlock>> {
trace!(target: "miner", "seal_block_internally: block has transaction - attempting internal seal.");
let s = self.engine.generate_seal(block.block(), match self.accounts {
Some(ref x) => Some(&**x),
None => None,
});
let s = self.engine.generate_seal(block.block());
if let Some(seal) = s {
trace!(target: "miner", "seal_block_internally: managed internal seal. importing...");
block.lock().try_seal(&*self.engine, seal).or_else(|_| {
Expand Down Expand Up @@ -1170,7 +1159,6 @@ mod tests {
},
GasPricer::new_fixed(0u64.into()),
&Spec::new_test(),
None, // accounts provider
)).ok().expect("Miner was just created.")
}

Expand Down
8 changes: 4 additions & 4 deletions parity/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,8 @@ pub fn execute(cmd: RunCmd, logger: Arc<RotatingLogger>) -> Result<(), String> {
sync_config.fork_block = spec.fork_block();
sync_config.warp_sync = cmd.warp_sync;

// prepare account provider
let account_provider = Arc::new(try!(prepare_account_provider(&cmd.dirs, cmd.acc_conf)));

// create miner
let miner = Miner::new(cmd.miner_options, cmd.gas_pricer.into(), &spec, Some(account_provider.clone()));
let miner = Miner::new(cmd.miner_options, cmd.gas_pricer.into(), &spec);
miner.set_author(cmd.miner_extras.author);
miner.set_gas_floor_target(cmd.miner_extras.gas_floor_target);
miner.set_gas_ceil_target(cmd.miner_extras.gas_ceil_target);
Expand Down Expand Up @@ -241,6 +238,9 @@ pub fn execute(cmd: RunCmd, logger: Arc<RotatingLogger>) -> Result<(), String> {
// create supervisor
let mut hypervisor = modules::hypervisor(&cmd.dirs.ipc_path());

// prepare account provider
let account_provider = Arc::new(try!(prepare_account_provider(&cmd.dirs, cmd.acc_conf)));

// create client service.
let service = try!(ClientService::start(
client_config,
Expand Down
6 changes: 3 additions & 3 deletions rpc/src/v1/tests/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn sync_provider() -> Arc<TestSyncProvider> {
}))
}

fn miner_service(spec: &Spec, accounts: Arc<AccountProvider>) -> Arc<Miner> {
fn miner_service(spec: &Spec) -> Arc<Miner> {
Miner::new(
MinerOptions {
new_work_notify: vec![],
Expand All @@ -69,7 +69,6 @@ fn miner_service(spec: &Spec, accounts: Arc<AccountProvider>) -> Arc<Miner> {
},
GasPricer::new_fixed(20_000_000_000u64.into()),
&spec,
Some(accounts),
)
}

Expand Down Expand Up @@ -116,7 +115,8 @@ impl EthTester {
fn from_spec(spec: Spec) -> Self {
let dir = RandomTempPath::new();
let account_provider = account_provider();
let miner_service = miner_service(&spec, account_provider.clone());
spec.engine.register_account_provider(account_provider.clone());
let miner_service = miner_service(&spec);
let snapshot_service = snapshot_service();

let db_config = ::util::kvdb::DatabaseConfig::with_columns(::ethcore::db::NUM_COLUMNS);
Expand Down