Code - FIFO #12

jkciw · 2024-03-15T10:39:02Z

Code to implement coin selection using first in first out (FIFO) algorithm.

delcin-raj · 2024-03-18T09:17:56Z

src/lib.rs

 ) -> Result<SelectionOutput, SelectionError> {
-    unimplemented!()
+    /* Using the value of the input as an identifier for the selected inputs, as the index of the vec<outputgroup> can't be used because the vector itself is sorted first. Ideally, txid of the input can serve as an unique identifier */


This is wrong. The indices of the Vec has to be the identifier. You can create a Vec<usize, OutGroup> first and then sort that Vec, this will track which OutGroup belongs to which index.
Mulltiple UTXOs can have same OutGroup value.

Right. Removed the comment.

delcin-raj · 2024-03-18T09:20:44Z

src/lib.rs

@@ -8,7 +8,7 @@ use rand::{seq::SliceRandom, thread_rng};
 /// single UTXO, or a group of UTXOs that should be spent together.
 /// The library user is responsible for crafting this structure correctly. Incorrect representation of this
 /// structure will cause incorrect selection result.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]


Multiple UTXOs can have same OutGroup, so deriving Equality doesn't make sense here.

Agreed. However, I need the PartialEq trait as I use it to compare the sorted and original OutputGroup vectors to extract the indices from the original OutputGroup vector.

But that's not semantically right, and your algorithm is running in O(n^2) while it should be optimally run in O(n logn) due to sorting.
You have to collect the elements with their indices using enumeration before you sort then later sort them according to the creation_sequence, so even after sorting each outputgroup will be associated with it's index in the inputs as given by the user.
You can check how I did that in SRD.

Understood. Made relevant changes and pushed a commit

jkciw

Changes:

Removed redundant Eq trait derived for OutputGroup
Removed comments regarding use of 'value' instead of 'index' to keep track of OutputGroup vector elements.

jkciw · 2024-03-20T01:40:39Z

src/lib.rs

@@ -8,7 +8,7 @@ use rand::{seq::SliceRandom, thread_rng};
 /// single UTXO, or a group of UTXOs that should be spent together.
 /// The library user is responsible for crafting this structure correctly. Incorrect representation of this
 /// structure will cause incorrect selection result.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]


Agreed. However, I need the PartialEq trait as I use it to compare the sorted and original OutputGroup vectors to extract the indices from the original OutputGroup vector.

jkciw · 2024-03-20T01:41:12Z

src/lib.rs

 ) -> Result<SelectionOutput, SelectionError> {
-    unimplemented!()
+    /* Using the value of the input as an identifier for the selected inputs, as the index of the vec<outputgroup> can't be used because the vector itself is sorted first. Ideally, txid of the input can serve as an unique identifier */


Right. Removed the comment.

delcin-raj · 2024-03-20T09:43:07Z

src/lib.rs

 /// Return NoSolutionFound, if no solution exists.
+
 pub fn select_coin_fifo(
    inputs: Vec<OutputGroup>,


Please merge from main.

delcin-raj · 2024-03-24T17:36:27Z

src/lib.rs


-    for i in sortedinputs.iter() {
+    for (index, inputs) in sorted_inputs {
        estimated_fees = (accumulated_weight as f32 * options.target_feerate).ceil() as u64;


Use calculate_fee function here

delcin-raj · 2024-03-24T17:37:45Z

src/lib.rs

-        }
+        accumulated_value += inputs.value;
+        accumulated_weight += inputs.weight;
+        selected_inputs.push(index);
    }
    if accumulated_value < options.target_value + estimated_fees + options.min_drain_value {


There could be cases where estimated_fees < min_absolute_fee, refer SRD to fix this.

delcin-raj · 2024-03-24T17:40:19Z

src/lib.rs

-        }
+        accumulated_value += inputs.value;
+        accumulated_weight += inputs.weight;
+        selected_inputs.push(index);
    }
    if accumulated_value < options.target_value + estimated_fees + options.min_drain_value {
        Err(SelectionError::NoSolutionFound)
    } else {
        let mut waste_score: u64 = 0;


Don't calculate waste manually, use calculate_waste function

delcin-raj · 2024-03-25T12:16:26Z

src/lib.rs

-        }
+        accumulated_value += inputs.value;
+        accumulated_weight += inputs.weight;
+        selected_inputs.push(index);
    }
    if accumulated_value < options.target_value + estimated_fees + options.min_drain_value {
        Err(SelectionError::NoSolutionFound)


InsufficientFunds will be the appropriate error for FIFO, as it tries to include all UTXOs until the desired target is met.
Because if sufficient fund exists fifo will give you a solution. This is not the case of BNB and Knapsack for example..

delcin-raj · 2024-03-30T13:55:57Z

src/lib.rs

+    for (index, inputs) in sorted_inputs {
+        estimated_fees = calculate_fee(accumulated_weight, options.target_feerate);
+        if accumulated_value
+            >= (options.target_value


Why 2 * options.target_value?

That is a typo.

delcin-raj · 2024-03-30T13:57:00Z

src/lib.rs

+            >= (options.target_value
+                + options.target_value
+                + estimated_fees
+                + options.min_drain_value)


Have to make sure that the accumulated_value has sufficient fee also, estimated_fee.max(options.min_absolute_fee)

delcin-raj

Thank you, looks good!

…uding feerate and weight calculation

Merged with main according to updates

…uding feerate and weight calculation

* Added FIFO * Added changes to use value of the input as identifier for FIFO algorithm * Added changes to use sort_by_key method and check for conditions including feerate and weight calculation * Removed Partial Imp and added one test case for fifo * Removed custom implementations of ordering * Added FIFO Merged with main according to updates * Added changes to use sort_by_key method and check for conditions including feerate and weight calculation * Removed Partial Imp and added one test case for fifo * Removed custom implementations of ordering * Performed fmt checks * Addressed issues raised by clippy and checked fmt

…cubator#11) This reverts commit 663e2bf.

* Single Random Draw rough implementation * fmt changes * Tests for SRD * fmt changes * Clippy suggestions * check for not empty * Spelling corrections and merge from upstream/main

* bnp function declaration * Clippy suggestion

* inputs type is changed to &[OutputGroup] Singalling the fact that selected_outputs should refer to indices of the given inputs and that the order of the given inputs are not changed with respect to the user. * fmt fix

…he existing utility function

delcin-raj reviewed Mar 18, 2024

View reviewed changes

jkciw commented Mar 20, 2024

View reviewed changes

delcin-raj reviewed Mar 20, 2024

View reviewed changes

delcin-raj requested changes Mar 24, 2024

View reviewed changes

delcin-raj requested changes Mar 25, 2024

View reviewed changes

delcin-raj requested changes Mar 30, 2024

View reviewed changes

delcin-raj approved these changes Apr 4, 2024

View reviewed changes

rajarshimaitra and others added 22 commits April 9, 2024 07:56

Initial commit

14c0610

initial structure and docs

69afb33

lint fixes

7979b8b

Added FIFO

1bd1206

Added changes to use value of the input as identifier for FIFO algorithm

4b611d2

Added changes to use sort_by_key method and check for conditions incl…

d776795

…uding feerate and weight calculation

Removed Partial Imp and added one test case for fifo

e2cab83

Removed custom implementations of ordering

336f956

Defining SelectionOutput type and some Error Variants

8de79c4

Allow unsed definitions

8558fe4

Changing the comments according to new types

b6d8eb0

Added FIFO

cf617f5

Merged with main according to updates

Added changes to use sort_by_key method and check for conditions incl…

5af83c0

…uding feerate and weight calculation

Removed Partial Imp and added one test case for fifo

8c1b1cc

Removed custom implementations of ordering

a4dbd56

Performed fmt checks

1e1333b

Addressed issues raised by clippy and checked fmt

43f0dfe

Revert "FIFO selection algorithm (Bitshala-Incubator#9)" (Bitshala-In…

b12d8a5

…cubator#11) This reverts commit 663e2bf.

Single Random Draw rough implementation (Bitshala-Incubator#7)

4a9fafe

* Single Random Draw rough implementation * fmt changes * Tests for SRD * fmt changes * Clippy suggestions * check for not empty * Spelling corrections and merge from upstream/main

Minor fmt edit

9082077

Addressed issues causing test to fail

92b5505

jkciw and others added 15 commits April 9, 2024 08:12

Removed comments about using values/indexes in FIFO

1febc55

bnp function declaration (Bitshala-Incubator#13)

3efacf7

* bnp function declaration * Clippy suggestion

inputs type is changed to &[OutputGroup] (Bitshala-Incubator#14)

85f53c8

* inputs type is changed to &[OutputGroup] Singalling the fact that selected_outputs should refer to indices of the given inputs and that the order of the given inputs are not changed with respect to the user. * fmt fix

Pulled changes from main and modified fifo accodingly

80bb02e

Removed a redundant utility function. Substituted its function with t…

3c427b9

…he existing utility function

Fmt fix

f18d4ba

including base weight of template in fee (Bitshala-Incubator#16)

1c269b8

Handle cases where estimated_fee < rbf value (Bitshala-Incubator#17)

6763852

Outline for knapsack selection (Bitshala-Incubator#18)

f387091

removed redundat codes and replaced them with utility functions.

17f3bd4

Modified SelectionError msg to Insufficient funds

ab0cbad

Made changes w.r.t estimated_fees in accumulated_value conditionals.

e512fb5

Removed unnecessary files committed earlier

36bf010

removed DS_Store and launch.json files

13393fe

signed commit

2afbb6d

jkciw closed this Apr 9, 2024

jkciw force-pushed the fifo branch from 1f1bfcd to 2afbb6d Compare April 9, 2024 02:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code - FIFO #12

Code - FIFO #12

jkciw commented Mar 15, 2024

delcin-raj Mar 18, 2024

jkciw Mar 20, 2024

delcin-raj Mar 18, 2024

jkciw Mar 20, 2024

delcin-raj Mar 20, 2024 •

edited

Loading

jkciw Mar 23, 2024

jkciw left a comment

jkciw Mar 20, 2024

jkciw Mar 20, 2024

delcin-raj Mar 20, 2024

delcin-raj Mar 24, 2024

jkciw Mar 28, 2024

delcin-raj Mar 24, 2024

jkciw Mar 28, 2024

delcin-raj Mar 24, 2024

delcin-raj Mar 25, 2024

delcin-raj Mar 30, 2024

jkciw Mar 31, 2024

delcin-raj Mar 30, 2024

jkciw Mar 31, 2024

delcin-raj left a comment

Code - FIFO #12

Code - FIFO #12

Conversation

jkciw commented Mar 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delcin-raj Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkciw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delcin-raj left a comment

Choose a reason for hiding this comment

delcin-raj Mar 20, 2024 •

edited

Loading