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.

Sign up for GitHub

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

Reduce number of unwarps in servers crate #2707

Merged
merged 4 commits into from
Mar 31, 2019
Merged

Reduce number of unwarps in servers crate #2707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18b86d7
Reduce number of unwarps in servers crate
hashmap Mar 25, 2019
ac64a0a
Use serde default in dandelion config
hashmap Mar 27, 2019
cf10f02
Revert "Use serde default in dandelion config"
hashmap Mar 27, 2019
b97dc8f
rename macro
hashmap Mar 31, 2019
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
50 changes: 16 additions & 34 deletions pool/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,19 @@ const DANDELION_STEM_PROBABILITY: usize = 90;
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct DandelionConfig {
/// Choose new Dandelion relay peer every n secs.
#[serde = "default_dandelion_relay_secs"]
pub relay_secs: Option<u64>,
#[serde(default = "default_dandelion_relay_secs")]
Copy link
Member

Choose a reason for hiding this comment

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

This is going to cause all kinds of conflicts with 1.1.0 branch.
It might be worth not touching the config here and just doing it on the 1.1.0 branch.
We fixed the "default" serde config like you do here, but we did not go the additional step and get rid of the options (which I agree is 👍).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antiochp ok, I've reverted this commit

Copy link
Member

Choose a reason for hiding this comment

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

I suspect #[serde = "foo"] vs. #[serde(default = "foo")] has bitten us and continues to bite us all over our config parsing code.

pub relay_secs: u64,
/// Dandelion embargo, fluff and broadcast tx if not seen on network before
/// embargo expires.
#[serde = "default_dandelion_embargo_secs"]
pub embargo_secs: Option<u64>,
#[serde(default = "default_dandelion_embargo_secs")]
pub embargo_secs: u64,
/// Dandelion patience timer, fluff/stem processing runs every n secs.
/// Tx aggregation happens on stem txs received within this window.
#[serde = "default_dandelion_patience_secs"]
pub patience_secs: Option<u64>,
#[serde(default = "default_dandelion_patience_secs")]
pub patience_secs: u64,
/// Dandelion stem probability (stem 90% of the time, fluff 10% etc.)
#[serde = "default_dandelion_stem_probability"]
pub stem_probability: Option<usize>,
}

impl DandelionConfig {
pub fn relay_secs(&self) -> u64 {
self.relay_secs.unwrap_or(DANDELION_RELAY_SECS)
}

pub fn embargo_secs(&self) -> u64 {
self.embargo_secs.unwrap_or(DANDELION_EMBARGO_SECS)
}

pub fn patience_secs(&self) -> u64 {
self.patience_secs.unwrap_or(DANDELION_PATIENCE_SECS)
}

pub fn stem_probability(&self) -> usize {
self.stem_probability.unwrap_or(DANDELION_STEM_PROBABILITY)
}
#[serde(default = "default_dandelion_stem_probability")]
pub stem_probability: usize,
}

impl Default for DandelionConfig {
Expand All @@ -88,20 +70,20 @@ impl Default for DandelionConfig {
}
}

fn default_dandelion_relay_secs() -> Option<u64> {
Some(DANDELION_RELAY_SECS)
fn default_dandelion_relay_secs() -> u64 {
DANDELION_RELAY_SECS
}

fn default_dandelion_embargo_secs() -> Option<u64> {
Some(DANDELION_EMBARGO_SECS)
fn default_dandelion_embargo_secs() -> u64 {
DANDELION_EMBARGO_SECS
}

fn default_dandelion_patience_secs() -> Option<u64> {
Some(DANDELION_PATIENCE_SECS)
fn default_dandelion_patience_secs() -> u64 {
DANDELION_PATIENCE_SECS
}

fn default_dandelion_stem_probability() -> Option<usize> {
Some(DANDELION_STEM_PROBABILITY)
fn default_dandelion_stem_probability() -> usize {
DANDELION_STEM_PROBABILITY
}

/// Transaction pool configuration
Expand Down
6 changes: 3 additions & 3 deletions servers/src/grin/dandelion_monitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn monitor_transactions(
}

// This is the patience timer, we loop every n secs.
let patience_secs = dandelion_config.patience_secs();
let patience_secs = dandelion_config.patience_secs;
thread::sleep(Duration::from_secs(patience_secs));

// Step 1: find all "ToStem" entries in stempool from last run.
Expand Down Expand Up @@ -199,7 +199,7 @@ fn process_fresh_entries(

for x in &mut fresh_entries.iter_mut() {
let random = rng.gen_range(0, 101);
if random <= dandelion_config.stem_probability() {
if random <= dandelion_config.stem_probability {
x.state = PoolEntryState::ToStem;
} else {
x.state = PoolEntryState::ToFluff;
Expand All @@ -214,7 +214,7 @@ fn process_expired_entries(
tx_pool: Arc<RwLock<TransactionPool>>,
) -> Result<(), PoolError> {
let now = Utc::now().timestamp();
let embargo_sec = dandelion_config.embargo_secs() + thread_rng().gen_range(0, 31);
let embargo_sec = dandelion_config.embargo_secs + thread_rng().gen_range(0, 31);
let cutoff = now - embargo_sec as i64;

let mut expired_entries = vec![];
Expand Down
2 changes: 1 addition & 1 deletion servers/src/grin/seed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ fn update_dandelion_relay(peers: Arc<p2p::Peers>, dandelion_config: DandelionCon
let dandelion_relay = peers.get_dandelion_relay();
if let Some((last_added, _)) = dandelion_relay {
let dandelion_interval = Utc::now().timestamp() - last_added;
if dandelion_interval >= dandelion_config.relay_secs() as i64 {
if dandelion_interval >= dandelion_config.relay_secs as i64 {
debug!("monitor_peers: updating expired dandelion relay");
peers.update_dandelion_relay();
}
Expand Down
18 changes: 9 additions & 9 deletions servers/tests/simulnet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,9 +924,9 @@ fn replicate_tx_fluff_failure() {
// Server 1 (mines into wallet 1)
let mut s1_config = framework::config(3000, "tx_fluff", 3000);
s1_config.test_miner_wallet_url = Some("http://127.0.0.1:33000".to_owned());
s1_config.dandelion_config.embargo_secs = Some(10);
s1_config.dandelion_config.patience_secs = Some(1);
s1_config.dandelion_config.relay_secs = Some(1);
s1_config.dandelion_config.embargo_secs = 10;
s1_config.dandelion_config.patience_secs = 1;
s1_config.dandelion_config.relay_secs = 1;
let s1 = servers::Server::new(s1_config.clone()).unwrap();
// Mine off of server 1
s1.start_test_miner(s1_config.test_miner_wallet_url, s1.stop_state.clone());
Expand All @@ -937,9 +937,9 @@ fn replicate_tx_fluff_failure() {
s2_config.p2p_config.seeds = Some(vec![PeerAddr(
"127.0.0.1:13000".to_owned().parse().unwrap(),
)]);
s2_config.dandelion_config.embargo_secs = Some(10);
s2_config.dandelion_config.patience_secs = Some(1);
s2_config.dandelion_config.relay_secs = Some(1);
s2_config.dandelion_config.embargo_secs = 10;
s2_config.dandelion_config.patience_secs = 1;
s2_config.dandelion_config.relay_secs = 1;
let _s2 = servers::Server::new(s2_config.clone()).unwrap();

let dl_nodes = 5;
Expand All @@ -950,9 +950,9 @@ fn replicate_tx_fluff_failure() {
s_config.p2p_config.seeds = Some(vec![PeerAddr(
"127.0.0.1:13000".to_owned().parse().unwrap(),
)]);
s_config.dandelion_config.embargo_secs = Some(10);
s_config.dandelion_config.patience_secs = Some(1);
s_config.dandelion_config.relay_secs = Some(1);
s_config.dandelion_config.embargo_secs = 10;
s_config.dandelion_config.patience_secs = 1;
s_config.dandelion_config.relay_secs = 1;
let _ = servers::Server::new(s_config.clone()).unwrap();
}

Expand Down