Skip to content

Implement BIP 349 validation (OP_INTERNALKEY) #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 16, 2025
Merged

Implement BIP 349 validation (OP_INTERNALKEY) #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7a59b4
consensus: implement OP_INTERNALKEY BIP349
instagibbs Aug 18, 2025
6c3fa1e
test: basic BIP349 unit test
instagibbs Mar 13, 2025
4287a3a
test: functional test for bip349
instagibbs Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/binana/internalkey.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"binana": [2024, 4, 0],
"deployment": "INTERNALKEY",
"scriptverify": true,
"scriptverify_discourage": true,
"opcodes": {
"INTERNALKEY": "0xcb"
}
}
13 changes: 13 additions & 0 deletions src/script/interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,19 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
break;
}

case OP_INTERNALKEY: {
// OP_INTERNALKEY is only available in Tapscript
if (sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0) {
Copy link

@ajtowns ajtowns Aug 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(sigversion != BASE && sigversion != WITNESS_V0) ?
assert(flags & SCRIPT_VERIFY_INTERNALKEY) ?

Copy link
Author

@instagibbs instagibbs Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(sigversion != BASE && sigversion != WITNESS_V0)

I don't believe this is correct; added test that fails with this change.

assert(flags & SCRIPT_VERIFY_INTERNALKEY)

This ends up being true post-activation, but I don't think would be prior?

We could add OP_INTERNALKEY to the disabled opcode area ala OP_CAT?

Copy link

@ajtowns ajtowns Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is correct; added test that fails with this change.

Right; disabled opcodes are checked beforehand so can have an assert, but everything needs to duplicate the set_error from the default: section. 👍

assert(flags & SCRIPT_VERIFY_INTERNALKEY)

This ends up being true post-activation, but I don't think would be prior?

Prior to activation, in the taproot case, it should be treated as OP_SUCCESS and this code shouldn't be reachable (CheckTapscriptOpSuccess will hit op_success_check via INQ_SUCCESS_OPCODES and either flags & discourage will force an error, or !(flags & enable) will force a success).

Copy link
Author

@instagibbs instagibbs Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, if I leave in the script context gating, it will be fine!

return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
}
// Always present in Tapscript
assert(flags & SCRIPT_VERIFY_INTERNALKEY);
assert(sigversion == SigVersion::TAPSCRIPT);
assert(execdata.m_internal_key);
Copy link

@ajtowns ajtowns Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(sigversion == TAPSCRIPT) in that case?

Copy link
Author

@instagibbs instagibbs Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

stack.emplace_back(execdata.m_internal_key->begin(), execdata.m_internal_key->end());
break;
}

default:
return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
}
Expand Down
8 changes: 8 additions & 0 deletions src/test/data/tx_valid.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,14 @@
[[["1111111111111111111111111111111111111111111111111111111111111111", 0, "0x00 0x14 0x751e76e8199196d454941c45d1b3a323f1433bd6", 5000000]],
"0100000000010111111111111111111111111111111111111111111111111111111111111111110000000000ffffffff0130244c0000000000fd02014cdc1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111175210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ac02483045022100c1a4a6581996a7fdfea77d58d537955a5655c1d619b6f3ab6874f28bb2e19708022056402db6fede03caae045a3be616a1a2d0919a475ed4be828dc9ff21f24063aa01210279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179800000000", "NONE"],

["Test OP_INTERNALKEY"],
[[["e2f2baee9c59389b34e39742ce05debf64aaa7a00fbdab88614f4d3c133186d5",
0,
"1 0x20 0xa9e62de0f9782710f702214fc81c0f0f90fb3537987b3685caad6d52db305447",
155000]],
"02000000000101d58631133c4d4f6188abbd0fa0a7aa64bfde05ce4297e3349b38599ceebaf2e20000000000ffffffff01f0490200000000002251202ca3bc76489a54904ad2507005789afc1e6b362b451be89f69de39ddf9ba8abf0223cb2050929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac08721c050929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac000000000",
"DISCOURAGE_INTERNALKEY"],

["Test OP_CHECKSIGFROMSTACK"],
[[["e2f2baee9c59389b34e39742ce05debf64aaa7a00fbdab88614f4d3c133186d5",
0,
Expand Down
65 changes: 54 additions & 11 deletions test/functional/feature_taproot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
OP_EQUAL,
OP_EQUALVERIFY,
OP_IF,
OP_INTERNALKEY,
OP_NOP,
OP_NOT,
OP_NOTIF,
Expand Down Expand Up @@ -666,6 +667,22 @@ def byte_popper(expr):

# === Actual test cases ===

def spenders_internalkey_active():

secs = [generate_privkey() for _ in range(8)]
Copy link

@benthecarman benthecarman Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit you're generating 8 keys here but only using [0] and [1]

Copy link
Author

@instagibbs instagibbs Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will touch if I do anything else

pubs = [compute_xonly_pubkey(sec)[0] for sec in secs]

spenders = []

scripts = [
("ik", CScript([OP_INTERNALKEY, OP_EQUAL])),
]

tap = taproot_construct(pubs[0], scripts)

add_spender(spenders, "ik/success", tap=tap, leaf="ik", inputs=[pubs[0]], failure={"inputs": [pubs[1]]})

return spenders

def spenders_taproot_active():
"""Return a list of Spenders for testing post-Taproot activation behavior."""
Expand Down Expand Up @@ -1255,7 +1272,7 @@ def predict_sigops_ratio(n, dummy_size):
# For the standard non-witness p2sh case, we need inputs to be minimal push opcodes (not witness stack elements)
# so we use arb non-0 byte push via valid pubkey
add_spender(spenders, "compat/nocsfs", p2sh=p2sh, witv0=witv0, standard=p2sh or witv0, script=CScript([OP_IF, b'', b'', pubs[0], OP_CHECKSIGFROMSTACK, OP_DROP, OP_ENDIF]), inputs=[pubs[0], b''], failure={"inputs": [pubs[0], pubs[0]]}, **ERR_UNDECODABLE)

add_spender(spenders, "compat/noik", p2sh=p2sh, witv0=witv0, standard=p2sh or witv0, script=CScript([OP_IF, OP_INTERNALKEY, OP_RETURN, OP_ENDIF]), inputs=[pubs[0], b''], failure={"inputs": [pubs[0], pubs[0]]}, **ERR_UNDECODABLE)
return spenders


Expand Down Expand Up @@ -1299,6 +1316,25 @@ def bip348_csfs_spenders_nonstandard():

return spenders

def bip349_ik_spenders_nonstandard():
"""Spenders for testing that pre-active INTERNALKEY usage is discouraged but valid"""

spenders = []

sec = generate_privkey()
pub, _ = compute_xonly_pubkey(sec)
scripts = [
("stilltrue", CScript([OP_INTERNALKEY])),
("still_opsuccess", CScript([OP_RETURN, OP_INTERNALKEY])),
]
tap = taproot_construct(pub, scripts)

# Valid prior to activation but nonstandard
add_spender(spenders, "discouraged_ik/stilltrue", tap=tap, leaf="stilltrue", standard=False)
add_spender(spenders, "discouraged_ik/still_opsuccess", tap=tap, leaf="still_opsuccess", standard=False)

return spenders

def bip348_csfs_spenders():
secs = [generate_privkey() for _ in range(2)]
pubs = [compute_xonly_pubkey(sec)[0] for sec in secs]
Expand Down Expand Up @@ -1417,7 +1453,8 @@ def skip_test_if_missing_module(self):

def set_test_params(self):
self.num_nodes = 1
self.extra_args = [["-vbparams=checksigfromstack:0:3999999999"]]
self.extra_args = [["-vbparams=checksigfromstack:0:3999999999",
"-vbparams=internalkey:0:3999999999"]]
self.setup_clean_chain = True

def block_submit(self, node, txs, msg, err_msg, cb_pubkey=None, fees=0, sigops_weight=0, witness=False, accept=False):
Expand Down Expand Up @@ -1890,25 +1927,31 @@ def pr(node):
def run_test(self):
self.gen_test_vectors()

self.log.info("CSFS Pre-activation tests...")
self.log.info("CSFS and IK Pre-activation tests...")
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["checksigfromstack"]["heretical"]["status"],"defined")
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["internalkey"]["heretical"]["status"],"defined")
self.generate(self.nodes[0], 144)
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["checksigfromstack"]["heretical"]["status"],"started")
signal_ver = int(self.nodes[0].getdeploymentinfo()["deployments"]["checksigfromstack"]["heretical"]["signal_activate"], 16)
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["internalkey"]["heretical"]["status"],"started")
signal_ver_csfs = int(self.nodes[0].getdeploymentinfo()["deployments"]["checksigfromstack"]["heretical"]["signal_activate"], 16)
signal_ver_ik = int(self.nodes[0].getdeploymentinfo()["deployments"]["internalkey"]["heretical"]["signal_activate"], 16)

self.test_spenders(self.nodes[0], bip348_csfs_spenders_nonstandard(), input_counts=[1, 2])
self.test_spenders(self.nodes[0], bip348_csfs_spenders_nonstandard() + bip349_ik_spenders_nonstandard(), input_counts=[1, 2])

self.log.info("Activating CSFS")
self.log.info("Activating CSFS and IK")
now = self.nodes[0].getblock(self.nodes[0].getbestblockhash())["time"]
coinbase_tx = create_coinbase(self.nodes[0].getblockcount() + 1)
block = create_block(hashprev=int(self.nodes[0].getbestblockhash(), 16), ntime=now, coinbase=coinbase_tx, version=signal_ver)
block.solve()
self.nodes[0].submitblock(block.serialize().hex())
for signal in [signal_ver_csfs, signal_ver_ik]:
coinbase_tx = create_coinbase(self.nodes[0].getblockcount() + 1)
block = create_block(hashprev=int(self.nodes[0].getbestblockhash(), 16), ntime=now, coinbase=coinbase_tx, version=signal)
block.solve()
self.nodes[0].submitblock(block.serialize().hex())
now += 1
self.generate(self.nodes[0], 288)
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["checksigfromstack"]["heretical"]["status"],"active")
assert_equal(self.nodes[0].getdeploymentinfo()["deployments"]["internalkey"]["heretical"]["status"],"active")

self.log.info("Post-activation tests...")
consensus_spenders = spenders_taproot_active() + bip348_csfs_spenders()
consensus_spenders = spenders_taproot_active() + bip348_csfs_spenders() + spenders_internalkey_active()
self.test_spenders(self.nodes[0], consensus_spenders, input_counts=[1, 2, 2, 2, 2, 3])
# Run each test twice; once in isolation, and once combined with others. Testing in isolation
# means that the standardness is verified in every test (as combined transactions are only standard
Expand Down
3 changes: 2 additions & 1 deletion test/functional/test_framework/script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ def __new__(cls, n):
OP_CHECKSIGADD = CScriptOp(0xba)

OP_CHECKSIGFROMSTACK = CScriptOp(0xcc)
OP_INTERNALKEY = CScriptOp(0xcb)

OP_INVALIDOPCODE = CScriptOp(0xff)

Expand Down Expand Up @@ -968,7 +969,7 @@ def taproot_construct(pubkey, scripts=None, *, keyver=None, treat_internal_as_in
return TaprootInfo(CScript([OP_1, tweaked]), pubkey, negated + 0, tweak, leaves, h, tweaked, keyver)

def is_op_success(o):
if o in [OP_CAT, OP_CHECKSIGFROMSTACK]:
if o in [OP_CAT, OP_CHECKSIGFROMSTACK, OP_INTERNALKEY]:
return False
Copy link

@ajtowns ajtowns Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess binana.json should be generating these too.

Copy link

@ajtowns ajtowns Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue, #93. Wasn't obvious to me how to sensibly make these functions depend on info in ../data/binana.json (or if that should get changed to binana.py like invalid_txs.py, or something else).


return o == 0x50 or o == 0x62 or o == 0x89 or o == 0x8a or o == 0x8d or o == 0x8e or (o >= 0x7e and o <= 0x81) or (o >= 0x83 and o <= 0x86) or (o >= 0x95 and o <= 0x99) or (o >= 0xbb and o <= 0xfe)
Loading