Skip to content
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

[feature]: Allow persisting dynamic configured alias when not set in lnd.conf or arguments #7123

Open
alexbosworth opened this issue Nov 7, 2022 · 12 comments · May be fixed by #8690
Open
Assignees
Labels
beginner Issues suitable for new developers bug fix config Parameters/arguments/config file related issues/PRs enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND
Milestone

Comments

@alexbosworth
Copy link
Contributor

If you set your alias using the UpdateNodeAnnouncement RPC call it will only maintain this as an update until a restart. Restarting clears all of the node announcement settings.

Instead, if a direct configuration is absent the node announcement settings could be persisted when set and used on restart

Otherwise there needs to be a new command issued on every restart to maintain the dynamic setting or the configuration elements must be changed

If the configuration is set at any point through lnd.conf or an argument, it does make sense for that to take precedence in a restart over the persisted dynamically set last setting

@alexbosworth alexbosworth added the enhancement Improvements to existing features / behaviour label Nov 7, 2022
@joostjager
Copy link
Collaborator

Related #5587 (comment) and also applies to #7094

@saubyk saubyk added the config Parameters/arguments/config file related issues/PRs label Nov 7, 2022
@Roasbeef Roasbeef added beginner Issues suitable for new developers good first issue Issues suitable for first time contributors to LND labels Apr 19, 2023
@Abdulkbk
Copy link

Abdulkbk commented Apr 9, 2024

Hi everyone! I will begin work on this issue @saubyk, @Roasbeef

@saubyk saubyk added this to the 0.19.0 milestone Apr 9, 2024
@Abdulkbk
Copy link

Hi @alexbosworth! thanks for opening this issue. I did some research and I have some questions that will clarify things and help me move forward.

Instead, if a direct configuration is absent the node announcement settings could be persisted when set and used on restart

This means we should only persist the alias when it's not set in lnd.conf right? And does it make sense to persist Alias in the db and retrieve on restart or is there a better way to do so?

Additionally, should we persist the entire announcement settings (including Alias, Color, ...) if they're updated using the updatenodeannouncement RPC or just Alias.

@alexbosworth
Copy link
Contributor Author

What should happen is debatable I guess. Persistence is nice for people who want to control their LND from the API rather than from both the API and the conf file. Lack of persistence is nice for simplicity. Potentially there could be the best of both worlds with a mutable preferences file that gets updated with API changes but I think that might be out of scope

A potential option for the db mode is to follow what happens for channel opens that use the channel policy settings as an initial value however once a policy update call is initiated they write the results of that call to the database and override the conf setting permanently and don't pay attention to the conf setting going forward

Yes for sure any setting that is overridden on startup could be considered equally

@Abdulkbk Abdulkbk linked a pull request Apr 25, 2024 that will close this issue
8 tasks
@Abdulkbk
Copy link

Hi @alexbosworth, I opened PR #8690 for this issue. Would you mind taking a look and sharing your thoughts on whether I'm heading in the right direction? Any suggestions for improvement would be greatly appreciated!

@alexbosworth
Copy link
Contributor Author

Looks good to me, i guess the problem might become more of documentation to let people know that they are only setting an initial value in the conf, and you can't really go backwards right to where the conf is in control again

@Chinwendu20
Copy link
Contributor

To be clear the order of checking for values when lnd starts up would be:
-1.) Check Lnd config, if empty go to step 2.
-2.)Check database, if empty go to step 3.
-3.) Let lnd create it.

Right?

@alexbosworth
Copy link
Contributor Author

hmm i didn't see that, ok but can i go back to total default, like remove both alias from conf and db? i guess the documentation aspect still stands

@Chinwendu20
Copy link
Contributor

Yes documentation is important I am just trying to be clear on the proposed flow. So, If user did not specify a node announcement field such as alias in their lnd config file and the user had not called the UpdateNodeAnnouncement rpc in the previous uptime of the lnd client, then lnd can go ahead and use the default which is for alias, using the serialised public key to create it:

alias = hex.EncodeToString(serializedPubKey[:10])

So, I guess we can still go back to the default in this case. Just wanted to be clear if this is the flow we are going for.

@Abdulkbk
Copy link

To be clear the order of checking for values when lnd starts up would be:
-1.) Check Lnd config, if empty go to step 2.
-2.)Check database, if empty go to step 3.
-3.) Let lnd create it.

Right?

Yes, this is the intended flow. check lnd.conf, then db, and if all not set, then use default (which is the first 10 chars of node's pubkey)

@Abdulkbk
Copy link

Yes documentation is important I am just trying to be clear on the proposed flow. So, If user did not specify a node announcement field such as alias in their lnd config file and the user had not called the UpdateNodeAnnouncement rpc in the previous uptime of the lnd client, then lnd can go ahead and use the default which is for alias, using the serialised public key to create it:

alias = hex.EncodeToString(serializedPubKey[:10])

So, I guess we can still go back to the default in this case. Just wanted to be clear if this is the flow we are going for.

Yes, I guess we can still go back to the default by clearing what we saved initially in the db and not specifying the fields such as alias in the config file.

@Abdulkbk
Copy link

hmm i didn't see that, ok but can i go back to total default, like remove both alias from conf and db? i guess the documentation aspect still stands

Yes, going back to the default is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner Issues suitable for new developers bug fix config Parameters/arguments/config file related issues/PRs enhancement Improvements to existing features / behaviour good first issue Issues suitable for first time contributors to LND
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants