Fee policy in the LSP#24
Merged
Merged
Conversation
Introduce the fee_policy module: a FeePolicy/FeeTier ADT and a single
pure resolve_skim mapping a policy plus an HTLC amount to the msat to
skim. Shared foundation for upcoming work that waives the JIT-channel
skim for grant recipients; nothing wires it up yet.
When the skim would eat the whole HTLC, resolve_skim waives it rather
than clamping to amount - 1: a residual that small may fall below
htlc_minimum_msat and be rejected anyway, so clamping would bank a fee
on a payment that never settles. This only triggers for a rate >= 100%
or an outsized Custom base; the standard 2% never reaches it.
This is also where resolve_skim deliberately diverges from the code it
replaces. The service computed the proportional fee in u64, so
amount * ppm overflowed for very large HTLCs and skimmed nothing,
forwarding the whole HTLC for free. resolve_skim multiplies in u128 and
skims correctly. At the standard 2% the result is identical for any
realistic HTLC; only the previously-overflowing range changes, from
"free" to "charged". Tests pin the standard tier against the legacy
computation for non-overflow sizes and document the large-HTLC case.
The other arms are encoding-only. ZeroFee resolves to zero; Custom
{ ppm, base_msat } adds a flat base to the proportional component.
Custom is kept though v1 never constructs it: it's free in the TLV and
documents the shape of an explicit rate.
Both enums serialize via impl_writeable_tlv_based_enum! with reserved
type bytes (FeeTier: Standard=0, ZeroFee=2, Custom=4; FeePolicy: Flat=0)
so future variants are additive. Flat is a length-prefixed tuple variant
wrapping FeeTier, letting a later commit store a policy as a TLV field
that defaults cleanly on old records.
Persist a FeePolicy per intercept SCID record so a later milestone can look up a peer's policy at the skim site. ScidWithPeer::new still hands every record Flat(Standard), so nothing changes yet. The field uses a length-delimited TLV with a default_value of Flat(Standard) at type 4. Records written before this field existed carry only types 0 and 2, so they read back as Standard with no migration. A test encodes a copy of the old two-field layout and decodes it through the new struct to pin that default.
| log_error!( | ||
| self.logger, | ||
| "Skimmed fee equaled the entire HTLC amount for {:?}. Skipping skim.", | ||
| "Skim would have consumed the entire HTLC {:?}; forwarding without a skim.", |
There was a problem hiding this comment.
this is partially wrong. it can also be 0 because the policy is ZeroFee. not only Skim would have consumed the entire HTLC {:?}; forwarding without a skim.
Author
There was a problem hiding this comment.
Will update these now so they don't end up stale when ZeroFee gets used
| /// | ||
| /// `standard_ppm` is the LSP's configured proportional rate, used only by [`FeeTier::Standard`]. | ||
| /// | ||
| /// The skim is waived in exactly one case: when it would consume the entire HTLC. A zero-value |
There was a problem hiding this comment.
The skim is waived in exactly one case
what about ZeroFee?
Author
There was a problem hiding this comment.
I think because the standard fee rate is hard coded still. These comments reflect that
martinsaposnic
left a comment
martinsaposnic
left a comment
There was a problem hiding this comment.
left comments, but overall looks good
martinsaposnic
approved these changes
Jun 9, 2026
Replace the inline compute_forward_fee block in calculate_htlc_actions_for_peer with a single resolve_skim call against a literal Flat(Standard) policy. This is the one place the LSP decides what to skim; routing it through the pure function is what later milestones need to swap the literal for a per-peer policy lookup. Not a strict no-op: it inherits the u128 overflow fix from resolve_skim, so a >9223-BTC HTLC is now skimmed 2% instead of overflowing and forwarding free. Every realistic HTLC is unchanged. The peer's stored policy is still ignored; every forward resolves Flat(Standard) until the lookup lands. The two old log lines (overflow, skim-ate-the-HTLC) collapse into one: resolve_skim can't overflow, so a zero skim on a non-zero HTLC can only mean the fee would have eaten the whole amount.
amackillop
force-pushed
the
austin_mdk-979_fee-policy
branch
from
efcef2d to
c2279f8
Compare
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MDK-979 — Fee policy in the LSP
Shared foundation for waiving the JIT-channel skim per peer. No
behaviour change for any realistic HTLC: every peer still resolves
Flat(Standard)and is skimmed 2%.
What changed
FeePolicy/FeeTierADT +resolve_skim(fee_policy.rs). One pure functionmaps a policy plus an HTLC amount to the msat to skim.
Flat(Standard)is the onlyarm v1 constructs;
ZeroFeeandCustom { ppm, base_msat }are encoding-only forlater work.
policyTLV onScidWithPeer(scid_store.rs). Length-delimited field at type4 defaulting to
Flat(Standard), so records written before it existed read back asStandard with no migration. Future work wires up the per-peer lookup.
resolve_skim(service.rs).calculate_htlc_actions_for_peernow calls
resolve_skim(&Flat(Standard), ...)instead of the inlinecompute_forward_feeblock.One deliberate behaviour change
resolve_skimcomputes the proportional fee in u128. The old code multiplied in u64,so
amount * ppmoverflowed above ~9223 BTC and silently skimmed nothing, forwardingthe whole HTLC for free. Such an HTLC is now skimmed the correct 2%. Every realistic
amount is byte-identical to before.
Tests
fee_policy.rs:Standardpinned against the legacycompute_forward_feefor allnon-overflow sizes; the large-HTLC case that used to forward free;
ZeroFee,Custom, the fee-eats-the-HTLC waiver, and TLV round-trips.scid_store.rs: full round-trip plus an old two-field record decoding toFlat(Standard).Validated with
just check(cargo test --workspace).Follow-ups
htlc_minimum_msaton a smallHTLC; needs investigation before any fix.
compute_forward_feeis now test-only (the parity oracle); remove oncethe equivalence is trusted.