Page MenuHomePhabricator
Differential D1621

Implement OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY
ClosedPublic
Actions

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

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGING9ba4bfca513d: Implement OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY
rABC9ba4bfca513d: Implement OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY
Summary

As per title.

Depends on D1619, D1620 and D1605

Test Plan

Added a ton of test cases.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
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
teamcity added a subscriber: teamcity.Aug 4 2018, 13:58

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Aug 4 2018, 14:36

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B3014: Diff 4494.Aug 4 2018, 14:37
deadalnix updated this revision to Diff 4506.Aug 5 2018, 19:28

rebase

teamcity added a comment.Aug 5 2018, 19:50

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Aug 5 2018, 20:29

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B3022: Diff 4506.Aug 5 2018, 20:29
deadalnix added a child revision: D1625: Add activation code for OP_CHECKDATASIG.Aug 5 2018, 20:55
jasonbcox added a subscriber: jasonbcox.Aug 5 2018, 23:30
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
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
teamcity added a comment.Aug 6 2018, 20:04

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Closed by commit rABC9ba4bfca513d: Implement OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY (authored by deadalnix). · Explain WhyAug 6 2018, 20:15
This revision was automatically updated to reflect the committed changes.
teamcity added a comment.Aug 6 2018, 20:42

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B3029: Diff 4517.Aug 6 2018, 20:42

Revision Contents
Changeset List

PathSize
src/
script/
60 lines
1 line
2 lines
test/
220 lines
data/
279 lines
191 lines

Diff 4518

View Options

src/script/interpreter.cpp

Loading...
View Options

src/script/script_error.h

Loading...
View Options

src/script/script_error.cpp

Loading...
View Options

src/test/checkdatasig_tests.cpp

Loading...
View Options

src/test/data/script_tests.json

Loading...
View Options

src/test/script_tests.cpp

Loading...