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

output script descriptors, part 1: change API of transaction.py #8230

Merged
merged 18 commits into from
Mar 4, 2023

Conversation

SomberNight
Copy link
Member

This is the first set of changes towards adding support for output script descriptors (#5694).

  • a new module, descriptor.py, is added, which is originally from bitcoin-core/HWI but non-trivially changed in parts and extended
  • transaction.py is adapted to use the new module, changing some APIs there, mostly for PartialTxInput
    • the logic in PartialTxInput re how to satisfy a locking script is mostly removed; that is now handled by descriptor.py instead
    • when creating a txin programmatically, instead of having to set script_type, num_sig, pubkeys, one will just now set a new field, script_descriptor instead
    • the {PartialTxInput,PartialTxOutput}.script_type field is removed
    • PartialTxInput.num_sig, which was a hack for solving multisig scripts, is removed
    • the .pubkeys field no longer has to be set explicitly, it is instead a method (property) derived from .script_descriptor
  • lots of files are touched to adapt to the changed transaction.py API
    • in particular, the HWW plugins are not fully adapted - I've added some clutches to help transition them instead, for now
  • there are no wallet changes here yet, that will come separately

@SomberNight SomberNight added topic-transactions 📑 related to logic in transaction.py topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py labels Feb 27, 2023
@SomberNight SomberNight marked this pull request as draft February 27, 2023 16:51
@SomberNight
Copy link
Member Author

SomberNight commented Feb 27, 2023

Changed to draft as I've found a regression in the trezor plugin.

EDIT: fixed in 9e7c366

@SomberNight SomberNight marked this pull request as ready for review February 28, 2023 19:28
@ecdsa ecdsa merged commit 798cd60 into spesmilo:master Mar 4, 2023
SomberNight added a commit that referenced this pull request May 9, 2023
fix signing txs when using old "bitcoin app" (pre-2.1) on the ledger device

```
 33.36 | W | transaction | heyheyhey. cp1. include_sigs=True force_legacy=False use_segwit_ser=True
 33.36 | W | transaction | heyheyhey. cp2. branch1
 33.37 | E | plugins.ledger.ledger |
Traceback (most recent call last):
  File "...\electrum\electrum\plugins\ledger\ledger.py", line 669, in sign_transaction
    rawTx = tx.serialize_to_network()
  File "...\electrum\electrum\transaction.py", line 945, in serialize_to_network
    witness = ''.join(self.serialize_witness(x, estimate_size=estimate_size) for x in inputs)
  File "...\electrum\electrum\transaction.py", line 945, in <genexpr>
    witness = ''.join(self.serialize_witness(x, estimate_size=estimate_size) for x in inputs)
  File "...\electrum\electrum\transaction.py", line 839, in serialize_witness
    sol = desc.satisfy(allow_dummy=estimate_size, sigdata=txin.part_sigs)
  File "...\electrum\electrum\descriptor.py", line 378, in satisfy
    sol = self._satisfy_inner(sigdata=sigdata, allow_dummy=allow_dummy)
  File "...\electrum\electrum\descriptor.py", line 574, in _satisfy_inner
    raise MissingSolutionPiece(f"no sig for {pubkey.hex()}")
electrum.descriptor.MissingSolutionPiece: no sig for 033e92e55923ea25809790f292ee9bd410355ee02492472d9a1ff1b364874d0679
 33.38 | I | plugins.ledger.ledger | no sig for 033e92e55923ea25809790f292ee9bd410355ee02492472d9a1ff1b364874d0679
```

fixes #8365
regression from #8230
SomberNight added a commit that referenced this pull request Jun 2, 2023
same as 9e13246

fixes #8463
regression from #8230
SomberNight added a commit that referenced this pull request Jun 2, 2023
same as 9e13246

fixes #8463
regression from #8230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-transactions 📑 related to logic in transaction.py topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants