Outgoing reputation and HTLC Accountability#1280
Outgoing reputation and HTLC Accountability#1280carlaKC wants to merge 14 commits intolightning:masterfrom
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.
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.
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. |
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>
h/t elnosh for pointing this out!
fe79559 to
2afbb47
Compare
We can just keep pulling from a single stream rather than needing multiple hashes.
Add support for the BOLT11 `a` tagged field (bech32 value 31) as specified in lightning/bolts#1280. This is a zero-length marker field that indicates the recipient is willing to be held accountable for timely resolution of the invoice (within 90 seconds of HTLC arrival). Includes: - TAG_ACCOUNTABLE constant and Accountable variant in TaggedField - Serialization/deserialization support - Builder method and accessors on RawBolt11Invoice and Bolt11Invoice - Roundtrip tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add support for the BOLT12 `invoice_accountable` TLV (type 161) as specified in lightning/bolts#1280. This is an empty marker field that indicates the recipient is willing to be held accountable for fast resolution of the HTLC (within 90 seconds of arrival). Includes: - New TLV record in InvoiceTlvStream - accountable field in InvoiceFields with default false - accountable() builder method on InvoiceBuilder - invoice_accountable() accessor on UnsignedBolt12Invoice and Bolt12Invoice - StaticInvoice ignores the accountable field on parse and never sets it on construction (out of scope for static invoices) - Roundtrip test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add support for the BOLT 4 `upgrade_accountability` TLV (type 19) in the non-blinded per-hop onion payload as specified in lightning/bolts#1280. This is a marker field set by the sender on every non-blinded hop of a route to a recipient that opted into accountability in their invoice. Includes: - upgrade_accountability field on InboundOnionForwardPayload and InboundOnionReceivePayload (read from the wire) - upgrade_accountability field on OutboundOnionPayload Forward and Receive variants (written to the wire as TLV 19 when set) - Decoding via TLV type 19 in InboundOnionPayload::read - Trampoline payloads default to false (the spec does not extend the trampoline payload format) - Roundtrip tests for both Forward and Receive - Default false at all current call sites; commit 5 will thread the value through from invoice signalling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_data Add support for the BOLT 4 `upgrade_accountability` TLV (type 3) in the blinded payment `encrypted_recipient_data` as specified in lightning/bolts#1280. This allows the recipient to authorize forwarding nodes inside a blinded route to set `accountable` on outgoing HTLCs. Includes: - upgrade_accountability field on ForwardTlvs and ReceiveTlvs - Serialization at TLV type 3 (between short_channel_id and payment_relay for ForwardTlvs; before payment_constraints for ReceiveTlvs) - Decoding via the BlindedPaymentTlvs read implementation - upgrade_accountability field on InboundOnionBlindedForwardPayload and InboundOnionBlindedReceivePayload (threaded through from the decrypted encrypted_recipient_data) - Trampoline ReceiveTlvs default to false (the spec does not extend the trampoline encrypted payload format) - Roundtrip test for ForwardTlvs and ReceiveTlvs - Default false at all current call sites; commit 5 will thread the value through from invoice signalling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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>
For a more accurate decaying average, particularly relevant for when we calculate the average across several windows.
Previous approach underestimated in early periods.
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).