Merge pull request #7264 from BitGo/BTC-2652.improve-p2tr-tests

feat(utxo-lib): improve test coverage for p2tr and p2trMusig

Author: lcovar

modules/utxo-lib/bip-0327/README.md

@@ -0,0 +1,179 @@
+# BIP 0327 with BitGo legacy p2tr variant
+
+This directory contains a modified version of the BIP-0327 MuSig2
+reference implementation by @jonasnick.
+
+The original code was taken from the following file:
+https://github.com/bitcoin/bips/blob/ab9d5b8/bip-0327/reference.py
+
+The modifications add support for an older aggregation method that is
+used at BitGo in a deprecated address type (`p2tr`, chain 30 and 31).
+
+The aggregation method is based on an older version of MuSig2 that predated this PR:
+https://github.com/jonasnick/bips/pull/37
+
+The recommended address type for taproot at BitGo is `p2trMusig2` (chains 40 and 41),
+which uses the standard MuSig2 aggregation scheme.
+
+## Implementation Differences
+
+### 1. X-Only Pubkey Support
+
+The `key_agg()` function has been enhanced to accept both 33-byte compressed pubkeys and 32-byte x-only pubkeys:
+
+```python
+def key_agg(pubkeys: List[bytes]) -> KeyAggContext:
+    for pk in pubkeys:
+        if len(pk) != len(pubkeys[0]):
+            raise ValueError('all pubkeys must be the same length')
+
+    # ...
+    for i in range(u):
+        # if the pubkey is 32 bytes, it is an xonly pubkey
+        if len(pubkeys[i]) == 32:
+            P_i = lift_x(pubkeys[i])
+        else:
+            P_i = cpoint(pubkeys[i])
+```
+
+This allows the implementation to work with both pubkey formats, checking the length to determine the appropriate parsing method.
+
+### 2. Legacy p2tr Aggregation Function
+
+A new function `key_agg_bitgo_p2tr_legacy()` implements the deprecated aggregation method:
+
+```python
+def key_agg_bitgo_p2tr_legacy(pubkeys: List[PlainPk]) -> KeyAggContext:
+    # Convert compressed pubkeys to x-only format
+    pubkeys = [pk[-32:] for pk in pubkeys]
+
+    # Sort keys AFTER x-only conversion
+    pubkeys = key_sort(pubkeys)
+
+    # Aggregate using standard algorithm
+    return key_agg(pubkeys)
+```
+
+**Key difference**: This method converts pubkeys to x-only format **before** sorting, whereas standard MuSig2 uses full 33-byte compressed keys throughout. This difference stems from the MuSig2 specification change documented in [jonasnick/bips#37](https://github.com/jonasnick/bips/pull/37).
+
+### 3. Enhanced Signing and Verification Functions
+
+Several functions were updated to handle x-only pubkeys properly:
+
+**`get_session_key_agg_coeff()`**: Detects x-only pubkeys and uses appropriate format for coefficient calculation:
+
+```python
+# If pubkeys are x-only, use x-only for coefficient calculation
+if len(pubkeys[0]) == 32:
+    pk_for_coeff = pk[-32:]
+else:
+    pk_for_coeff = pk
+return key_agg_coeff(pubkeys, pk_for_coeff)
+```
+
+**`sign()`**: Validates the secnonce against both compressed and x-only pubkey formats:
+
+```python
+if not pk == secnonce[64:97] and not pk[-32:] == secnonce[64:97]:
+    raise ValueError('Public key does not match nonce_gen argument')
+```
+
+**`partial_sig_verify_internal()`**: Handles x-only pubkeys by prepending the appropriate prefix:
+
+```python
+# prepend a 0x02 if the pk is 32 bytes
+P = cpoint(b'\x02' + pk) if len(pk) == 32 else cpoint(pk)
+```
+
+## Testing Differences
+
+The testing code has been significantly restructured to validate BitGo-specific behavior.
+
+### Refactored Test Helpers
+
+The previous monolithic `test_sign_and_verify_random()` function has been broken down into reusable components:
+
+**`sign_and_verify_with_aggpk()`**: Core signing workflow that:
+
+- Generates nonces for two signers
+- Supports both random nonce generation and deterministic signing
+- Performs partial signature verification
+- Tests nonce reuse protection
+- Verifies the final aggregated signature
+
+**`sign_and_verify_with_keys()`**: Simplified wrapper that generates random tweaks and calls the core signing function.
+
+**`sign_and_verify_with_aggpk_bitgo()`**: BitGo-specific wrapper with no tweaks applied (BitGo doesn't use tweaks in production).
+
+**`sign_and_verify_with_aggpk_bitgo_legacy()`**: Special handler for legacy p2tr that:
+
+- Normalizes secret keys to produce even y-coordinate pubkeys
+- Converts to x-only format
+- Sorts by x-only pubkey order
+- Validates the expected aggregate pubkey
+- Performs full signing workflow
+
+### BitGo-Specific Test Cases
+
+Three new test functions validate BitGo's taproot implementations:
+
+#### `test_agg_bitgo_derive()`
+
+Sanity check that the test fixture private keys correctly derive to their expected public keys.
+
+#### `test_agg_bitgo_p2tr_legacy()`
+
+Tests the legacy p2tr aggregation (chains 30/31):
+
+- Expected aggregate key: `cc899cac29f6243ef481be86f0d39e173c075cd57193d46332b1ec0b42c439aa`
+- Verifies order-independence: aggregating `[user, bitgo]` and `[bitgo, user]` produce the same result (due to sorting after x-only conversion)
+- Tests complete signing and verification workflow
+
+#### `test_agg_bitgo_p2tr_musig2()`
+
+Tests the standard MuSig2 aggregation (chains 40/41):
+
+- Expected aggregate key `[user, bitgo]`: `c0e255b4510e041ab81151091d875687a618de314344dff4b73b1bcd366cdbd8`
+- Expected aggregate key `[bitgo, user]`: `e48d309b535811eb0b148c4b0600a10e82e289899429e40aee05577504eca356`
+- Verifies order-dependence: different key orders produce different aggregate keys (standard MuSig2 behavior)
+- Tests both orderings with complete signing workflows
+
+### Shared Test Fixtures
+
+All BitGo tests use consistent keypairs:
+
+```python
+# Private keys from test fixtures
+privkey_user = bytes.fromhex("a07e682489dad68834f7df8a5c8b34f3b9ff9fdd8809e2ba53ae29df65fc146b")
+privkey_bitgo = bytes.fromhex("2d210ff6703d0fae0e9ca91e1d0bbab006b03e8e699f49becbaf554066fa79aa")
+
+# Corresponding public keys
+pubkey_user = PlainPk(bytes.fromhex("02d20a62701c54f6eb3abb9f964b0e29ff90ffa3b4e3fcb73e7c67d4950fa6e3c7"))
+pubkey_bitgo = PlainPk(bytes.fromhex("03203ab799ce28e2cca044f594c69275050af4bb0854ad730a8f74622342300e64"))
+```
+
+**Important note**: These pubkeys have different sort orders depending on whether comparison is done on the full 33-byte compressed format or the 32-byte x-only format. This is precisely why the legacy and standard methods produce different aggregate keys.
+
+## Running Tests
+
+Execute all tests including BitGo-specific ones:
+
+```bash
+cd modules/utxo-lib/bip-0327
+python3 reference.py
+```
+
+The test suite runs:
+
+1. Standard BIP327 test vectors (key sorting, aggregation, nonces, signing, tweaks, deterministic signing, signature aggregation)
+2. Random signing/verification tests (6 iterations)
+3. BitGo derivation tests
+4. BitGo legacy p2tr tests
+5. BitGo standard p2trMusig2 tests
+
+## References
+
+- [BIP327 Specification](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki)
+- [BIP340 Schnorr Signatures](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki)
+- [MuSig2 32-byte to 33-byte key change](https://github.com/jonasnick/bips/pull/37)
+- [Original BIP327 reference implementation](https://github.com/bitcoin/bips/blob/ab9d5b8/bip-0327/reference.py)

modules/utxo-lib/bip-0327/gen_vectors_helper.py

@@ -0,0 +1,185 @@
+from reference import *
+
+def gen_key_agg_vectors():
+    print("key_agg_vectors.json: Intermediate tweaking result is point at infinity")
+    sk = bytes.fromhex("7FB9E0E687ADA1EEBF7ECFE2F21E73EBDB51A7D450948DFE8D76D7F2D1007671")
+    pk = individual_pk(sk)
+    keygen_ctx = key_agg([pk])
+    aggpoint, _, _ = keygen_ctx
+    aggsk = key_agg_coeff([pk], pk)*int_from_bytes(sk) % n
+    t = n - aggsk
+    assert point_add(point_mul(G, t), aggpoint) == None
+    is_xonly = False
+    tweak = bytes_from_int(t)
+    assert_raises(ValueError, lambda: apply_tweak(keygen_ctx, tweak, is_xonly), lambda e: True)
+    print("  pubkey:", pk.hex().upper())
+    print("  tweak: ", tweak.hex().upper())
+
+def check_sign_verify_vectors():
+    with open(os.path.join(sys.path[0], 'vectors', 'sign_verify_vectors.json')) as f:
+        test_data = json.load(f)
+    X = fromhex_all(test_data["pubkeys"])
+    pnonce = fromhex_all(test_data["pnonces"])
+    aggnonces = fromhex_all(test_data["aggnonces"])
+    msgs = fromhex_all(test_data["msgs"])
+
+    valid_test_cases = test_data["valid_test_cases"]
+    for (i, test_case) in enumerate(valid_test_cases):
+        pubkeys = [X[i] for i in test_case["key_indices"]]
+        pubnonces = [pnonce[i] for i in test_case["nonce_indices"]]
+        aggnonce = aggnonces[test_case["aggnonce_index"]]
+        assert nonce_agg(pubnonces) == aggnonce
+        msg = msgs[test_case["msg_index"]]
+        signer_index = test_case["signer_index"]
+        expected = bytes.fromhex(test_case["expected"])
+
+        session_ctx = SessionContext(aggnonce, pubkeys, [], [], msg)
+        (Q, _, _, _, R, _) = get_session_values(session_ctx)
+        # Make sure the vectors include tests for both variants of Q and R
+        if i == 0:
+           assert has_even_y(Q) and not has_even_y(R)
+        if i == 1:
+           assert not has_even_y(Q) and has_even_y(R)
+        if i == 2:
+           assert has_even_y(Q) and has_even_y(R)
+
+def check_tweak_vectors():
+    with open(os.path.join(sys.path[0], 'vectors', 'tweak_vectors.json')) as f:
+        test_data = json.load(f)
+
+    X = fromhex_all(test_data["pubkeys"])
+    pnonce = fromhex_all(test_data["pnonces"])
+    tweak = fromhex_all(test_data["tweaks"])
+    valid_test_cases = test_data["valid_test_cases"]
+
+    for (i, test_case) in enumerate(valid_test_cases):
+        pubkeys = [X[i] for i in test_case["key_indices"]]
+        tweaks = [tweak[i] for i in test_case["tweak_indices"]]
+        is_xonly = test_case["is_xonly"]
+
+        _, gacc, _ = key_agg_and_tweak(pubkeys, tweaks, is_xonly)
+        # Make sure the vectors include tests for gacc = 1 and -1
+        if i == 0:
+           assert gacc == n - 1
+        if i == 1:
+           assert gacc == 1
+
+def sig_agg_vectors():
+    print("sig_agg_vectors.json:")
+    sk = fromhex_all([
+        "7FB9E0E687ADA1EEBF7ECFE2F21E73EBDB51A7D450948DFE8D76D7F2D1007671",
+        "3874D22DE7A7290C49CE7F1DC17D1A8CD8918E1F799055139D57FC0988D04D10",
+        "D0EA1B84481ED1BCFAA39D6775F97BDC9BF8D7C02FD0C009D6D85BAE5EC7B87A",
+        "FC2BF9E056B273AF0A8AABB815E541A3552C142AC10D4FE584F01D2CAB84F577"])
+    pubkeys = list(map(lambda secret: individual_pk(secret), sk))
+    indices32 = [i.to_bytes(32, 'big') for i in range(6)]
+    secnonces, pnonces = zip(*[nonce_gen_internal(r, None, pubkeys[0], None, None, None) for r in indices32])
+    tweaks = fromhex_all([
+        "B511DA492182A91B0FFB9A98020D55F260AE86D7ECBD0399C7383D59A5F2AF7C",
+        "A815FE049EE3C5AAB66310477FBC8BCCCAC2F3395F59F921C364ACD78A2F48DC",
+        "75448A87274B056468B977BE06EB1E9F657577B7320B0A3376EA51FD420D18A8"])
+    msg = bytes.fromhex("599C67EA410D005B9DA90817CF03ED3B1C868E4DA4EDF00A5880B0082C237869")
+
+    psigs = [None] * 9
+
+    valid_test_cases = [
+        {
+            "aggnonce": None,
+            "nonce_indices": [0, 1],
+            "key_indices": [0, 1],
+            "tweak_indices": [],
+            "is_xonly": [],
+            "psig_indices": [0, 1],
+        }, {
+            "aggnonce": None,
+            "nonce_indices": [0, 2],
+            "key_indices": [0, 2],
+            "tweak_indices": [],
+            "is_xonly": [],
+            "psig_indices": [2, 3],
+        }, {
+            "aggnonce": None,
+            "nonce_indices": [0, 3],
+            "key_indices": [0, 2],
+            "tweak_indices": [0],
+            "is_xonly": [False],
+            "psig_indices": [4, 5],
+        }, {
+            "aggnonce": None,
+            "nonce_indices": [0, 4],
+            "key_indices": [0, 3],
+            "tweak_indices": [0, 1, 2],
+            "is_xonly": [True, False, True],
+            "psig_indices": [6, 7],
+        },
+    ]
+    for (i, test_case) in enumerate(valid_test_cases):
+        is_xonly = test_case["is_xonly"]
+        nonce_indices = test_case["nonce_indices"]
+        key_indices = test_case["key_indices"]
+        psig_indices = test_case["psig_indices"]
+        vec_pnonces = [pnonces[i] for i in nonce_indices]
+        vec_pubkeys = [pubkeys[i] for i in key_indices]
+        vec_tweaks = [tweaks[i] for i in test_case["tweak_indices"]]
+
+        aggnonce = nonce_agg(vec_pnonces)
+        test_case["aggnonce"] = aggnonce.hex().upper()
+        session_ctx = SessionContext(aggnonce, vec_pubkeys, vec_tweaks, is_xonly, msg)
+
+        for j in range(len(key_indices)):
+            # WARNING: An actual implementation should _not_ copy the secnonce.
+            # Reusing the secnonce, as we do here for testing purposes, can leak the
+            # secret key.
+            secnonce_tmp = bytearray(secnonces[nonce_indices[j]][:64] + pubkeys[key_indices[j]])
+            psigs[psig_indices[j]] = sign(secnonce_tmp, sk[key_indices[j]], session_ctx)
+        sig = partial_sig_agg([psigs[i] for i in psig_indices], session_ctx)
+        keygen_ctx = key_agg_and_tweak(vec_pubkeys, vec_tweaks, is_xonly)
+        # To maximize coverage of the sig_agg algorithm, we want one public key
+        # point with an even and one with an odd Y coordinate.
+        if i == 0:
+            assert(has_even_y(keygen_ctx[0]))
+        if i == 1:
+            assert(not has_even_y(keygen_ctx[0]))
+        aggpk = get_xonly_pk(keygen_ctx)
+        assert schnorr_verify(msg, aggpk, sig)
+        test_case["expected"] = sig.hex().upper()
+
+    error_test_case = {
+        "aggnonce": None,
+        "nonce_indices": [0, 4],
+        "key_indices": [0, 3],
+        "tweak_indices": [0, 1, 2],
+        "is_xonly": [True, False, True],
+        "psig_indices": [7, 8],
+        "error": {
+            "type": "invalid_contribution",
+            "signer": 1,
+            "contrib": "psig",
+        },
+        "comment": "Partial signature is invalid because it exceeds group size"
+    }
+
+    psigs[8] = bytes.fromhex("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141")
+
+    vec_pnonces = [pnonces[i] for i in error_test_case["nonce_indices"]]
+    aggnonce = nonce_agg(vec_pnonces)
+    error_test_case["aggnonce"] = aggnonce.hex().upper()
+
+    def tohex_all(l):
+        return list(map(lambda e: e.hex().upper(), l))
+
+    print(json.dumps({
+        "pubkeys": tohex_all(pubkeys),
+        "pnonces": tohex_all(pnonces),
+        "tweaks": tohex_all(tweaks),
+        "psigs": tohex_all(psigs),
+        "msg": msg.hex().upper(),
+        "valid_test_cases": valid_test_cases,
+        "error_test_cases": [error_test_case]
+    }, indent=4))
+
+gen_key_agg_vectors()
+check_sign_verify_vectors()
+check_tweak_vectors()
+print()
+sig_agg_vectors()