Add a test case to feature_taproot which involves an output that is (incorrectly) constructed, using an invalid internal public key and valid script tree. It is designed to detect cases where the script path spending validation logic does not detect this case, and instead treats the internal public key as the point at infinity.
Equivalent unit test case added in https://github.com/bitcoin-core/qa-assets/pull/98.
871 | @@ -872,7 +872,7 @@ def taproot_tree_helper(scripts): 872 | # - merklebranch: the merkle branch to use for this leaf (32*N bytes) 873 | TaprootLeafInfo = namedtuple("TaprootLeafInfo", "script,version,merklebranch,leaf_hash") 874 | 875 | -def taproot_construct(pubkey, scripts=None): 876 | +def taproot_construct(pubkey, scripts=None, treat_internal_as_infinity=False):
needs to be described in below comments
Let me try explaining in comments here before updating the PR.
X-only public keys are 32 bytes, but not every 32-byte array is a valid public key; only around 50% of them are. This does not affect users using correct software; these "keys" have no corresponding private key, and thus will never appear as output of key generation/derivation/tweaking.
Using an invalid public key as P2TR output key makes the UTXO unspendable. Revealing an invalid public key as internal key in a P2TR script path spend also makes the spend invalid. These conditions are explicitly spelled out in BIP341.
Nometheless, there are no validation test vectors for these. It's hard to create such vectors, because it involves "guesses" how a potential incorrect implementation deals with an obviously-invalid condition, and making sure that guessed behavior (accepting it in certain condition) doesn't occur.
The test case added here tries to detect a very specific bug a verifier could have: if they don't verify whether or not a revealed internal public key in a script path spend is valid, and instead implement output_key == tweak(internal_key, tweakval) but such that tweak(invalid_key, tweakval) equals the public key corresponding to private key tweakval. This may seem like a far-fetched edge condition to test for, but in fact, the BIP341 wallet pseudocode does exactly that (but obviously only triggerable by someone invoking the tweaking function with an invalid public key, which shouldn't happen).
if they don't verify whether or not a revealed internal public key in a script path spend is valid, and instead implement output_key == tweak(internal_key, tweakval) but such that tweak(invalid_key, tweakval) equals the public key corresponding to private key tweakval
Ok so you're saying an implementation could, instead of fail detecting it's not on the curve, could instead just consider the pubkey as the point at infinity, which would result in the tweak alone being the key, and in fact this may be a common mistake.
Right, exactly.
I'm not sure it's common (no verification implementation (which is where this matters) I know of has this issue). But it is testable.
Included a big comment in the code with slightly-reworked version of my explanation above.
675 | + invalid_pub = random_bytes(32) 676 | + if not SECP256K1.is_x_coord(int.from_bytes(invalid_pub, 'big')): 677 | + break 678 | + 679 | + # Implement a test case that detects validation logic which maps invalid public keys to the 680 | + # point at infinity in the tweaking logic.
can this be dumbed down for those of us who treat EC things in a black box manner? It's not on the curve, what does treating it as a point at infinity mean in layman?
ACK 6ca24638d0b70f3d40166ca1894f2481260a1b5f I understand that an invalid x-only pubkey can be treated as a point to infinity as both lead to invalid results. This is only testing the test framework though correct? Which is still useful.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | instagibbs, aureleoules |
Rebased, and added a big comment.
ACK 5d413c8e793a439540d064d24fddfc868e1817d0
reACK 5d413c8e793a439540d064d24fddfc868e1817d0
