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

Use psbt to fund and sign transactions #1876

Closed
wants to merge 8 commits into from
Closed

Use psbt to fund and sign transactions #1876

wants to merge 8 commits into from

Conversation

sstone
Copy link
Member

@sstone sstone commented Jul 15, 2021

No description provided.

@sstone sstone force-pushed the use-psbt branch 3 times, most recently from 17032dc to f9478c1 Compare July 22, 2021 17:20
@sstone sstone force-pushed the use-psbt branch 3 times, most recently from cb31307 to 44dbaf3 Compare August 16, 2021 16:20
@sstone sstone force-pushed the use-psbt branch 7 times, most recently from e36d2e4 to ad60adf Compare August 24, 2021 07:03
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2021

Codecov Report

Merging #1876 (b4eb3cc) into master (273fae9) will decrease coverage by 0.02%.
The diff coverage is 81.25%.

@@            Coverage Diff             @@
##           master    #1876      +/-   ##
==========================================
- Coverage   87.65%   87.63%   -0.03%     
==========================================
  Files         158      158              
  Lines       12406    12467      +61     
  Branches      515      525      +10     
==========================================
+ Hits        10875    10925      +50     
- Misses       1531     1542      +11     
Impacted Files Coverage Δ
...-core/src/main/scala/fr/acinq/eclair/package.scala 65.57% <37.50%> (-9.99%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 88.27% <66.66%> (ø)
...ir/blockchain/bitcoind/rpc/BitcoinCoreClient.scala 88.75% <88.23%> (-5.49%) ⬇️
...clair/channel/publish/ReplaceableTxPublisher.scala 87.70% <92.85%> (+1.81%) ⬆️
...ir-core/src/main/scala/fr/acinq/eclair/Setup.scala 80.45% <100.00%> (ø)
...main/scala/fr/acinq/eclair/router/Validation.scala 92.30% <0.00%> (+1.53%) ⬆️

@t-bast
Copy link
Member

t-bast commented Sep 6, 2021

While reading this PR, I realized that:

  • the separation between ExtendedBitcoinClient and BitcoinCoreWallet doesn't make sense anymore, so I removed it in Clean up inconsistency between bitcoin client and wallet #1939 (which should be a pre-requisite for this PR)
  • I added the address conversion utilities to bitcoin-lib in Add support for Bech32m (BIP 350) bitcoin-lib#55 (with compatibility with future segwit versions)
  • handling psbts is a bit messy: we only have a single PartiallySignedInput type which doesn't reflect the state of the input and has optional fields everywhere, which makes it hard to check the progress of a psbt's inputs. I've done a big refactoring to add more types and make psbts more pleasant to work with in Improve PSBT types bitcoin-lib#60, I'm curious to have your feedback. I think it's important to nail down the right abstraction for psbt before we start using it widely inside eclair (e.g. we need to be very careful when we sign psbts, we must for example check sighash flags in case the other party modified it)

…bitcoin core

We add:
- the bip32 path for our local funding pubkey, without the first 2 items (that are based on which chain we're on)
- a dummy 1/2/3/4 path for the remote funding pubkey
This is used to fund PSBTs.
@sstone
Copy link
Member Author

sstone commented Feb 2, 2022

Superseded by #2120

@sstone sstone closed this Feb 2, 2022
@sstone sstone deleted the use-psbt branch June 28, 2022 11:53
@sstone sstone restored the use-psbt branch August 10, 2022 16:30
@sstone sstone deleted the use-psbt branch November 16, 2022 17:19
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