Page MenuHomePhabricator


Authored by deadalnix on Aug 4 2018, 13:55.



As per title.

Depends on D1619, D1620 and D1605

Test Plan

Added a ton of test cases.

Diff Detail

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

Event Timeline

deadalnix created this revision.Aug 4 2018, 13:55
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 4 2018, 13:55
jasonbcox added inline comments.
108 ↗(On Diff #4506)

epxected -> expected

jasonbcox requested changes to this revision.Aug 6 2018, 19:26

I've gone over this diff twice now. It's quite large, but looks pretty great so far. Should be good once PushSig is clarified. It took me 10-20 minutes to figure out what was going on there.

345 ↗(On Diff #4506)

This needs some differentiation from the other PushSig(), since the other one always includes the SigHashType. Differentiation can be either a rename from PushSig() to PushSigWithoutHashType() or just adding a comment that explains that SigHashType is not appended when hash is provided directly.

1206 ↗(On Diff #4506)

TestBuilder's API is hilariously weird. Opened up a few tasks to fix that so it makes more sense:

This revision now requires changes to proceed.Aug 6 2018, 19:26
deadalnix updated this revision to Diff 4517.Aug 6 2018, 20:02
  • Fix typo
  • PushSig -> PushDataSig
jasonbcox accepted this revision.Aug 6 2018, 20:03
This revision is now accepted and ready to land.Aug 6 2018, 20:03
This revision was automatically updated to reflect the committed changes.