Skip to content

Commit c161563

Browse files
sasa-tomicsa-github-api
sasa-tomic
and
sa-github-api
authored
feat(decentralization): Provide more details on the node replacement proposals (#816)
Co-authored-by: sa-github-api <138766536+sa-github-api@users.noreply.github.com>
1 parent 3752142 commit c161563

File tree

4 files changed

+128
-42
lines changed

4 files changed

+128
-42
lines changed

‎rs/cli/src/runner.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -729,11 +729,7 @@ pub fn replace_proposal_options(change: &SubnetChangeResponse) -> anyhow::Result
729729
Ok(ic_admin::ProposeOptions {
730730
title: format!("Replace {replace_target} in subnet {subnet_id_short}",).into(),
731731
summary: format!("# Replace {replace_target} in subnet {subnet_id_short}",).into(),
732-
motivation: Some(format!(
733-
"{}\n\n```\n{}\n```\n",
734-
change.motivation.as_ref().unwrap_or(&String::new()),
735-
change
736-
)),
732+
motivation: Some(format!("{}\n\n{}\n", change.motivation.as_ref().unwrap_or(&String::new()), change)),
737733
})
738734
}
739735

‎rs/decentralization/src/lib.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ impl Display for SubnetChangeResponse {
8080
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
8181
writeln!(
8282
f,
83-
"Decentralization Nakamoto coefficient changes for subnet {}:\n",
83+
"Decentralization Nakamoto coefficient changes for subnet `{}`:\n```",
8484
self.subnet_id.unwrap_or_default()
8585
)?;
8686
let before_individual = self.score_before.scores_individual();
@@ -105,14 +105,14 @@ impl Display for SubnetChangeResponse {
105105
let total_before = self.score_before.score_avg_linear();
106106
let total_after = self.score_after.score_avg_linear();
107107
let output = format!(
108-
"**Mean Nakamoto comparison:** {:.2} -> {:.2} ({:+.0}%)\n\nOverall replacement impact: {}",
108+
"\n**Mean Nakamoto comparison:** {:.2} -> {:.2} ({:+.0}%)\n\nOverall replacement impact: {}",
109109
total_before,
110110
total_after,
111111
((total_after - total_before) / total_before) * 100.,
112112
self.score_after.describe_difference_from(&self.score_before).1
113113
);
114114

115-
writeln!(f, "\n{}\n\n# Details\n\n", output)?;
115+
writeln!(f, "```\n{}\n\n# Details\n\n", output)?;
116116

117117
writeln!(f, "Nodes removed:")?;
118118
for (id, desc) in &self.removed_with_desc {
@@ -121,7 +121,7 @@ impl Display for SubnetChangeResponse {
121121
.get(id)
122122
.map(|h| h.to_string().to_lowercase())
123123
.unwrap_or("unknown".to_string());
124-
writeln!(f, " --> {} [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
124+
writeln!(f, "- `{}` [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
125125
}
126126
writeln!(f, "\nNodes added:")?;
127127
for (id, desc) in &self.added_with_desc {
@@ -130,7 +130,7 @@ impl Display for SubnetChangeResponse {
130130
.get(id)
131131
.map(|h| h.to_string().to_lowercase())
132132
.unwrap_or("unknown".to_string());
133-
writeln!(f, " ++> {} [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
133+
writeln!(f, "- `{}` [health: {}, impact on decentralization: {}]", id, health, desc).expect("write failed");
134134
}
135135

136136
let rows = self.feature_diff.values().map(|diff| diff.len()).max().unwrap_or(0);
@@ -166,10 +166,10 @@ impl Display for SubnetChangeResponse {
166166
}));
167167
}
168168

169-
writeln!(f, "\n\n{}", table)?;
169+
writeln!(f, "\n\n```\n{}```\n", table)?;
170170

171171
if let Some(comment) = &self.comment {
172-
writeln!(f, "*** Note ***\n{}", comment)?;
172+
writeln!(f, "### Business rules analysis\n{}", comment)?;
173173
}
174174

175175
Ok(())

‎rs/decentralization/src/nakamoto/mod.rs

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -348,11 +348,19 @@ impl NakamotoScore {
348348
if cmp != Some(Ordering::Equal) {
349349
return (
350350
cmp,
351-
format!(
352-
"the minimum Nakamoto coefficient across all features changes from {} to {}",
353-
other.score_min(),
354-
self.score_min()
355-
),
351+
if cmp == Some(Ordering::Less) {
352+
format!(
353+
"(gets worse) the minimum Nakamoto coefficient across all features decreases from {} to {}",
354+
other.score_min(),
355+
self.score_min()
356+
)
357+
} else {
358+
format!(
359+
"(gets better) the minimum Nakamoto coefficient across all features increases from {} to {}",
360+
other.score_min(),
361+
self.score_min()
362+
)
363+
},
356364
);
357365
}
358366

@@ -362,11 +370,19 @@ impl NakamotoScore {
362370
if cmp != Some(Ordering::Equal) {
363371
return (
364372
cmp,
365-
format!(
366-
"the average log2 of Nakamoto Coefficients across all features changes from {:.4} to {:.4}",
367-
other.score_avg_log2().unwrap_or(0.0),
368-
self.score_avg_log2().unwrap_or(0.0)
369-
),
373+
if cmp == Some(Ordering::Less) {
374+
format!(
375+
"(gets worse) the average log2 of Nakamoto Coefficients across all features decreases from {:.4} to {:.4}",
376+
other.score_avg_log2().unwrap_or(0.0),
377+
self.score_avg_log2().unwrap_or(0.0)
378+
)
379+
} else {
380+
format!(
381+
"(gets better) the average log2 of Nakamoto Coefficients across all features increases from {:.4} to {:.4}",
382+
other.score_avg_log2().unwrap_or(0.0),
383+
self.score_avg_log2().unwrap_or(0.0)
384+
)
385+
},
370386
);
371387
}
372388

@@ -380,13 +396,25 @@ impl NakamotoScore {
380396
return (
381397
cmp,
382398
if val_self[0] != val_other[0] {
399+
if val_self[0] > val_other[0] {
400+
format!(
401+
"(gets better) the number of nodes controlled by dominant NPs decreases from {} to {}",
402+
val_other[0], val_self[0]
403+
)
404+
} else {
405+
format!(
406+
"(gets worse) the number of nodes controlled by dominant NPs increases from {} to {}",
407+
val_other[0], val_self[0]
408+
)
409+
}
410+
} else if val_self[1] > val_other[1] {
383411
format!(
384-
"the number of nodes controlled by dominant NPs changes from {} to {}",
385-
val_other[0], val_self[0]
412+
"(gets better) the number of nodes controlled by dominant Country actors decreases from {} to {}",
413+
val_other[1], val_self[1]
386414
)
387415
} else {
388416
format!(
389-
"the number of nodes controlled by dominant Country actors changes from {} to {}",
417+
"(gets worse) the number of nodes controlled by dominant Country actors increases from {} to {}",
390418
val_other[1], val_self[1]
391419
)
392420
},
@@ -405,9 +433,27 @@ impl NakamotoScore {
405433
return (
406434
cmp,
407435
if val_self[0] != val_other[0] {
408-
format!("the number of different NP actors changes from {} to {}", val_other[0], val_self[0])
436+
if val_other[0] < val_self[0] {
437+
format!(
438+
"(gets better) the number of different NP actors increases from {} to {}",
439+
val_other[0], val_self[0]
440+
)
441+
} else {
442+
format!(
443+
"(gets worse) the number of different NP actors decreases from {} to {}",
444+
val_other[0], val_self[0]
445+
)
446+
}
447+
} else if val_other[1] < val_self[1] {
448+
format!(
449+
"(gets better) the number of different Country actors increases from {} to {}",
450+
val_other[1], val_self[1]
451+
)
409452
} else {
410-
format!("the number of different Country actors changes from {} to {}", val_other[1], val_self[1])
453+
format!(
454+
"(gets worse) the number of different Country actors decreases from {} to {}",
455+
val_other[1], val_self[1]
456+
)
411457
},
412458
);
413459
}
@@ -421,10 +467,17 @@ impl NakamotoScore {
421467
if cmp != Some(Ordering::Equal) {
422468
return (
423469
cmp,
424-
format!(
425-
"the number of Nakamoto coefficients with extremely low values changes from {} to {}",
426-
c2, c1
427-
),
470+
if cmp == Some(Ordering::Less) {
471+
format!(
472+
"(gets better) the number of Nakamoto coefficients with extremely low values decreases from {} to {}",
473+
c2, c1
474+
)
475+
} else {
476+
format!(
477+
"(gets worse) the number of Nakamoto coefficients with extremely low values increases from {} to {}",
478+
c2, c1
479+
)
480+
},
428481
);
429482
}
430483

@@ -491,12 +544,12 @@ impl Eq for NakamotoScore {}
491544
impl Display for NakamotoScore {
492545
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
493546
let avg_log2_str = match self.avg_log2 {
494-
Some(v) => format!("{:0.2}", v),
547+
Some(v) => format!("{:0.4}", v),
495548
None => "undefined".to_string(),
496549
};
497550
write!(
498551
f,
499-
"NakamotoScore: min {:0.2} avg log2 {} #crit nodes {:?} # crit uniq {:?} #crit coeff {} avg linear {:0.2}",
552+
"NakamotoScore: min {:0.2} avg log2 {} #crit nodes {:?} # crit uniq {:?} #crit coeff {} avg linear {:0.4}",
500553
self.min,
501554
avg_log2_str,
502555
self.critical_features_num_nodes(),

‎rs/decentralization/src/network.rs

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -631,18 +631,26 @@ impl DecentralizedSubnet {
631631
best_result
632632
.business_rules_log
633633
.iter()
634-
.map(|s| format!("node {}/{} ({}): {}", i + 1, how_many_nodes, best_result.node.id, s))
634+
.map(|s| {
635+
format!(
636+
"- adding node {} of {} ({}): {}",
637+
i + 1,
638+
how_many_nodes,
639+
best_result.node.id.to_string().split('-').next().unwrap_or_default(),
640+
s
641+
)
642+
})
635643
.collect::<Vec<String>>(),
636644
);
637645
if i + 1 == how_many_nodes {
638646
if total_penalty != 0 {
639647
comment = Some(format!(
640-
"Subnet extension with {} nodes finished with the total penalty {}. Penalty causes throughout the extension:\n{}\n\n{}",
648+
"Subnet extension with {} nodes finished with the total penalty {}. Penalty causes throughout the extension:\n\n{}\n\n{}",
641649
how_many_nodes,
642650
total_penalty,
643651
business_rules_log.join("\n"),
644652
if how_many_nodes > 1 {
645-
"Note that the penalty for nodes before the last node may not be relevant in the end. We leave this to humans to assess."
653+
"Business rules analysis is calculated on each operation. Typically only the last operation is relevant, although this may depend on the case."
646654
} else { "" }
647655
));
648656
} else {
@@ -714,18 +722,26 @@ impl DecentralizedSubnet {
714722
best_result
715723
.business_rules_log
716724
.iter()
717-
.map(|s| format!("node {}/{} ({}): {}", i + 1, how_many_nodes, best_result.node.id, s))
725+
.map(|s| {
726+
format!(
727+
"- removing node {} of {} ({}): {}",
728+
i + 1,
729+
how_many_nodes,
730+
best_result.node.id.to_string().split('-').next().unwrap_or_default(),
731+
s
732+
)
733+
})
718734
.collect::<Vec<String>>(),
719735
);
720736
if i + 1 == how_many_nodes {
721737
if total_penalty != 0 {
722738
comment = Some(format!(
723-
"Subnet removal of {} nodes finished with the total penalty {}. Penalty causes throughout the removal:\n{}\n\n{}",
739+
"Subnet removal of {} nodes finished with the total penalty {}. Penalty causes throughout the removal:\n\n{}\n\n{}",
724740
how_many_nodes,
725741
total_penalty,
726742
business_rules_log.join("\n"),
727743
if how_many_nodes > 1 {
728-
"Note that the penalty for nodes before the last node may not be relevant in the end. We leave this to humans to assess."
744+
"Business rules analysis is calculated on each operation. Typically only the last operation is relevant, although this may depend on the case."
729745
} else {
730746
""
731747
}
@@ -1332,19 +1348,40 @@ impl NetworkHealRequest {
13321348
"- {} additional node{}: {}",
13331349
num_opt,
13341350
if num_opt > 1 { "s" } else { "" },
1335-
change.score_after.describe_difference_from(&changes[num_opt - 1].score_after).1
1351+
change
1352+
.score_after
1353+
.describe_difference_from(&changes[num_opt.saturating_sub(1)].score_after)
1354+
.1
13361355
)
13371356
})
13381357
.collect::<Vec<_>>();
1358+
info!("Max score: {}", changes_max_score.score_after);
13391359

13401360
let change = changes
13411361
.iter()
1342-
.find(|change| change.score_after == changes_max_score.score_after)
1362+
.find(|change: &&SubnetChangeResponse| change.score_after == changes_max_score.score_after)
13431363
.expect("No suitable changes found");
13441364

1365+
info!(
1366+
"Replacing {} nodes in subnet {} gives Nakamoto coefficient: {}\n",
1367+
change.removed_with_desc.len(),
1368+
subnet.decentralized_subnet.id,
1369+
change.score_after
1370+
);
1371+
13451372
let num_opt = change.removed_with_desc.len() - unhealthy_nodes_len;
13461373
let reason_additional_optimizations = if num_opt == 0 {
1347-
"\n\nNot replacing any additional nodes to improve optimization.\n\n".to_string()
1374+
format!(
1375+
"
1376+
1377+
Calculated impact on subnet decentralization if replacing:
1378+
1379+
{}
1380+
1381+
Based on the calculated impact, not replacing additional nodes to improve optimization.
1382+
",
1383+
optimizations_desc.join("\n")
1384+
)
13481385
} else {
13491386
format!("
13501387

0 commit comments

Comments
 (0)