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.

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

Fix an infinite loop with auto allotment #2202

Merged
merged 2 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 1 addition & 6 deletions sdk/src/client/api/block_builder/transaction_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ pub(crate) struct MinManaAllotment {
required_allotment: Option<u64>,
}

#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) struct Remainders {
address: Option<Address>,
data: Vec<RemainderData>,
Expand Down Expand Up @@ -545,11 +545,6 @@ impl TransactionBuilder {
.expect("expiration unlockable outputs already filtered out");
self.selected_inputs.insert(required_address, input);

// Remove the cached allotment value because it's no longer valid
if let Some(MinManaAllotment { required_allotment, .. }) = self.min_mana_allotment.as_mut() {
*required_allotment = None;
}

Ok(added_output)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ use crate::{
};

impl TransactionBuilder {
/// Updates the remainders, overwriting old values.
pub(crate) fn update_remainders(&mut self) -> Result<(), TransactionBuilderError> {
self.remainders = Remainders {
address: self.remainders.address.take(),
/// Updates the remainders, overwriting old values. Returns whether any changes were made.
pub(crate) fn update_remainders(&mut self) -> Result<bool, TransactionBuilderError> {
// Swap the remainders so we can keep the old ones to compare to later.
let mut old_remainders = Remainders {
address: self.remainders.address.clone(),
..Default::default()
};
core::mem::swap(&mut self.remainders, &mut old_remainders);
let (input_amount, output_amount, inputs_sdr, outputs_sdr) = self.amount_sums();

for (address, amount) in inputs_sdr {
Expand Down Expand Up @@ -120,12 +122,11 @@ impl TransactionBuilder {

if amount_diff == 0 && mana_diff == 0 && native_tokens_diff.is_empty() {
log::debug!("No remainder required");
return Ok(());
} else {
self.create_remainder_outputs(amount_diff, mana_diff, native_tokens_diff, remainder_address, chain)?;
}

self.create_remainder_outputs(amount_diff, mana_diff, native_tokens_diff, remainder_address, chain)?;

Ok(())
Ok(self.remainders != old_remainders)
}

/// Gets the remainder address from configuration or finds one from the inputs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ impl TransactionBuilder {
}

should_recalculate |= self.reduce_account_output()?;
} else {
should_recalculate = true;
}

// Remainders can only be calculated when the input mana is >= the output mana
let (input_mana, output_mana) = self.mana_sums(false)?;
if input_mana >= output_mana {
self.update_remainders()?;
should_recalculate |= self.update_remainders()?;
}

should_recalculate |= self.get_inputs_for_mana_balance()?;
Expand All @@ -91,7 +89,7 @@ impl TransactionBuilder {
return Ok(None);
};

if required_allotment.is_none() && !self.selected_inputs.is_empty() && self.all_outputs().next().is_some() {
if !self.selected_inputs.is_empty() && self.all_outputs().next().is_some() {
let inputs = self
.selected_inputs
.sorted_iter()
Expand Down Expand Up @@ -303,7 +301,10 @@ impl TransactionBuilder {

pub(crate) fn mana_sums(&mut self, include_remainders: bool) -> Result<(u64, u64), TransactionBuilderError> {
let allotments_sum = if let Some(MinManaAllotment { issuer_id, .. }) = self.min_mana_allotment {
let required_allotment = self.required_allotment()?.unwrap_or_default();
let mut required_allotment = self.min_mana_allotment.and_then(|a| a.required_allotment);
if required_allotment.is_none() {
required_allotment = self.required_allotment()?;
}
self.mana_allotments
.iter()
.filter_map(|(id, value)| (id != &issuer_id).then_some(value))
Expand All @@ -313,7 +314,7 @@ impl TransactionBuilder {
.get(&issuer_id)
.copied()
.unwrap_or_default()
.max(required_allotment)
.max(required_allotment.unwrap_or_default())
} else {
self.mana_allotments.values().sum::<u64>()
};
Expand Down
Loading