Repository navigation
Conversation
4c35777 to
1bab2ce
Compare
morehouse
left a comment
There was a problem hiding this comment.
Concept ACK.
This is a big improvement over #1071:
- The inverted reputation scheme protects against sink attacks.
- The inverted reputation scheme enables new nodes to immediately send payments to reputable destinations when the network is under attack (better UX).
- The hash-based general slot assignment provides some discouragement for general jamming attacks.
- The tit-for-tat congestion bucket provides a mechanism to build reputation even when being general-jammed.
We probably also want to tweak the general slot assignment recommendations a bit to improve the strength of the hash-based defense:
- Scale
general_bucket_slot_countbased onmax_accepted_htlcsinstead of the commitment type. - Increase the recommended percentage of total slots allocated to the general bucket.
There's also still an open question about how to address the issues with high payment fanout. For example, LSPs often have high payment fan-out, which would prevent their clients from ever getting a good reputation with the current algorithm.
| Reputation relies on the fees charged by the local node rather than the fee | ||
| offered by the sender to prevent over-payment of advertised fees from | ||
| contributing to reputation. This is unlikely to impact honest senders who will | ||
| abide by advertised fee policies, and complicates (but does not prevent) | ||
| attempts to artificially inflate reputation through excessive fee payments. |
There was a problem hiding this comment.
Why should we care about reputation inflation if it's paid for? Fees paid are sunk costs to an attacker regardless of whether they happen in one payment or many.
Basing reputation on charged rather than offered fees could be good for a different reason though -- to avoid certain reputation destruction attacks. Suppose payments like this: M1 -> A -> B -> M2, where the attacker is trying to destroy B's reputation with A. The attacker could offer insanely high fees to A and minimal fees to B, then hold HTLCs until expiry. As a result, B's reputation would drop way more than M2's.
There was a problem hiding this comment.
Why should we care about reputation inflation if it's paid for?
We generally don't want an attacker to be able to take unusual actions that give them an advantage over honest behaving nodes. This just forces an attacker to align more with the behavior of honest nodes, even if it's trivial for them to do (multiple payments).
to avoid certain reputation destruction attacks
Good point! I'll include this in rationale.
| A HTLC is eligible to use the general bucket if for its | ||
| `(incoming scid, outgoing scid)`'s assigned resources: | ||
| - Currently occupied slots < `general_bucket_slot_count` | ||
| - Currently occupied liquidity + `amt_msat` <= `general_bucket_liquidity_allocation` |
There was a problem hiding this comment.
We should explicitly state that the general bucket can only be used when one of the assigned slots for that channel pair is unoccupied.
There was a problem hiding this comment.
This is covered by the general_bucket_slot_count check above?
It's badly named, I"ll change it to general_bucket_slot_allocation so that it's clearer.
| it difficult for an attacker to crowd out honest traffic. With these defaults, | ||
| an attacker will need to open approximately 50 channels in expectation to gain | ||
| access to all general resources. |
There was a problem hiding this comment.
As noted above, the number of channels required is a lot less with typical values for max_accepted_htlcs. If we have 16 general slots, we'd expect them to be fully occupied after ~11 or ~3 opened channels for general_bucket_slot_counts of 5 and 20, respectively (coupon collector expectation).
| average, expressed in seconds. | ||
| - `decaying_average`: stores the value of the decaying average. | ||
| - `decay_rate`: a constant rate of decay based on the rolling window chosen, | ||
| calculated as: `((1/2)^(2/window_length_seconds))`. |
There was a problem hiding this comment.
Should we provide a recommendation for window_length_seconds?
It looks like this will decay by 75% every window, so I'm guessing the same size as the fixed window would be reasonable here...
There was a problem hiding this comment.
window_length_seconds is a placeholder for the period that you want a rolling average over - for revenue you'd use revenue_window and for reputation you'd use revenue_window * reputation_multiplier, for example.
I'll update the wording - let me know if it clarifies it!
1bab2ce to
c25f7b1
Compare
|
Thanks for taking a look @morehouse! Addressed some of your feedback - diff here.
Agreed, this could definitely use some refining! Also interested to see whether we can think about larger default
Now that we look at reputation only in the outgoing direction, a sending LSP wouldn't need to worry about its clients (only the node it is forwarding out to). For a receiving LSP that needs to decide whether the client that they're sending a HTLC to has reputation, I imagine there are some LSP-related heuristics or different set of parameters that they could use? |
| - `general_bucket_slot_count`: | ||
| - If the channel type allows a maximum of 483 HTLCs: 20 | ||
| - If the channel type allows a maximum of 120 HTLCs: 5 |
There was a problem hiding this comment.
LND and LDK both have defaults on 483, so most of the network is probably running with this default.
That's true for LND, though it looks like LDK defaults to 50.
Assuming that these values are set to protect nodes from the on-chain cost of resolution, I'm hopeful that other impls will be able to increase the number of slots they allow now that they can't be so trivially filled up.
Good point. Using the suggested defaults with zero-fee commits would put the general bucket size at 120 * .4 = 48, which is essentially the same as current max_accepted_htlcs defaults. Usage above that amount would be possible but would require reputation.
| 1. type: 0 (`blinded_path`) | ||
| 2. data: | ||
| * [`point`:`path_key`] | ||
| 1. type: 1 (`accountable`) |
There was a problem hiding this comment.
Why not allow multiple accountability level instead of it being just a boolean flag? Just as with the endorsement before.
There was a problem hiding this comment.
The accountable signal being set should be interpreted as: "I will hold your reputation responsible for the timely resolution of this HTLC", which is a boolean statement.
By contrast, endorsement signals meant "I think that this HTLC will resolve in a timely manner", which makes more sense to interpret as a range.
c25f7b1 to
84ab329
Compare
thomash-acinq
left a comment
There was a problem hiding this comment.
Passing upgrade_accountability in the payload is not compatible with blinded routes, I've fixed it in carlaKC#6
Add accountability signal for HTLCs, it replaces endorsement. See lightning/bolts#1280
|
|
||
| ##### Rationale | ||
|
|
||
| Fees are used to accumulate reputation because they are an unforegable, |
Add accountability signal for HTLCs, it replaces endorsement. See lightning/bolts#1280
@carlaKC Thomas has a good point here that this is currently incompatible with the blinded path requirements on what is allowed in the onion, we need to either update the blinded path requirements to allow setting the accountability TLV or let the creator of the blinded path set that TLV in the encrypted route data (which is what Thomas proposed in carlaKC#6). Let us know which option you like best! |
|
Updating the blinded path requirements to allow setting the accountability TLV would mean that only nodes that support accountability would be able to relay payments with
Letting the recipient add |
Add accountability signal for HTLCs, it replaces endorsement. See lightning/bolts#1280
This seems like the right approach to me, since it's a recipient chosen value. Thanks for the PR, I'll upstream it! Update: diff adds signal in blinded routes + fixes a few typos. |
When an incoming onion payload includes the `upgrade_accountability` marker (BOLT 4 TLV type 19, or type 3 in `encrypted_recipient_data` for blinded hops), the forwarding node is permitted to set `accountable` on the outgoing `update_add_htlc` even if the incoming HTLC did not have it set. We always opt-in for maximum jamming protection (lightning/bolts#1280). Includes: - New `upgrade_accountability: bool` field on `PendingHTLCInfo`, serialized as TLV type 13 with default `false` for backwards compatibility - `create_fwd_pending_htlc_info` extracts the marker from the decoded `InboundOnionForwardPayload` or `InboundOnionBlindedForwardPayload` and stores it on the resulting `PendingHTLCInfo` - Trampoline forward variants set the field to `false` (the spec does not extend trampoline) - The forwarding dispatch in `process_pending_htlc_forwards` computes `outgoing_accountable = incoming_accountable || upgrade_accountability` and passes it to `queue_add_htlc` - The receive case sets `upgrade_accountability: false` on the `PendingHTLCInfo` since this node is the recipient and won't forward - Functional test that builds an onion with `invoice_accountable: true` and verifies the forwarding node upgrades the outgoing HTLC Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
BOLT 4 receiver validation rule (lightning/bolts#1280): if `accountable` is set on the incoming `update_add_htlc`, the sender should have either included `upgrade_accountability` in the onion payload (or, for blinded hops, in `encrypted_recipient_data`) or honored the recipient's invoice accountability marker. In this read-only mode we don't reject the HTLC; we log a warning when the marker is absent so operators can detect upstream tampering. A follow-up can change this to a hard error once the spec stabilizes. Includes: - `create_recv_pending_htlc_info` now binds `upgrade_accountability` from the decoded `InboundOnionReceivePayload` / `InboundOnionBlindedReceivePayload` (the trampoline receive case defaults to `false` since the spec doesn't extend trampoline) - New `logger: &dyn Logger` parameter on `create_recv_pending_htlc_info`, threaded through from each call site in `channelmanager` - Inline `log_warn!` when `incoming_accountable && !upgrade_accountability` - Functional test that simulates upstream tampering and asserts the warning is logged on the receiving node Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
erickcestari
left a comment
There was a problem hiding this comment.
Did a pass over the recommendation doc and the BOLT 02/04/11/12 changes, design reads well and the permission (BOLT-02) vs. policy (recommendation) split is coherent.
One real issue: the BOLT-11 a field is numbered 31, but 31 is l in bech32; a is 29. Worth fixing before merge.
Everything else is nits: typos, broken cross-file links (leading #), missing TOC entry, and tightening the sender-mimicry wording.
4acb09c to
c2771fb
Compare
The recipient of a payment is ultimately responsible for the fast resolution of a payment (though intermediate node can, of course, slow it down). We add a signal from the final recipient that the sender can use to reason about the amount of time it should take to resolve. Co-authored-by: Thomas HUET <thomas.huet@acinq.fr>
Once we have a signal from the recipient, we need a way to propagate this information throughout the route. Including a signal in the onion provides an uncorruptable way for the sender to propagate this signal. If the sender is dishonest, this will eventually be detected by the final recipient. In the commits that follow we'll add reputation and resource management that will allow nodes to use this signal to protect against jamming attacks. Co-authored-by: Thomas HUET <thomas.huet@acinq.fr>
75ef141 to
2e09bcd
Compare
|
Addressed @erickcestari's comments - diff here + here Squashed fixups because they were getting a bit unwieldy. |
2e09bcd to
b881c0b
Compare
Add a self-contained, log-only subsystem implementing the local resource conservation scheme from lightning/bolts#1280 (outgoing reputation + HTLC accountability), modelled on rust-lightning's resource_manager. The subsystem observes forwarded HTLCs and computes, per channel: an outgoing-channel reputation (decaying average of effective fees), an incoming-channel revenue threshold, a sufficiency verdict, and a general/congestion/protected resource-bucket assignment. It is strictly observational: it never fails, delays, or re-prioritises an HTLC and never touches the wire. Reputation is built only from live traffic (no historical read); decaying-average state is persisted so it survives restarts, while in-flight HTLC tracking is discarded on restart (a documented log-only trade-off). Arithmetic mirrors the LDK reference so the two agree numerically.
Add a self-contained, log-only subsystem implementing the local resource conservation scheme from lightning/bolts#1280 (outgoing reputation + HTLC accountability), modelled on rust-lightning's resource_manager. The subsystem observes forwarded HTLCs and computes, per channel: an outgoing-channel reputation (decaying average of effective fees), an incoming-channel revenue threshold, a sufficiency verdict, and a general/congestion/protected resource-bucket assignment. It is strictly observational: it never fails, delays, or re-prioritises an HTLC and never touches the wire. Reputation is built only from live traffic (no historical read); decaying-average state is persisted so it survives restarts, while in-flight HTLC tracking is discarded on restart (a documented log-only trade-off). Arithmetic mirrors the LDK reference so the two agree numerically.
Add a self-contained, log-only subsystem implementing the local resource conservation scheme from lightning/bolts#1280 (outgoing reputation + HTLC accountability), modelled on rust-lightning's resource_manager. The subsystem observes forwarded HTLCs and computes, per channel: an outgoing-channel reputation (decaying average of effective fees), an incoming-channel revenue threshold, a sufficiency verdict, and a general/congestion/protected resource-bucket assignment. It is strictly observational: it never fails, delays, or re-prioritises an HTLC and never touches the wire. Reputation is built only from live traffic (no historical read); decaying-average state is persisted so it survives restarts, while in-flight HTLC tracking is discarded on restart (a documented log-only trade-off). Arithmetic mirrors the LDK reference so the two agree numerically. When a channel needs every slot in the general bucket (small channels whose per-channel allocation meets or exceeds the bucket's total slot count), the slots are now assigned deterministically rather than drawn from the ChaCha20 keystream, which could otherwise fail to converge within the attempt budget and make slot assignment intermittently error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a self-contained, log-only subsystem implementing the local resource conservation scheme from lightning/bolts#1280 (outgoing reputation + HTLC accountability), modelled on rust-lightning's resource_manager. The subsystem observes forwarded HTLCs and computes, per channel: an outgoing-channel reputation (decaying average of effective fees), an incoming-channel revenue threshold, a sufficiency verdict, and a general/congestion/protected resource-bucket assignment. It is strictly observational: it never fails, delays, or re-prioritises an HTLC and never touches the wire. Reputation is built only from live traffic (no historical read); decaying-average state is persisted so it survives restarts, while in-flight HTLC tracking is discarded on restart (a documented log-only trade-off). Arithmetic mirrors the LDK reference so the two agree numerically. When a channel needs every slot in the general bucket (small channels whose per-channel allocation meets or exceeds the bucket's total slot count), the slots are now assigned deterministically rather than drawn from the ChaCha20 keystream, which could otherwise fail to converge within the attempt budget and make slot assignment intermittently error.
|
Woops apparently a commit that mentions this PR notifies this page, sorry for the spam, removing the URL from the above commit. |
| - `decay_rate`: a constant rate of decay based on the rolling window chosen, | ||
| calculated as: `(1/2)^(1/ln(2)*window_length_seconds)` |
There was a problem hiding this comment.
I think the decay_rate formula in the spec is missing a parenthesis: (1/2)^(1/ln(2)*window_length_seconds) as written parses as 0.5^(W/ln 2) = e^(-W), which is essentially zero for any realistic window. I guess the intended formula was (1/2)^(1/(ln(2)*window_length_seconds)), which simplifies to e^(-1/W).
One thing I noticed is that LND and LDK are currently using 0.5^(2/W) (half-life = W/2), which decays a bit faster than e^(-1/W). With the 24-week window, 0.5^(2/W) leaves 25% weight at the end vs 37% for e^(-1/W).
So which one should be the recommended decay_rate here? Should the spec just get the parenthesis fix and keep e^(-1/W), or would it make sense to align it with what the implementations are already doing, i.e. (1/2)^(2/window_length_seconds)?
| - `decay_rate`: a constant rate of decay based on the rolling window chosen, | |
| calculated as: `(1/2)^(1/ln(2)*window_length_seconds)` | |
| - `decay_rate`: a constant rate of decay based on the rolling window chosen, | |
| calculated as: `(1/2)^(1/(ln(2)*window_length_seconds))` |
Each general bucket slot represents a fixed share of the bucket's capacity, so an HTLC consumes max(1, ceil(amt_msat / general_bucket_slot_liquidity)) slots. A slot holds at most one HTLC, bounding the bucket to its slot count, and a pair is admitted only while enough of its assigned slots are unoccupied. Expressing a pair's limit purely in slots means one check covers both slot and liquidity exhaustion, and makes a large HTLC cost a pair proportionally more of its allocation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Slot indexes are drawn from a ChaCha20 stream keyed with a node-wide salt and nonced with SHA256(incoming_channel_id || outgoing_channel_id)[0:12], so a pair's assignment is unpredictable without the salt and an attacker cannot open channels to land on a victim's slots. The channel_id identifies the pair because it is stable for the channel's lifetime, whereas the short_channel_id changes on splice and would silently re-assign slots. Committing to both ids in full lets a single salt be shared across all channels, leaving one value to persist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An outgoing channel qualifies for an incoming channel's congestion bucket only while it holds no congestion HTLC anywhere on the node, and has not resolved a HTLC in that bucket slower than resolution_period in the last two weeks. An attacker must therefore open one channel for every congestion slot it wants to hold, while a slow resolution costs it access to just the one bucket it abused. Access additionally requires the general bucket to be fully occupied, reserving congestion resources for periods of apparent attack, and caps amount_msat at one slot's worth of the bucket's liquidity so that no single HTLC consumes a disproportionate share. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The decay_rate is (1/2)^(1/(ln(2) * window_length_seconds)), giving a half life of ln(2) * window_length_seconds and a mean contribution lifetime of one window. Stating the half life alongside the formula lets implementers check their reading of the exponent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The aggregated decaying average is divided by max(warmup_factor, 1), so until roughly one revenue_window has elapsed the reported value is the revenue accumulated so far rather than an extrapolation of it to a full window. This keeps a short burst of traffic on a new channel from inflating its revenue threshold, and avoids dividing by a near-zero factor immediately after initialization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This PR replaces #1071 with our latest proposal for a reputation scheme and resource management to mitigate slow jamming attacks.
Key differences for the new approach are:
accountablesignal to our outgoing peer to indicate whether we'll hold their reputation accountable because scarce resources have been used (rather than anendorsedsignal that indicates scarce resources may be used).