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

lncli: Allow lncli to read binary PSBTs from a file #7122

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

antonilol
Copy link
Contributor

@antonilol antonilol commented Nov 6, 2022

BIP 174 states that PSBTs can be encoded in 2 formats:

  • plain text as base64
  • in a file as binary

With this change, lncli will also try to read a PSBT as binary

Change Description

Description of change / link to associated issue.

closes #7119

Read PSBT/hex transaction based on the prefix, which is different for every of the 3:

  • binary psbt: "psbt\xff"
  • base64 psbt: "cHNidP"
  • hex tx: at least 2 hex chars, none of the above do

Steps to Test

Steps for reviewers to follow to test the change.

lncli openchannel <pubkey> <amount> --psbt

the first input box (funded psbt) allows to enter:

  • a base64 psbt
  • file path to a base64 psbt
  • file path to a binary psbt (new)

the second input box (signed psbt/tx) also allows the above 3, and hex encoded transactions

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

@antonilol antonilol force-pushed the read-binary-psbt branch 2 times, most recently from 89781b7 to ae5316f Compare November 6, 2022 11:49
@guggero guggero self-requested a review November 7, 2022 10:15
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

tACK, very nice!

One small thing that we could also update now is the message in the CLI that now shows (or path to text file): , that could just be or path to file):

cmd/lncli/cmd_open_channel.go Outdated Show resolved Hide resolved
@antonilol
Copy link
Contributor Author

One small thing that we could also update now is the message in the CLI that now shows (or path to text file): , that could just be or path to file):

makes sense, changed it

@guggero
Copy link
Collaborator

guggero commented Nov 17, 2022

Needs a rebase, then this is good to go.

BIP 174 states that PSBTs can be encoded in 2 formats:
- plain text as base64
- in a file as binary

With this change, lncli will also try to read a PSBT as binary
@guggero guggero merged commit 00006ad into lightningnetwork:master Nov 17, 2022
@antonilol antonilol deleted the read-binary-psbt branch November 17, 2022 12:38
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.

allow reading .psbt files during psbt funding flow as specified in bip 174
3 participants