Просмотр исходного кода

Inline posting selection into resolve so the debit set is summed once

resolve_envelope already computed the total positive balance for a debit
before delegating to select_postings, which then re-filtered and re-summed
the identical posting set and carried an InsufficientFunds path that resolve
could never reach (it only called into selection once funds sufficed; the
shortfall case is handled by resolve's own overdraft branch).

Fold the greedy largest-first pick into resolve's has-funds branch, reusing
the running sum for change instead of a second pass over the postings. The
posting_selection module now holds only SelectionError, whose sole surviving
variant is InsufficientFunds; the Overflow variant went with the function
that was its only constructor. Selection behaviour that lost its unit tests
is now exercised through resolve_envelope.
Cesar Rodas 1 день назад
Родитель
Сommit
0b21203919

+ 1 - 1
crates/kuatia-core/src/lib.rs

@@ -18,5 +18,5 @@ pub use hash::{
 pub use posting_resolution::{
     Debit, MovementDraft, ResolveError, ResolveInput, draft_movements, resolve_envelope,
 };
-pub use posting_selection::{SelectionError, select_postings};
+pub use posting_selection::SelectionError;
 pub use validate::{Plan, PlanInput, ValidationError, validate_and_plan};

+ 49 - 11
crates/kuatia-core/src/posting_resolution.rs

@@ -15,7 +15,7 @@
 
 use std::collections::HashMap;
 
-use crate::posting_selection::{SelectionError, select_postings};
+use crate::posting_selection::SelectionError;
 use kuatia_types::{
     AccountId, AccountPolicy, AssetId, Cent, Envelope, EnvelopeBuilder, NewPosting, OverflowError,
     Posting, PostingId, Transfer,
@@ -178,17 +178,23 @@ pub fn resolve_envelope(input: ResolveInput<'_>) -> Result<Envelope, ResolveErro
         )?;
 
         if total_positive >= debit.amount {
-            // Enough positive postings: select a subset and compute change.
-            let selected = select_postings(avail, debit.asset, debit.amount)?;
-            let consumed_sum = Cent::checked_sum(
-                avail
-                    .iter()
-                    .filter(|p| selected.contains(&p.id))
-                    .map(|p| p.value),
-            )?;
-            let change = consumed_sum.checked_sub(debit.amount)?;
+            // Enough positive postings: greedily take them largest-first to
+            // minimise the number consumed (and therefore the change postings
+            // created), summing as we go so we stop once the debit is covered.
+            let mut candidates: Vec<&Posting> =
+                avail.iter().filter(|p| p.value.is_positive()).collect();
+            candidates.sort_by_key(|p| std::cmp::Reverse(p.value));
+
+            let mut consumed_sum = Cent::ZERO;
+            for posting in candidates {
+                consumes.push(posting.id);
+                consumed_sum = consumed_sum.checked_add(posting.value)?;
+                if consumed_sum >= debit.amount {
+                    break;
+                }
+            }
 
-            consumes.extend_from_slice(&selected);
+            let change = consumed_sum.checked_sub(debit.amount)?;
             if change.is_positive() {
                 creates.push(NewPosting {
                     owner: debit.account,
@@ -344,6 +350,38 @@ mod tests {
     }
 
     #[test]
+    fn selects_largest_first_to_minimise_consumed() {
+        // With 10, 90 and 50 available, a debit of 80 is covered by the single
+        // 90 posting rather than several smaller ones — one consumed, 10 change.
+        let transfer = pay(acct(1), acct(2), 80);
+        let draft = draft_movements(&transfer).unwrap();
+        let available = HashMap::from([(
+            (acct(1), AssetId::new(1)),
+            vec![
+                posting(acct(1), 0, 10),
+                posting(acct(1), 1, 90),
+                posting(acct(1), 2, 50),
+            ],
+        )]);
+        let policies = HashMap::new();
+        let env = resolve_envelope(ResolveInput {
+            transfer: &transfer,
+            draft,
+            available: &available,
+            policies: &policies,
+        })
+        .unwrap();
+        assert_eq!(env.consumes().len(), 1);
+        let change: Vec<_> = env
+            .creates()
+            .iter()
+            .filter(|p| p.owner == acct(1))
+            .collect();
+        assert_eq!(change.len(), 1);
+        assert_eq!(change[0].value, Cent::from(10));
+    }
+
+    #[test]
     fn insufficient_funds_without_overdraft_fails() {
         let transfer = pay(acct(1), acct(2), 100);
         let draft = draft_movements(&transfer).unwrap();

+ 6 - 141
crates/kuatia-core/src/posting_selection.rs

@@ -1,13 +1,13 @@
-//! Posting selection for the intent layer.
+//! The error raised when an intent cannot be covered by the available postings.
 //!
 //! When a caller uses `pay` or `withdraw`, they specify an amount — not which
-//! postings to consume. This module picks the smallest set of postings that
-//! covers the requested amount, so the intent layer can build the transfer
-//! automatically without exposing UTXO mechanics to the caller.
+//! postings to consume. Resolution ([`crate::posting_resolution::resolve_envelope`])
+//! picks the postings; when a non-overdraft account cannot cover the requested
+//! amount it fails with [`SelectionError::InsufficientFunds`].
 
-use kuatia_types::{AssetId, Cent, Posting, PostingId};
+use kuatia_types::Cent;
 
-/// Error returned when posting selection fails.
+/// Error returned when an amount cannot be covered by the available postings.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum SelectionError {
     /// Available postings do not cover the requested amount.
@@ -17,8 +17,6 @@ pub enum SelectionError {
         /// Amount the caller asked for.
         requested: Cent,
     },
-    /// Summing posting values would overflow `Cent`.
-    Overflow,
 }
 
 impl std::fmt::Display for SelectionError {
@@ -33,141 +31,8 @@ impl std::fmt::Display for SelectionError {
                     "insufficient funds: available {available}, requested {requested}"
                 )
             }
-            Self::Overflow => write!(f, "monetary amount overflow"),
         }
     }
 }
 
 impl std::error::Error for SelectionError {}
-
-/// Picks postings to cover `target`, using largest-first greedy to minimise
-/// the number of postings consumed (and therefore the number of change postings
-/// created). Only positive postings of the right asset are considered; the
-/// caller supplies an already-active `available` set.
-pub fn select_postings(
-    available: &[Posting],
-    asset: AssetId,
-    target: Cent,
-) -> Result<Vec<PostingId>, SelectionError> {
-    assert!(target.is_positive(), "target must be positive");
-
-    let mut candidates: Vec<&Posting> = available
-        .iter()
-        .filter(|p| p.asset == asset && p.value.is_positive())
-        .collect();
-
-    // Largest first
-    candidates.sort_by_key(|p| std::cmp::Reverse(p.value));
-
-    let mut total_available = Cent::ZERO;
-    for p in &candidates {
-        total_available = total_available
-            .checked_add(p.value)
-            .map_err(|_| SelectionError::Overflow)?;
-    }
-    if total_available < target {
-        return Err(SelectionError::InsufficientFunds {
-            available: total_available,
-            requested: target,
-        });
-    }
-
-    let mut selected = Vec::new();
-    let mut sum = Cent::ZERO;
-    for posting in candidates {
-        selected.push(posting.id);
-        sum = sum
-            .checked_add(posting.value)
-            .map_err(|_| SelectionError::Overflow)?;
-        if sum >= target {
-            break;
-        }
-    }
-
-    Ok(selected)
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use kuatia_types::*;
-
-    fn make_posting(index: u16, value: i64) -> Posting {
-        Posting::new(
-            PostingId {
-                transfer: EnvelopeId([1; 32]),
-                index,
-            },
-            AccountId::new(1),
-            AssetId::new(1),
-            Cent::from(value),
-        )
-    }
-
-    #[test]
-    fn exact_match() {
-        let postings = vec![make_posting(0, 50), make_posting(1, 50)];
-        let result = select_postings(&postings, AssetId::new(1), Cent::from(100)).unwrap();
-        assert_eq!(result.len(), 2);
-    }
-
-    #[test]
-    fn largest_first() {
-        let postings = vec![
-            make_posting(0, 10),
-            make_posting(1, 90),
-            make_posting(2, 50),
-        ];
-        let result = select_postings(&postings, AssetId::new(1), Cent::from(80)).unwrap();
-        // Should pick 90 first (enough on its own)
-        assert_eq!(result.len(), 1);
-        assert_eq!(result[0].index, 1);
-    }
-
-    #[test]
-    fn insufficient_funds() {
-        let postings = vec![make_posting(0, 30), make_posting(1, 20)];
-        let err = select_postings(&postings, AssetId::new(1), Cent::from(100)).unwrap_err();
-        assert_eq!(
-            err,
-            SelectionError::InsufficientFunds {
-                available: Cent::from(50),
-                requested: Cent::from(100)
-            }
-        );
-    }
-
-    #[test]
-    fn ignores_wrong_asset() {
-        // Selection receives an already-active set (state lives in the store's
-        // index tables, not on the posting), so it only has to skip the wrong
-        // asset and negative values.
-        let mut wrong_asset = make_posting(1, 1000);
-        wrong_asset.asset = AssetId::new(2);
-
-        let good = make_posting(2, 50);
-
-        let postings = vec![wrong_asset, good];
-        let result = select_postings(&postings, AssetId::new(1), Cent::from(50)).unwrap();
-        assert_eq!(result.len(), 1);
-        assert_eq!(result[0].index, 2);
-    }
-
-    #[test]
-    fn ignores_negative_postings() {
-        let negative = Posting::new(
-            PostingId {
-                transfer: EnvelopeId([1; 32]),
-                index: 0,
-            },
-            AccountId::new(1),
-            AssetId::new(1),
-            Cent::from(-100),
-        );
-        let good = make_posting(1, 50);
-        let postings = vec![negative, good];
-        let result = select_postings(&postings, AssetId::new(1), Cent::from(50)).unwrap();
-        assert_eq!(result.len(), 1);
-        assert_eq!(result[0].index, 1);
-    }
-}