Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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/policy/policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
// Check policy limits for Taproot spends:
// - MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE limit for stack item size
// - No annexes
// ELEMENTS: allow annexes for simplicity transactions
if (witnessversion == 1 && witnessprogram.size() == WITNESS_V1_TAPROOT_SIZE && !p2sh) {
// Missing witness; invalid by consensus rules
if (i >= tx.witness.vtxinwit.size()) {
Expand All @@ -282,8 +283,16 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
// Taproot spend (non-P2SH-wrapped, version 1, witness program size 32; see BIP 341)
Span stack{tx.witness.vtxinwit[i].scriptWitness.stack};
if (stack.size() >= 2 && !stack.back().empty() && stack.back()[0] == ANNEX_TAG) {
SpanPopBack(stack); // drop the annex
const auto& control_block = SpanPopBack(stack);
if (!control_block.empty() && (control_block[0] & TAPROOT_LEAF_MASK) == TAPROOT_LEAF_TAPSIMPLICITY) {
// Annexes are allowed for Simplicity spends
// checks for zero padding and exact size are done in CheckSimplicity
return true;
} else {
// Annexes are nonstandard as long as no semantics are defined for them.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This block needs to be indented.

return false;
}
}
if (stack.size() >= 2) {
// Script path spend (2 or more stack elements after removing optional annex)
Expand Down
3 changes: 2 additions & 1 deletion src/policy/policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ static constexpr unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VE
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION |
SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS |
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE |
SCRIPT_VERIFY_SIMPLICITY;
SCRIPT_VERIFY_SIMPLICITY |
SCRIPT_VERIFY_ANNEX_PADDING;


/** For convenience, standard but not mandatory verify flags. */
Expand Down
26 changes: 23 additions & 3 deletions src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3119,14 +3119,14 @@ uint32_t GenericTransactionSignatureChecker<T>::GetnIn() const
}

template <class T>
bool GenericTransactionSignatureChecker<T>::CheckSimplicity(const valtype& program, const valtype& witness, const rawElementsTapEnv& simplicityRawTap, int64_t budget, ScriptError* serror) const
bool GenericTransactionSignatureChecker<T>::CheckSimplicity(const valtype& program, const valtype& witness, const rawElementsTapEnv& simplicityRawTap, int64_t minCost, int64_t budget, ScriptError* serror) const
{
simplicity_err error;
elementsTapEnv* simplicityTapEnv = simplicity_elements_mallocTapEnv(&simplicityRawTap);

assert(txdata->m_simplicity_tx_data);
assert(simplicityTapEnv);
if (!simplicity_elements_execSimplicity(&error, 0, txdata->m_simplicity_tx_data.get(), nIn, simplicityTapEnv, txdata->m_hash_genesis_block.data(), 0, budget, 0, program.data(), program.size(), witness.data(), witness.size())) {
if (!simplicity_elements_execSimplicity(&error, 0, txdata->m_simplicity_tx_data.get(), nIn, simplicityTapEnv, txdata->m_hash_genesis_block.data(), minCost, budget, 0, program.data(), program.size(), witness.data(), witness.size())) {
assert(!"simplicity_elements_execSimplicity internal error");
}
simplicity_elements_freeTapEnv(simplicityTapEnv);
Expand Down Expand Up @@ -3278,11 +3278,15 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
// BIP341 Taproot: 32-byte non-P2SH witness v1 program (which encodes a P2C-tweaked pubkey)
if (!(flags & SCRIPT_VERIFY_TAPROOT)) return set_success(serror);
if (stack.size() == 0) return set_error(serror, SCRIPT_ERR_WITNESS_PROGRAM_WITNESS_EMPTY);
valtype padding;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my opinion, we can just hoist the declaration of annex below out to here and assign to it below and make the annex available to everyone in this scope.

i.e.

valtype annex;
if (stack.size() >= 2 && !stack.back().empty() && stack.back()[0] == ANNEX_TAG) {
    // Drop annex (this is non-standard; see IsWitnessStandard)
    annex = SpanPopBack(stack);

subject to whatever crazy C++ isms you need to make this sound.

This would mean cloning the annex below in the minCost calculation because we would ideally not mutate the annex value, so I can sort of see why you might want to keep this code the way it is.

if (stack.size() >= 2 && !stack.back().empty() && stack.back()[0] == ANNEX_TAG) {
// Drop annex (this is non-standard; see IsWitnessStandard)
const valtype& annex = SpanPopBack(stack);
execdata.m_annex_hash = (CHashWriter(SER_GETHASH, 0) << annex).GetSHA256();
execdata.m_annex_present = true;
if (!stack.back().empty() && (stack.back()[0] & TAPROOT_LEAF_MASK) == TAPROOT_LEAF_TAPSIMPLICITY) {
padding = annex;
}
} else {
execdata.m_annex_present = false;
}
Expand Down Expand Up @@ -3322,7 +3326,23 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
simplicityRawTap.controlBlock = control.data();
simplicityRawTap.pathLen = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE;
simplicityRawTap.scriptCMR = script_bytes.data();
return checker.CheckSimplicity(simplicity_program, simplicity_witness, simplicityRawTap, budget, serror);
int64_t minCost = 0;
if ((flags & SCRIPT_VERIFY_ANNEX_PADDING) && padding.size() > 0) {
valtype zero_padding(padding.size(), 0);
zero_padding[0] = ANNEX_TAG;
if (padding != zero_padding) {
return set_error(serror, SCRIPT_ERR_SIMPLICITY_PADDING_NONZERO);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feel free to push back on this idea, but I think it would be marginally better to move this zero-padding check into policy.cpp. That is to say that IsWitnessStandard says that an annex is standard only when it is both

  1. all zeros and
  2. is part of a Simplicity redemption.

The all zero check can be validated without parsing of the Simplicity program, and moving the code there removes the need for an explicit SCRIPT_ERR_SIMPLICITY_PADDING_NONZERO error. It just simply becomes non-standard to have a non-zero annex.

}
// Compute what the budget would have been without the padding.
// budget includes the padding cost, so subtracting this stack item won't underflow.
minCost = budget - ::GetSerializeSize(padding);
if (!zero_padding.empty()) {
Copy link
Copy Markdown
Contributor

@roconnor-blockstream roconnor-blockstream Mar 31, 2026

Choose a reason for hiding this comment

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

!zero_padding.empty() can never be false here because zero_padding.size() > 0. The condition should be 1 < zero_padding.size().

Preferably we'd add a test case to catch this programming error before fixing it. It would be something like a program that needs exactly one byte of padding in order to be spendable. With the code as currently written, such program would fail to be spendable. One byte of padding would require a minimal annex with 0 zero bytes. But the serialization of the stack item 0x50 requires two bytes, one for the 0x50 and one for the prefix. Two bytes would put the program overweight with the current logic.

When I was last looking at how to construct such a specific example, I had asked @apoelstra to use his fuzzer to find one.

// Set the minCost to what the budget would have been if the padding were one byte smaller.
zero_padding.pop_back();
minCost += ::GetSerializeSize(zero_padding);
}
}
return checker.CheckSimplicity(simplicity_program, simplicity_witness, simplicityRawTap, minCost, budget, serror);
}
if (flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION) {
return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION);
Expand Down
8 changes: 6 additions & 2 deletions src/script/interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ enum : uint32_t {
//
SCRIPT_VERIFY_SIMPLICITY = (1U << 23),

// Check exact annex padding policy for simplicity spends
//
SCRIPT_VERIFY_ANNEX_PADDING = (1U << 24),

// Constants to point to the highest flag in use. Add new flags above this line.
//
SCRIPT_VERIFY_END_MARKER
Expand Down Expand Up @@ -343,7 +347,7 @@ class BaseSignatureChecker
return std::numeric_limits<uint32_t>::max();
}

virtual bool CheckSimplicity(const std::vector<unsigned char>& witness, const std::vector<unsigned char>& program, const rawElementsTapEnv& simplicityRawTap, int64_t budget, ScriptError* serror) const
virtual bool CheckSimplicity(const std::vector<unsigned char>& witness, const std::vector<unsigned char>& program, const rawElementsTapEnv& simplicityRawTap, int64_t minCost, int64_t budget, ScriptError* serror) const
{
return false;
}
Expand Down Expand Up @@ -391,7 +395,7 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker

const PrecomputedTransactionData* GetPrecomputedTransactionData() const override;
uint32_t GetnIn() const override;
bool CheckSimplicity(const std::vector<unsigned char>& program, const std::vector<unsigned char>& witness, const rawElementsTapEnv& simplicityRawTap, int64_t budget, ScriptError* serror) const override;
bool CheckSimplicity(const std::vector<unsigned char>& program, const std::vector<unsigned char>& witness, const rawElementsTapEnv& simplicityRawTap, int64_t minCost, int64_t budget, ScriptError* serror) const override;
};

using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>;
Expand Down
2 changes: 2 additions & 0 deletions src/script/script_error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ std::string ScriptErrorString(const ScriptError serror)
return "EC scalar mult verify fail";
case SCRIPT_ERR_SIMPLICITY_WRONG_LENGTH:
return "Simplicity witness has incorrect length";
case SCRIPT_ERR_SIMPLICITY_PADDING_NONZERO:
return "Simplicity annex padding must be all zeros";
case SCRIPT_ERR_SIMPLICITY_DATA_OUT_OF_RANGE:
return SIMPLICITY_ERR_MSG(SIMPLICITY_ERR_DATA_OUT_OF_RANGE);
case SCRIPT_ERR_SIMPLICITY_DATA_OUT_OF_ORDER:
Expand Down
1 change: 1 addition & 0 deletions src/script/script_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ typedef enum ScriptError_t

/* Elements: Simplicity related errors */
SCRIPT_ERR_SIMPLICITY_WRONG_LENGTH,
SCRIPT_ERR_SIMPLICITY_PADDING_NONZERO,
SCRIPT_ERR_SIMPLICITY_NOT_YET_IMPLEMENTED,
SCRIPT_ERR_SIMPLICITY_DATA_OUT_OF_RANGE,
SCRIPT_ERR_SIMPLICITY_DATA_OUT_OF_ORDER,
Expand Down
1 change: 1 addition & 0 deletions src/test/transaction_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ static std::map<std::string, unsigned int> mapFlagNames = {
{std::string("DISCOURAGE_OP_SUCCESS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS},
{std::string("DISCOURAGE_UPGRADABLE_TAPROOT_VERSION"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION},
{std::string("SIMPLICITY"), (unsigned int)SCRIPT_VERIFY_SIMPLICITY},
{std::string("ANNEX_PADDING"), (unsigned int)SCRIPT_VERIFY_ANNEX_PADDING},
};

unsigned int ParseScriptFlags(std::string strFlags)
Expand Down
145 changes: 145 additions & 0 deletions test/functional/mempool_annex_padding.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
#!/usr/bin/env python3
# Copyright (c) 2021 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
from decimal import Decimal
from io import BytesIO

from test_framework.messages import CTransaction, CTxInWitness
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_equal, assert_raises_rpc_error


class AnnexPaddingTest(BitcoinTestFramework):
"""Test exact annex padding for Simplicity spends."""

def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 1
self.extra_args = [["-evbparams=simplicity:-1:::"]] * self.num_nodes

def skip_test_if_missing_module(self):
self.skip_if_no_wallet()

def run_test(self):
self.log.info("Check that Simplicity is active")
simplicity = self.nodes[0].getdeploymentinfo()["deployments"]["simplicity"]
assert simplicity["active"]
self.generate(self.nodes[0], 101)

utxo = self.nodes[0].listunspent()[0]
assert_equal(utxo["amount"], Decimal("50.00000000"))
fee = Decimal("0.00001000")
amount = utxo["amount"] - fee

# this elementsregtest address is generated from the "hash loop" SimplicityHL template
addr = "ert1pzp4xccn92zvhh44z9qwh3ap3jnv677ympuaafmyv4urgfrp2lafsdap5ha"
# ---
# fn hash_counter_8(ctx: Ctx8, unused: (), byte: u8) -> Either<u256, Ctx8> {
# let new_ctx: Ctx8 = jet::sha_256_ctx_8_add_1(ctx, byte);
# match jet::all_8(byte) {
# true => Left(jet::sha_256_ctx_8_finalize(new_ctx)),
# false => Right(new_ctx),
# }
# }
# fn main() {
# // Hash bytes 0x00 to 0xff
# let ctx: Ctx8 = jet::sha_256_ctx_8_init();
# let out: Either<u256, Ctx8> = for_while::<hash_counter_8>(ctx, ());
# let expected: u256 = 0x40aff2e9d2d8922e47afd4648e6967497158785fbd1da870e7110266bf944880;
# assert!(jet::eq_256(expected, unwrap_left::<Ctx8>(out)));
# }
# ---

self.log.info("Fund the contract address")
raw = self.nodes[0].createrawtransaction([{"txid": utxo["txid"], "vout": utxo["vout"]}], [{addr: amount}, {"fee": fee}])
signed = self.nodes[0].signrawtransactionwithwallet(raw)
assert signed["complete"]
txid = self.nodes[0].sendrawtransaction(signed["hex"])
self.generate(self.nodes[0], 1)

in_witness = CTxInWitness()
simplicity_witness = ""
simplicity_program = "e8144eac81081420c48a0f9128a0590da107248150b21b79b8118720e30e3e070a85a02d8370c41c920542c86e2341c920542c86e2a36e30e3f0b30e3f0e38542cc2d6371b1b8e4c39071871f038542d016c1b906839240a8590dc8a41c920542c86e489b71871f90461c7e429c2a16616b1b93a839240a8590dca441c920542c86e559b71871f93861c7e4f9c2a16616b1b96e6e3430e3f204c38fc8438542cc2d6373066e618c39071871f038542d016c1b99041c70b06aa0420507cb3650420506e678e2b5620a203801a00dc0708038980e33039001390ac5f8bdd59a0d0ed8d3bb22cb0ef50f71e3a577040de5bfe99608095e7d53356765e430b9101722c0661c40cc0e4804e4a9792a2e4b85c9a01907681901c9f03958139625e588b966172d80641e0c064072ec273005e6005cc105cc280c83c380c80e6280e6600e694273545e6a85cd605cd780c83c5006407368139b92f3722e6ec2e6f80641e30032039c3039d109ceb179d6173b0173b60320f1c81901cf004e790bcf38b9e60b9ea01907902064073d840a136940aff2e9d2d8922e47afd4648e6967497158785fbd1da870e7110266bf944880042050831061c9160366ce8867390b3cffedf1a67f35f4e6e69c39210d09fddb9189a14c225a77e6c262e1806006616b66dd008c0212283f4060201c180e180740780"
cmr = "988c6d7a1c50012028523debc8ec575ce96920c46a45f663051aa3309f6fc539"
control_block = "be50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0"

self.log.info("Try to spend without no annex")
addr = self.nodes[0].getnewaddress(address_type="bech32")
fee = Decimal("0.00003000")
raw = self.nodes[0].createrawtransaction([{"txid": txid, "vout": 0}], [{addr: amount - fee}, {"fee": fee}])
ctx = CTransaction()
ctx.deserialize(BytesIO(bytes.fromhex(raw)))

in_witness.scriptWitness.stack = [
bytes.fromhex(simplicity_witness),
bytes.fromhex(simplicity_program),
bytes.fromhex(cmr),
bytes.fromhex(control_block),
]
ctx.wit.vtxinwit.append(in_witness)
raw = ctx.serialize().hex()
assert_raises_rpc_error(-26, "Program's execution cost could exceed budget", self.nodes[0].sendrawtransaction, raw)

EXACT_PADDING = 7423
# FIXME: investigate why rust-simplicity `Cost::get_padding` gives exact padding as 7426
# https://github.com/BlockstreamResearch/rust-simplicity/blob/d28440bc0c6be333aa84fa441844541c14dbb563/src/analysis.rs#L148

self.log.info("Try to spend with non-zero padded annex")
annex = [0x50] + [0x01] * EXACT_PADDING
in_witness.scriptWitness.stack = [
bytes.fromhex(simplicity_witness),
bytes.fromhex(simplicity_program),
bytes.fromhex(cmr),
bytes.fromhex(control_block),
bytes(annex),
]
ctx.wit.vtxinwit[0] = in_witness
raw = ctx.serialize().hex()
assert_raises_rpc_error(-26, "Simplicity annex padding must be all zeros", self.nodes[0].sendrawtransaction, raw)

self.log.info("Try to spend with under-padded annex")
annex = [0x50] + [0x00] * (EXACT_PADDING - 1)
in_witness.scriptWitness.stack = [
bytes.fromhex(simplicity_witness),
bytes.fromhex(simplicity_program),
bytes.fromhex(cmr),
bytes.fromhex(control_block),
bytes(annex),
]
ctx.wit.vtxinwit[0] = in_witness
raw = ctx.serialize().hex()
assert_raises_rpc_error(-26, "Program's execution cost could exceed budget", self.nodes[0].sendrawtransaction, raw)

self.log.info("Try to spend with over-padded annex")
annex = [0x50] + [0x00] * (EXACT_PADDING + 1)
in_witness.scriptWitness.stack = [
bytes.fromhex(simplicity_witness),
bytes.fromhex(simplicity_program),
bytes.fromhex(cmr),
bytes.fromhex(control_block),
bytes(annex),
]
ctx.wit.vtxinwit[0] = in_witness
raw = ctx.serialize().hex()
assert_raises_rpc_error(-26, "Program's budget is too large", self.nodes[0].sendrawtransaction, raw)

self.log.info("Spend with exact padded annex")
annex = [0x50] + [0x00] * EXACT_PADDING
in_witness.scriptWitness.stack = [
bytes.fromhex(simplicity_witness),
bytes.fromhex(simplicity_program),
bytes.fromhex(cmr),
bytes.fromhex(control_block),
bytes(annex),
]
ctx.wit.vtxinwit[0] = in_witness
raw = ctx.serialize().hex()
txid = self.nodes[0].sendrawtransaction(raw)
self.generate(self.nodes[0], 1)
tx = self.nodes[0].gettransaction(txid)
assert_equal(tx["confirmations"], 1)


if __name__ == "__main__":
AnnexPaddingTest().main()
1 change: 1 addition & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@
'feature_help.py',
'feature_shutdown.py',
'p2p_ibd_txrelay.py',
'mempool_annex_padding.py',
'feature_blockfilterindex_prune.py'
# Don't append tests at the end to avoid merge conflicts
# Put them in a random line within the section that fits their approximate run-time
Expand Down