[anchors] zero-fee HTLC secondlevel transactions#4840
[anchors] zero-fee HTLC secondlevel transactions#4840Roasbeef merged 7 commits intolightningnetwork:masterfrom
Conversation
Roasbeef
left a comment
There was a problem hiding this comment.
Very straight forward change! No major comments, I think if we're in agreement re 0 fees vs 1 sat/byte, then this can go in as soon as the dependent PRs are merged.
lnwallet/commitment.go
Outdated
There was a problem hiding this comment.
EZ 😎
Could also instead have the param be passed in (0 in this case) if we want to make it a constant so it can be modified easily later. Based on recent discussions though, it seems like the zero fee route may be the best way forward.
There was a problem hiding this comment.
I prefer zero, since that mitigates the fee leak completely.
8f6be06 to
f6404f6
Compare
chanbackup/single.go
Outdated
There was a problem hiding this comment.
We don't strictly need a new version, since on recovery we don't handle HTLCs anyway.
2627fc7 to
d7f942b
Compare
|
Updated with latest terminology used in the spec PR: lightning/bolts#824 |
d7f942b to
d63df17
Compare
|
Needs a rebase! |
b81b862 to
387adc6
Compare
feature/deps.go
Outdated
There was a problem hiding this comment.
Remove optional signal as well? Can defer perhaps until the rc phase as well
There was a problem hiding this comment.
This is just the dependency check, I don't think it affects the signalling, @cfromknecht ?
There was a problem hiding this comment.
yeah this can be removed, this is equivalent to not having an entry in the dep map
There was a problem hiding this comment.
Ok, removed! Should we remove the lnwire.AnchorsOptional entry also?
feature/deps.go
Outdated
There was a problem hiding this comment.
yeah this can be removed, this is equivalent to not having an entry in the dep map
fundingmanager.go
Outdated
lncfg/protocol.go
Outdated
There was a problem hiding this comment.
not sure how specific we want to be here, but does it make sense to clarify that this is anchor_zero_fee_htlc vs just anchor_outputs?
There was a problem hiding this comment.
I think users interacting with this flag don't have to know about the difference.
lncfg/protocol_rpctest.go
Outdated
There was a problem hiding this comment.
before this was NoAnchors, now it's back to Anchors?
There was a problem hiding this comment.
So there's two different versions of the config now: one for the itests, and the one for the rest. This commit makes the default one have anchor on, while the itest one it's off by default.
…es support We assume the legacy anchor type is no longer advertised by us.
e90dedb to
64659e6
Compare
This PR enables an extension to the
anchorcommitment type that makes second-level HTLC transactions pay a zero fee, meaning they will have to have fees added as in #4779.This PR adds this type to the already existing
anchorstype, meaning all new anchor channels being opened will have this enabled (as long as both nodes support it). Existing anchor channels will stay the same.TODO
Builds on
#4779