Earlier this delay was needed to increase the likelihood that the DLP
scanario was successfully completed. Since we would risk the connection
being torn down, and the link exit, we could end up with the remote
marking the channel borked, but not finishing the force close.
With the previous set of commits, we should now trigger the force close
before we merk the channel borked, which should ensure we'll resume the
orocess on next restart/connect.
Before publishing the close tx to the network and commit to the
StateCommitmentBroadcasted state, we mark the commitment as broadcasted
and store it to the db. This ensures it will get re-published on startup
if we go down.
Instead of marking the channel Borked in cases where we want to force
close it, we immediately let the peer fail the link. The channel state
will instead be updated by the channel arbitrator, which will transition
to StateBroadcastCommit, marking the channel borked, then marking the
commitment tx broadcasted right before publishing the force close tx. We
do this to avoid the case where we would mark it Borked, but go down
before being able to publish the closing tx.
Storing the force close tx ensures it will be re-published on startup.
Instead of marking the database state when processing the channel
reestablishment message, we wait for the result of this processing to
arrive in the link, and mark it accordingly in the database here.
We do this move the logic determining whether we should force close the
channel or not, and what state to mark it in the DB, to the same place,
as these need to be consistent.
This commit converts the ErrCommitSyncLocalDataLoss error into a struct,
that also holds the received last unrevoked commit point from the remote
party.
TestChainArbitratorRepulishCommitment testst that the chain arbitrator
will republish closing transactions for channels marked
CommitementBroadcast in the database at startup.
When loading active channels for a connected peer, we gather channel
sync messages for all borked channels, and send them to the peer. This
should help a peer realize that the state is irreconcible, as we have
already realized.
Checks that we get ErrDoubleSpend as expected when publishing a
conflicting mempool transaction with the same fee as the existing one,
and that we can publish a replacement with a higher fee successfully.
error
Since btcwallet will return typed errors now, we can simplify the
matching logic in order to return ErrDoubleSpend.
In case a transaction cannot be published since it did not satisfy the
requirements for a valid replacement, return ErrDoubleSpend to indicate
it was not propagated.
This commit fixes a potential issue within the fundingmanager, where
failing to write the channel opening state could cause the channel being
marked open in the DB, but the opening state not being set. On startup
this would cause the channel state machine to not be able to resume.
We fix this by saving the channel opening state _first_. This works
because saving the opening state is idempotent, and in case a channel is
found pending at startup, it will re-register for confirmation
notifications and re-do the process.
Since the advanceFundingState now can handle pending channels, we'll
call it for both pending and non-pending channels, just making sure that
we re-initialize the channel barriers and re-publish the funding tx fro
pending channels.
This commit makes advanceFundingState check whether a channel is still
pending before checking the channel opening state. This lets us call it
directly, without checking whether a channel has confirmed first.
This commit makes the waitForFundingTimeout method synchronous, and
return ErrConfirmationTimeout in case the timeout is reached.
We also simplify the internals by using waitForTimout defined earlier.
This commit defines a new method waitForTimeout, that will be used to
listen for channels timing out. It handles a subset of what is already
handled by waitForFundingWithTimeout, but we want to break that one up
in smaller parts, and waitForTimeout is the first of these.
Since waitForFundingConfirmation is always called in a goroutine, we
make this explicit by requireing the caller to always increment the
waitgroup before calling it.
Similarly to what we did in the previous commit, we move the
responsibility of marking the channel open by calling
handleFundingConfirmation out from waitForFundingWithTimeout to the
caller.
This commit moves the handling of a funding confirmation out of
waitForFundingConfirmation, and instead let the caller handle marking
the channel opened.
This commit moves the opening logic found within
waitForFundingConfirmation into a new method handleFundingConfirmation.
This will make it easier to later break up waitForFundingConfirmation,
and avoid code duplication.
This commit removes the handleFundingConfirmation method, and instead
hands the newly confirmed channel of to advanceFundingState, which will
take the channel through the rest of the channel opening flow.
Since the advanceFundingSigned now can resume a channel from any state,
we resue the logic in handleFundingSigned instead of manually executing
each step of the funding flow.
This commit make the advanceFundingStateMethod synchronous. It will now
query the database for a channel's opening state, and call the method
stateStep until the channel has finished the opening procedure.
This commit extracts the funding state check we do at startup into a new
method advanceFundingState. In later commits we will modify this method
to work for all funding state machine flows, not only on restart.
This commit checks that the size of the bech32 encoded invoice is not
greater than 7092 bytes, which is the maximum number of bytes that can
fit into a QR code. This mitigates a potential DoS vector where an attacker
could craft a very large bech32 invoice string containing an absurd amount
of route and/or hop hints. If sent to an application that processes
payment requests, this would allocate a burdensome amount of memory
due to the public key parsing for each route/hop hint.
For a 1.7MB payment request, this yielded about 38MB in allocations
from just parsing public keys:
```
45.51MB 7.31% 92.07% 45.51MB 7.31% math/big.nat.make
25.50MB 4.09% 96.16% 25.50MB 4.09% github.com/lightningnetwork/lnd/zpay32.bech32VerifyChecksum
1MB 0.16% 96.32% 39.50MB 6.34% github.com/lightningnetwork/lnd/zpay32.parseRouteHint
1MB 0.16% 96.48% 33.50MB 5.38% github.com/btcsuite/btcd/btcec.decompressPoint
0.50MB 0.08% 96.56% 7.50MB 1.20% crypto/elliptic.(*CurveParams).doubleJacobian
0.50MB 0.08% 96.64% 38MB 6.10% github.com/btcsuite/btcd/btcec.ParsePubKey
0 0% 96.64% 12MB 1.93% crypto/ecdsa.Verify
0 0% 96.64% 8MB 1.28% crypto/elliptic.(*CurveParams).ScalarBaseMult
0 0% 96.64% 12MB 1.93% crypto/elliptic.(*CurveParams).ScalarMult
```
With this change, memory usage will be far lower as decoding will exit
early with an error if the invoice is too large.
