Make Anchor Output Sweeps configurable. #7939
Conversation
ziggie1984
force-pushed
the
anchorsweep-configurable
branch
from
13b2820 to
9b47249
Compare
When an anchor output was not used to bump the commitment transaction (CPFP) we allow the behaviour whether the anchor output will be recovered later to be configurable.
Now that the default behaviour does not recover unused anchor outputs tests had to be adopted.
ziggie1984
force-pushed
the
anchorsweep-configurable
branch
from
9b47249 to
6691202
Compare
| @@ -771,8 +773,6 @@ func (c *ChannelArbitrator) relaunchResolvers(commitSet *CommitSet, | |||
| anchorResolver.SupplementState(chanState) | |||
|
|
|||
| unresolvedContracts = append(unresolvedContracts, anchorResolver) | |||
|
|
|||
| // TODO(roasbeef): this isn't re-launched? | |||
There was a problem hiding this comment.
not sure what you meant by this todo, according to the code the resolvers are relaunched ? The linter would complain about this comment so I removed it, happy to add it back if it still holds true.
There was a problem hiding this comment.
IIUC, if all the contract for a resolution are resolved, but the anchor isn't, then it's never re-created.
|
I don't think we want to never sweep anchors. Never sweeping them means we can never use it to automatically fee bump. In that case, then I think something like this could be a middleground: #7965 (comment) |
|
This change would still need one or two integration tests to finalize, if we wanna go this route. |
There was a problem hiding this comment.
I think if we want to go with this config, we need to provide a mechanism to sweep them back when requested - the easiest way is to provide a new RPC to do this, but then we'll need to persist the anchor sweep on disk, still need more thinking in this area.
Tho we know there are anchor collectors out there, I don't think we should rely on them to keep the utxo set from bloating.
| @@ -7,7 +7,8 @@ import ( | |||
|
|
|||
| //nolint:lll | |||
| type Sweeper struct { | |||
| BatchWindowDuration time.Duration `long:"batchwindowduration" description:"Duration of the sweep batch window. The sweep is held back during the batch window to allow more inputs to be added and thereby lower the fee per input."` | |||
| BatchWindowDuration time.Duration `long:"batchwindowduration" description:"Duration of the sweep batch window. The sweep is held back during the batch window to allow more inputs to be added and thereby lower the fee per input."` | |||
| NotRecoverAnchorOutputs bool `long:"notrecoveranchoroutputs" description:"Not recovering anchor outputs when they were not used to bump their commitment transaction."` | |||
There was a problem hiding this comment.
nit: maybe call it SkipAnchorRecover?
|
Further to this change, it should be granted that any zero-conf wallet input which is the result of a |
|
Obsolete with the new sweeper subsystem. |
The sweeping of anchor outputs is not really aligned with incentives when mempool conditions do not allow 1 sat/vbyte transactions to be confirmed in a reasonable amount of time. Always sweeping anchor outputs although they have no real chance to be confirmed will cost more eventually when one needs to use the funds which were used to sweep this output. In addition anchor sweeps are mostly bumped by third parties which can lead to a mandatory restart because lnd does not monitor the mempool when RBFs happen.
Therefore noderunners have now the possibility to switch off the anchor recovery. I did not switch it off by default because a lot of unit test would fail as a consequence.
--sweeper.notrecoveranchoroutputs=true
fixes #7602
Checking for the tests to pass ....