MDK-980: Claim wire + verification in the LSP#25
Merged
Conversation
MDK-980 lets a registering node carry a signed grant for a
non-standard FeePolicy that the LSP verifies locally before honouring.
This is just the pure core: the claim ADT and the verifier. Nothing
calls it yet, so behaviour is unchanged.
A claim is a versioned TLV pair. ClaimPayload binds {scheme, node_id,
policy}; SignedFeeClaim wraps the encoded payload as an opaque byte
string plus a detached BIP340 signature over SHA256 of those bytes.
Keeping the payload opaque means the verifier hashes exactly what was
signed and never reproduces the payload's TLV layout to check the
signature. TLV rather than a fixed concatenation keeps later schemes
(more policy arms, an issuer key-id) additive: a new tag, not a re-issue.
verify_claim takes a slice of issuer keys, not a single Option. An empty
slice rejects every claim, which is how the feature stays inert until a
key is configured. A non-empty slice accepts a signature from any one of
the keys. That covers a no-key-id claim today and key rotation later,
and per-key scoping can slot in without a breaking config change.
The verifier borrows a caller-supplied secp context rather than building
its own, so the verifier stays allocation free and context lifetime is
the caller's call. The handler wiring that follows will own a long-lived
verify context on the service and hand it in. The scheme byte is read
and matched before the signature check because it selects which
verification rules apply, so an unknown scheme is rejected before any
crypto runs.
The wire format is the contract MDK-981 (ldk-node) and MDK-982 (the TS
minter) must reproduce byte-for-byte, so an in-tree test vector pins a
known issuer key and claim hex. Drift in the TLV layout or the signing
input fails that test instead of silently diverging across repos. The
signing helper is test-only; the verifier does no I/O.
Adds an optional fee_claim string to RegisterNodeRequest so a node can
present a signed grant when it registers. The field is hex of the
SignedFeeClaim bytes from the previous commit. Nothing reads it yet; the
service-side verify and persist land later.
serde(default, skip_serializing_if = "Option::is_none") keeps the wire
backward compatible both ways. An old client that sends {} still decodes
to fee_claim: None through the existing params.unwrap_or(json!({})) path,
and a node with no claim emits {} rather than an explicit null, so the
request bytes match today exactly when no claim is set.
The client constructs the request with fee_claim: None; only a node that
has been granted a claim populates it, which is wired up in a later
change.
The persisted ScidWithPeer already carries a policy field, but the store only built peer<->scid lookups, so the granted policy was written to disk and then lost in memory. This adds a policy_by_peer map kept in lockstep with the existing two: populated on load from the decoded records, upserted on insert, dropped on remove. A get_policy accessor reads it back. ScidWithPeer::new now takes the policy explicitly rather than defaulting it, so there is one way to build the record and the policy is always named at the construction site. add_intercepted_scid passes Flat(Standard), the path a peer with no claim takes; the register handler will pass a verified grant. The on-disk default is unchanged: it comes from the TLV default_value, not the constructor. get_policy warns as dead code until the handler reads it. Keeping policy in its own map rather than handing out whole ScidWithPeer records keeps the skim site's read to a single lookup by node id, which is all it needs.
Wires the claim verifier into register_node. The service now holds a set of trusted issuer keys and a long-lived verification context; when a peer registers, any fee_claim it presented is verified against those keys and the granted policy is persisted onto the peer's SCID record before the response is enqueued, so the policy is in place by the time the SCID is handed out. The feature is inert by default. An empty issuer_pubkeys set short-circuits before any crypto and resolves every peer to the standard policy, byte for byte identical to today. Population of the key set is the operator's job downstream; nothing in-tree sets it. A grant is only ever upserted, never downgraded. An absent or unverifiable claim returns None and leaves an existing record untouched, so a transient miss or a malformed claim can't wipe a live grant; a brand-new record falls back to standard. This deviates from a literal "else standard" on purpose: once a node has been granted zero-fee, a dropped claim on a later re-registration must not silently restore the 2% skim. The verifier borrows the service's context rather than allocating per call, and resolve_claim_policy logs and swallows verification failures rather than failing the registration: a bad claim should cost the node its discount, not its channel. add_intercepted_scid is removed. It only built a standard-policy record, which the new persist path now does directly with the resolved policy, so the old helper had no remaining caller.
The forwarding skim now reads the peer's granted policy from the SCID store instead of hard-coding Flat(Standard). A peer with no grant resolves to Standard, so the skim is byte-for-byte the historical 2% for everyone the issuer set has not waived. This is the read side that makes the persisted grant actually take effect; before it, a verified ZeroFee was stored and then ignored. The lookup is hoisted out of the per-HTLC loop because the policy is keyed by peer, not by HTLC, so it is one store read per batch rather than one per forward. A zero skim is now two distinct cases, so the log is split. ZeroFee is the expected path and logs at info. Any other tier skimming nothing means the fee would have consumed the whole HTLC, which keeps the error-level log: a forward of the full amount that we expected to take a cut on. Previously ZeroFee was unreachable, so the single error log was correct; now that a grant can legitimately waive the fee, the error would be noise.
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
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.
Summary
Lets a registering node carry a signed grant for a non-standard
FeePolicy.The LSP verifies the grant locally (no network I/O), persists the granted
policy onto the peer's SCID record before the SCID is handed out, and honors
it at the skim site. Inert in production: with no issuer key configured every
peer resolves
Flat(Standard)and is skimmed the same 2% as today.Builds on MDK-979 (#24), which
landed the
FeePolicy/FeeTierADT,resolve_skim, and thepolicyTLV onScidWithPeer. That work deferred the per-peer lookup; this PR makes the grantreal and wires that lookup up.
Companion PR: the issuer side (MDK-982,
the open-money TS minter) is CumuloGlobal/open-money#468. It mints the
fee_claimthis PR verifies and pins the same in-tree test vector, so the two are validated
against each other.
What changed
Five atomic commits,
just checkgreen after each.claim.rs, new).ClaimPayload { scheme, node_id, policy }andSignedFeeClaim { payload, sig }, both TLV. The signature isBIP340 over
SHA256(ClaimPayload.encode()); the outer container keeps thepayload as opaque bytes so the verifier hashes exactly what was signed and
never re-derives the TLV layout.
verify_claimtakes a slice of issuer keys(empty rejects everything), checks the scheme, the signature against any one
key, and the
node_idbinding.fee_claimonRegisterNodeRequest(msgs.rs,client.rs). Optionalhex string,
skip_serializing_if = "Option::is_none". An old client sending{}still decodes toNone, and a node with no claim emits{}, so the wireis unchanged when no claim is set.
scid_store.rs). Apolicy_by_peermap kept in lockstep with the existing peer<->scid maps: built on load,
upserted on insert, dropped on remove.
get_policyreads it back.service.rs).issuer_pubkeys: Vec<XOnlyPublicKey>onLSPS4ServiceConfig, a long-livedverify context on the handler, and
register_noderesolving any presentedclaim into the SCID record before the response is enqueued.
service.rs).calculate_htlc_actions_for_peerreads the peer's policy from the storeinstead of the literal
Flat(Standard).Decisions
and upserts the policy in either direction. An absent or unverifiable claim
returns
Noneand leaves a live record untouched, so a transient miss or amalformed claim on a later re-registration can't silently restore the 2% skim
on a node that was granted zero-fee. This deviates from a literal "else
Standard" on purpose.
Vec, not a singleOption. Empty disables the feature.A non-empty set accepts a signature from any one key, which covers a no-key-id
claim today and key rotation later without a breaking config change. Per-key
policy scope (a promo key that must not grant permanent zero-fee) is a known
gap: it slots in additively as a keyed
&[(key, scope)]plus a payload key-id.claim.rsships an in-treetest vector (fixed issuer secret, fixed node secret, a
ZeroFeeclaim pinnedto its hex and issuer x-only key). The TS minter (open-money#468) already
reproduces it byte-for-byte, and
MDK-981 (ldk-node) will too;
drift fails the test instead of diverging silently.
tags, additive, no re-issue.
call; the service owns one long-lived verify context.
Behaviour preservation
Empty
issuer_pubkeysshort-circuits before any crypto, so every peer resolvesFlat(Standard)and the skim is byte-for-byte the historical 2%. The on-diskScidWithPeerlayout is unchanged (thepolicyTLV default predates this work),so no migration. A node only populates
fee_claimonceMDK-981 and
MDK-982 land, so even with the
code merged nothing presents a claim until then.
Tests
claim.rs: payload round-trip, valid claim yields the granted policy, emptyissuer set rejects, wrong key rejects, forged signature rejects, node_id
mismatch, unknown scheme, malformed hex/TLV, multi-issuer second key verifies,
a
Custompolicy survives verify, and the in-tree test vector.msgs.rs: round-trip with a claim, field omitted whenNone, legacy{}decodes to
None.scid_store.rs: insert-with-policy thenget_policy, load rebuilds the map,default record resolves to
Standard, plus the existing serialization tests.The handler glue (resolve + persist) and the skim read are not unit-tested: the
pure pieces they compose are, and exercising the handler needs the full node
harness which is saved for future integration test work.
Follow-ups
a
fee_claimvalue intoregister_node(client role) and surfaceissuer_pubkeysconfig (LSP role).lsps4_integration_tests.rsis untracked and fully commented out; whoeverrevives that harness adds
issuer_pubkeys: Vec::new()to its config literal.