diff --git a/node/src/accountant/payment_adjuster/adjustment_runners.rs b/node/src/accountant/payment_adjuster/adjustment_runners.rs index a02a0cee2..059b91274 100644 --- a/node/src/accountant/payment_adjuster/adjustment_runners.rs +++ b/node/src/accountant/payment_adjuster/adjustment_runners.rs @@ -46,6 +46,7 @@ impl AdjustmentRunner for TransactionAndServiceFeeAdjustmentRunner { } } +// TODO: GH-711: This could be deleted (the code is directly used) pub struct ServiceFeeOnlyAdjustmentRunner {} impl AdjustmentRunner for ServiceFeeOnlyAdjustmentRunner { diff --git a/node/src/accountant/payment_adjuster/miscellaneous/data_structures.rs b/node/src/accountant/payment_adjuster/miscellaneous/data_structures.rs index 0f3fd705f..753cc067b 100644 --- a/node/src/accountant/payment_adjuster/miscellaneous/data_structures.rs +++ b/node/src/accountant/payment_adjuster/miscellaneous/data_structures.rs @@ -31,7 +31,7 @@ impl WeightedPayable { #[derive(Debug, PartialEq, Eq)] pub struct AdjustmentIterationResult { pub decided_accounts: DecidedAccounts, - pub remaining_undecided_accounts: Vec, + pub undecided_accounts: Vec, } pub struct RecursionResults { @@ -40,16 +40,6 @@ pub struct RecursionResults { } impl RecursionResults { - pub fn new( - here_decided_accounts: Vec, - downstream_decided_accounts: Vec, - ) -> Self { - Self { - here_decided_accounts, - downstream_decided_accounts, - } - } - pub fn merge_results_from_recursion(self) -> Vec { self.here_decided_accounts .into_iter() @@ -64,6 +54,15 @@ pub enum DecidedAccounts { SomeAccountsProcessed(Vec), } +impl From for Vec { + fn from(decided_accounts: DecidedAccounts) -> Self { + match decided_accounts { + DecidedAccounts::LowGainingAccountEliminated => vec![], + DecidedAccounts::SomeAccountsProcessed(accounts) => accounts, + } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub struct AdjustedAccountBeforeFinalization { pub original_account: PayableAccount, diff --git a/node/src/accountant/payment_adjuster/mod.rs b/node/src/accountant/payment_adjuster/mod.rs index 878bd9721..91dd4c749 100644 --- a/node/src/accountant/payment_adjuster/mod.rs +++ b/node/src/accountant/payment_adjuster/mod.rs @@ -16,7 +16,7 @@ mod test_utils; use crate::accountant::db_access_objects::payable_dao::PayableAccount; use crate::accountant::payment_adjuster::adjustment_runners::{ - AdjustmentRunner, ServiceFeeOnlyAdjustmentRunner, TransactionAndServiceFeeAdjustmentRunner, + AdjustmentRunner, TransactionAndServiceFeeAdjustmentRunner, }; use crate::accountant::payment_adjuster::criterion_calculators::balance_calculator::BalanceCriterionCalculator; use crate::accountant::payment_adjuster::criterion_calculators::CriterionCalculator; @@ -31,10 +31,7 @@ use crate::accountant::payment_adjuster::inner::{ use crate::accountant::payment_adjuster::logging_and_diagnostics::log_functions::{ accounts_before_and_after_debug, }; -use crate::accountant::payment_adjuster::miscellaneous::data_structures::DecidedAccounts::{ - LowGainingAccountEliminated, SomeAccountsProcessed, -}; -use crate::accountant::payment_adjuster::miscellaneous::data_structures::{AdjustedAccountBeforeFinalization, AdjustmentIterationResult, RecursionResults, WeightedPayable}; +use crate::accountant::payment_adjuster::miscellaneous::data_structures::{AdjustedAccountBeforeFinalization, WeightedPayable}; use crate::accountant::payment_adjuster::miscellaneous::helper_functions::{ dump_unaffordable_accounts_by_transaction_fee, exhaust_cw_balance_entirely, find_largest_exceeding_balance, @@ -195,10 +192,15 @@ impl PaymentAdjusterReal { analyzed_accounts: Vec, ) -> Result, PaymentAdjusterError> { let weighted_accounts = self.calculate_weights(analyzed_accounts); - let processed_accounts = self.propose_adjustments_recursively( - weighted_accounts, - TransactionAndServiceFeeAdjustmentRunner {}, - )?; + diagnostics!( + "\nUNRESOLVED QUALIFIED ACCOUNTS IN CURRENT ITERATION:", + &weighted_accounts + ); + let processed_accounts = if let Some(limit) = self.inner.transaction_fee_count_limit_opt() { + self.begin_with_adjustment_by_transaction_fee(weighted_accounts.clone(), limit)? + } else { + Either::Left(self.propose_possible_adjustment_recursively(weighted_accounts)) + }; if zero_affordable_accounts_found(&processed_accounts) { return Err(PaymentAdjusterError::AllAccountsEliminated); @@ -218,6 +220,7 @@ impl PaymentAdjusterReal { } } + // TODO: GH-711: This can be removed. The code for AdjustmentRunners is now used directly fn propose_adjustments_recursively( &mut self, unresolved_accounts: Vec, @@ -274,25 +277,35 @@ impl PaymentAdjusterReal { } } + // TODO: GH-711: This is the only recursive fn that is being called now. + // It's sibling function propose_adjustment_recursively() and it's + // subsequent adjustment runners can be safely removed fn propose_possible_adjustment_recursively( &mut self, weighed_accounts: Vec, ) -> Vec { - let disqualification_arbiter = &self.disqualification_arbiter; - let unallocated_cw_service_fee_balance = - self.inner.unallocated_cw_service_fee_balance_minor(); - let logger = &self.logger; - let current_iteration_result = self.service_fee_adjuster.perform_adjustment_by_service_fee( weighed_accounts, - disqualification_arbiter, - unallocated_cw_service_fee_balance, - logger, + &self.disqualification_arbiter, + self.inner.unallocated_cw_service_fee_balance_minor(), + &self.logger, ); + let undecided_accounts = current_iteration_result.undecided_accounts; + let decided_accounts: Vec = + current_iteration_result.decided_accounts.into(); - let recursion_results = self.resolve_current_iteration_result(current_iteration_result); + if undecided_accounts.is_empty() { + return decided_accounts; + } - let merged = recursion_results.merge_results_from_recursion(); + self.deduct_unallocated_cw_balance(&decided_accounts); + let remaining_accounts = if self.is_cw_balance_under_limit(&undecided_accounts) { + // Fast return after a direct conversion into the expected type + convert_collection(undecided_accounts) + } else { + self.propose_possible_adjustment_recursively(undecided_accounts) + }; + let merged = Self::add_accounts(remaining_accounts, decided_accounts); diagnostics!( "\nFINAL SET OF ADJUSTED ACCOUNTS IN CURRENT ITERATION:", @@ -302,35 +315,27 @@ impl PaymentAdjusterReal { merged } - fn resolve_current_iteration_result( - &mut self, - adjustment_iteration_result: AdjustmentIterationResult, - ) -> RecursionResults { - let remaining_undecided_accounts = adjustment_iteration_result.remaining_undecided_accounts; - let here_decided_accounts = match adjustment_iteration_result.decided_accounts { - LowGainingAccountEliminated => { - if remaining_undecided_accounts.is_empty() { - return RecursionResults::new(vec![], vec![]); - } - - vec![] - } - SomeAccountsProcessed(decided_accounts) => { - if remaining_undecided_accounts.is_empty() { - return RecursionResults::new(decided_accounts, vec![]); - } + fn is_cw_balance_under_limit( + &self, + remaining_undecided_accounts: &Vec, + ) -> bool { + let check_sum: u128 = sum_as(remaining_undecided_accounts, |weighted_account| { + weighted_account.balance_minor() + }); - self.adjust_remaining_unallocated_cw_balance_down(&decided_accounts); - decided_accounts - } - }; + let unallocated_cw_balance = self.inner.unallocated_cw_service_fee_balance_minor(); - let down_stream_decided_accounts = self.propose_adjustments_recursively( - remaining_undecided_accounts, - ServiceFeeOnlyAdjustmentRunner {}, - ); + check_sum <= unallocated_cw_balance + } - RecursionResults::new(here_decided_accounts, down_stream_decided_accounts) + fn add_accounts( + remaining_accounts: Vec, + decided_accounts: Vec, + ) -> Vec { + decided_accounts + .into_iter() + .chain(remaining_accounts.into_iter()) + .collect() } fn calculate_weights(&self, accounts: Vec) -> Vec { @@ -369,10 +374,14 @@ impl PaymentAdjusterReal { .collect() } - fn adjust_remaining_unallocated_cw_balance_down( + fn deduct_unallocated_cw_balance( &mut self, processed_outweighed: &[AdjustedAccountBeforeFinalization], ) { + if processed_outweighed.is_empty() { + return; + } + let subtrahend_total: u128 = sum_as(processed_outweighed, |account| { account.proposed_adjusted_balance_minor }); @@ -2211,7 +2220,7 @@ mod tests { // We care only for the params captured inside the container from above .perform_adjustment_by_service_fee_result(AdjustmentIterationResult { decided_accounts: SomeAccountsProcessed(vec![]), - remaining_undecided_accounts: vec![], + undecided_accounts: vec![], }); subject.service_fee_adjuster = Box::new(service_fee_adjuster_mock); diff --git a/node/src/accountant/payment_adjuster/service_fee_adjuster.rs b/node/src/accountant/payment_adjuster/service_fee_adjuster.rs index 5954a224d..08846b892 100644 --- a/node/src/accountant/payment_adjuster/service_fee_adjuster.rs +++ b/node/src/accountant/payment_adjuster/service_fee_adjuster.rs @@ -99,13 +99,12 @@ impl ServiceFeeAdjusterReal { if thriving_competitors.is_empty() { Either::Left(losing_competitors) } else { - let remaining_undecided_accounts: Vec = - convert_collection(losing_competitors); + let undecided_accounts: Vec = convert_collection(losing_competitors); let pre_processed_decided_accounts: Vec = convert_collection(thriving_competitors); Either::Right(AdjustmentIterationResult { decided_accounts: SomeAccountsProcessed(pre_processed_decided_accounts), - remaining_undecided_accounts, + undecided_accounts, }) } } @@ -127,7 +126,7 @@ impl ServiceFeeAdjusterReal { AdjustmentIterationResult { decided_accounts: LowGainingAccountEliminated, - remaining_undecided_accounts: remaining_reverted, + undecided_accounts: remaining_reverted, } }