This fixes tests that were surfaced as flaky. Usually these tests had
an off-by-one error when mining blocks while waiting for a
CSV-encumbered output.
This fixes itest flakes that happen when a payment is attempted that
ends up causing a channel closure.
completePaymentRequests() attempts to monitor the open channels after a
payment is attempted in order to identify that payment was actually
dispatched to a remote node before returning.
However, when the payment actually causes a channel closure (for
example, because the receiver sent an incorrect preimage) this logic
fails in that the channel will no longer the found in the list of open
channels. This could cause a flake when there was enough time for the
channel to close before performing the check.
One example of such a flaky test is failing_link.
This fixes the issue by also checking whether the total number of
channels was reduced, which indicates (assuming itest operations are
being executed serially) that one of the attempted payments affected at
least one channel.
This ensures the Carol and Dave nodes created in the single_hop series
of tests are synced to the latest chain tip generated by the miner
before creating the route that will eventually be used for payments.
This prevents a flake during CI tests where slow processing of blocks by
Carol could cause the generated route to have a final CLTV delta too
short for Dave to accept.
While at it, we switch the test to use the default CLTV delta provided
by QueryRoutes which is now the global default CLTV.
SIGABRT is used by the Go runtime to forcefully terminate all
goroutines, even if they are in a deadlocked state.
It is useful in development (to get a glimpse of any potential race or
hang conditions) and in production to forcefully terminate execution
when a standard SIGQUIT won't do.
This modifies the signal package to _not_ trap SIGABRT and let it be
handled in the standard way by the runtime.
This reduces the flakiness of the CPFP test by asserting the wallet has
seen the unspent output before attempting to perform the walletkit's
BumpFee method.
Previously the attempt to bump the fee of the target transaction could
be made before the wallet had had a chance to fully process the
transaction, causing a flaky error.
This switches a few call sites that used a different timeout when
openening channels to the correct openChannelTimeout, which better deal
with flakes in the CI.
This replaces an outstanding sleep for a check for a specific state
during the test for watchtower use: specifically, that the backup has
been sent to the watchtower prior to shutting down Dave.
This reduces flakiness in the test that could occur if the Dave shutdown
without the backup being comitted to the watchtower, causing the rest of
the test to fail.
This changes the wait during node connection to check both for the
existance as well as for the validity of the tls cert and macaroon
files.
This ensures that nodes in the process of starting up don't inadvertedly
cause a connection error due to not yet having written the entire file.
During the channel_backup_restore/restore_during_unlock itest, the node
is restored from seed and immediately restarted. Depending on specific
timing of the machine, the test harness might not have had the graph
subscription processed before the node shuts down, causing the harness
to trigger a panic.
Reducing this to a synchronous subscription attempt means node
initialization necessarily waits until the subscription is done before
attempting to restart, reducing flakiness and ensuring correct behavior.
This forces the Dial attempt to succeed or fail before proceeding with
node setup.
We also log on the node a failure to establish the graph subscription
before panicking so that we can more easily find issues.
This improves the error reporting for the harness' CloseChannel so that
the exact step where closure fails can be better indicated.
This is to help debug some flaky failures in the CI.
This fixes a decoding error when the list of short channel ids within a
QueryShortChanIDs message started with a zero sid.
BOLT-0007 specifies that lists of short channel ids should be sorted in
ascending order. Previously, this was checked within lnwire by comparing
two consecutive sids in the list, starting at the empty (zero) sid.
This meant that a list that started with a zero sid couldn't be decoded
since the first element would _not_ be greater than the last one
(namely: also zero).
Given that one can only check for ordering starting at the second
element, we add a check to ensure the proper behavior.
A unit test is also added to ensure no future regressions on this
behavior.
This fixes a possible race condition during TestBreachSpends that could
cause the test to fail in a flaky way.
The error returned in PublishTransaction (publErr) could be modified by
the main test goroutine before PublishTransaction had a chance to
return, causing the wrong error value to be returned.
This was mostly visible as a flake during TestBreachSpends/all_spends
where adding a one second delay in the old code between the send in
publTx and the call to publMtx.Lock() would cause the last iteration of
the test loop to fail.
This is fixed by moving the lock and locally storing the expected error
value to before the send so that each call to PublishTransaction is
guaranteed to return the correct error value.
This reworks the locking behavior of the Gossiper so that a race
condition on channel updates and block notifications doesn't cause any
loss of messages.
This fixes an issue that manifested mostly as flakes on itests during
WaitForNetworkChannelOpen calls.
The previous behavior allowed ChannelUpdates to be missed if they
happened concurrently to block notifications. The
processNetworkAnnoucement call would check for the current block height,
then lock the gossiper and add the msg to the prematureAnnoucements
list. New blocks would trigger an update to the current block height
then a lock and check of the aforementioned list.
However, specially during itests it could happen that the missing lock
before checking the height could case a race condition if the following
sequence of events happened:
- A new ChannelUpdate message was received and started processing on a
separate goroutine
- The isPremature() call was made and verified that the ChannelUpdate
was in fact premature
- The goroutine was scheduled out
- A new block started processing in the gossiper. It updated the block
height, asked and was granted the lock for the gossiper and verified
there was zero premature announcements. The lock was released.
- The goroutine processing the ChannelUpdate asked for the gossiper lock
and was granted it. It added the ChannelUpdate in the
prematureAnnoucements list. This can never be processed now.
The way to fix this behavior is to ensure that both isPremature checks
done inside processNetworkAnnoucement and best block updates are made
inside the same critical section (i.e. while holding the same lock) so
that they can't both check and update the prematureAnnoucements list
concurrently.
This adds a test to the commit sweeper resolver to ensure it behaves
properly if the local node breaches a channel.
In this situation the remote party is expected to sweep the breached
output to itself and therefore the local party won't be able to recover
any funds.
This fixes an issue where the contract court could leave a completely
swept commit tx unresolved if it was swept by the remote party.
This could happen if (our) commit tx just published was actually a
previously revoked state, in which case the remote party would claim the
funds via a justice transaction.
This manifested itself in the testRevokedCloseRetribution integration
test where at the end of the test Bob was left with a pending channel
that never resolved itself.
This changes the HarnessNode structure to hold onto the client grpc
connection made during startup so that it can close it during shutdown.
This is needed because the grpc.Dial function spins a new goroutine that
attempts to maintain an open connection to the target endpoint and
without calling Close() in the connection while shutting down the node
we leak this goroutine to the rest of the tests.
This changes TCP port selection in integration tests from being
sequential, based on the node ID to being sequential but tested before
assigment.
This should reduce the number of flaky tests that fail due to the port
already being used by another process in the CI server.
This fixes an issue where the last tagged field of an invoice could get
broken due to the malleability of bech32 checksums.
The addition of a specific character in the second to last position of
the checksum could cause the previous signature field to mutate and thus
point to a different public node.
This changes the defer function in the test for channel backups to
correctly close over the 'dave' variable.
Without this closure, the shutdownAndAssert call would attempt to
shutdown the original (non-restored) dave instead of the most recently
created (restored) dave, causing a leak of a node during tests.
Matheus Degiovani