Allow nodes to overshoot final htlc amount and expiry #1032
Conversation
04-onion-routing.md
Outdated
| @@ -292,7 +292,7 @@ The writer: | |||
| - otherwise: | |||
| - MUST set `total_msat` to the amount it wishes to pay. | |||
| - MUST ensure that the total `amount_msat` of the HTLC set which arrives at the payee | |||
| is equal to `total_msat`. | |||
| is equal to or greater than `total_msat`. | |||
There was a problem hiding this comment.
I don't think this change is necessary anymore after the change to allow htlc_amt > amt_to_fwd in the later commit?
There was a problem hiding this comment.
I think we have to keep it, don't we? Because if we overpay one HTLC in the set, that means that the total HTLC set will overpay the total_amount?
There was a problem hiding this comment.
I don't think so, because you only count amt_to_forward towards total_msat. The overpayment is already absorbed at the htlc level.
There was a problem hiding this comment.
I don't think so, because you only count amt_to_forward towards total_msat. The overpayment is already absorbed at the htlc level.
I see what you mean, that depends on what you use to compute your "HTLC set amount". Currently the spec does it using amount_msat, which is the value from update_add_htlc: in that case, we do need to explicitly allow over-paying.
What you suggest is that we should define the "HTLC set amount" to be the sum of the onion's amt_to_fwd, in which case we don't necessarily need to allow over-paying at this layer.
That would be a bit awkward for the scenario highlighted in #1031 though. Let's imagine that you retry a partial payment of 500 msat and are forced to pay 510 msat to satisfy min_htlc constraints in the route. With your proposal, the sender would be forced to put 500 msat in the onion amt_to_fwd, which means giving the additional fee to intermediate nodes instead of the final node (and feels a bit weird, doesn't it?). Whereas if we allow over-paying the total_amount_msat, the sender can decide whether they want to give that extra fee to intermediate nodes or to the final node.
I understand your point of view, but my current feeling is to go for the extra flexibility of allowing the total_amount_msat over-payment. Let's see what others prefer, I can change that if that's the consensus.
There was a problem hiding this comment.
Let's imagine that you retry a partial payment of 500 msat and are forced to pay 510 msat to satisfy min_htlc constraints in the route. With your proposal, the sender would be forced to put 500 msat in the onion amt_to_fwd, which means giving the additional fee to intermediate nodes instead of the final node.
I don't think you are giving it to the intermediate node straight away? The instruction is still to forward 510 sat to the final node. Only if the intermediate node would first try to pay lower amounts, they could steal some of those sats.
Also I think this only applies to the final shard. All the other shards would just put 510 msat in the onion amt_to_fwd? It satisfies min_htlc and makes sure that a maximum of sats is delivered to the final destination. The overshoot can be compensated in the final shard to end up with the exact pre-determined total_msat.
What remains then is a sender being forced to overpay on the final shard because of a min_htlc constraint and the possibility of a routing node trying to steal the difference. It wouldn't increase the total cost of the payment though, because all prior shards were already 'optimal' in that they don't allow any fee stealing. The difference is just in where that final overpayment ends up IF the routing node decides to probe.
Even if we fix this for the final hop, it is still possible to shift the fee distribution for the intermediate hops.
If senders would always assume that overpayment of total_msat is acceptable, there wouldn't be a way anymore to reject such payments. You can fail the htlc set if it overpays, but the sender wouldn't know what's wrong. As mentioned before, overpayment isn't always desirable and can complicate accounting.
There was a problem hiding this comment.
if we would let the per-hop payload hmac also cover the required incoming htlc amount
That would just be an alternative from adding a required_incoming_htlc_amt field, right? But that removes all possibility of giving some leeway for intermediate nodes to decide to forward more to pay an inbound fee for example.
Also, this would require a feature bit to signal the change in the sphinx mac logic and would require the whole path to support it, which is annoying.
There was a problem hiding this comment.
Yes, a zero bytes alternative indeed for required_incoming_htlc_amt. Why would an intermediate node need leeway to pay for example an inbound fee? I'd think this is all decided upfront by the sender. It's the leeway that we don't want because it allows the probing. So if there is leeway, it is bad?
The feature bit is required indeed, but does the whole path need to support this? I thought the sender can calculate the hmac for each hop and cover the required incoming amt under the hmac only if the feature bit is present.
There was a problem hiding this comment.
Starting to magically include more data that's not be present in the HMAC sounds like something that would make similar changes harder later.
What's bad about just adding an extra (optional) field require_incoming_amount and as more nodes start checking this, probing gets harder and harder. I still thinks this is the cleanest way of ensuring intermediate nodes can't do anything other than what the sender tells them to, which sounds like it would fix most of these probing vectors (maybe even some we haven't thought of).
Should also be totally backwards compatible IIUC.
I agree with t-bast that this would give intermediate nodes less leeway to overpay (for some reason), but as Joost is saying, shouldn't senders really be in charge of deciding that?
There was a problem hiding this comment.
Why would an intermediate node need leeway to pay for example an inbound fee? I'd think this is all decided upfront by the sender.
I thought it could be interesting to let nodes only tell their own peers about inbound fees, so that the sender doesn't have to know about it. But that's a separate topic, so maybe we should discuss it somewhere else or later.
Starting to magically include more data that's not be present in the HMAC sounds like something that would make similar changes harder later.
Agreed.
I agree with t-bast that this would give intermediate nodes less leeway to overpay (for some reason), but as Joost is saying, shouldn't senders really be in charge of deciding that?
I think that whatever senders do, if it doesn't align with the right incentives, it will eventually be ignored: that's why I think the required_incoming_htlc_amt would not be useful in practice. As @thomash-acinq correctly points out, as an intermediate node or a final node, if I receive more money than expected, I will take it, regardless of what is inside the onion. If I receive less money than I expect, I will reject it. Anything else doesn't really matter to my decision to accept or reject an HTLC. The current spec correctly does that for intermediate nodes, but incorrectly restricts final nodes, which doesn't align with their incentives: that's what this PR is fixing (and it incidentally fixes probing vectors, except in rare scenarios of recently updated fees as @joostjager pointed out).
There was a problem hiding this comment.
as an intermediate node or a final node, if I receive more money than expected, I will take it
Another way to look at this is that failing could lead to more money on the longer term. By failing you're teaching your predecessor that there is no point in trying to steal anything and next time they might just pay the instructed (higher) amount?
| - if it is the final node: | ||
| - MUST treat `total_msat` as if it were equal to `amt_to_forward` if it | ||
| is not present. | ||
| - MUST return an error if: | ||
| - incoming `amount_msat` != `amt_to_forward`. | ||
| - incoming `cltv_expiry` != `cltv_expiry_delta`. | ||
| - incoming `amount_msat` < `amt_to_forward`. |
There was a problem hiding this comment.
If we relax this field we must also start counting payment_amt = SUM(htlcs.amt_to_forward) at the final node. If we don't, a second-to-last node can overpay the htlc amount and mess with the tracking of MPP shards (that would lead to a premature settle of the HTLC set).
This would become even worse with the relaxation of the total_msat check, as it would get very easy to cause such a premature settle for a malicious node.
I think the ultimate solution would be what is mentioned in https://github.com/lightning/bolts/pull/1032/files#r993085640, adding arequired_incoming_htlc_amt field. This would restrict all sorts of funny business routing nodes can do to the HTLC amt they send to the next hop, and would require them to always forward exactly amt_to_fwd. To me that sounds like it would solve all the probing issues also, as there would not be possible for a routing node to under- nor overpay, regardless of the next hop is the final or not.
There was a problem hiding this comment.
I think you're over-complicating this. From the recipient's point of view, they should be happy as soon as they receive at least total_amount_msat: they've been paid what they wanted and should release the preimage. From the sender's point of view, they should be happy as soon as they receive the preimage while paying at most total_amount_msat.
If intermediate nodes overpay an HTLC, neither the sender not the recipient are negatively impacted. The sender may even get the preimage for less than what they intended to pay: this is great, why would they prevent that?
as there would not be possible for a routing node to under- nor overpay
It should never be possible for an intermediate node to underpay, I agree with that. However, I don't see how overpaying can be an issue, and it's actually necessary to allow overpaying if you want to enable inbound fees?
There was a problem hiding this comment.
I agree with the theory, but are implementations properly doing this in practice? I guess it should be handled anyway, since there are never any guarantees all shards will be settled.
There was a problem hiding this comment.
That's at least how eclair handles it ;)
|
The objections to this change seem to only consider the privacy aspect: preventing a malicious node from learning more information than it should, assuming all other nodes follow the spec. However for this to work, we need all other nodes to follow the spec, and for that we need to make sure that incentives are aligned. Currently, the spec asks us to refuse free money to protect the privacy of others. Any economically rational actor should take the free money as there is no downside for them, the downside is only for the privacy of the others. |
Note that this depends on #1032 which clarifies the final node requirements for non-blinded payments.
When nodes receive HTLCs, they verify that the contents of those HTLCs match the intructions that the sender provided in the onion. It is important to ensure that intermediate nodes and final nodes have similar requirements, otherwise a malicious intermediate node could easily probe whether the next node is the final recipient or not. Unfortunately, the requirements for intermediate nodes were more lenient than the requirements for final nodes. Intermediate nodes allowed overpaying and increasing the CLTV expiry, whereas final nodes required a perfect equality between the HTLC values and the onion values. This provided a trivial way of probing: when relaying an HTLC, nodes could relay 1 msat more than what the onion instructed (or increase the outgoing expiry by 1). If the next node was an intermediate node, they would accept this HTLC, but if the next node was the recipient, they would reject it. We update those requirements to fix this probing attack vector.
This is actually a cltv_expiry_delta, not an absolute cltv_expiry, so the field name should reflect that. Recipients require incoming HTLC expiry to comply with that expiry delta.
t-bast
force-pushed
the
recipient-overshoot
branch
from
befe93f
to
391ed84
Compare
|
Rebased on |
Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See lightning/bolts#1032 for more details.
When nodes receive HTLCs, they verify that the contents of those HTLCs match the intructions that the sender provided in the onion. It is important to ensure that intermediate nodes and final nodes have similar requirements, otherwise a malicious intermediate node could easily probe whether the next node is the final recipient or not. Unfortunately, the requirements for intermediate nodes were more lenient than the requirements for final nodes. Intermediate nodes allowed overpaying and increasing the CLTV expiry, whereas final nodes required a perfect equality between the HTLC values and the onion values. This provided a trivial way of probing: when relaying an HTLC, nodes could relay 1 msat more than what the onion instructed (or increase the outgoing expiry by 1). If the next node was an intermediate node, they would accept this HTLC, but if the next node was the recipient, they would reject it. We update those requirements to fix this probing attack vector. See lightning/bolts#1032
|
I am not so sure about this change anymore, #1031 (comment) I know the spec already allows overpayment, but it's undeniable that in the traditional world overpayments can lead to problems. Companies not knowing what to do with the excess, requiring refunds, perhaps tax implications even, etc. |
As was pointed out in response to your comment, I believe this isn't a valid reason to reject the PR as we already support over-payment. We can add a recipient feature to require strict payment amounts (at the expense of privacy) in future work, but that shouldn't have any impact on acceptance of this PR. |
I am wondering if we can have strict payment amounts without sacrificing privacy. I think we can have that if we'd go the other previously discussed way and instead of relaxing the final amt_to_fwd requirement (this pr), we'd tighten acceptance for routing nodes via a new onion field or inclusion of the amount in the hmac'ed data. The downside of that is, and we also discussed this before, is that an economically rational routing node would ignore the accept instruction in the onion and maximize profit without caring about privacy, which brings us back to the current situation. So yes, maybe the outcome is that there is no other option than relaxing indeed. Unless anyone has a new creative idea? |
|
I think that strict payments should not be the default as they're not what an economically rational actor would want. But there's room for making this an opt-in feature, signaled in the invoice, most likely at the expense of privacy, and maybe this is a trade-off that some nodes are ok with? Anyway implementing strict payments is a separate proposal, and as you say hopefully by the time we really commit to implementing it we'll have found ways of doing it that doesn't sacrifice too much privacy. |
|
If our understanding is that we cannot easily get route-privacy with strict payments then I don't think we should ever add strict payments. The recipient deciding that the sender is not allowed to have privacy is not a good tradeoff, IMO. |
When nodes receive HTLCs, they verify that the contents of those HTLCs match the intructions that the sender provided in the onion. It is important to ensure that intermediate nodes and final nodes have similar requirements, otherwise a malicious intermediate node could easily probe whether the next node is the final recipient or not. Unfortunately, the requirements for intermediate nodes were more lenient than the requirements for final nodes. Intermediate nodes allowed overpaying and increasing the CLTV expiry, whereas final nodes required a perfect equality between the HTLC values and the onion values. This provided a trivial way of probing: when relaying an HTLC, nodes could relay 1 msat more than what the onion instructed (or increase the outgoing expiry by 1). If the next node was an intermediate node, they would accept this HTLC, but if the next node was the recipient, they would reject it. We update those requirements to fix this probing attack vector. See lightning/bolts#1032
Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See lightning/bolts#1032 for more details.
|
Yes. Consistency FTW. Ack. |
These requirements were missed when integrating lightning#1032
* Use onion amount in MPP set calculation The sender chooses the amounts that are set in the onion payload (`amt_to_forward`) but cannot predict what amounts will be set in the HTLCs (`amount_msat`) since intermediate nodes are allowed to overpay. * Fix error requirements for final node These requirements were missed when integrating #1032
Note that this depends on #1032 which clarifies the final node requirements for non-blinded payments.
In lightning#1032 we allowed overshooting the final amount and expiry, but forgot to update the onion error descriptions which make reference thereto.
In #1032 we allowed overshooting the final amount and expiry, but forgot to update the onion error descriptions which make reference thereto.
Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See lightning/bolts#1032 for more details.
Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See lightning/bolts#1032 for more details.
Recipients must allow HTLC values greater than what the onion payload contains, otherwise they can be trivially identified as being the final recipient. See lightning/bolts#1032 for more details.
When nodes receive HTLCs, they verify that the contents of those HTLCs match the intructions that the sender provided in the onion. It is important to ensure that intermediate nodes and final nodes have similar requirements, otherwise a malicious intermediate node could easily probe whether the next node is the final recipient or not.
Unfortunately, the requirements for intermediate nodes were more lenient than the requirements for final nodes. Intermediate nodes allowed overpaying and increasing the CLTV expiry, whereas final nodes required a perfect equality between the HTLC values and the onion values.
This provided a trivial way of probing: when relaying an HTLC, nodes could relay 1 msat more than what the onion instructed (or increase the outgoing expiry by 1). If the next node was an intermediate node, they would accept this HTLC, but if the next node was the recipient, they would reject it.
We update those requirements to fix this probing attack vector. We also rename the
min_final_cltv_expiryfield, which is actually an expiry delta.