Skip to content

Remove UserConfig::accept_mpp_keysend #3439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

valentinewallace
Copy link
Contributor

This option was added to force users to opt into breaking compat with < 0.0.116, so it should be fine to remove for the 0.1 release. Otherwise, receiving to static invoices would always require flipping this on.

@valentinewallace valentinewallace added this to the 0.1 milestone Dec 4, 2024
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Needs pending changelog

This option was added to force users to opt into breaking compat with
< 0.0.116, so it should be fine to remove for the 0.1 release. Otherwise,
receiving to static invoices would always require flipping this on.
@valentinewallace valentinewallace force-pushed the 2024-12-remove-mpp-keysend-cfg branch from 05a59a4 to 2e685ff Compare December 4, 2024 20:57
@valentinewallace
Copy link
Contributor Author

Needs pending changelog

Thanks, done

Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

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

LGTM. Linting and fuzz CI failures seem unrelated. Can't see if they're tracked anywhere else.

@TheBlueMatt TheBlueMatt merged commit f33738d into lightningdevkit:main Dec 5, 2024
16 of 19 checks passed
@@ -901,7 +889,6 @@ impl Readable for UserConfig {
accept_inbound_channels: Readable::read(reader)?,
manually_accept_inbound_channels: Readable::read(reader)?,
accept_intercept_htlcs: Readable::read(reader)?,
accept_mpp_keysend: Readable::read(reader)?,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey sorry, I think this might be why fuzz failed? Looking into it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably the no-changes test failed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants