policy: allow exact annex padding for simplicity spends #1539
policy: allow exact annex padding for simplicity spends #1539delta1 wants to merge 2 commits intoElementsProject:masterfrom
Conversation
|
I'll have some more comment later but first and foremost there is no need to amend the Simplicity C code. Instead there is a Have a careful read of the documentation for |
delta1
force-pushed
the
2026-02-annex-policy
branch
2 times, most recently
from
4cdb88c to
1918260
Compare
|
Thanks @roconnor-blockstream updated |
Cost::get_padding previously always assumed only 1 byte for compactsize encoding when calculating required padding size. For larger differences in cost/budget, this incorrectly resulted in an additional 1 or 2 bytes of padding depending on the difference. I found this when calculating padding for the SimplicityHL hash loop example, where rust-simplicity was calculating a 7426 byte annex padding while libsimplicity required a 7424 byte padding, since the compactsize encoding requires an additional 2 bytes. See ElementsProject/elements#1539
Cost::get_padding previously always assumed only 1 byte for compactsize encoding when calculating required padding size. For larger differences in cost/budget, this incorrectly resulted in an additional 1 or 2 bytes of padding depending on the difference. I found this when calculating padding for the SimplicityHL hash loop example, where rust-simplicity was calculating a 7426 byte annex padding while libsimplicity required a 7424 byte padding, since the compactsize encoding requires 2 additional bytes. See ElementsProject/elements#1539
Cost::get_padding previously always assumed only 1 byte for compactsize encoding when calculating required padding size. For larger differences in cost/budget, this incorrectly resulted in an additional 1 or 2 bytes of padding depending on the difference. I found this when calculating padding for the SimplicityHL hash loop example, where rust-simplicity was calculating a 7426 byte annex padding while libsimplicity required a 7424 byte padding, since the compactsize encoding requires 2 additional bytes. See ElementsProject/elements#1539
Cost::get_padding previously always assumed only 1 byte for compactsize encoding when calculating required padding size. For larger differences in cost/budget, this incorrectly resulted in an additional 1 or 2 bytes of padding depending on the difference. I found this when calculating padding for the SimplicityHL hash loop example, where rust-simplicity was calculating a 7426 byte annex padding while libsimplicity required a 7424 byte padding, since the compactsize encoding requires 2 additional bytes. See ElementsProject/elements#1539
Cost::get_padding previously always assumed only 1 byte for compactsize encoding when calculating required padding size. For larger differences in cost/budget, this incorrectly resulted in an additional 1 or 2 bytes of padding depending on the difference. I found this when calculating padding for the SimplicityHL hash loop example, where rust-simplicity was calculating a 7426 byte annex padding while libsimplicity required a 7424 byte padding, since the compactsize encoding requires 2 additional bytes. See ElementsProject/elements#1539
delta1
force-pushed
the
2026-02-annex-policy
branch
from
1918260 to
ddf712e
Compare
|
Force push to fix a null-pointer-use identified by the fuzz task |
|
This looks approximately like what I'm expecting, but I'll need some time to go over it in more detail. |
| // 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; |
There was a problem hiding this comment.
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.
| valtype zero_padding(padding.size(), 0); | ||
| zero_padding[0] = ANNEX_TAG; | ||
| if (padding != zero_padding) { | ||
| return set_error(serror, SCRIPT_ERR_SIMPLICITY_PADDING_NONZERO); |
There was a problem hiding this comment.
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
- all zeros and
- 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()) { |
There was a problem hiding this comment.
!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.
2 participants
Implements BlockstreamResearch/simplicity#290 for Elements standardness policy.
Something concerning to be investigated is thatCost::get_paddingin rust-simplicity is returning a padding 2 bytes bigger than this calculation.. presumably an error in that method?Fix for rust-simplicity
get_paddingat BlockstreamResearch/rust-simplicity#356