Page MenuHomePhabricator

Count OP_CHECKDATASIG and OP_CHECKDATASIGVERIFY as sigops when the proper flag is set.
ClosedPublic

Authored by deadalnix on Jul 26 2018, 20:56.

Details

Summary

As per title. Once the opcode activates, it needs to count as a sigops.

Depends on D1601

Test Plan

Added test cases.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
countcheckdatasig
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3010
Build 4113: Bitcoin ABC Buildbot (legacy)
Build 4112: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Jul 27 2018, 17:24
jasonbcox requested changes to this revision.Jul 27 2018, 17:30
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/test/sigopcount_tests.cpp
31 ↗(On Diff #4451)

This entire patch will have to be cleaned up once the hardfork activates. Rather than add a datasigops param, you could have a flags param instead. Then, copy the GetSigOpCount test, but use the STANDARD_CHECKDATASIG_VERIFY_FLAGS instead of STANDARD_SCRIPT_VERIFY_FLAGS. The benefits of this are: more concise CheckScriptSigOps function, easier to read tests, and cleanup is as simple as deleting one of the tests (if necessary).

This revision now requires changes to proceed.Jul 27 2018, 17:30

Rebase.

Having an API that do force one to specify the checkdatasig count is good. I don't see any situation where we clean that up, unless we change the way sigops are counted in the future and in that case, the whole test goes anyway.

This revision is now accepted and ready to land.Aug 5 2018, 21:33
This revision was automatically updated to reflect the committed changes.