Skip to content

GH-711: review three #710

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

Open
wants to merge 11 commits into
base: GH-711-review-two-initial-state
Choose a base branch
from
Open

GH-711: review three #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6c8b103
GH-711: knocking off commented findings; interim commit
Mar 20, 2025
1239a15
GH-711: fixing percentage.rs; yet more to go with
Mar 22, 2025
b7c7900
GH-711: percentage.rs finished
Mar 23, 2025
4fc1a1a
GH-711: some progress in verify_bill_payment
Mar 27, 2025
8dd256d
GH-711: a few more fixes in verify_bill_payments.rs
Apr 7, 2025
d306718
GH-711: a few more shovels
Apr 8, 2025
e43f655
GH-711: interim commit
Sep 24, 2025
86b5a9a
GH-711: fixes in the non-unit test
Sep 24, 2025
baf2745
GH-711: interim commit
Sep 25, 2025
3499edd
GH-711: interim commit
Sep 25, 2025
2e150b4
GH-711: review two finished
Sep 25, 2025
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
605 changes: 350 additions & 255 deletions masq_lib/src/percentage.rs
View file
Open in desktop

Large diffs are not rendered by default.

41 changes: 22 additions & 19 deletions multinode_integration_tests/tests/verify_bill_payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::verify_bill_payment_utils::utils::{
test_body, to_wei, AssertionsValues, Debt, DebtsSpecs, FinalServiceFeeBalancesByServingNodes,
NodeProfile, Ports, TestInputsBuilder, WholesomeConfig,
NodeProfile, TestInputBuilder, UiPorts, WholesomeConfig,
};
use itertools::Itertools;
use masq_lib::blockchains::chains::Chain;
Expand All @@ -23,7 +23,7 @@ use std::u128;
mod verify_bill_payment_utils;

#[test]
fn full_payments_were_processed_for_sufficient_balances() {
fn payments_processed_fully_as_balances_were_sufficient() {
// Note: besides the main objectives of this test, it relies on (and so it proves) the premise
// that each Node, after it achieves an effective connectivity as making a route is enabled,
// activates the accountancy module whereas the first cycle of scanners is unleashed. That's
Expand All @@ -46,7 +46,7 @@ fn full_payments_were_processed_for_sufficient_balances() {
let owed_to_serving_node_2_minor = debt_threshold_wei + 456_789;
let owed_to_serving_node_3_minor = debt_threshold_wei + 789_012;
let consuming_node_initial_service_fee_balance_minor = debt_threshold_wei * 4;
let test_inputs = TestInputsBuilder::default()
let test_input = TestInputBuilder::default()
.consuming_node_initial_service_fee_balance_minor(
consuming_node_initial_service_fee_balance_minor,
)
Expand Down Expand Up @@ -75,7 +75,7 @@ fn full_payments_were_processed_for_sufficient_balances() {
};

test_body(
test_inputs,
test_input,
assertions_values,
stimulate_consuming_node_to_pay_for_test_with_sufficient_funds,
activating_serving_nodes_for_test_with_sufficient_funds,
Expand All @@ -87,7 +87,8 @@ fn stimulate_consuming_node_to_pay_for_test_with_sufficient_funds(
real_consuming_node: &MASQRealNode,
_wholesome_config: &WholesomeConfig,
) {
for _ in 0..6 {
// 1 + 4 Nodes should be enough to compose a route, right?
for _ in 0..4 {
cluster.start_real_node(
NodeStartupConfigBuilder::standard()
.chain(Chain::Dev)
Expand Down Expand Up @@ -145,7 +146,8 @@ fn set_old_debts(
.into_iter()
.map(|balance_minor| Debt::new(balance_minor, quite_long_ago))
.collect_vec();
DebtsSpecs::new(debts[0], debts[1], debts[2])
let debt_array = debts.try_into().unwrap();
DebtsSpecs::new(debt_array)
}

#[test]
Expand All @@ -161,14 +163,15 @@ fn payments_were_adjusted_due_to_insufficient_balances() {
// Assuming all Nodes rely on the same set of payment thresholds
let owed_to_serv_node_1_minor = to_wei(payment_thresholds.debt_threshold_gwei + 5_000_000);
let owed_to_serv_node_2_minor = to_wei(payment_thresholds.debt_threshold_gwei + 20_000_000);
// Account of Node 3 will be a victim of tx fee insufficiency and will fall away, as its debt
// is the heaviest, implying the smallest weight evaluated and the last priority compared to
// those two others.
// Account of Node 3 will be a victim of tx fee insufficiency and will drop out, as its debt
// is the heaviest, implying the smallest weight evaluated and the smallest priority compared to
// the two others.
let owed_to_serv_node_3_minor = to_wei(payment_thresholds.debt_threshold_gwei + 60_000_000);
let enough_balance_for_serving_node_1_and_2 =
owed_to_serv_node_1_minor + owed_to_serv_node_2_minor;
let missing_portion_to_the_full_amount = to_wei(2_345_678);
let consuming_node_initial_service_fee_balance_minor =
enough_balance_for_serving_node_1_and_2 - to_wei(2_345_678);
enough_balance_for_serving_node_1_and_2 - missing_portion_to_the_full_amount;
let gas_price_major = 60;
let tx_fee_needed_to_pay_for_one_payment_major = {
// We'll need littler funds, but we can stand mild inaccuracy from assuming the use of
Expand All @@ -183,15 +186,15 @@ fn payments_were_adjusted_due_to_insufficient_balances() {
.unwrap()
};
let transaction_fee_margin = PurePercentage::try_from(15).unwrap();
transaction_fee_margin.add_percent_to(gas_limit_dev_chain * gas_price_major)
transaction_fee_margin.increase_by_percent_for(gas_limit_dev_chain * gas_price_major)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to keep the word _for in the functions's name. Please eliminate it.

};
const AFFORDABLE_PAYMENTS_COUNT: u128 = 2;
let affordable_payments_count_by_tx_fee = 2;
let tx_fee_needed_to_pay_for_one_payment_minor: u128 =
to_wei(tx_fee_needed_to_pay_for_one_payment_major);
let consuming_node_transaction_fee_balance_minor =
AFFORDABLE_PAYMENTS_COUNT * tx_fee_needed_to_pay_for_one_payment_minor;
let test_inputs = TestInputsBuilder::default()
.ui_ports(Ports::new(
affordable_payments_count_by_tx_fee * tx_fee_needed_to_pay_for_one_payment_minor;
let test_input = TestInputBuilder::default()
.ui_ports(UiPorts::new(
find_free_port(),
find_free_port(),
find_free_port(),
Expand All @@ -202,7 +205,7 @@ fn payments_were_adjusted_due_to_insufficient_balances() {
.consuming_node_initial_service_fee_balance_minor(
consuming_node_initial_service_fee_balance_minor,
)
.debts_config(DebtsSpecs::new(
.debts_config(DebtsSpecs::new([
// This account will be the most significant and will deserve the full balance
Debt::new(
owed_to_serv_node_1_minor,
Expand All @@ -219,7 +222,7 @@ fn payments_were_adjusted_due_to_insufficient_balances() {
owed_to_serv_node_3_minor,
payment_thresholds.maturity_threshold_sec + 30_000,
),
))
]))
.payment_thresholds_all_nodes(payment_thresholds)
.consuming_node_gas_price_major(gas_price_major)
.build();
Expand All @@ -240,7 +243,7 @@ fn payments_were_adjusted_due_to_insufficient_balances() {
};

test_body(
test_inputs,
test_input,
assertions_values,
stimulate_consuming_node_to_pay_for_test_with_insufficient_funds,
activating_serving_nodes_for_test_with_insufficient_funds,
Expand Down Expand Up @@ -286,7 +289,7 @@ fn activating_serving_nodes_for_test_with_insufficient_funds(
node_config,
);
let ui_port = serving_node_attributes
.serving_node_profile
.node_profile
.ui_port()
.expect("ui port missing");

Expand Down
Loading