rpc: Add script verification flags to getdeploymentinfo #28806

ajtowns · 2023-11-07T00:59:07Z

Adds a script_flags field to the output of the getdeploymentinfo RPC that lists the flags selected by GetBlockScriptFlags(). This can be useful for seeing the detailed consequences of soft-fork activation behaviour, eg:

$ bitcoin-cli getdeploymentinfo | jq -j '.deployments | to_entries | .[] | (.value.height, " ", .key, "\n")'
227931 bip34
363725 bip66
388381 bip65
419328 csv
481824 segwit
709632 taproot
$ for a in 1 170060 227931 363725 388381 419328 481824 692261 709632; do echo -n "$a: "; bitcoin-cli getdeploymentinfo $(bitcoin-cli getblockhash $a) | jq -r .script_flags; done
1: P2SH,TAPROOT,WITNESS
170060: 
227931: P2SH,TAPROOT,WITNESS
363725: DERSIG,P2SH,TAPROOT,WITNESS
388381: CHECKLOCKTIMEVERIFY,DERSIG,P2SH,TAPROOT,WITNESS
419328: CHECKLOCKTIMEVERIFY,CHECKSEQUENCEVERIFY,DERSIG,P2SH,TAPROOT,WITNESS
481824: CHECKLOCKTIMEVERIFY,CHECKSEQUENCEVERIFY,DERSIG,NULLDUMMY,P2SH,TAPROOT,WITNESS
692261: CHECKLOCKTIMEVERIFY,CHECKSEQUENCEVERIFY,DERSIG,NULLDUMMY,P2SH,WITNESS
709632: CHECKLOCKTIMEVERIFY,CHECKSEQUENCEVERIFY,DERSIG,NULLDUMMY,P2SH,TAPROOT,WITNESS

This allows you to see that P2SH, WITNESS and TAPROOT are enforced since genesis; that the p2sh exception block (170060) has none of these extra rules enforced, that the taproot exception block (692261) is exempted from taproot enforcement, and exactly which rules were activated by each soft fork (eg, NULLDUMMY as part of the segwit soft fork).

DrahtBot · 2023-11-07T00:59:10Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	kristapsk

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29745 (bench: Adds a benchmark for CheckInputScripts by kevkevinpal)
#29280 (Implement OP_CHECKTEMPLATEVERIFY by reardencode)
#29270 (Implement OP_CHECKSIGFROMSTACK(VERIFY) by reardencode)
#29269 (Add OP_INTERNALKEY for Tapscript by reardencode)
#29039 (versionbits refactoring by ajtowns)

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 · 2023-11-07T01:02:34Z

This patch has been included in bitcoin-inquisition for a few releases (bitcoin-inquisition#5, bitcoin-inquisition#15 and bitcoin-inquisition#31). I used the same technique when in #23536 (review) and found it helpful.

maflcko

left some nits/questions. Feel free to ignore.

src/deploymentinfo.cpp

maflcko · 2023-11-07T09:05:59Z

src/deploymentinfo.cpp

+
+std::string FormatScriptFlags(uint32_t flags)
+{
+    if (flags == 0) return "";


Is this ever hit? If not, maybe it can be removed, as the remaining code should reach the same conclusion anyway in the rare case that this is hit?

cc @aureleoules for the coverage report

No coverage data available for this pull request and commit.

Thanks for the ping, I re-ran the job. AWS preempted the job and I haven't figured out how to rerun them automatically yet.
Anyone in this org can re-run corecheck jobs by signing in as well!

Is this ever hit? If not, maybe it can be removed, as the remaining code should reach the same conclusion anyway in the rare case that this is hit?

It's hit for block 170060. I think it's worth having it be explicit, as it would also be reasonable to return "NONE" (as in SCRIPT_VERIFY_NONE) for that case.

src/deploymentinfo.h

kristapsk · 2023-11-08T10:58:40Z

Concept ACK

luke-jr

Getting some deja vu here - is there another PR for this already?

luke-jr · 2023-11-08T16:38:38Z

src/deploymentinfo.cpp

+
+#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
+const std::map<std::string, uint32_t> g_verify_flag_names{
+    FLAG_NAME(P2SH),


If we're exposing this, probably better to call it "BIP16" since p2wsh and p2tr(sometimes) are also p2sh.

"Pay to script hash" is the title of BIP16, and we use both "pay-to-script-hash" and "P2SH" as descriptions of the sh() descriptor in doc/descriptors.md, so I don't think this is confusing or needs any changes.

src/rpc/blockchain.cpp

luke-jr · 2023-11-08T16:40:07Z

src/rpc/blockchain.cpp

@@ -1359,6 +1360,7 @@ RPCHelpMan getdeploymentinfo()
            UniValue deploymentinfo(UniValue::VOBJ);
            deploymentinfo.pushKV("hash", blockindex->GetBlockHash().ToString());
            deploymentinfo.pushKV("height", blockindex->nHeight);
+            deploymentinfo.pushKV("script_flags", FormatScriptFlags(GetBlockScriptFlags(*blockindex, chainman)));


Maybe number form should also be available for users of libbitcoinconsensus?

Numbers aren't a standard (they'll readily change between releases; also true of the names) but they also aren't particularly useful as documentation, so don't think it's sensible to export them here.

luke-jr · 2023-11-08T16:42:30Z

Ah, #10730

ajtowns · 2023-11-08T23:22:11Z

Ah, #10730

"When we last discussed making scripts debuggable" -- this and #28802 are prereqs for "bitcoin-util evalscript" which I'm poking at on https://github.com/ajtowns/bitcoin/commits/202309-evalscript . Real debugging seems better done with smarter external tools like https://github.com/dgpv/bsst though.

Moves FormatScriptFlags logic into GetScriptFlagNames which returns a vector of strings. For completeness, also has GetScriptFlagNames report on any bits that do not match a known script flag.

achow101 · 2024-04-09T15:45:26Z

The PR didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open PRs.

Closing due to lack of interest.

DrahtBot added the RPC/REST/ZMQ label Nov 7, 2023

maflcko reviewed Nov 7, 2023

View reviewed changes

ajtowns force-pushed the 202311-depinfo-scriptflags branch from c8ea84c to 02d8461 Compare November 7, 2023 20:44

luke-jr suggested changes Nov 8, 2023

View reviewed changes

ajtowns force-pushed the 202311-depinfo-scriptflags branch from 02d8461 to 7f3ee40 Compare November 9, 2023 07:22

This was referenced Dec 9, 2023

versionbits refactoring #29039

Merged

Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes #29050

Closed

DrahtBot mentioned this pull request Dec 18, 2023

Covenant tools softfork #28550

Closed

This was referenced Jan 5, 2024

refactor(tidy): Use C++20 contains method #29191

Closed

OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE) #29198

Closed

DrahtBot added the CI failed label Jan 16, 2024

This was referenced Jan 18, 2024

Implement OP_CHECKSIGFROMSTACK(VERIFY) #29270

Closed

Add OP_INTERNALKEY for Tapscript #29269

Closed

Implement OP_CHECKTEMPLATEVERIFY #29280

Closed

ajtowns added 3 commits February 7, 2024 08:22

Move mapFlagNames and FormatScriptFlags logic to deploymentinfo.h

4cd4f10

Moves FormatScriptFlags logic into GetScriptFlagNames which returns a vector of strings. For completeness, also has GetScriptFlagNames report on any bits that do not match a known script flag.

validation: export GetBlockScriptFlags()

aa1f379

rpc: have getdeploymentinfo report script verify flags

b295371

ajtowns force-pushed the 202311-depinfo-scriptflags branch from 7f3ee40 to b295371 Compare February 6, 2024 22:51

DrahtBot removed the CI failed label Feb 6, 2024

DrahtBot mentioned this pull request Mar 27, 2024

bench: Adds a benchmark for CheckInputScripts #29745

Closed

ajtowns mentioned this pull request Apr 9, 2024

Backports for 27.x bitcoin-inquisition/bitcoin#52

Merged

achow101 closed this Apr 9, 2024

ajtowns mentioned this pull request May 8, 2024

bitcoin-util evalscript subcommand bitcoin-inquisition/bitcoin#58

Merged

bitcoin locked and limited conversation to collaborators Apr 9, 2025

rpc: Add script verification flags to getdeploymentinfo #28806

rpc: Add script verification flags to getdeploymentinfo #28806

Uh oh!

Conversation

ajtowns commented Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

ajtowns commented Nov 7, 2023

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

maflcko Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aureleoules Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

ajtowns Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kristapsk commented Nov 8, 2023

Uh oh!

luke-jr left a comment

Choose a reason for hiding this comment

Uh oh!

luke-jr Nov 8, 2023

Choose a reason for hiding this comment

Uh oh!

ajtowns Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

luke-jr Nov 8, 2023

Choose a reason for hiding this comment

Uh oh!

ajtowns Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Nov 8, 2023

Uh oh!

ajtowns commented Nov 8, 2023

Uh oh!

achow101 commented Apr 9, 2024

Uh oh!

Uh oh!

ajtowns commented Nov 7, 2023 •

edited

Loading

DrahtBot commented Nov 7, 2023 •

edited

Loading

maflcko Nov 7, 2023 •

edited

Loading