loopout: ignore saved HTLC txid, add an option to skip some HTLC transactions #958
Conversation
hieblmi
left a comment
hieblmi
left a comment
There was a problem hiding this comment.
Thank you for the quick fix @starius. Iiuc this provids a fix for the current set of clients that have the precondition where different versions of batches were reorg'd, but not for future occurrences of this scenario, correct?
Just in case future clients might have to use SkippedTxs maybe it would be easier to persist the affected txns to the db and load it on restart rather than having it as a command flag to loopd or in a section of the config file.
| LoopOutMaxParts uint32 | ||
|
|
||
| // SkippedTxs is the list of existing HTLC txids to skip when starting | ||
| // Loop. This should only be used if affected by the historical bug. |
There was a problem hiding this comment.
nit: maybe mention reorg here, so the historical reorg bug. Should we also include reorg here to make it easier to understand, like SkippedReorgdTxns?
client.go
Outdated
| } | ||
|
|
||
| if len(cfg.SkippedTxs) != 0 { | ||
| skippedTxs := make(map[chainhash.Hash]struct{}) |
There was a problem hiding this comment.
nit: I think usually we name it txns instead of txs.
loopd/config.go
Outdated
|
|
||
| LoopOutMaxParts uint32 `long:"loopoutmaxparts" description:"The maximum number of payment parts that may be used for a loop out swap."` | ||
|
|
||
| SkippedTxs []string `long:"skippedtxs" description:"The list of existing HTLC txids to skip when starting Loop. This should only be used if affected by the historical bug."` |
There was a problem hiding this comment.
Does this only need to be provided once on a restart and can then be skipped for future runs of loopd. Maybe we document the requirement here.
Could we also add a section for this new value in the sample-loopd.conf?
There was a problem hiding this comment.
This option is intended for testing mostly. It is not expected to be set in prod. I added hidden:"true" to it.
If a reorg happens, the saved txid prevents loop-out from discoveing new txid.
Added option WithSkippedTxns, which has one historical problematic tx by default. Sweeps originating from these transactions are omitted when reading from DB. loopdb: add column sweep_batches.cancelled and replaced DropBatch with CancelBatch. It is needed, because sweep.batch_id is a foreign key to batch. Changed StoreMock.InsertSweepBatch not to reuse batch_id. This is needed by the test, which checks that new batch has fresh ID.
4 participants
If a reorg happens, the saved txid prevents loop-out from discoveing new txid. So do not use this data after restart.
Added option WithSkippedTxns, which has one historical problematic tx by default. Sweeps originating from these transactions are omitted when reading from DB.
loopdb: add column sweep_batches.cancelled and replaced DropBatch with CancelBatch. It is needed, because sweep.batch_id is a foreign key to batch.
Changed StoreMock.InsertSweepBatch not to reuse batch_id. This is needed by the test, which checks that new batch has fresh ID.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes