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

FundPsbt and EstimateFee improvements #4845

Closed
wants to merge 1 commit into from
Closed

FundPsbt and EstimateFee improvements #4845

wants to merge 1 commit into from

Conversation

jalavosus
Copy link

FundPsbt and EstimateFee now support spending from the underlying
wallet using unconfirmed inputs or inputs with a specific number of
minumum confirmations; this is made possible by adding the
params min_confs and spend_unconfirmed to the underlying proto
messages, and implementing checks in their function calls.

This commit also changes the interface of WalletController slightly; it
adds the param minConfs to CreateSimpleTx inside the
WalletController interface, thus allowing the underlying
implementation of the function to pass minConfs to its
underlying wallet's createsimpletx implementation (if possible and/or desired).

Pull Request Checklist

  • If this is your first time contributing, we recommend you read the Code
    Contribution Guidelines
  • All changes are Go version 1.12 compliant
  • The code being submitted is commented according to Code Documentation and Commenting
  • For new code: Code is accompanied by tests which exercise both
    the positive and negative (error paths) conditions (if applicable)
  • For bug fixes: Code is accompanied by new tests which trigger
    the bug being fixed to prevent regressions
  • Any new logging statements use an appropriate subsystem and
    logging level
  • Code has been formatted with go fmt
  • Protobuf files (lnrpc/**/*.proto) have been formatted with
    make rpc-format and compiled with make rpc
  • New configuration flags have been added to sample-lnd.conf
  • For code and documentation: lines are wrapped at 80 characters
    (the tab character should be counted as 8 characters, not 4, as some IDEs do
    per default)
  • Running make check does not fail any tests
  • Running go vet does not report any issues
  • Running make lint does not report any new issues that did not
    already exist
  • All commits build properly and pass tests. Only in exceptional
    cases it can be justifiable to violate this condition. In that case, the
    reason should be stated in the commit message.
  • Commits have a logical structure according to Ideal Git Commit Structure

`FundPsbt` and `EstimateFee` now support spending from the underlying
wallet using unconfirmed inputs or inputs with a specific number of
minumum confirmations; this is made possible by adding the
params `min_confs` and `spend_unconfirmed` to the underlying proto
messages, and implementing checks in their function calls.

This commit also changes the interface of `WalletController` slightly; it
adds the param `minConfs` to `CreateSimpleTx` inside the
`WalletController` interface, thus allowing the underlying
implementation of the function to pass `minConfs` to its
underlying wallet's `createsimpletx` implementation (if possible and/or desired).
@guggero guggero added this to the 0.13.0 milestone Dec 8, 2020
@guggero guggero linked an issue Dec 8, 2020 that may be closed by this pull request
@guggero guggero added psbt rpc Related to the RPC interface wallet The wallet (lnwallet) which LND uses enhancement Improvements to existing features / behaviour labels Dec 8, 2020
@guggero guggero self-requested a review December 8, 2020 09:33
@guggero
Copy link
Collaborator

guggero commented Jan 11, 2021

@jalavosus was the repo for this PR removed by accident? Or what happened?
Was going to review this now, since 0.12.0 is almost out. Or should I instead close this and review #4905?

@jakesylvestre
Copy link
Contributor

@jalavosus I went ahead and applied the patch for this commit to xplorfin#11 so if you want to change the base branch to xplorfin/fundpsbt we should be good to go here

@Roasbeef
Copy link
Member

We should either proceed with this or #4905

@guggero guggero removed their request for review February 8, 2021 08:30
@guggero
Copy link
Collaborator

guggero commented Feb 8, 2021

Closing this due to inactivity and in favor of #4905.

@guggero guggero closed this Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features / behaviour psbt rpc Related to the RPC interface wallet The wallet (lnwallet) which LND uses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add spend_unconfirmed param to estimateFee
4 participants