Add PSBT derivation info for remote signer

[aakselrod]

Change Description

Motivation

Currently, lnd as remote signer and external projects such as lndsigner are restricted to acting as non-validating signers. Other projects, notably VLS, are capable of validating channel state transitions but don't work with lnd. I've started working on similar policy enforcement abilities in lndsigner. I've started a PR against lndsigner to allow validating channel state transitions.

Problem at hand

In order to validate that it's signing transactions that are sending money to the right places, a signer needs to know how the outputs of the transactions are derived. Currently, lnd doesn't send derivation information to the signer, leaving it unable to ensure the node will be able to correctly claim its outputs in the future.

Currently, NYDIG-OSS/lndsigner#23 partially gets around this by importing static channel backups. This lets it derive some outputs, such as anchor and to-local/to-remote outputs in commitments. However, that doesn't give us details for HTLCs, such as the rHash, or make it possible to validate funding transactions.

This solution

This PR proposes to send derivation information to the remote signer in the partial outputs of the PSBT. This involves:

• Updates to the input package's SignDescriptor struct for creating [][]*psbt.Unknown to populate the psbt.POutput struct.
• Updates to the lnwallet package and subpackages to ensure PSBT partial outputs are populated with derivation info, including full channel configuration info for the funding output. This means remote signers will no longer need to import a channel backup file.
• Updates to signer mocks in input, lnwallet, and lntest/mock to allow checking that the partial outputs have correct derivation info in them.
• Updates to tests in lnwallet and subpackages, contractcourt, htlcswitch, and peer to ensure that any test channels that are created now check PSBTs' output derivation when making signing requests, including funding transactions.

Potential alternatives

We discussed alternative approaches, notably:

• Retrieving wallet and channel state directly from the walletdb and channeldb. This would only be possible with some DB backends, such as Postgres. Boltdb and SQLite are locked exclusively by lnd. In addition, the database structure in the SQL-based backends is expected to change significantly to improve performance by moving away from emulating KVDB to using fully relational tables and queries.
• Updating the SignPSBTRequest to add derivation information. However, there are already fields available in the PSBT itself to provide this information.
• Allowing full derivation information to be retrieved via RPC. This would involve more round trips per signing request and would require the signer to have network access and permissions to make RPC requests against the node.

Steps to Test

The easiest way to test the change is to:

• Run make unit to ensure everything works correctly with the changes
• Comment out lines where unknowns are added, for example in CreateCommitTx
• Run make unit again and verify that tests fail as expected

Pull Request Checklist

Testing

• [x] Your PR passes all CI checks.
• [x] Tests covering the positive and negative (error paths) are included.

Code Style and Documentation

• [x] The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• [x] Commits follow the Ideal Git Commit Structure.
• [x] Any new logging statements use an appropriate subsystem and logging level.
• [ ] There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[aakselrod]

Thanks for the initial review, @guggero! I've addressed most of the comments, but am still working on the PR.

I think I need to do a bit more work on the chanfunding side, including passing the information required to construct the entire BIP32 derivation. I was working on the assumption that the signer would have the master key fingerprint and coin type already, but that isn't standard for PSBTs. I'll add a preparatory commit for passing that into the assemblers. I think I also need to update one more SignDescriptor that I missed in that package, and add some more testing here.

[Roasbeef]

Hi @aakselrod, this looks really cool! Can you add some more detail in the PR body which outlines: the motivation, problem at land, this solution, and potential alternatives? This'll help future reviewers get up to speed with the path here if they weren't listening on all the private/offline conversations.

[aakselrod]

I've done a lot of work in lnwallet/chanfunding to ensure that:

• The funding TX contains all the information needed for the remote signer to validate subsequent commitment TXs
• The funding tests check for this info

I think this PR is ready for review. I'm starting work on the lndsigner side to start validating signing requests using this new metadata.

[aakselrod]

Thanks for the review, Oliver! I was out for a few days, will update code to address your comments over the next few days.

[lightninglabs-deploy]

@aakselrod, remember to re-request review from reviewers when ready

[lightninglabs-deploy]

Closing due to inactivity

[lightninglabs-deploy]

Closing due to inactivity

[guggero]

!lightninglabs-deploy mute