Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1/?] - lnwallet: ensure that SignOutputRaw can sign w/ non-default sighash f… #7332

Merged
merged 5 commits into from Jan 26, 2023

Conversation

Roasbeef
Copy link
Member

…or schnorr sigs

Before this commit, we'd end up failing in schnorr.ParseSignature if a non-default sighash was used. To fix that, we'll slice the signature to only pass in the sig w/o the sighash flag.

]

@saubyk saubyk added this to the v0.16.0 milestone Jan 17, 2023
@Roasbeef Roasbeef changed the title lnwallet: ensure that SignOutputRaw can sign w/ non-default sighash f… [1/?] - lnwallet: ensure that SignOutputRaw can sign w/ non-default sighash f… Jan 19, 2023
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice fixes, LGTM 🎉

@@ -364,7 +379,7 @@ func testTaprootSignOutputRawScriptSpend(ht *lntemp.HarnessTest,
// also be spent using the key spend path through the SignOutputRaw RPC using a
// BIP0086 key spend only commitment.
func testTaprootSignOutputRawKeySpendBip86(ht *lntemp.HarnessTest,
alice *node.HarnessNode) {
alice *node.HarnessNode, sigHashType ...txscript.SigHashType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

An alternative would be a pointer and a nil check (instead of a len check).

@Roasbeef Roasbeef force-pushed the sign-output-raw-bug-fix branch 2 times, most recently from 4bb7194 to f59766f Compare January 25, 2023 03:06
@Roasbeef
Copy link
Member Author

Pushed up a new version pointing to the merged btcd commit.

…or schnorr sigs

Before this commitment, we'd end up failing in `schnorr.ParseSignature`
if a non-default sighash was used. To fix that, we'll slice the
signature to only pass in the sig w/o the sighash flag.
Calling ht.Helper() means we'll get the proper line number if an error
hapens.
…ault sighashes

Without the prior commit, this test fails with:
```
rpc error: code = Unknown desc = malformed signature: too long: 65 > 64
```
Copy link
Collaborator

@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 📝 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants