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

Commit

Permalink
Merge branch 'master' into refactor/hashdb-generic
Browse files Browse the repository at this point in the history
* master:
  ethstore: retry deduplication of wallet file names until success (#8910)
  Update ropsten.json (#8926)
  Include node identity in the P2P advertised client version. (#8830)
  Allow disabling local-by-default for transactions with new config entry (#8882)
  Allow Poll Lifetime to be configured via CLI (#8885)
  • Loading branch information
dvdplm committed Jun 20, 2018
2 parents b48e2c8 + cf5ae81 commit c366b2a
Show file tree
Hide file tree
Showing 16 changed files with 184 additions and 27 deletions.
4 changes: 2 additions & 2 deletions ethcore/res/ethereum/ropsten.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
"maximumExtraDataSize": "0x20",
"minGasLimit": "0x1388",
"networkID" : "0x3",
"forkBlock": 641350,
"forkCanonHash": "0x8033403e9fe5811a7b6d6b469905915de1c59207ce2172cbcf5d6ff14fa6a2eb",
"forkBlock": 3383558,
"forkCanonHash": "0x6b4b80d65951375a70bc1ecf9a270d152dd355454d57869abbae2e42c213e0f3",
"maxCodeSize": 24576,
"maxCodeSizeTransition": 10,
"eip150Transition": 0,
Expand Down
80 changes: 78 additions & 2 deletions ethcore/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ pub struct MinerOptions {
pub tx_queue_strategy: PrioritizationStrategy,
/// Simple senders penalization.
pub tx_queue_penalization: Penalization,
/// Do we want to mark transactions recieved locally (e.g. RPC) as local if we don't have the sending account?
pub tx_queue_no_unfamiliar_locals: bool,
/// Do we refuse to accept service transactions even if sender is certified.
pub refuse_service_transactions: bool,
/// Transaction pool limits.
Expand All @@ -148,6 +150,7 @@ impl Default for MinerOptions {
infinite_pending_block: false,
tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,
tx_queue_penalization: Penalization::Disabled,
tx_queue_no_unfamiliar_locals: false,
refuse_service_transactions: false,
pool_limits: pool::Options {
max_count: 8_192,
Expand Down Expand Up @@ -794,8 +797,9 @@ impl miner::MinerService for Miner {
fn import_own_transaction<C: miner::BlockChainClient>(
&self,
chain: &C,
pending: PendingTransaction,
pending: PendingTransaction
) -> Result<(), transaction::Error> {
// note: you may want to use `import_claimed_local_transaction` instead of this one.

trace!(target: "own_tx", "Importing transaction: {:?}", pending);

Expand All @@ -816,6 +820,28 @@ impl miner::MinerService for Miner {
imported
}

fn import_claimed_local_transaction<C: miner::BlockChainClient>(
&self,
chain: &C,
pending: PendingTransaction,
trusted: bool
) -> Result<(), transaction::Error> {
// treat the tx as local if the option is enabled, or if we have the account
let sender = pending.sender();
let treat_as_local = trusted
|| !self.options.tx_queue_no_unfamiliar_locals
|| self.accounts.as_ref().map(|accts| accts.has_account(sender)).unwrap_or(false);

if treat_as_local {
self.import_own_transaction(chain, pending)
} else {
// We want to replicate behaviour for external transactions if we're not going to treat
// this as local. This is important with regards to sealing blocks
self.import_external_transactions(chain, vec![pending.transaction.into()])
.pop().expect("one result per tx, as in `import_own_transaction`")
}
}

fn local_transactions(&self) -> BTreeMap<H256, pool::local_transactions::Status> {
self.transaction_queue.local_transactions()
}
Expand Down Expand Up @@ -1161,6 +1187,7 @@ mod tests {
infinite_pending_block: false,
tx_queue_penalization: Penalization::Disabled,
tx_queue_strategy: PrioritizationStrategy::GasPriceOnly,
tx_queue_no_unfamiliar_locals: false,
refuse_service_transactions: false,
pool_limits: Default::default(),
pool_verification_options: pool::verifier::Options {
Expand All @@ -1175,8 +1202,10 @@ mod tests {
)
}

const TEST_CHAIN_ID: u64 = 2;

fn transaction() -> SignedTransaction {
transaction_with_chain_id(2)
transaction_with_chain_id(TEST_CHAIN_ID)
}

fn transaction_with_chain_id(chain_id: u64) -> SignedTransaction {
Expand Down Expand Up @@ -1250,6 +1279,53 @@ mod tests {
assert_eq!(miner.ready_transactions(&client, 10, PendingOrdering::Priority).len(), 1);
}

#[test]
fn should_treat_unfamiliar_locals_selectively() {
// given
let keypair = Random.generate().unwrap();
let client = TestBlockChainClient::default();
let account_provider = AccountProvider::transient_provider();
account_provider.insert_account(keypair.secret().clone(), "").expect("can add accounts to the provider we just created");

let miner = Miner::new(
MinerOptions {
tx_queue_no_unfamiliar_locals: true,
..miner().options
},
GasPricer::new_fixed(0u64.into()),
&Spec::new_test(),
Some(Arc::new(account_provider)),
);
let transaction = transaction();
let best_block = 0;
// when
// This transaction should not be marked as local because our account_provider doesn't have the sender
let res = miner.import_claimed_local_transaction(&client, PendingTransaction::new(transaction.clone(), None), false);

// then
// Check the same conditions as `should_import_external_transaction` first. Behaviour should be identical.
// That is: it's treated as though we added it through `import_external_transactions`
assert_eq!(res.unwrap(), ());
assert_eq!(miner.pending_transactions(best_block), None);
assert_eq!(miner.pending_receipts(best_block), None);
assert_eq!(miner.ready_transactions(&client, 10, PendingOrdering::Priority).len(), 0);
assert!(miner.prepare_pending_block(&client));
assert_eq!(miner.ready_transactions(&client, 10, PendingOrdering::Priority).len(), 1);

// when - 2nd part: create a local transaction from account_provider.
// Borrow the transaction used before & sign with our generated keypair.
let local_transaction = transaction.deconstruct().0.as_unsigned().clone().sign(keypair.secret(), Some(TEST_CHAIN_ID));
let res2 = miner.import_claimed_local_transaction(&client, PendingTransaction::new(local_transaction, None), false);

// then - 2nd part: we add on the results from the last pending block.
// This is borrowed from `should_make_pending_block_when_importing_own_transaction` and slightly modified.
assert_eq!(res2.unwrap(), ());
assert_eq!(miner.pending_transactions(best_block).unwrap().len(), 2);
assert_eq!(miner.pending_receipts(best_block).unwrap().len(), 2);
assert_eq!(miner.ready_transactions(&client, 10, PendingOrdering::Priority).len(), 2);
assert!(!miner.prepare_pending_block(&client));
}

#[test]
fn should_not_seal_unless_enabled() {
let miner = miner();
Expand Down
6 changes: 6 additions & 0 deletions ethcore/src/miner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ pub trait MinerService : Send + Sync {
-> Result<(), transaction::Error>
where C: BlockChainClient;

/// Imports transactions from potentially external sources, with behaviour determined
/// by the config flag `tx_queue_allow_unfamiliar_locals`
fn import_claimed_local_transaction<C>(&self, chain: &C, transaction: PendingTransaction, trusted: bool)
-> Result<(), transaction::Error>
where C: BlockChainClient;

/// Removes transaction from the pool.
///
/// Attempts to "cancel" a transaction. If it was not propagated yet (or not accepted by other peers)
Expand Down
19 changes: 16 additions & 3 deletions ethstore/src/accounts_dir/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,22 @@ impl<T> DiskDirectory<T> where T: KeyFileManager {

// check for duplicate filename and append random suffix
if dedup && keyfile_path.exists() {
let suffix = ::random::random_string(4);
filename.push_str(&format!("-{}", suffix));
keyfile_path.set_file_name(&filename);
const MAX_RETRIES: usize = 500;
let mut retries = 0;
let mut deduped_filename = filename.clone();

while keyfile_path.exists() {
if retries >= MAX_RETRIES {
return Err(Error::Custom(format!("Exceeded maximum retries when deduplicating account filename.")));
}

let suffix = ::random::random_string(4);
deduped_filename = format!("{}-{}", filename, suffix);
keyfile_path.set_file_name(&deduped_filename);
retries += 1;
}

filename = deduped_filename;
}

// update account filename
Expand Down
14 changes: 14 additions & 0 deletions parity/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,10 @@ usage! {
"--remove-solved",
"Move solved blocks from the work package queue instead of cloning them. This gives a slightly faster import speed, but means that extra solutions submitted for the same work package will go unused.",

FLAG flag_tx_queue_no_unfamiliar_locals: (bool) = false, or |c: &Config| c.mining.as_ref()?.tx_queue_no_unfamiliar_locals.clone(),
"--tx-queue-no-unfamiliar-locals",
"Transactions recieved via local means (RPC, WS, etc) will be treated as external if the sending account is unknown.",

FLAG flag_refuse_service_transactions: (bool) = false, or |c: &Config| c.mining.as_ref()?.refuse_service_transactions.clone(),
"--refuse-service-transactions",
"Always refuse service transactions.",
Expand Down Expand Up @@ -732,6 +736,10 @@ usage! {
"--gas-price-percentile=[PCT]",
"Set PCT percentile gas price value from last 100 blocks as default gas price when sending transactions.",

ARG arg_poll_lifetime: (u32) = 60u32, or |c: &Config| c.mining.as_ref()?.poll_lifetime.clone(),
"--poll-lifetime=[S]",
"Set the lifetime of the internal index filter to S seconds.",

ARG arg_author: (Option<String>) = None, or |c: &Config| c.mining.as_ref()?.author.clone(),
"--author=[ADDRESS]",
"Specify the block author (aka \"coinbase\") address for sending block rewards from sealed blocks. NOTE: MINING WILL NOT WORK WITHOUT THIS OPTION.", // Sealing/Mining Option
Expand Down Expand Up @@ -1241,6 +1249,7 @@ struct Mining {
relay_set: Option<String>,
min_gas_price: Option<u64>,
gas_price_percentile: Option<usize>,
poll_lifetime: Option<u32>,
usd_per_tx: Option<String>,
usd_per_eth: Option<String>,
price_update_period: Option<String>,
Expand All @@ -1254,6 +1263,7 @@ struct Mining {
tx_queue_strategy: Option<String>,
tx_queue_ban_count: Option<u16>,
tx_queue_ban_time: Option<u16>,
tx_queue_no_unfamiliar_locals: Option<bool>,
remove_solved: Option<bool>,
notify_work: Option<Vec<String>>,
refuse_service_transactions: Option<bool>,
Expand Down Expand Up @@ -1664,11 +1674,13 @@ mod tests {
arg_min_gas_price: Some(0u64),
arg_usd_per_tx: "0.0001".into(),
arg_gas_price_percentile: 50usize,
arg_poll_lifetime: 60u32,
arg_usd_per_eth: "auto".into(),
arg_price_update_period: "hourly".into(),
arg_gas_floor_target: "4700000".into(),
arg_gas_cap: "6283184".into(),
arg_extra_data: Some("Parity".into()),
flag_tx_queue_no_unfamiliar_locals: false,
arg_tx_queue_size: 8192usize,
arg_tx_queue_per_sender: None,
arg_tx_queue_mem_limit: 4u32,
Expand Down Expand Up @@ -1924,6 +1936,7 @@ mod tests {
relay_set: None,
min_gas_price: None,
gas_price_percentile: None,
poll_lifetime: None,
usd_per_tx: None,
usd_per_eth: None,
price_update_period: Some("hourly".into()),
Expand All @@ -1936,6 +1949,7 @@ mod tests {
tx_queue_strategy: None,
tx_queue_ban_count: None,
tx_queue_ban_time: None,
tx_queue_no_unfamiliar_locals: None,
tx_gas_limit: None,
tx_time_limit: None,
extra_data: None,
Expand Down
1 change: 1 addition & 0 deletions parity/cli/tests/config.full.toml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ tx_queue_ban_count = 1
tx_queue_ban_time = 180 #s
tx_gas_limit = "6283184"
tx_time_limit = 100 #ms
tx_queue_no_unfamiliar_locals = false
extra_data = "Parity"
remove_solved = false
notify_work = ["http://localhost:3001"]
Expand Down
26 changes: 25 additions & 1 deletion parity/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ impl Configuration {
logger_config: logger_config.clone(),
miner_options: self.miner_options()?,
gas_price_percentile: self.args.arg_gas_price_percentile,
poll_lifetime: self.args.arg_poll_lifetime,
ntp_servers: self.ntp_servers(),
ws_conf: ws_conf,
http_conf: http_conf,
Expand Down Expand Up @@ -549,6 +550,7 @@ impl Configuration {

tx_queue_penalization: to_queue_penalization(self.args.arg_tx_time_limit)?,
tx_queue_strategy: to_queue_strategy(&self.args.arg_tx_queue_strategy)?,
tx_queue_no_unfamiliar_locals: self.args.flag_tx_queue_no_unfamiliar_locals,
refuse_service_transactions: self.args.flag_refuse_service_transactions,

pool_limits: self.pool_limits()?,
Expand Down Expand Up @@ -754,7 +756,15 @@ impl Configuration {
ret.config_path = Some(net_path.to_str().unwrap().to_owned());
ret.reserved_nodes = self.init_reserved_nodes()?;
ret.allow_non_reserved = !self.args.flag_reserved_only;
ret.client_version = version();
ret.client_version = {
let mut client_version = version();
if !self.args.arg_identity.is_empty() {
// Insert name after the "Parity/" at the beginning of version string.
let idx = client_version.find('/').unwrap_or(client_version.len());
client_version.insert_str(idx, &format!("/{}", self.args.arg_identity));
}
client_version
};
Ok(ret)
}

Expand Down Expand Up @@ -1398,6 +1408,7 @@ mod tests {
logger_config: Default::default(),
miner_options: Default::default(),
gas_price_percentile: 50,
poll_lifetime: 60,
ntp_servers: vec![
"0.parity.pool.ntp.org:123".into(),
"1.parity.pool.ntp.org:123".into(),
Expand Down Expand Up @@ -1787,6 +1798,19 @@ mod tests {
}
}

#[test]
fn test_identity_arg() {
let args = vec!["parity", "--identity", "Somebody"];
let conf = Configuration::parse_cli(&args).unwrap();
match conf.into_command().unwrap().cmd {
Cmd::Run(c) => {
assert_eq!(c.name, "Somebody");
assert!(c.net_conf.client_version.starts_with("Parity/Somebody/"));
}
_ => panic!("Should be Cmd::Run"),
}
}

#[test]
fn should_apply_ports_shift() {
// given
Expand Down
6 changes: 5 additions & 1 deletion parity/rpc_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ pub struct FullDependencies {
pub remote: parity_reactor::Remote,
pub whisper_rpc: Option<::whisper::RpcFactory>,
pub gas_price_percentile: usize,
pub poll_lifetime: u32,
}

impl FullDependencies {
Expand Down Expand Up @@ -288,12 +289,13 @@ impl FullDependencies {
allow_pending_receipt_query: !self.geth_compatibility,
send_block_number_in_get_work: !self.geth_compatibility,
gas_price_percentile: self.gas_price_percentile,
poll_lifetime: self.poll_lifetime
}
);
handler.extend_with(client.to_delegate());

if !for_generic_pubsub {
let filter_client = EthFilterClient::new(self.client.clone(), self.miner.clone());
let filter_client = EthFilterClient::new(self.client.clone(), self.miner.clone(), self.poll_lifetime);
handler.extend_with(filter_client.to_delegate());

add_signing_methods!(EthSigning, handler, self, nonces.clone());
Expand Down Expand Up @@ -447,6 +449,7 @@ pub struct LightDependencies<T> {
pub whisper_rpc: Option<::whisper::RpcFactory>,
pub private_tx_service: Option<Arc<PrivateTransactionManager>>,
pub gas_price_percentile: usize,
pub poll_lifetime: u32,
}

impl<C: LightChainClient + 'static> LightDependencies<C> {
Expand Down Expand Up @@ -504,6 +507,7 @@ impl<C: LightChainClient + 'static> LightDependencies<C> {
self.secret_store.clone(),
self.cache.clone(),
self.gas_price_percentile,
self.poll_lifetime,
);
handler.extend_with(Eth::to_delegate(client.clone()));

Expand Down
3 changes: 3 additions & 0 deletions parity/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub struct RunCmd {
pub logger_config: LogConfig,
pub miner_options: MinerOptions,
pub gas_price_percentile: usize,
pub poll_lifetime: u32,
pub ntp_servers: Vec<String>,
pub ws_conf: rpc::WsConfiguration,
pub http_conf: rpc::HttpConfiguration,
Expand Down Expand Up @@ -350,6 +351,7 @@ fn execute_light_impl(cmd: RunCmd, logger: Arc<RotatingLogger>) -> Result<Runnin
whisper_rpc: whisper_factory,
private_tx_service: None, //TODO: add this to client.
gas_price_percentile: cmd.gas_price_percentile,
poll_lifetime: cmd.poll_lifetime
});

let dependencies = rpc::Dependencies {
Expand Down Expand Up @@ -779,6 +781,7 @@ fn execute_impl<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq:
whisper_rpc: whisper_factory,
private_tx_service: Some(private_tx_service.clone()),
gas_price_percentile: cmd.gas_price_percentile,
poll_lifetime: cmd.poll_lifetime,
});

let dependencies = rpc::Dependencies {
Expand Down
Loading

0 comments on commit c366b2a

Please sign in to comment.