[custom channels]: update to unmerged release branch of lnd 0.18.4-beta #1130
Conversation
guggero
changed the title
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
78fb484 to
ed97350
Compare
Pull Request Test Coverage Report for Build 11460101149Details
💛 - Coveralls |
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
ed97350 to
ed3e7b3
Compare
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
ed3e7b3 to
5f1e5b7
Compare
guggero
force-pushed
the
update-to-lnd-18-4
branch
2 times, most recently
from
0a37fcf to
45d3a96
Compare
guggero
changed the title
guggero
force-pushed
the
update-to-lnd-18-4
branch
2 times, most recently
from
95064dd to
32bff76
Compare
|
Similar to lightninglabs/loop#828, I bumped to |
rfqmsg/messages.go
Outdated
| // We need to make sure the block height is within the range of valid | ||
| // SCID block heights. | ||
| scid := lnwire.NewShortChanIDFromInt(scidInteger) | ||
|
|
||
| minBlock := uint32(aliasmgr.AliasStartBlockHeight) | ||
| maxBlock := uint32(aliasmgr.AliasEndBlockHeight) | ||
|
|
||
| // If we're within the valid range, we can return the SCID as is. | ||
| if aliasmgr.IsAlias(scid) { | ||
| return SerialisedScid(scid.ToUint64()) | ||
| } | ||
|
|
||
| // Normalize the value relative to min, then wrap within blockRange, and | ||
| // finally shift back by min. Generated by ChatGPT and I couldn't find a | ||
| // better way to do this. | ||
| blockRange := maxBlock - minBlock + 1 | ||
| scid.BlockHeight = ((scid.BlockHeight-minBlock)%blockRange+blockRange)% | ||
| blockRange + minBlock | ||
|
|
||
| return SerialisedScid(scid.ToUint64()) |
There was a problem hiding this comment.
I think this shifting will need to be described in the BLIP:
Given a valid `rfq_id`, the RFQ specific SCID `tap_rfq_scid` is defined by
taking the last `8` bytes of the `rfq_id` and interpreting them as a 64-bit
integer.
It would probably be simpler at this point to just generate a SCID independent of RFQ ID and pass it to our peer in the RFQ request message.
There might be an advantage in separating the RFQ message ID from the SCID. We might be able to re-use the same SCID across multiple quotes for example. So every payment would correspond to a single RFQ quote but then multiple payments can share the same SCID.
There may be an advantage there when we come up with a formulation that avoids quote re-use.
There was a problem hiding this comment.
I don't think separating the two values is a good idea at this point. I went with the approach to generate random IDs until they fall within the correct range.
So the bLIP could just say: "The rfq_id must be drawn in a way that the derived tap_rfq_scid falls within the correct range for an SCID alias (16_000_000 <= block height < 16_250_000).
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
32bff76 to
118d738
Compare
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
118d738 to
fc1a430
Compare
There was a problem hiding this comment.
Lgtm fc1a430
i think the htlc intercept itest might be flaky, kicked it
Is this PR going to get more commits as we move forward, or is this it's final-ish state?
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
fc1a430 to
1bbe907
Compare
There will be one more round of |
A recent change to the lnd SCID alias RPCs requires us to generate SCID aliases in a certain range. Because we derive the alias from the randomly generated RFQ ID, we need to make sure we derive a random ID that can be successfully transformed into a valid SCID alias.
We now get custom_channel_data fields in the ListInvoices/LookupInvoice as well as in the ListPayments RPC. We add those fields to our data parser implementation.
This commit unifies the use of the lfn.Result return type as a preparation for the final commit in this PR, where we change a bunch of function/method signatures to use the Result type as well. We use lnf.Err(fnt.Errorf()) instead of lnf.Errf because the intellisense of some IDEs complain about the '%w' verb only being usable with fmt.Errorf().
The lnd version we're going to use in the next commit requires a slightly more up-to-date version of Golang.
With this commit we switch the testing branch to the currently unmerged 'update-to-lnd-18-4' branch which will soon be in master. But because that branch depends on the commits in this PR, we first have to point to it, get this PR merged, then merge update-to-lnd-18-4, then change this to 'master'.
guggero
force-pushed
the
update-to-lnd-18-4
branch
from
1bbe907 to
d74dd65
Compare
Due to the new way we calculate asset rates, the rounding works slightly differently. That's why we need to add a single milli-satoshi to the total inbound margin.
|
I had to add 2 commits to address some of the RFQ changes (and to make the itests pass with the new way asset to BTC conversions now happen with more precision). |
This PR prepares all compile time changes that will come with
lnd v0.18.4-beta. We don't have a tag forv0.18.4-betayet, but the release branch will remain stable (meaning only new PRs will be added).So we'll still need a version bump PR later, but it should only be a
go.mod/go.sumbump, which means the main review can be done in this PR.Full serialized dependency list can be seen here: lightninglabs/lightning-terminal#848