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

PSBT v2 #330

Merged
merged 122 commits into from
Sep 19, 2022
Merged

PSBT v2 #330

merged 122 commits into from
Sep 19, 2022

Conversation

jgriffiths
Copy link
Contributor

No description provided.

@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 4 times, most recently from 76fb7dd to 8c7ec7f Compare June 3, 2022 23:11
@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 10 times, most recently from 71ad950 to b8b79a3 Compare June 13, 2022 22:16
@jgriffiths jgriffiths changed the title WIP: PSBT v2 PSBT v2 Jun 14, 2022
@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 2 times, most recently from 9c7afdb to 7d035ed Compare June 16, 2022 09:43
@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 2 times, most recently from 56aa789 to f8dac44 Compare July 5, 2022 11:09
@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 3 times, most recently from 5a14d04 to 26aef05 Compare July 19, 2022 02:58
@jgriffiths jgriffiths force-pushed the psbt_v2_merge branch 3 times, most recently from 742652e to df3e69a Compare July 22, 2022 00:16
Allen Piscitello and others added 4 commits July 22, 2022 13:22
The addition of new allocations for v2 inputs and outputs means the
error handling logic must be updated. Some new checks are needed for v2
which in the v0 path are caught by the tx modification path.

Additionally, there was a long-existing bug in the old code where memory
allocation failure would cause the number of in/outputs to be off-by-one.
This flag allows internal functions to be exposed through the Python
CFFI tests.
This follows the convention that cryptographically unfortunate or
malicious inputs are rejected with this error, allow the caller to retry
with new inputs. In the Elements case where ECDH is used during
blinding, this indicates that a new ephemeral key should be generated,
for example.
This allows SWIG exposed languages to avoid a lot of boilerplate copying
txout values over.
Some shared-blinding scenarios require them.
Also fix compilation for non-elements builds.
As noted, these calls may be used for other purposes by callers.
@JamieDriver
Copy link
Contributor

I'm happy to call it a utack and iterate/fix-forward as required. (e1fc3cd)

@jgriffiths jgriffiths merged commit e1fc3cd into master Sep 19, 2022
@Sjors
Copy link
Contributor

Sjors commented Nov 1, 2022

@Sjors Would you mind taking a look at these changes (at least, the interface changes for PSBT v0 in CHANGES.md)?

Please let me know if you find any issues or would like anything changed, thanks.

Oops, I missed this :-)

When trying to bump to v0.8.6 in LibWallySwift, something did break and I didn't find CHANGES.md very clear on what to do:

  • wally_psbt_output.witness_script no longer exists, since 8f8481a; still figuring out how to use the psbt_fields map where this moved

@jgriffiths
Copy link
Contributor Author

@Sjors include/wally_psbt_members.h has functions to get and set all members through the top-level psbt object, for the sub objects like inputs and outputs there are only setter functions so that you can create/populate one and add it (then use the top level calls to modify it once added).

This interface is because most wrapped languages can't return a reference to sub-objects once they are embedded in a parent (shared ownership semantics don't transfer well, and taking/returning sub-objects by value is very inefficient).

I'm not against adding getters for the sub-objects for non-swig wrapped language use although it may be a while before I have time to so so.

@Sjors
Copy link
Contributor

Sjors commented Nov 24, 2022

I ended up using wally_psbt_get_output_witness_script(_len) (might be good to add them to the docs). It's not too bad. Although it's slightly inelegant to have to pass wally_psbt around, it's probably not a performance issue on a typical iOs device.

Sjors added a commit to Sjors/libwally-swift that referenced this pull request Nov 24, 2022
The secp256k1-zkp submodule at CLibWally/libwally-core/src/secp256k1 is updated along with libwally-core. In build-libwally.sh script we switch back to a vanilla  libwally at the same commit that Bitcoin Core uses.

Although Bitcoin Core bumped that commit a few months ago, we don't change it here because secp256k1-zkp has not yet been rebased on it.

There were two breaking change in libwally-core:

1. wally_psbt_from_bytes got an additional FLAGS argument in 0fb94751def8b2c767680e2428ebe2fdabd436c7 (ElementsProject/libwally-core#336): trivially fixed by setting it to 0

2. wally_psbt_output no longer has a witness_script field as of 8f8481a3c509fc4a02425db3839f92594e6ea852  (ElementsProject/libwally-core#330). This required a more tedious workaround.
Sjors added a commit to Sjors/libwally-swift that referenced this pull request Nov 25, 2022
The secp256k1-zkp submodule at CLibWally/libwally-core/src/secp256k1 is updated along with libwally-core. In build-libwally.sh script we switch back to a vanilla  libwally at the same commit that Bitcoin Core uses.

Although Bitcoin Core bumped that commit a few months ago, we don't change it here because secp256k1-zkp has not yet been rebased on it.

There were two breaking change in libwally-core:

1. wally_psbt_from_bytes got an additional FLAGS argument in 0fb94751def8b2c767680e2428ebe2fdabd436c7 (ElementsProject/libwally-core#336): trivially fixed by setting it to 0

2. wally_psbt_output no longer has a witness_script field as of 8f8481a3c509fc4a02425db3839f92594e6ea852  (ElementsProject/libwally-core#330). This required a more tedious workaround.
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.

4 participants