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

Adds coin select flags to defid with option to override from CLI #1820

Merged
merged 10 commits into from
Mar 17, 2023

Conversation

Jouzo
Copy link
Collaborator

@Jouzo Jouzo commented Mar 14, 2023

Summary

  • Adds coin select flags to defid
  • Adds GetOptionalBoolArg for when a flag should not have any default value. This is here used to override defid flag iff flag is also set via defi-cli

Examples

defid -walletfastselect=1

defi-cli sendtoaddress <address> 1
  • Will use value from defid

defid -walletfastselect=1

defi-cli -walletfastselect=0 sendtoaddress <address> 1
  • Value will be overridden and set to 0 from defi-cli

Flags

-walletfastselect - Faster coin select - Enables walletcoinoptskipsolvable and walletcoinopteagerselect. This ends up in faster selection but has the disadvantage of not being able to pick complex input scripts. Default: false

-walletcoinoptskipsolvable - Coin select option: Skips IsSolvable signable UTXO check. Default: false

-walletcoinopteagerselect - Coin select option: Take fast path and eagerly exit on match even without having through the entire set. Default: false

@prasannavl prasannavl added the v/next-release Items ready or targeted for upcoming release(s) label Mar 15, 2023
@prasannavl
Copy link
Member

prasannavl commented Mar 15, 2023

CreateDefault will still always initialize everything. Also ToHTTPHeader shouldn't be calling out to the gArgs. It should only be contained within the FromArgs.

Other motivations behind keeping ToHTTPHeader, FromHTTPHeader dependency-free, is that they are now individually testable. Shouldn't depend on gArgs that destroys test ability of these methods. Let's use FromArgs to handle this.

We'll need to tweak this a bit.

@prasannavl
Copy link
Member

prasannavl commented Mar 15, 2023

Added changes here: https://github.com/DeFiCh/ain/compare/pvl/coinselect-cli?expand=1. Believe that should provide a better base and mostly solve it.

To-do on that branch:

  • Find the best way to detect defi-cli or defid (best to use it statically if we have an optional define from somewhere)
  • Adapt rest of the coinselect usage to work with optional
  • Port the request metadata pass on to the rest of methods in this branch

@Jouzo Jouzo dismissed a stale review via 5902881 March 15, 2023 10:11
DocteurPing
DocteurPing previously approved these changes Mar 17, 2023
@prasannavl
Copy link
Member

LGTM

@prasannavl prasannavl merged commit b781611 into master Mar 17, 2023
@prasannavl prasannavl deleted the fix/cli_flag branch March 17, 2023 06:36
@dcorral dcorral mentioned this pull request Mar 17, 2023
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v/next-release Items ready or targeted for upcoming release(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants