BIP-376: Silent Payment input signing behavior

[lucia-w]

• This PR adds a reference implementation and test vectors for Silent Payment input signing behavior.

[murchandamus]

Thanks for your submission. cc'ing the author @nymius for review.

[nymius]

Approach NACK 8c38c9e

Thanks for working on this.
Although your changes are exercising the behavior of the signer role, my expectations for the test cases are inclined to what we already have in BIP 174 or is currently being worked on for BIP 375 (#2046)
There we have full serialized PSBT tests.
As this BIP is focused towards PSBTs, I think that's the way to proceed.

[nymius]

I recommend you to follow the approach taken in #2087 and #2046 , use secp256k1lab and the components from bitcoin_test that you need. That would focus the review process on BIP 376.

[lucia-w]

Makes sense, thank you

[nymius]

These are unnecessary and constraint the implementation, I would remove them.

[lucia-w]

Done, could you please review again

[nymius]

Thanks for the update. Let me explain this better:

I would create a dependencies or deps dir with the secp256k1lab@1.0.0 source code and the code from bitcoin core test framework like BIP 375 does: https://github.com/bitcoin/bips/tree/master/bip-0375

And I recommend you to do it with git subtree add --prefix=bip-0352/secp256k1lab --squash https://github.com/secp256k1lab/secp256k1lab v1.0.0 command, like in: #2087, it is redundant, but is self contained.

For the test implementation, I don't think the spend seckey checks belong here, because they are in the domain of BIP 340.

The test cases I have in mind for now are:

• Updater adds PSBT_IN_SP_TWEAK.
• Updater adds PSBT_IN_SP_TWEAK and PSBT_IN_SP_SPEND_BIP32_DERIVATION.
• Signer sets the PSBT_IN_TAP_KEY_SIG with expected signature for input with PSBT_IN_SP_TWEAK set using PSBT_IN_SP_SPEND_BIP32_DERIVATION.
• Signer sets the PSBT_IN_TAP_KEY_SIG with expected signature for input with PSBT_IN_SP_TWEAK without PSBT_IN_SP_SPEND_BIP32_DERIVATION field.
• Signer sets the PSBT_IN_TAP_KEY_SIG with expected signature with d.G odd, for input with PSBT_IN_SP_TWEAK set.
• Signer fails if the x-coordinate of d.G does not equal the output key in the P2TR output script.
• Input finalizer fails to verify PSBT_IN_TAP_KEY_SIG for PSBT_IN_SP_TWEAK (i.e., wrong signature).
• Input finalizer removes PSBT_IN_SP_TWEAK, PSBT_IN_SP_SPEND_BIP32_DERIVATION, PSBT_IN_TAP_KEY_SIG, and PSBT_IN_WITNESS_UTXO fields for an input where PSBT_IN_SP_TWEAK is set, and the PSBT_IN_FINAL_SCRIPTWITNESS is present.

I have been thinking in a way to standarize the PSBT test vectors, following BIP 375 approach, and this is the structure I came up with:

{
  "description": "string", // BIP 376 test vectors
  "version": "string", // "0.1.0"
  "notes": [],
  "cases": [
    {
      "description": "string",
      "psbt": {
        "hex": "string",
        "base64": "string"
      },
      "supplementary": { // Supplementary can hold multiple optional fields, which depend of the case you are testing, I've added the ones already required for signing
        "spend_seckey": "string",
        "message": "string",
        "aux_rand": "string" // I'm not sure about this one, we can do like BIP 340 reference and derive one each time using incremental numbers
        "task": "string", // This is required, but to keep compatibility with BIP 375 test vectors I placed it here. It can be one of [fail, fail_sign, update, finalize]
      }
    }
  ]
}

[nymius]

This is brittle, I would do exactly what bip-0375 does and copy the whole secp256k1lab source code in this directory, and then do the Path magic.

[nymius]

BIP 376 is backwards compatible with PSBTv2. It isn't compatible with PSBTv0, as the new fields are marked as excluded.