The channel should only be considered active when its pending tasks are
finished, which includes,
1. sync channel state via reestablish.
2. send previous shutdown msg.
3. reset the mailbox's packets.
4. reforwarding logs loaded from restart.
When the above tasks are finished, the channel can be considered as
fully resumed from its previous disconnection.
If there's an error occured when updating the peer's status after the
channel status is changed, we now make sure we log the error instead of
letting it interrupt the channel open/close flow.
There's no guarantee that the `b.beat` is initialized when the
dispatcher shuts down, especially with the case in the remote signer
where no chainbackend is created.
For outbound connections, since they are initialized by the users, we
can relax on the restriction. A future global limit may be added - as
for now, we will let them to be managed by the users.
When the callback is called in `scheduledPeerConnection`, it is
referencing the old `access` variable which was created when the peer
was first connected. However, if this peer opens a channel with us and
goes offline, or another inbound connection is made from this peer, we
may still use the old `access` value. To fix it, we need to make sure we
always get the fresh perm by calling `assignPeerPerms` inside
`peerConnected`.
When a peer already has a connection with us, there's no need to check
for available slots as we will either close the old conn or refuse the
new conn.
This commit removes the `batchReplayBkt` as its only effect is to allow
reforwarding htlcs during startup.
Normally for every incoming htlc added, their shared secret is used as
the key to be saved into the `sharedHashBucket`, which will be used for
check for replays. In addition, the fwdPkg's ID, which is SCID+height is
also saved to the bucket `batchReplayBkt`. Since replays of HTLCs cannot
happen at the same commitment height, when a replay happens,
`batchReplayBkt` simply doesn't have this info, and we again rely on
`sharedHashBucket` to detect it. This means most of the time the
`batchReplayBkt` is a list of SCID+height with empty values.
The `batchReplayBkt` was previously used as a mechanism to check for
reforwardings during startup - when reforwarding htlcs, it quries this
bucket and finds an empty map, knowing this is a forwarding and skips
the check in `sharedHashBucket`. Given now we use a bool flag to
explicitly skip the replay check, this bucket is no longer useful.
We now rely on the forwarding package's state to decide whether a given
packet is a reforwarding or not. If we know it's a reforwarding packet,
there's no need to check for replays in the `sharedHashes` bucket, which
behaves the same as if we are querying the `batchReplayBkt`.
Previously when deciding whether a UTXO is spent or not, we accept a
height hint as the starting block to look for the spending tx in the
rescan process. When it's already found spent before the rescan starts,
we will update the height hint to be the spent height, only if the
latter is greater. This means if the user-specified hint is greater than
the actual spending height, this UTXO will never be found as spent. We
now fix it by always using the spent height as the hint.
Previously we'd skip returning immature inputs to stay backwards
compatible. This has the downside as the node operator cannot take
actions on this input, like some customized batching strategy via
`bumpfee` RPC.
After this change, it means when calling `bumpfee` RPC, it will now
update the params for the immature input instead of returning an error
saying `output does not belong to wallet`.
We also add a `MaturityHeight` field so the user knows when this input
will be swept.
For legacy payments, the hash field will be nil, and we need to use the
payment identifier instead. We have multiple ways to fix this:
A trivial solution is we can simply call `sharder.GetHash` in
`collectResult`, and pass this hash to `attempt.Circuit()`, which ends
up multiple methods taking the hash. This is bad as it's confusing why
the methods of `HTLCAttempt` need to take another hash value, while
itself already has the info via `HTLCAttempt.Hash`. We don't want an
exceptional case to influence our main flow.
We can then patch the field `HTLCAttempt.Hash`, and set it to the
payment hash if it's nil, which can be done in `collectResult`. This is
also less optimal as it means every htlc attempts, either legacy or not,
now need to bear this context.
The best way to do this is to patch the field in
`reloadInflightAttempts`. As we are sure any new payments made won't be
legacy, and the only source of legacy payments comes from reloading
existing payments.
yyforyongyu