lnwallet+channeldb: add new AuxLeafStore for dynamic aux leaves #8562
Conversation
In this commit, we rename the files as assembler.go houses the primary interfaces/abstractions of the package. In the rest of the codebase, this file is near uniformly called interface.go, so we rename the file to make the repo more digestible at a scan.
In this commit, we consolidate the root bucket TLVs into a new struct. This makes it easier to see all the new TLV fields at a glance. We also convert TLV usage to use the new type param based APis.
This'll allow us to create a funding output that uses musig2, but uses a tapscript tweak rather than a normal BIP 86 tweak.
In most cases, we won't yet be passing a root. The option usage helps us keep the control flow mostly unchanged.
This isn't hooked up yet to the funding manager, but with this commit, we can now start to write internal unit tests that handle musig2 channels with a tapscript root.
With this commit, the channel is now aware of if it's a musig2 channel, that also has a tapscript root. We'll need to always pass in the tapscript root each time we: make the funding output, sign a new state, and also verify a new state.
In this commit, we update all the taproot scripts to also accept an optional aux leaf. This aux leaf can be used to add more redemption paths for advanced channels, or just as an extra commitment space.
In this commit, we add a new type alias for a blob type. This type can be used in areas where a byte slice is used to store a TLV value, which may be a fully opaque nested TLV. We also commit our go.work file to ensure the changes that follow can always build.
In this commit, for each channel, we'll now start to store an optional custom blob. This can be used to store extra information for custom channels in an opauqe manner.
In this commit, we add a new AuxLeafStore which can be used to dynamically fetch the latest aux leaves for a given state. This is useful for custom channel types that will store some extra information in the form of a custom blob, then will use that information to derive the new leaf tapscript leaves that may be attached to reach state.
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent update introduces enhancements to the handling of custom data and auxiliary script paths within the Lightning Network Daemon (LND). Key changes include the addition of a Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| // TestTaprootSenderHtlcSpend tests that all the positive and negative paths | ||
| // for the sender HTLC tapscript tree work as expected. |
There was a problem hiding this comment.
Was the removal of the comment intentional? Occurs a few times
ah I see it's moved
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
go.workis excluded by:!**/*.workgo.work.sumis excluded by:!**/*.sumtlv/go.modis excluded by:!**/*.modtlv/go.sumis excluded by:!**/*.sum
Files selected for processing (12)
- channeldb/channel.go (8 hunks)
- channeldb/channel_test.go (1 hunks)
- input/script_utils.go (25 hunks)
- input/size_test.go (14 hunks)
- input/taproot.go (2 hunks)
- input/taproot_test.go (20 hunks)
- lnwallet/channel.go (11 hunks)
- lnwallet/commitment.go (18 hunks)
- lnwallet/wallet.go (4 hunks)
- tlv/record.go (1 hunks)
- watchtower/blob/justice_kit.go (3 hunks)
- watchtower/blob/justice_kit_test.go (3 hunks)
Additional comments: 51
input/taproot.go (1)
- 25-27: The introduction of
AuxTapLeafas a type alias for an optional tapscript leaf is a welcome addition, enhancing the flexibility and customization capabilities of the LND. However, it would be beneficial to add a brief documentation comment above theAuxTapLeafdeclaration to explain its purpose and usage for future maintainability.tlv/record.go (1)
- 20-22: The introduction of
Blobas a type alias for a byte slice to represent an encoded TLV stream is a straightforward and effective change, improving code readability. However, adding a brief documentation comment above theBlobdeclaration to explain its purpose and usage would enhance future maintainability.watchtower/blob/justice_kit.go (4)
- 13-13: The import of
fnis justified and aligns with the PR objectives, enabling the use of modern Go features for handling optional values effectively.- 311-311: The TODO comment could benefit from additional context or a reference to an issue tracker to provide clarity on the required auxiliary leaf tower updates and facilitate tracking and accountability.
- 315-315: The use of
fn.None[txscript.TapLeaf]()inNewTaprootJusticeKitis appropriate and aligns with the PR objectives of integrating optional auxiliary leaves. This change enhances the flexibility and customization capabilities of the LND.- 423-423: The inclusion of
fn.None[txscript.TapLeaf]()inToRemoteOutputSpendInfosupports the integration of optional auxiliary leaves, consistent with the PR's objectives. This enhances the LND's flexibility and customization capabilities.watchtower/blob/justice_kit_test.go (3)
- 15-15: The addition of the
fnpackage fromgithub.com/lightningnetwork/lnd/fnintroduces functional programming utilities, specifically for handling optional values. This is a modern approach to dealing with optional data and can enhance code readability and safety by avoidingnilchecks.- 308-310: Using
fn.None[txscript.TapLeaf]()as an argument forinput.NewRemoteCommitScriptTreeis a clean and safe way to handle optionalTapLeafvalues. This approach enhances code readability and safety by explicitly indicating the absence of a value, avoiding potentialnilpointer dereferences.- 466-466: The use of
fn.None[txscript.TapLeaf]()for specifying an optionalTapLeafininput.NewLocalCommitScriptTreeis consistent with modern coding practices. It clearly communicates the intention and avoids the pitfalls associated withnilvalues, enhancing both code safety and readability.input/size_test.go (2)
- 14-14: The introduction of the
fnpackage and itsNonefunction for handlingtxscript.TapLeafinstances is a notable change. This approach simplifies the handling of optional taproot leaves by providing a clear way to represent the absence of a leaf. This is a good use of functional programming concepts to improve code readability and maintainability.- 855-855: The consistent use of
fn.None[txscript.TapLeaf]()across various test cases for generating taproot script trees without specific leaves is a good practice. It ensures that the tests remain focused on the intended aspects of transaction and script handling without unnecessary complexity. This approach enhances the clarity and maintainability of the test code.Also applies to: 889-889, 923-923, 990-990, 1030-1030, 1079-1079, 1121-1121, 1163-1163, 1210-1210, 1271-1271, 1317-1317, 1403-1403, 1472-1472
lnwallet/commitment.go (7)
- 227-233: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [230-240]
The introduction of the
auxLeafparameter in theCommitScriptToSelffunction enhances the flexibility of commitment transactions by allowing the inclusion of auxiliary tapscript leaves. This change aligns with the PR's objective to introduce dynamic auxiliary leaves based on custom channel information. However, it's crucial to ensure that all calls to this function throughout the codebase have been updated to include the new parameter to avoid compilation errors or unintended behavior.
- 291-299: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [294-325]
The addition of the
auxLeafparameter in theCommitScriptToRemotefunction, allowing for the inclusion of auxiliary tapscript leaves in the to_remote output of commitment transactions, is a significant enhancement. This change supports the PR's goal of introducing dynamic auxiliary leaves. Similar to the previous comment, it's essential to verify that all invocations of this function have been updated accordingly.
- 616-634: The
CommitAuxLeavesstruct, which stores potential auxiliary leaves for the remote and local output, is a well-structured way to manage these leaves. This struct facilitates the handling of auxiliary leaves in a structured manner, contributing to the maintainability and readability of the code.- 636-650: The
AuxLeafStoreinterface is a crucial addition for fetching and refreshing auxiliary tapscript leaves based on an opaque blob. This interface abstracts the underlying mechanism for managing auxiliary leaves, promoting modularity and flexibility in the codebase. It's important to ensure that the implementation of this interface is robust and thoroughly tested, especially theFetchLeavesandRefreshBlobmethods, to prevent potential issues in dynamic leaf management.- 729-746: The
auxLeavesFromBlobfunction, which attempts to fetch auxiliary leaves given a blob and a leaf store, is a critical component for integrating auxiliary leaves into the commitment transaction process. This function effectively bridges the gap between the opaque blob and the auxiliary leaves needed for the commitment transaction. It's important to ensure that error handling and edge cases are adequately addressed in the implementations of theFetchLeavesmethod within theAuxLeafStoreinterface to prevent issues during runtime.- 964-989: The modifications in the
CreateCommitTxfunction to include auxiliary leaves in the commitment transaction are aligned with the PR's objectives. The handling ofauxLeavesto dynamically include auxiliary tapscript leaves based on the channel's state enhances the transaction's flexibility. It's crucial to ensure comprehensive testing of these changes, particularly the correct inclusion of auxiliary leaves in various commitment transaction scenarios.- 1324-1362: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1296-1359]
The
findOutputIndexesFromRemotefunction's modifications to support auxiliary leaves when finding output indexes from the remote commitment transaction are a necessary extension of the auxiliary leaves feature. This function's changes ensure that the auxiliary leaves are considered when determining the output indexes, which is crucial for the correct functioning of the feature. It's recommended to add tests covering the scenarios where auxiliary leaves are present and correctly identified in the remote commitment transaction.Would you like assistance in creating these tests or should I open a GitHub issue for tracking?
input/taproot_test.go (6)
- 4-6: The addition of imports for
bytesandfmtis appropriate given their usage in the newly introduced tests and functions. This change aligns with the need to manipulate byte slices and format strings within the test cases.- 34-42: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [37-56]
The modification of
newTestSenderHtlcScriptTreeto accept anauxLeafparameter and its usage within the function is a significant enhancement. This change allows for testing the behavior of HTLC script trees with auxiliary leaves, which is crucial for the new functionality introduced in the PR. However, it's important to ensure that all calls to this function throughout the test suite have been updated to include theauxLeafparameter.Verification successful
The verification process confirms that the modification of
newTestSenderHtlcScriptTreeto include anauxLeafparameter has been correctly implemented in the codebase, with all calls to this function updated accordingly. There is only one occurrence of this function call, and it includes the requiredauxLeafparameter.* 434-462: > 📝 **NOTE** > This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [215-457]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all calls to newTestSenderHtlcScriptTree include the auxLeaf parameter. ast-grep --lang go --pattern $'newTestSenderHtlcScriptTree($_, $_)'Length of output: 155
Script:
#!/bin/bash # Verify all occurrences of newTestSenderHtlcScriptTree, regardless of the number of parameters. ast-grep --lang go --pattern $'newTestSenderHtlcScriptTree($$$)'Length of output: 152
The refactoring of
TestTaprootSenderHtlcSpendto iterate overhasAuxLeafvalues and test with different configurations is a robust approach to ensure the functionality works as expected with and without auxiliary leaves. This pattern is consistent across several test functions, enhancing the test coverage for the new feature. It's crucial to maintain this pattern for all relevant tests to ensure comprehensive coverage.
- 477-485: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [480-501]
Similar to the
newTestSenderHtlcScriptTree, thenewTestReceiverHtlcScriptTreefunction has been modified to accept anauxLeafparameter. This consistency in handling auxiliary leaves across sender and receiver script trees is commendable and necessary for thorough testing of the new functionality. Ensure that all invocations of this function are updated accordingly.Verification successful
The verification process confirms that the modification to include the
auxLeafparameter in thenewTestReceiverHtlcScriptTreefunction is correctly applied in its call, as indicated by the script output. This aligns with the expectations set in the review comment.* 975-985: > 📝 **NOTE** > This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [956-982]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all calls to newTestReceiverHtlcScriptTree include the auxLeaf parameter. ast-grep --lang go --pattern $'newTestReceiverHtlcScriptTree($_, $_)'Length of output: 159
The introduction of the
newTestCommitScriptTreefunction with anauxLeafparameter for both local and remote commit script trees is a critical update. This change allows for testing the commit script trees with auxiliary leaves, aligning with the PR's objectives to enhance flexibility and customization in channel and transaction management. It's essential to verify that the function is utilized correctly across the test suite.Verification successful
The verification process confirms that all calls to
newTestCommitScriptTreewithin the test suite correctly include theauxLeafparameter, aligning with the objectives mentioned in the review comment. This ensures the function's intended use for testing commit script trees with auxiliary leaves is adhered to.* 1690-1710: > 📝 **NOTE** > This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1693-1716]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all calls to newTestCommitScriptTree include the auxLeaf parameter correctly. ast-grep --lang go --pattern $'newTestCommitScriptTree($_, $_)'Length of output: 253
The
newTestSecondLevelHtlcTreefunction's adaptation to include anauxLeafparameter is a significant enhancement. This modification ensures that the second-level HTLC script trees can be tested with auxiliary leaves, further extending the test coverage for the new feature. Consistency in handling auxiliary leaves across different script trees demonstrates a thorough approach to testing.Verification successful
The verification process confirms that the
newTestSecondLevelHtlcTreefunction is indeed called with theauxLeafparameter, as indicated by the review comment. The script output shows that all identified calls to this function include theauxLeafparameter, aligning with the enhancement described in the review. This adaptation ensures that the second-level HTLC script trees can be tested with auxiliary leaves, demonstrating a thorough approach to testing.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all calls to newTestSecondLevelHtlcTree include the auxLeaf parameter. ast-grep --lang go --pattern $'newTestSecondLevelHtlcTree($_, $_)'Length of output: 154
channeldb/channel_test.go (1)
- 361-361: The addition of the
CustomBlobfield to theOpenChannelstruct in thecreateTestChannelStatefunction is a significant change. This field allows for the storage of custom information related to channels, enhancing flexibility and customization for developers working with LND.lnwallet/wallet.go (6)
- 1458-1467: The introduction of
createCommitOptsanddefaultCommitOptsprovides a structured way to handle options for creating commitment transactions. This approach enhances readability and maintainability by encapsulating options within a struct.- 1469-1471: The
CreateCommitOpttype definition as a functional option is a good practice. It allows for flexible and extensible configuration of commitment transactions without changing the function signature.- 1479-1490: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1482-1513]
The
CreateCommitmentTxnsfunction has been modified to accept functional options for creating commitment transactions. This change improves the function's flexibility and extensibility by allowing additional parameters to be passed without altering the function's signature. However, ensure that all calls to this function throughout the codebase have been updated to use the new functional options pattern.
- 1482-1487: The pattern of using functional options (
CreateCommitOpt) for configuring the creation of commitment transactions is a solid design choice. It enhances code readability and maintainability by allowing for more flexible function invocation.- 1479-1490: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1482-1513]
The addition of
leaseExpiryandoptsparameters toCreateCommitmentTxnsfunction indicates an enhancement in the functionality related to commitment transactions. Ensure that the handling ofleaseExpiryand the application of options throughoptsare thoroughly tested, especially in scenarios whereleaseExpiryimpacts the transaction's validity or behavior.
- 1479-1490: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1482-1513]
The
CreateCommitmentTxnsfunction now includes handling forleaseExpiryand functional options (opts). This enhancement allows for more flexible and dynamic creation of commitment transactions. It's important to ensure that all existing and new calls to this function are correctly updated to utilize these new capabilities.input/script_utils.go (7)
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3050-3058]
Looks good, correctly implements single tweak generation.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3060-3084]
The
TweakPubKeyfunction correctly tweaks a public key using a commit point. It's essential for the security of off-chain contracts in the Lightning Network.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3086-3093]
The
TweakPubKeyWithTweakfunction provides flexibility by allowing direct tweak bytes input, which is useful for various operations in the Lightning Network.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3095-3108]
The
TweakPrivKeyfunction correctly tweaks a private key, which is crucial for deriving the correct private key corresponding to the tweaked public key in the Lightning Network.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3110-3142]
The
DeriveRevocationPubkeyfunction correctly derives the revocation public key, a key component in the penalty mechanism of the Lightning Network.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3144-3158]
The
DeriveRevocationPrivKeyfunction correctly derives the revocation private key, enabling a node to claim funds from a counterparty who broadcasts a revoked state.
- 673-684: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [3160-3164]
The
ComputeCommitmentPointfunction correctly generates a commitment point from a secret, essential for key randomization in the Lightning Network.channeldb/channel.go (5)
- 242-242: The
customBlobfield has been added to thechanAuxDatastruct.This addition allows for the storage of custom information related to channels, enhancing flexibility and extensibility.
- 245-273: The
encodemethod inchanAuxDatahas been updated to include the serialization of thecustomBlobfield.The method correctly appends the
customBlobto the TLV records if present, ensuring the custom data is serialized along with other channel data.
- 276-310: The
decodemethod inchanAuxDatahas been updated to include the deserialization of thecustomBlobfield.The method correctly extracts the
customBlobfrom the TLV stream if present, ensuring the custom data is deserialized along with other channel data.
- 327-329: The
toOpenChanmethod inchanAuxDatanow includes the assignment of thecustomBlobfield to theOpenChannelstruct.This ensures that the custom data is correctly transferred from the auxiliary data structure to the main channel structure.
- 359-363: The
newChanAuxDataFromChanfunction has been updated to include thecustomBlobfield.This ensures that the custom data is correctly included when creating a new
chanAuxDatainstance from anOpenChannelstruct.lnwallet/channel.go (8)
- 1258-1260: The addition of the
leafStorefield to theLightningChannelstruct is a key part of integrating theAuxLeafStore. This change allows theLightningChannelto retrieve extra tapscript leaves for special custom channel types. Ensure that theAuxLeafStoreinterface is well-defined and that its methods are implemented correctly to avoid runtime errors.- 1337-1337: The
leafStorefield is also added to thechannelOptsstruct, which is consistent with the addition to theLightningChannelstruct. This ensures that the custom leaf store can be passed through the channel options during initialization. It's important to verify that all code paths that createchannelOptsinstances properly handle this new field.- 1369-1374: The
WithLeafStorefunction is a well-structured way to specify a custom leaf store for the channel. This approach follows the functional options pattern, which is a good practice for extending functionality without breaking existing code. Ensure that this function is used wherever aLightningChannelis initialized with a custom leaf store.- 1417-1426: The initialization of
LightningChannelnow includes theleafStorefrom options, which is a necessary change to support the new functionality. Additionally, passing theleafStoreto theNewCommitmentBuilderfunction is crucial for integrating the custom leaf retrieval into the commitment transaction generation process. Ensure that theNewCommitmentBuilderfunction properly utilizes theleafStore.- 2484-2484: The use of
fn.None[txscript.TapLeaf]()in theCommitScriptToSelfcall suggests that the auxiliary leaves are optional for certain script generation calls. This is a good approach to maintain backward compatibility and flexibility. However, ensure that thefn.Noneandfn.Somefunctions are well-tested, especially in the context of optional types in Go.- 5824-5824: The modification to include
leafStorein thefindOutputIndexesFromRemotefunction call is crucial for ensuring that auxiliary leaves are considered when finding output indexes. This change integrates the custom leaf retrieval into a critical part of the channel state management. Verify that thefindOutputIndexesFromRemotefunction correctly handles theleafStore.- 7129-7129: The use of
fn.None[txscript.TapLeaf]()in theTaprootSecondLevelScriptTreecall is consistent with the optional nature of auxiliary leaves. This approach allows for flexibility in script tree generation. Ensure that theTaprootSecondLevelScriptTreefunction is prepared to handle cases where auxiliary leaves are not provided.- 7374-7374: This is a duplicate of the previous comment regarding the use of
fn.None[txscript.TapLeaf]()in theTaprootSecondLevelScriptTreecall. Ensure that the handling of optional auxiliary leaves is consistent across all relevant function calls.
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Cool cool cool, I can see where things are going, very exciting. Did a first pass only as this is still in draft. But looking pretty, good, just need to finish drawing the owl :)
lnwallet/commitment.go
Outdated
| type AuxLeafStore interface { | ||
| // FetchLeaves attempts to fetch the auxiliary leaves for the | ||
| // commitment. | ||
| FetchLeaves(chanPoint wire.OutPoint, |
There was a problem hiding this comment.
I'm not sure I understand this method signature. Why would you pass in the auxBlob? Isn't that what contains the aux leaves already? So it would be more of a parse/extract than a fetch? Or what am I misunderstanding?
There was a problem hiding this comment.
From lines 636-637
// AuxLeafStore is used to optionally fetch auxiliary tapscript leaves for the
// commitment transaction given an opaque blob.
So seems like it's expected to provide the blob. I do see your argument tho, perhaps worth renaming something here
There was a problem hiding this comment.
Yeh I can probably rename things a bit, it's more of a parse/extract, although the implementor could also store that information, but my latest thinking is that it's derived on the fly.
I think the "shape" of these interfaces will likely change a bit as we start on the concrete implementations elsewhere.
The layer of indirection here is that lnd doesn't know exactly what's in the blob, but given the blob and the commitment it can obtain the aux tap leaves.
GeorgeTsagk
left a comment
GeorgeTsagk
left a comment
There was a problem hiding this comment.
Good stuff 🧑🦲 🥕 🥕 🧑🦱 , really looking forward to the tests for 0d18ae6
lnwallet/commitment.go
Outdated
| type AuxLeafStore interface { | ||
| // FetchLeaves attempts to fetch the auxiliary leaves for the | ||
| // commitment. | ||
| FetchLeaves(chanPoint wire.OutPoint, |
There was a problem hiding this comment.
From lines 636-637
// AuxLeafStore is used to optionally fetch auxiliary tapscript leaves for the
// commitment transaction given an opaque blob.
So seems like it's expected to provide the blob. I do see your argument tho, perhaps worth renaming something here
This type is useful when one wants to encode an integer as an underlying BigSize record. It wraps any integer, then handles the transformation into and out of the BigSize encoding on disk.
This'll be useful for custom channel types that want to store extra information that'll be useful to help handle channel revocation cases.
This may be useful for custom channel types that base everything off the index (a global value) rather than the output index (can change with each state).
This blob can be used to store funding specific information.
In this commit, we add some useful type definitions for the aux leaf, and a helper function to cut down on some line noise.
In this commit, we add a TLV blob to the PaymentDescriptor struct. We also now thread through this value from the UpdateAddHTLC message to the PaymentDescriptor mapping, and the other way around.
We'll need this later on to ensure we can always interact with the new aux blobs at all stages of commitment transaction construction.
In addition to the leaves for the commitment outputs, we'll also need to interact with leaves for all the HTLC outputs as well.
When constructing or validating a commitment transaction, we'll need to be able to obtain the aux leaves when: * we're making a new state * we're making a resolution for an existing state * we're handling an on chain breach The three top level methods now handle these three cases. The final method `ApplyHtlcView` will be used when we've made a new state, and need to derive the opaque blob that we'll store in the db for that respective commitment.
In this commit, we also add the custom TLV blob to the internal commitment struct that we use within the in-memory commitment linked list. This'll be useful to ensure that we're tracking the current blob for our in memory commitment for when we need to write it to disk.
In this commit, we start to thread thru the new aux tap leaf structures to all relevant areas. This includes: commitment outputs, resolution creation, breach handling, and also HTLC scripts. Given the aux leaf store, and a struct that describes the current commitment, we can obtain the CommitAuxLeaves struct, then use that to derive the aux leaves for the commitment outputs and also HTLCs as well. When restoring commitments and pay descs from disk, we'll store the aux leaves as custom TLV blobs on disk, then use the aux leaf store to map back to the concrete aux leaves when needed.
|
Pushed up some more commits, tentatively removing from draft. Commits need a good bit of clean up and consolidation though. After the next PR, will be able to go back and meld some commits into themselves. |
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Looks very good! Was able to hook everything up in litd (with some small tweaks, see inline comments).
Still working on integrating everything, so will probably have more comments at a later date.
guggero
left a comment
guggero
left a comment
There was a problem hiding this comment.
Did a deep dive while starting on the implementation for the aux leaf store.
Have a couple of questions around the method signatures of the aux leaf store.
|
@Roasbeef I think we also need error return values on the interface methods of the aux leaf store, otherwise it's hard to communicate if something goes wrong (and we definitely don't want to burn the assets by returning I implemented that in this branch: https://github.com/lightningnetwork/lnd/tree/custom-channels-integration (see last 4 commits, I also hooked up the aux leaf store everywhere in those commits, feel free to take over). |
|
@GeorgeTsagk: review reminder |
guggero
force-pushed
the
funding-tapcript
branch
from
33e0cea to
08a6c9c
Compare
|
Replacement version of this PR with all current review comments applied and rebased on the 0-19-staging branch created here: #8684 |
5 participants
In this commit, we add a new abstraction, the
AuxLeafStorethat's capable of dynamically deriving hte aux leaves for a new commitment state given the channel point and custom blob stored. The custom blob can be used by external programers to store custom information for a channel which can then be used to derive the aux leaves for each new state. All added features in this PR as optional, and if not set, don't alter the normal channel flow.NOTE: In this PR, I've started to also commit my local
go.workfiles, as I have some submodules change, and committing thego.workfile allows the PR to build w/o any temporary commits or PRs.TODO
Summary by CodeRabbit