Minor Bugfixes and new sweeptimelockmanual logic #63
Conversation
ziggie1984
force-pushed
the
sweep-tolocal-bugfix
branch
2 times, most recently
from
28e5e52
to
7314657
Compare
| // Now we try to increase the index by 1 to account for the situation | ||
| // where the node was started with a version after (including) | ||
| // v0.13.0-beta | ||
| revPath3 := []uint32{ |
There was a problem hiding this comment.
So this commit doesn't really change anything in the logic, it just changes the order in which things are tried? Increasing the success probability?
There was a problem hiding this comment.
The commit changes the logic a bit, in prior versions you tried to increase the index but were using the old shachain root revocation scheme. But this case cannot happen, we need to test the 2 indices exactly with the new shachain root scheme to make sure we account for the case where the node was started with the old scheme and updated. There the new scheme is not 1 off.
There was a problem hiding this comment.
Efficiency logic could be added later, but this makes sure we definitely are able to derive the to_local for every channel of a noderunner. Otherwise when a node started with 0.12 and wanted to sweep the to_local output created after updating to 0.13. or later then it would not be able to derive the secrets with the prior chantool version
There was a problem hiding this comment.
Ah, now I see it. Thanks a lot for catching this!
Currently chantools crashes when trying to get the force-close summary because the new btcd version needs a prevoutInput fetcher otherwise we get a nil error.
When channel before the new shachain root creation scheme were created we do not need to increase the idx for the shachain generation.
ziggie1984
force-pushed
the
sweep-tolocal-bugfix
branch
from
7314657
to
f56f426
Compare
| // PubKey: (string) (len=5) "<nil>" | ||
| // | ||
| // For more details: | ||
| // https://github.com/lightningnetwork/lnd/commit/bb84f0ebc88620050dec7cf4be6283f5cba8b920 |
There was a problem hiding this comment.
this breaks the 80 line rule, should I split this comment into 2 lines , looks weird imo with 2 lines?
There was a problem hiding this comment.
It's okay, I haven't enabled the line length linter rule in this repo.
This PR solves two Things.
The first one is a small bugfix where the forceclose command would crash because a new btcd logic introduced a new logic for the sighashes
The second one is that it adds a more detailed description regarding the shachain generation and also fixes a bug were we would not be able to derive the to_local script in case we opened channels when the old shachain root creation scheme was still active. It also adds tests to verify this new logic.