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

VG-13274 - Confirmed-first ordering of utxos when crafting a transaction + add configuration #933

Merged
merged 13 commits into from
Feb 22, 2023

Conversation

jcoatelen-ledger
Copy link
Contributor

@jcoatelen-ledger jcoatelen-ledger commented Feb 21, 2023

Changes:

  • add CONFIRMED_UTXO_FIRST configuration to enable (default) or disable the use of confirmed utxos as first candidates for the different utxo picking strategies
  • implementation of confirmed first in all utxo picking strategies
  • unit tests to cover the logic

Testing

The new logic is tested with some new unit tests. To test the new behavior at integration level, prepare an account with utxos and remove the block_height value from most of the entries in DB, then craft a transaction. The expected result is that those with block_height should be proposed in priority.

References

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@jcoatelen-ledger jcoatelen-ledger changed the title VG-13274 - Confirmed first ordering when querying UTXOs + add configuration VG-13274 - Confirmed-first ordering of utxos when crafting a transaction + add configuration Feb 21, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Contributor

@BertrandD BertrandD left a comment

Choose a reason for hiding this comment

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

LGTM

@jcoatelen-ledger jcoatelen-ledger merged commit e35f2ba into main Feb 22, 2023
@jcoatelen-ledger jcoatelen-ledger deleted the order-utxo-confirmed-first branch February 22, 2023 10:26
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.

3 participants