Page MenuHomePhabricator

Implement OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY
ClosedPublic

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

Details

Summary

As per title.

Depends on D1619, D1620 and D1605

Test Plan

Added a ton of test cases.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
checkdatasig
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3014
Build 4121: Bitcoin ABC Buildbot (legacy)
Build 4120: arc lint + arc unit

Event Timeline

jasonbcox added inline comments.
src/test/checkdatasig_tests.cpp
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.

src/test/script_tests.cpp
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: https://reviews.bitcoinabc.org/T392

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