OP_CHECKCONTRACTVERIFY

[bigspider]

This is a port of the previous PR for OP_CHECKCONTRACTVERIFY in core, matching the specifications in BIP-443.

Implementation with mutex

There are several ways the opcode's cross-input logic could be implemented, some without changing the semantics and some that make functionality tradeoffs to simplify the implementation. In this PR, I chose to use the keep the most direct implementation that uses a mutex for the synchronization of the cross-input logic, like in the linked bitcoin-core PR. The analysis of the various options will certainly deserve more careful consideration for an implementation in bitcoin-core, as cross-input Script evaluation logic is currently a new concept in the Script interpreter (although implicitly introduced in taproot, if Schnorr batch validation is implemented)

Sigops budget (draft)

Apart from rebasing and some minor style changes in the code, this PR adds a simple benchmark to compare with Schnorr signature checks, and a rough sigops budget pricing (which is still left as a TODO in the BIP). This will also need deserve more care in view of the fact that overpricing might cause some reasonable use cases to run out of sigops budget (forcing to stuff the witness just for the sake of gaining some more budget). I am currently not aware of such situations, but I hope that having the opcode available in signet might help with discovery. Underpricing it, of course, would cause the usual DoS concerns.

In this PR, CCV is priced as a signature check in the heaviest case with two tweaks, half if one tweak is skipped, and 0 otherwise.

[bigspider]

This is currently failing the script_tests/script_assets_test, as redefining 0xbb from OP_SUCCESS187 to OP_CHECKCONTRACTVERIFY makes some testcases fail. The failing tests are testing the scripts OP_SUCCESS187 and 0 OP_IF OP_SUCCESS187 OP_ENDIF - so basically testing the OP_SUCCESS behavior.

I assume I must be doing something wrong in the activation logic - any advice would be greatly appreciated.

[Christewart]

Hey @bigspider , glad to see this show up here! Hopefully I can be a little helpful

I assume I must be doing something wrong in the activation logic - any advice would be greatly appreciated.

I didn't look at this super closely, but taking your word for it, if the Script that is failing is

0 OP_IF OP_SUCCESS187 OP_ENDIF

That means nothing ends up on the stack, which is equivalent to the empty byte vector and thus a Script failure correct? Previously [IsOpSuccess()] would return true, and now it returns false with the changes in 8b34c8c. The previous behavior would see the OP_SUCCESS187 (rather than OP_CCV) and trivially make the Script succeed. So I believe that this test case is failing as expected.

I believe you would want to make the IsOpSuccess() dependent on a new sigversion. See this PR which hopefully is helpful bitcoin#29265

Implementation with mutex

I'm still interested in this subject and will come back later and take a look at this. This is an interesting design space imo

[bigspider]

That means nothing ends up on the stack, which is equivalent to the empty byte vector and thus a Script failure correct? Previously [IsOpSuccess()] would return true, and now it returns false with the changes in 8b34c8c. The previous behavior would see the OP_SUCCESS187 (rather than OP_CCV) and trivially make the Script succeed. So I believe that this test case is failing as expected.

Yes, the test is just checking the expected OP_SUCCESS behaviour of 0xbb, so it should only be run without SCRIPT_ENABLE_CHECKCONTRACTVERIFY. I think binana's magic in inquisition should be able handle that, therefore I probably did something wrong.

Summoning @ajtowns if you kindly have time to check.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #108 (PoC: tooling for OP_TEMPLATEHASH by darosior)
• #104 (TAPSCRIPT_V2 (Great Script Restoration) by jmoik)
• #100 (BIP446 "OP_TEMPLATEHASH" implementation by darosior)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[ajtowns]

Hmm, fyi I've been ignoring this since the lint CI is complaining and those are usually trivial fixes.

I think to make the op_success tests pass, you generally need to make sure that the op_success being used to test op_success behaviour isn't being redefined via activation params -- inquisition regtest tends to automatically enable every activation ASAP, so test cases will see the new behaviour, not the old op_success behaviour. Just bumping the op_success-test-case opcode to one that hasn't been redefined yet is probably best.

[bigspider]

@ajtowns thanks for the reply!
I rebased to the latest 29.x, fixed the previous linting errors (I'll be on the lookout if there's any new ones), and implemented some minor simplifications in the python functional tests.

Unless something changed, I still expect the script_assets_test to fail.

I think to make the op_success tests pass, you generally need to make sure that the op_success being used to test op_success behaviour isn't being redefined via activation params -- inquisition regtest tends to automatically enable every activation ASAP, so test cases will see the new behaviour, not the old op_success behaviour. Just bumping the op_success-test-case opcode to one that hasn't been redefined yet is probably best.

The testcase is pulled from bitcoin-core/qa-assets. The failing testcases are:

• opsuccess_bare with Script 0xbb
• opsuccess_unexecif with Script 0x0063bb68, that is 0 IF 0xbb ENDIF

Should I look into how to regenerate script_assets_test.json without the tests above (or switching to another unused OP_SUCCESS)? Note that this is shared with bitcoin-core, where of course the test remains correct.