`pyln-testing` depends on `pyln-client`, and the API for the latter has changed
in version 24.0. Fix our dependency to 23.11 to prevent test from breaking
Clippy is complaining about using `.get(0)` instead of `.fist()` in methods
where we are getting more than just the first item. Suppress those warning.
Also fixes some actual issues.
Currently, when mocking sendrawtransaction so it does nothing, the client will raise an exception
given the picked lambda does not accept any params (and sendrawtransaction has some). Patches it
so this does not happen.
This patch is pretty minimal, given the behavior wrt CLN does not change, but it feels better not to
except here.
Implements ping/pong logic for the tower and CLN plugin
Modification in test as suggested and some more
Some fixed
Signed-off-by: Anmol Agrawal <anmol2002.aa1@gmail.com>
Also:
- Replaced time based waits for loops with the expected conditions to
get given the former is way more error prone.
- Updates tests that needed a response based on the request (main reason why
were're switching to mockito)
The `DBM` methods related to loading data return `Result<T, E>` where `E` is always `dbm::Error::NotFound`.
It may makes more sense make them return `Option<T>`.
The pooling time for the Retrier was hardcoded to 1, let's at least use a constant for that.
Also, `retrier::tests::test_manage_retry_while_idle` was randomly failing (for Ubuntu) when checking whether the Retrier was
idle after giving up on a retry. This is due to the time of running a round not being taken into account.
- `watchtower-proxy` was still part of constants but it was not being used anymore
- `watchtower-auto-retry-delay` was not part of the README
Also redefines defaults so a retry strategy lasts at most 1h, every interval is at most 15 minutes
and automatic retries are triggered every 8 hours.
The `watchtower-plugin` was specifying a custom tor flag to signal whether Tor may be used by the client. This was due to `cln-plugin (v0.1.1-)` not allowing plugins to access the CoreLN configuration options and, therefore, our plugin was unable to fetch the `proxy` / `always-use-proxy` options.
This fetches the aforementioned options and revamps the logic to comply with the `always-use-proxy` requirements, that is, if the flag is set all communications must be performed using Tor. Also, it replaces some of the currently used `String`s for more meaningful types to store network data (such as `AddressType`, `NetAddress`, or `ProxyInfo`).
This drops our custom `watchtower-proxy` config option
Options include helper functions to convert them to their proper type after
`cln-plugin=0.1.2`. Use that to reduce the option parsing boilerplate.
Also move all names, descriptions and default values for options, rpc_methods and hooks
to a new file.
test_get_appointment fails when the plugin is asked about a given tracker before the
tower is able to process the appointment to tracker transition. This doesn't happen consistently,
but we've seen the error popping up every now and then.
Simply add a log based waiting to make sure the state transition is performed before asserting.
The re-register logic had a bug where the `TowerSummary` that was kept in memory
after re-registering was sweeping the references to both the pending and invalid appointments.
While this data was still in the database, re-registering may have made it look like the data was not there.
- Updates `Retrier::run` to return more meaningful errors. `Retrier::run` used to simply return a message,
revamps it to return RetryError variants so we can handle return cases better.
- Adds an additional state to `RetrierStatus`: Idle. Retries that fail due to an accumulation of transient errors will be flagged
as Idle instead of Failed and retried later on (based on `auto_retry_delay`). Notice Retrier data is not kept in memory while a retrier is Idle. Instead, data is cleared and loaded again from the database when the `Retrier` is ready to run again.
- Revamps how revocation data is sent to the `RetryManager`: The RetrierManager used to received locators one by one via unreachable_towers. This is due to them being mainly fed by `on_commitment_revocation`, which generates them one by one. However, both when manually retrying or when bootstrapping from an already populated database, multiple appointments may be pending for the same tower, hence needing to call `unreachable_towers.send` multiple times for the same tower. This itself was not a big deal, given we didn't really needed to differentiate between the cases. We do now though.
In order to implement periodic retries while allowing manual retries we need to be able to signal the state transition to the
`Retrier` without providing any new data:
- If a Retrier is idle and we receive data trough `on_commitment_revocation` we need to append that data to the `Retrier`.
- If a Retrier is iddle and we receive data trough a manual retry, we need to change the state of the `Retrier` without
adding any new data to it.
In order to implement this we've added an additional map to `WTClient` that reports the state of the active retriers. Retriers are active only if they are running or idle.
- Also reworks `WTClient::set_tower_status` to update the status only if the new one does not match the old one.
This is simply to reduce the boiler plate of having to perform this check in other pats of the plugin codebase.
`TeosD` outputDir was being set to the same directory use by the test suite.
That cause it to be logging in the same log file, potentially colluding for
read/write operations. Setting the outputDir to it's own location fixed the issue.
Replaces time.sleep in the `watchtower-plugin` tests to wait for certain things to happen
on the backend by wait_for_logs, which should be less error prone
Also does some reformating and removes unnecessary imports
Using --locked with cargo install(s) will instruct cargo to use the lock file in the repo (Cargo.lock).
So all the users' builds will be identical to builds in the master branch.
Signed-off-by: Omer Yacine <mariocynicys@gmail.com>
One thing I really disliked about #89 was having a method that cloned its caller to be able to work around spawning a task inside it that called a method of the same class.
Turns out you can have self as Arc<Self> which would completely prevent having to do such a thing, plus it also reduces the number of things being cloned.
This is an attempt to rework the retrier logic to simplify how it works
and make it less error prone. This is done by making the retry manager
object responsible for both:
1- adding new retriers and extending current ones
2- removing retriers when they finish their work
This way, we don't need a mutex to gaurd the retriers hashmap & we are
sure there is no adding/extending retriers and removing them happending
at the same time, because only the retry manager does it and not
single retriers (i.e. retriers can't remove themselves from the retriers
hashmap).
The retry manager logic goes as follows:
1- drain the unreachable towers channel till it's empty, and store the pending appointments (locators to be exact) in the pending appointments set for each retrier.
2- remove any finished retrier (ones that succeeded and have no more pending appointments) and failed retriers (ones that failed to send their appointments).
3- start all the non-running retriers left after removing failed and finished retrieres.
Retriers will signal thier status so that the retry manager could
determine which retriers to keep, which to remove, and which to re-start.
We also set tower as unreachable when destroying the tower's retrier
and not after completing backoff. This makes it so that the tower is
unreachable until its retrier is destroyed, thus manual tower retry
by the user will fail with an error till the tower's retrier is destroyed.
If we were to set the unreachable tower status after the backoff, then manual
user retries might get discarded completely without an error because retrier
set the tower state to unreachable too early thus allowing the user to
perform manual retries, but if the user does manual retry, it won't get
carried out, since the retry manager will remove that retrier anyway as
it failed to deliver its pending appointments.
`watchtower-plugin::tests::test::test_watchtower` was not properly checking that
an appointment could not be found after resolving. This was due to an off-by-one error
that made the resolution not actually happening.
