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

FIFO Success Test #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

aruokhai
Copy link
Contributor

Objective

Theoretically, a coin select algorithm should be selected as the best candidate by the select_coin function, if it produced the minimum amount of waste. Verification of the expected behaviour is needed with the help a crafted inputs that are biased towards a specific coin_select algorithm.

Changes

  • Added a test_select_coin_fifo_successful unit test function to validate the correcteness of FIFO algorithm
  • Added a generator function to specifically create FIFO output groups from vectors.

Scope Of Change

This is a semi critical change because the test is expected to acts a verifier of the coin-select and FIFO logic, such test logic should therefore be correct.

aruokhai added 2 commits July 17, 2024 15:53
- added unit test to validate the success criteria of FIFO Select coin logic
- added a generator function to create FIFO output Groups from vectors
@@ -315,6 +315,10 @@ pub fn select_coin_fifo(

for (index, inputs) in sorted_inputs {
estimated_fees = calculate_fee(accumulated_weight, options.target_feerate);
let outter = (options.target_value
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you need this?

Copy link
Collaborator

@delcin-raj delcin-raj left a comment

Choose a reason for hiding this comment

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

Assert with hand picked inputs.
Do not call select_coin_fifo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants