Skip to content

Conversation

@guggero
Copy link
Contributor

@guggero guggero commented Jun 2, 2025

This was incorrect all along, seems like we haven't actually used the VerifySchnorr() option in any project yet.

This is needed for lightninglabs/taproot-assets#1502.

Pull Request Checklist

  • PR is opened against correct version branch.
  • Version compatibility matrix in the README and minimal required version
    in lnd_services.go are updated.
  • Update macaroon_recipes.go if your PR adds a new method that is called
    differently than the RPC method it invokes.

This was incorrect all along, seems like we haven't actually used the
VerifySchnorr option in any project yet.
@guggero guggero requested review from GeorgeTsagk and bhandras June 2, 2025 15:42
Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

LGTM, one nit

// If the signature is a Schnorr signature, we need to set the public
// key as the 32-byte x-only key, as mentioned in the RPC docs.
if rpcIn.IsSchnorrSig {
rpcIn.Pubkey = pubkey[1:]
Copy link
Member

Choose a reason for hiding this comment

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

could also check for the len(pubkey) here to avoid a potential panic?

Copy link
Member

@ellemouton ellemouton Jun 2, 2025

Choose a reason for hiding this comment

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

i dont think it can panic. The param is [33]byte and so is guaranteed to have 33 bytes

@guggero guggero merged commit fe0bdd6 into master Jun 2, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants