-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add XFindBaseLocalChanAlias
RPC
#10133
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -242,11 +242,12 @@ func (m *Manager) populateMaps() error { | |||||||||||||||||||||||||||||||||||||||
// AddLocalAlias adds a database mapping from the passed alias to the passed | ||||||||||||||||||||||||||||||||||||||||
// base SCID. The gossip boolean marks whether or not to create a mapping | ||||||||||||||||||||||||||||||||||||||||
// that the gossiper will use. It is set to false for the upgrade path where | ||||||||||||||||||||||||||||||||||||||||
// the feature-bit is toggled on and there are existing channels. The linkUpdate | ||||||||||||||||||||||||||||||||||||||||
// flag is used to signal whether this function should also trigger an update | ||||||||||||||||||||||||||||||||||||||||
// on the htlcswitch scid alias maps. | ||||||||||||||||||||||||||||||||||||||||
// the feature-bit is toggled on and there are existing channels. The base | ||||||||||||||||||||||||||||||||||||||||
// lookup flag indicates whether we want store a mapping from the alias to its | ||||||||||||||||||||||||||||||||||||||||
// base scid. The linkUpdate flag is used to signal whether this function should | ||||||||||||||||||||||||||||||||||||||||
// also trigger an update on the htlcswitch scid alias maps. | ||||||||||||||||||||||||||||||||||||||||
func (m *Manager) AddLocalAlias(alias, baseScid lnwire.ShortChannelID, | ||||||||||||||||||||||||||||||||||||||||
gossip, linkUpdate bool) error { | ||||||||||||||||||||||||||||||||||||||||
gossip, baseLookup, linkUpdate bool) error { | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
// We need to lock the manager for the whole duration of this method, | ||||||||||||||||||||||||||||||||||||||||
// except for the very last part where we call the link updater. In | ||||||||||||||||||||||||||||||||||||||||
|
@@ -302,8 +303,9 @@ func (m *Manager) AddLocalAlias(alias, baseScid lnwire.ShortChannelID, | |||||||||||||||||||||||||||||||||||||||
// Update the aliasToBase and baseToSet maps. | ||||||||||||||||||||||||||||||||||||||||
m.baseToSet[baseScid] = append(m.baseToSet[baseScid], alias) | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
// Only store the gossiper map if gossip is true. | ||||||||||||||||||||||||||||||||||||||||
if gossip { | ||||||||||||||||||||||||||||||||||||||||
// Only store the gossiper map if gossip is true, or if the caller | ||||||||||||||||||||||||||||||||||||||||
// explicitly asked to store this reverse mapping. | ||||||||||||||||||||||||||||||||||||||||
if gossip || baseLookup { | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it the desired behaviour that this reverse lookup is not persisted? currently, all these mappings added will be lost on restart There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we add a local alias we also store a (kvdb) entry that maps the alias to its base Line 297 in d821318
Then on restart when we call the helper Line 178 in d821318
and populate Lines 225 to 234 in d821318
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok now I realise that After looking over the
If we make sure to also delete the alias entries from the kvdb aliasToBase bucket here Lines 385 to 391 in d821318
Then we can totally strip away the
Note: if an RPC user added custom aliases before six confs, then those would also be deleted by above approach. In this case: I lean towards a) for the last note. Also worth investigating how this affects interaction with other systems |
||||||||||||||||||||||||||||||||||||||||
m.aliasToBase[alias] = baseScid | ||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
@@ -342,7 +344,9 @@ func (m *Manager) GetAliases( | |||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
// FindBaseSCID finds the base SCID for a given alias. This is used in the | ||||||||||||||||||||||||||||||||||||||||
// gossiper to find the correct SCID to lookup in the graph database. | ||||||||||||||||||||||||||||||||||||||||
// gossiper to find the correct SCID to lookup in the graph database. It can | ||||||||||||||||||||||||||||||||||||||||
// also be used to look up the base for manual aliases that were added over the | ||||||||||||||||||||||||||||||||||||||||
// RPC. | ||||||||||||||||||||||||||||||||||||||||
func (m *Manager) FindBaseSCID( | ||||||||||||||||||||||||||||||||||||||||
alias lnwire.ShortChannelID) (lnwire.ShortChannelID, error) { | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
|
@@ -446,7 +450,7 @@ func (m *Manager) DeleteLocalAlias(alias, | |||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
// Finally, we'll delete the aliasToBase mapping from the Manager's | ||||||||||||||||||||||||||||||||||||||||
// cache (but this is only set if we gossip the alias). | ||||||||||||||||||||||||||||||||||||||||
// cache. | ||||||||||||||||||||||||||||||||||||||||
delete(m.aliasToBase, alias) | ||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||
// We definitely need to unlock the Manager before calling the link | ||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,12 @@ a certain amount of msats. | |
- Added support for [P2TR Fallback Addresses]( | ||
https://github.com/lightningnetwork/lnd/pull/9975) in BOLT-11 invoices. | ||
|
||
- A new experimental RPC endpoint | ||
[XFindBaseLocalChanAlias](https://github.com/lightningnetwork/lnd/pull/10133) | ||
GeorgeTsagk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
was added for looking up the base scid for an scid alias. Aliases that were | ||
manually created via the `XAddLocalChanAliases` endpoint will get lost on | ||
restart. | ||
Comment on lines
+67
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted back to the non-persistent approach, turns out there's too many things to deal with to properly support persistence for manually added aliases. Added the note on the release doc as per your suggestion @ellemouton. Will re-iterate in the future for persistence. |
||
|
||
## Functional Enhancements | ||
* [Add](https://github.com/lightningnetwork/lnd/pull/9677) | ||
`ConfirmationsUntilActive` and `ConfirmationHeight` field to the | ||
|
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.
i dont think we can do this cause now it will be stored in the
m.aliasToBase
map ifgossip
is false butbaseLookup
is true.. meaning it will be available in the map when the gossiper callsFindBaseByAlias
which we explicitly dont want, right?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.
Yeah, now
baseLookup
flag alone is sufficient to store this in the map.The gossiper will only look up scids from received channel updates, so whether we have more values there stored for reverse look-up doesn't seem like an issue as it's unlikely that the gossiper will look-up an scid that we manually created.
W.r.t conflicts in the alias scid range:
we will currently fail if the alias that we're trying to add via RPC already exists for another base scid, so it's even more unlikely that the gossiper will get a bad value out of the lookup, as we explicitly care about the uniqueness of the alias (see related code)