decred / dcrlnd

Decred Lightning Network Daemon ⚡️
MIT License
36 stars 24 forks source link

channeldb: Fix migration 20 incorrect outpoint serialization #203

Closed matheusd closed 7 months ago

matheusd commented 7 months ago

This migration fixes the prior migration 20, introduced when porting the upstream code.

That migration erroneously encoded outpoints without the Tree field that exists in decred code, thus rendering the index incorrect.

The new migration corrects the issue by assuming the tree of every entry is zero, which is true because the channels can only reside in regular (as opposed to stake) transactions.

One way this could cause problems was when closing or detecting closed channels that were created before the migration took place. Closing them could fail with "missing outpoint from index".

While this fixes the issue for nodes that upgrade, this does not handle channels that were already closed for nodes that are already running v0.5.0.

matheusd commented 7 months ago

Manually tested by running a v0.4.0 simnet node connect to v0.5.0 nodes, create channels, go offline and have those channels closed (SPV mode).

After this runs, eventually the channels are detected as closed and other running channels can be run without issues.

Unfortunately, since this requires a migration, the current integration tests don't catch this sorts of errors (channel created on one version being closed on another). I'll investigate the possibility of creating a new set of integration tests that exercise these types of scenarios, but for this particular issue I'll expedite merging and releasing a new version with the fix (v0.5.1) in order avoid having users with broken channels.

Bison Relay devs will also want to release a new version soon(ish). To reiterate: funds are safe and any channels that get closed in the mean time are eventually fixed, but it's not great to have these channels hanging around in the wallet, compromising pathfinding and breaking channel listings.

cc @dajohi @alexlyp @miki-totefu