Page MenuHomePhabricator

Implement PSBT Structures and un/serialization methods per BIP 174
ClosedPublic

Authored by deadalnix on Tue, Oct 15, 14:20.

Details

Summary
Test Plan

Unfortunately, The PR dump all the tests at once at the end, so there are n test for the new code :/

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Tue, Oct 15, 14:20
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Oct 15, 14:20
Fabien requested changes to this revision.Fri, Oct 18, 08:31
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/script/sign.h
121 ↗(On Diff #13563)

Nit: missing dot.
stream. The resulting

437 ↗(On Diff #13563)

Nit: trailing space

453 ↗(On Diff #13563)

Simply use a.tx == b.tx ? The == operator is defined for CMutableTransaction.

524 ↗(On Diff #13563)

Nit: remove scriptWitness

530 ↗(On Diff #13563)

Dito.

562 ↗(On Diff #13563)

Why did you remove the check here ?

This revision now requires changes to proceed.Fri, Oct 18, 08:31
deadalnix added inline comments.Tue, Oct 22, 15:32
src/script/sign.h
562 ↗(On Diff #13563)

We only do what they call witness utxo. While we don't have segwit, we do use a similar sighash algorithm.

deadalnix updated this revision to Diff 13643.Tue, Oct 22, 16:45

Addres comments

deadalnix updated this revision to Diff 13664.Wed, Oct 23, 16:36

Rebase, ping?

jasonbcox added inline comments.
src/script/sign.h
104 ↗(On Diff #13664)

If our UTXOs are "witness utxos" then is it more accurate to have this value be 0x01 to match Core? This caused some mild confusion while comparing to the original PR.

463 ↗(On Diff #13664)

nit: unnecessary newline

437 ↗(On Diff #13563)

Nit: If you're going to reformat this comment, do it to the rest in this file that are the form /** ... */

jasonbcox requested changes to this revision.Wed, Oct 23, 22:34
jasonbcox added inline comments.
src/script/sign.h
169 ↗(On Diff #13664)

unsigned int -> uint64_t

This revision now requires changes to proceed.Wed, Oct 23, 22:34
deadalnix added inline comments.Tue, Oct 29, 13:46
src/script/sign.h
104 ↗(On Diff #13664)

There is no reason to set it to 1.

deadalnix updated this revision to Diff 13742.Tue, Oct 29, 16:23

Address various comments

Fabien accepted this revision.Wed, Oct 30, 10:32
jasonbcox accepted this revision.Wed, Oct 30, 16:44
This revision is now accepted and ready to land.Wed, Oct 30, 16:44