Page MenuHomePhabricator

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

Authored by deadalnix on Oct 15 2019, 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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested changes to this revision.Oct 18 2019, 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.Oct 18 2019, 08:31
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.

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.Oct 23 2019, 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.Oct 23 2019, 22:34
src/script/sign.h
104 ↗(On Diff #13664)

There is no reason to set it to 1.

This revision is now accepted and ready to land.Oct 30 2019, 16:44