Skip to content
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

xpay: emulate maxfeepercent and exemptfee when xpay-handle-pay used #7942

Merged

Conversation

rustyrussell
Copy link
Contributor

maxfeepercent is use by Zeus, so let's make that work.

maxfee is more precise, so it's the only xpay option (maxfee was added to pay later).

Fixes: #7926

I prefer this to #7936

@rustyrussell rustyrussell added this to the v24.11.1 milestone Dec 16, 2024
Copy link
Collaborator

@Lagrang3 Lagrang3 left a comment

Choose a reason for hiding this comment

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

This should work.

} else
maxfeepercent_ppm = 500000 / 100;

if (!amount_msat_fee(&maxfee, amount, 0, maxfeepercent_ppm))
Copy link
Collaborator

@Lagrang3 Lagrang3 Dec 16, 2024

Choose a reason for hiding this comment

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

maxfeepercent is a percent in the way this argument is treated by pay and the users of this api expect that.

Suggested change
if (!amount_msat_fee(&maxfee, amount, 0, maxfeepercent_ppm))
if (!amount_msat_fee(&maxfee, amount, 0, maxfeepercent_ppm/100))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

&maxfeepercent_ppm))
return false;
} else
maxfeepercent_ppm = 500000 / 100;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here. If this is a percent then 500'000 ppm means 0.5%

Suggested change
maxfeepercent_ppm = 500000 / 100;
maxfeepercent_ppm = 500000;

maxfeepercent is use by Zeus, so let's make that work.

maxfee is more precise, so it's the only xpay option (maxfee was added
to pay later).

[ Fix to ppm logic by Lagrang3, thanks! --RR ]

Fixes: ElementsProject#7926
Changelog-Changed: JSON-RPC: With `xpay-handle-pay` set, xpay will now be used even if `pay` uses maxfeeprecent or exemptfee parameters (e.g. Zeus)
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the xpay-as-pay-maxfeepercent branch from 1369f2c to 49aff81 Compare December 16, 2024 22:07
@rustyrussell rustyrussell merged commit 428c760 into ElementsProject:master Dec 17, 2024
35 of 39 checks passed
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.

plugin-cln-xpay: Not redirecting pay (unknown arg \"maxfeepercent\") and overly verbose log entries
2 participants