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 clarify `min_final_cltv_expiry`: 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.
Bolt 11 invoices must contain a `payment_secret`, which means that the
`features` field must set the `payment_secret` feature (and its dependency,
`var_onion_optin`).
Fixes#897
Update Bolt 11 test vectors to always include a payment secret.
We want to make it mandatory in invoices which would make the existing
test vectors invalid.
Judging from the comment
https://github.com/bitcoin/bitcoin/pull/18267/files#r491150895 in the
Signet PR all test networks should have the same bech32_hrp prefix (even
regtest). That's why 'tb' was chosen for Signet as well.
This is not optimal for LN as invoices shouldn't be vague in
what network they were issued for.
Therefore we add the explicit prefix 'lntbs' for Signet invoices.
We added a requirement on the writer, not the reader. We can't really add
a test vector without a new requirement, though.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
One for uppercase, and one with should-be-ignored fields.
The first of these addresses #659 (#677 directly changes the text
to make it clear this is allowed, and should also be applied).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As a final step, we now can remove several of the BOLT 11 writer's
requirements now that it builds on BOLT 9's, particularly:
- setting the even bit if a feature is required.
- only setting a feature if the node supports a given feature.
The lone requirement that remains pertains to setting the `s` value if
and only if the `payment_secret` feature is set.
This commit:
- Adds a new Dependencies column to the BOLT 9 feature table
populated with existing feature dependencies.
- Requires that all valid feature vectors set transitive dependencies.
- Requires checking transitive dependencies when validating init
messages and payment request.
- Removes transitive feature requiremetns from the BOLT 11 writer, now
that they are implicit by needing to comply with the BOLT 9 origin
requirements.
This means the BOLT11 invoice must offer it (we already say it must
set the field if it offers it), and that the receiving node must
require it (again, we already say it must check it if it requires it).
Without the payment_secret, MPP payments are especially vulnerable to
probing attacks: unlike normal payments (with amounts) they can be
detected with 1msat payment probes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When the `p` multiplier is used, the amount MUST be divisible
by 10 since the resolution used internally is millisatoshi.
This addresses but does not close#692.
Most obviously, we want this for BASE AMP, but it's useful in future.
Even though even bits won't cause existing implementations to know
they can't pay the invoice, it will allow it in future once everyone
has upgraded.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I was trying to compactly indicate that the considered alternative to
multiplier postfixes was just express everything in millisatoshi, but
it's just confusing, and anyway there are other notations we didn't use
so it seems like a weird thing to explain.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
a.k.a. landonmutch style, where requirements are explicit and bullet-pointed.
This also tightens requirements:
1. New requirement (previously implied) that writer use correct prefix.
2. Reader MUST rather than SHOULD fail malformed `amount`.
It also makes it clear that writer can omit the multiplier.
Fixes: #442
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The routing tagged field of one test vector was extracted from the
invoice incorrectly. The route included in the invoice has as base
fee of 1 resp. 2 msat (as described in the explanation), but the
extracted bech32 part had a 0 msat base fee.
