Skip to content

feature: if a bit is unset, then all other features that dep it should be #6007

Closed
Roasbeef wants to merge 2 commits intolightningnetwork:masterfrom
Roasbeef:transitive-dep-protocol-config-removal
Closed

feature: if a bit is unset, then all other features that dep it should be #6007
Roasbeef wants to merge 2 commits intolightningnetwork:masterfrom
Roasbeef:transitive-dep-protocol-config-removal

Conversation

@Roasbeef
Copy link
Copy Markdown
Member

@Roasbeef Roasbeef commented Nov 20, 2021

This fixes an issue where if one tries to unset a feature like anchors,
and other feature depend on it, then lnd fails to start as it realizes
that its dependnacy set is inconsistent.

Fixes #6002

…d be

This fixes an issue where if one tries to unset a feature like anchors,
and other feature depend on it, then `lnd` fails to start as it realizes
that its dependnacy set is inconsistent.

Fixes lightningnetwork#6002
raw.Unset(lnwire.AnchorsZeroFeeHtlcTxOptional)
raw.Unset(lnwire.AnchorsZeroFeeHtlcTxRequired)

// If anchors are disabled, then we also need to
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, seems we could convert all other instances above to use logic like this as well, particularly the notlvonion part of it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

if test.cfg.NoStaticRemoteKey {
assertUnset(lnwire.StaticRemoteKeyOptional)
}
if test.cfg.NoAnchors {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked this and I think ScriptEnforcedLeaseRequired and ScriptEnforcedLeaseOptional are disabled by default, so this test passes even without your fix.

@positiveblue positiveblue self-requested a review November 23, 2021 06:19
Copy link
Copy Markdown
Contributor

@positiveblue positiveblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM, only a small problem with the tests

raw.Unset(lnwire.AnchorsZeroFeeHtlcTxOptional)
raw.Unset(lnwire.AnchorsZeroFeeHtlcTxRequired)

// If anchors are disabled, then we also need to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@guggero
Copy link
Copy Markdown
Collaborator

guggero commented Nov 24, 2021

Replaced by #6026.

@guggero guggero closed this Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"missing feature dependency: 23" after upgrade to 0.14.0-beta

3 participants