Implements auto-register for the watchtower-client #164
Conversation
sr-gi
added
feature request
sr-gi
force-pushed
the
69-auto-renew-v2
branch
4 times, most recently
from
3b33da8 to
ab44760
Compare
sr-gi
force-pushed
the
69-auto-renew-v2
branch
from
ab44760 to
9812118
Compare
|
The necessity of an additional backoff cycle to address subscription issues when restarting can be addressed by implementing cln rpc logic so the plugin can be aware of the current chain tip. Once that data is available the client will be able to do client-side subscription expiry validation, so we'll be able to properly flag towers without probing them. I've already left a TODO comment on the code regarding this. |
|
Also, something that came to mind when implementing this: maybe we should put this feature under a conf flag so it can be disabled (specially for testing). If so, I think it should be enabled by default. |
sr-gi
force-pushed
the
69-auto-renew-v2
branch
4 times, most recently
from
0c271c9 to
c0c49c6
Compare
watchtower-plugin/src/main.rs
Outdated
| state.set_tower_status(tower_id, TowerStatus::TemporaryUnreachable); | ||
| } | ||
| let e = to_cln_error(e); | ||
| log::info!("{}", e); |
There was a problem hiding this comment.
This line is redundant since to_cln_error already logs it.
There was a problem hiding this comment.
Oh, that comes from a rebase :(
watchtower-plugin/src/wt_client.rs
Outdated
| assert!(matches!( | ||
| wt_client.add_update_tower(tower_id, &updated_tower_info.net_addr, &receipt_same_slots), | ||
| Err(SubscriptionError::Slots) | ||
| )); |
There was a problem hiding this comment.
This assertion is repeated. Similar one is in the line above.
There was a problem hiding this comment.
Same, comes from a rebase, will fix it
@sr-gi The additional backoff cycle need not to be handled, right? it's done by the backoff strategy already? |
Currently it is handled by the backoff strategy, but it properly implemented it won't require a cycle that will always fail. We need client-side subscription validation for that, which requires querying |
sr-gi
force-pushed
the
69-auto-renew-v2
branch
from
c0c49c6 to
658ff39
Compare
This helps reducing some of the WTClient is queried only to get the tower status
sr-gi
force-pushed
the
69-auto-renew-v2
branch
from
658ff39 to
e816d9e
Compare
|
@mariocynicys issues should have been fixed |
Description
The current version of the
watchtower-plugindoes not auto-renew subscriptions once they expire or get out of appointment slots. This may be annoying and some users may even be unaware that the data is not being back-up anymore due to the subscription needing a renewal.This PR fixes this. The approach for the fix is to use the
Retirerfor it given the logic for unreachable towers and towers with a subscription issue was pretty similar. The way this works from now on is the following:If an appointment is sent to the tower and there is a subscription error, the data is added to pending, the tower is flagged as
subscription_errorand theRetrieris notified (so far nothing changed). TheRetirerwill now try to renew the subscription if the state of the tower isSubscriptionErrorbefore sending any further data though. On failure, the behavior will be the same as before, just giving up, however, on success the pending data will be sent to the tower.Notice that, from now on, if the
Retriergets a subscription error from the tower it won't give up until a re-register has been attempted, therefore this also covers subscription issues faced while retrying pending data.Notice there is one case in which the
Retrierwill need an additional cycle to fix a subscription issue and that is if the tower is stopped in such a state. This is due to the tower state not being stored in the database but deduced based on what data is loaded from it on bootstrap. In the case of some pending appointments being found, the tower is flagged asTemporaryUnreachableand retried straightaway. This means that, under these conditions, theRetrierwill:TempoaryUnreachablewhen it is reallySubscriptionErrorSubscriptionErrorfrom the towerBug fixes
This PR is also fixing a client-side bug related to re-registering. The client was checking that, on re-register, the available slot count assigned to the new subscription was higher than the total count from the last subscription instead of the remaining count.