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

Add joinpsbts functionality #76

Merged
merged 13 commits into from Jan 9, 2023
Merged

Add joinpsbts functionality #76

merged 13 commits into from Jan 9, 2023

Conversation

St333p
Copy link
Collaborator

@St333p St333p commented Dec 13, 2022

Bitcoin Core has a function that allows to merge multiple unsigned PSBTs into one which contains the union of inputs and outputs from the input PSBTs. This is relevant for every usecase in which the set of inputs and outputs of a transaction is collaboratively defined by a set of entities that do not necessarily have a coordinator; for instance it is often used in coinjoin/payjoin transactions.

This PR aims to bring a similar functionality to btclib, choosing flexibility where possible but striving to keep compliance with bitcoin core as the default behavior.

giacomocaironi
giacomocaironi previously approved these changes Dec 27, 2022
@giacomocaironi giacomocaironi marked this pull request as ready for review December 27, 2022 14:14
@fametrano
Copy link
Member

fametrano commented Dec 31, 2022

please rebase and check that all new 5 GitHib actions succed

@fametrano fametrano dismissed giacomocaironi’s stale review January 2, 2023 15:02

should be rebased before being merged

@fametrano fametrano removed the request for review from giacomocaironi January 2, 2023 15:02
@btclib-org btclib-org deleted a comment from sourcery-ai bot Jan 8, 2023
btclib/psbt/psbt.py Outdated Show resolved Hide resolved
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 9, 2023

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 1.35%.

Quality metrics Before After Change
Complexity 1.96 ⭐ 2.08 ⭐ 0.12 👎
Method Length 85.88 🙂 93.24 🙂 7.36 👎
Working memory 7.51 🙂 7.71 🙂 0.20 👎
Quality 68.86% 🙂 67.51% 🙂 -1.35% 👎
Other metrics Before After Change
Lines 1728 2205 477
Changed files Quality Before Quality After Quality Change
btclib/bip32/key_origin.py 88.58% ⭐ 88.02% ⭐ -0.56% 👎
btclib/psbt/__init__.py 80.59% ⭐ 69.66% 🙂 -10.93% 👎
btclib/psbt/psbt.py 60.51% 🙂 61.12% 🙂 0.61% 👍
btclib/tx/__init__.py 94.23% ⭐ 91.08% ⭐ -3.15% 👎
btclib/tx/tx.py 80.51% ⭐ 74.62% 🙂 -5.89% 👎
tests/bip32/test_key_origin.py 68.39% 🙂 68.53% 🙂 0.14% 👍
tests/psbt/test_psbt.py 74.03% 🙂 71.98% 🙂 -2.05% 👎
tests/tx/test_tx.py 60.10% 🙂 59.35% 🙂 -0.75% 👎

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
btclib/psbt/psbt.py Psbt.assert_signable 22 😞 163 😞 11 😞 40.42% 😞 Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
btclib/psbt/psbt.py Psbt.parse 10 🙂 188 😞 15 😞 41.77% 😞 Try splitting into smaller methods. Extract out complex expressions
tests/tx/test_tx.py test_tx 0 ⭐ 697 ⛔ 12 😞 46.17% 😞 Try splitting into smaller methods. Extract out complex expressions
btclib/psbt/psbt.py join_psbts 5 ⭐ 158 😞 16 ⛔ 48.14% 😞 Try splitting into smaller methods. Extract out complex expressions
tests/psbt/test_psbt.py test_vectors_bip174 5 ⭐ 208 ⛔ 12 😞 49.23% 😞 Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

St333p and others added 12 commits January 9, 2023 18:09
add method to merge psbts

list expansion in function inputs

fix outpoint is not hashable

add initial basic test for join_txs

test few error cases

add minimal test for join_psbts

raise proper error messages in case of join failures instead of AssertionError

avoid privacy leak from input/output ordering, see: bitcoin/bitcoin#16512

refactor to reduce method complexity

fix linting issues and error messages

add failure test cases to bring coverage back to 100%

'Refactored by Sourcery'

allow to sort inputs and outputs when joining

also provide methods to sort psbt_{in|out} inplace

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

cleaned up

cleaned up

make HdKeyPaths a MutableMapping

cleaned up _sort_together, added tests to complete coverage

fixed checking of errors

fixed error messages
@fametrano fametrano merged commit bdb3674 into master Jan 9, 2023
@fametrano fametrano deleted the joinpsbts branch January 29, 2023 12:01
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.

None yet

3 participants