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
[5/5]: lnwallet: extensions to aux leaf store to integrate custom channels #8641
base: aux-leaf-signing
Are you sure you want to change the base?
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
Pull reviewers statsStats of the last 30 days for lnd:
|
lnwallet/commitment.go
Outdated
@@ -654,25 +654,29 @@ type AuxLeafStore interface { | |||
// FetchLeavesFromView attempts to fetch the auxiliary leaves that | |||
// correspond to the passed aux blob, and pending fully evaluated HTLC | |||
// view. | |||
FetchLeavesFromView(prevBlob tlv.Blob, view *HtlcView, | |||
FetchLeavesFromView(chanState *channeldb.OpenChannel, prevBlob tlv.Blob, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively could just be that special funding blob (the one in the open channel struct), we could also use a type alias here. Unsure though if we do need all that other information or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the local/remote channel config, custom blob, thaw height, initiator and channel type. So it's easiest to just pass in the whole channel state. Not sure what a type alias would give us? Other than perhaps being a bit shorter.
lnwallet/commitment.go
Outdated
// of the pkScript and CLTV timeout, and the value is the index the | ||
// output should be placed at. The CLTV timeout is encoded as a big | ||
// endian before hashing and is assumed to be 0 for non-HTLC outputs. | ||
// sha256(pkScript || bigEndian(cltvDelta)) -> outputIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this also incudes the value as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, good catch. Fixed.
lnwallet/commitment.go
Outdated
// lets us skip sending the entire transaction over, instead we'll just | ||
// send signatures. | ||
if auxLeaves.IsSome() && | ||
auxLeaves.UnsafeFromSome().CustomOutputOrder != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to mask this a bit more, then we can have a functional option to pass in a new custom sorting interface/type. Default is the existing one, then we know to override it at the config level for these special channels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'm meeting you half way: Instead of passing in a function, we now require a sort function to be returned. But that way at least the sorting logic is not in the lnd
code base anymore.
lnwallet/commitment.go
Outdated
@@ -660,7 +660,8 @@ type AuxLeafStore interface { | |||
// correspond to the passed aux blob, and pending original (unfiltered) | |||
// HTLC view. | |||
FetchLeavesFromView(chanState *channeldb.OpenChannel, prevBlob tlv.Blob, | |||
unfilteredView *HtlcView, | |||
unfilteredView *HtlcView, isOurCommit bool, ourBalance, | |||
theirBalance lnwire.MilliSatoshi, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Balances needed for the new sorting algo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for sorting, making sure we have non-dust BTC amounts for any asset amounts and for determining whether we need commitment anchor outputs (which we need to create exclusion proofs for).
d288b6a
to
f7f875b
Compare
cb514a1
to
f35aead
Compare
f35aead
to
cf965f0
Compare
1858e19
to
ffa19c0
Compare
cf965f0
to
0e6a9dd
Compare
ebfde14
to
0b991e2
Compare
0b991e2
to
a5d1e43
Compare
8ed50cf
to
cfd4314
Compare
a5d1e43
to
15abb14
Compare
15abb14
to
09b34f1
Compare
09b34f1
to
fedcac2
Compare
fedcac2
to
f9b366e
Compare
This PR should now contain everything from the PoC branch and is ready for review. |
f9b366e
to
5a834c1
Compare
This commit adds an optional data parser that can inspect and in-place format custom data of certain RPC messages.
5a834c1
to
7b9e627
Compare
Depends on #8632.
Extends the aux leaf store with more required parameters.
Link to all PRs in the saga: