wtclient: automatically initialise while creating a watchtower server #5596
Conversation
There was a problem hiding this comment.
Thanks for the PR!
Ideally we find a way to address this without a breaking change to config.
What do you think about leaving the active flag as is + defaulting to true? We can then add a deactivate, as done in this PR, for people who'd like to turn it off.
Also noticed that the CI "every commit builds" and "sample config" checks failed on this one (likely need to add the deactivate flag to our sample config).
|
Sure! Making active to default would definitely not break things and is the safest way to proceed. Can we add a deprecation notice and warn the user about that they do not need to pass it anymore and eventually clean it up in future versions (stable)? I will also look into the config file and resolve the conflicts! |
Yeah, was thinking exactly this :) |
- This commit adds the command wtclient.deactivate inorder to facilitate for automatic creation of wtclient Signed-off-by: DarthBenro008 <hkpdev008@gmail.com>
DarthBenro008
force-pushed
the
fix/wtclient-config
branch
from
6972e54
to
8dd1b6b
Compare
|
Hi @carlaKC , sorry for the delay, apparently the codebase that handles the database for watchtower and wtclient had completely changed, hence the merge conflicts. I re-wrote the implementation and have force pushed the codebase.
|
Initially, the `--wtclient.active` had to be passed alongside `--watchtower.active` for lncli to be able to connect to the watchtower This commit inverts the functionality, now `wtclient` is automatically created, however if you do not want it it to create automatically, you can pass `--watchtower.deactivate` now instead. Signed-off-by: DarthBenro008 <hkpdev008@gmail.com>
DarthBenro008
force-pushed
the
fix/wtclient-config
branch
from
8dd1b6b
to
91cab67
Compare
| @@ -4,9 +4,9 @@ import "fmt" | |||
|
|
|||
| // WtClient holds the configuration options for the daemon's watchtower client. | |||
| type WtClient struct { | |||
| // Active determines whether a watchtower client should be created to | |||
| // Deactivate determines whether a watchtower client should not be created to | |||
There was a problem hiding this comment.
Rather than make this flag Deactivate, could you leave Active as in and then just add the deprecation warning here rather than moving the Active flag below? Makes the diff a bit clearer.
I think we could also update the description here that this will always be set to true by default, and is just here for the sake of backwards compatibility?
| @@ -906,16 +906,20 @@ litecoin.node=ltcd | |||
|
|
|||
| [wtclient] | |||
|
|
|||
| ; Activate Watchtower Client. To get more information or configure watchtowers | |||
| ; Deactivate Watchtower Client. To get more information or configure watchtowers | |||
There was a problem hiding this comment.
Same here re change order, leave this as Activate and add Deactivate below.
|
|
||
| // defaultWatchTowerClientStartup sets the watchTower client to start | ||
| // automatically along with lnd | ||
| defaultWatchTowerClientStartup = true |
There was a problem hiding this comment.
defaultWatchTowerClientStartup -> deafultWtClientActive?
|
@Crypt-iQ: review reminder |
|
Closing due to inactivity |
Initially, the
--wtclient.activehad to be passed alongside--watchtower.activeforlnclito be able to connect to the watchtowerThis PR inverts the functionality, now
wtclientis automatically created, however if you do not want it it to create automatically, you can pass--watchtower.deactivatenow instead.Old Command:
lnd --watchtower.active --wtclient.activateNew Command (automatically creates a wtclient):
lnd --watchtower.activeNew Command to disable creating wtclient:
lnd --watchtower.active --wtclient.deactivate[Test]: personally tested on local machine and passes
go test .../..Pull Request Checklist