Skip to content

Commit

Permalink
lnaddr: make payment_secret field mandatory, in both directions
Browse files Browse the repository at this point in the history
we now require payment_secret both for sending and for receiving
(previously was optional for both)

see
lightning/bolts#898
ACINQ/eclair#1810
ElementsProject/lightning#4646

note: payment_secret depends on var_onion_optin, so that becomes mandatory as well,
however this commit does not yet remove the ability of creating legacy onions
  • Loading branch information
SomberNight committed Jun 29, 2023
1 parent a66b0c6 commit fc6486e
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 72 deletions.
17 changes: 16 additions & 1 deletion electrum/lnaddr.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ def lnencode(addr: 'LnAddr', privkey) -> str:
tags_set = set()

# Payment hash
assert addr.paymenthash is not None
data += tagged_bytes('p', addr.paymenthash)
tags_set.add('p')

Expand All @@ -196,7 +197,7 @@ def lnencode(addr: 'LnAddr', privkey) -> str:
# BOLT #11:
#
# A writer MUST NOT include more than one `d`, `h`, `n` or `x` fields,
if k in ('d', 'h', 'n', 'x', 'p', 's'):
if k in ('d', 'h', 'n', 'x', 'p', 's', '9'):
if k in tags_set:
raise LnEncodeException("Duplicate '{}' tag".format(k))

Expand Down Expand Up @@ -317,6 +318,17 @@ def get_features(self) -> 'LnFeatures':
from .lnutil import LnFeatures
return LnFeatures(self.get_tag('9') or 0)

def validate_and_compare_features(self, myfeatures: 'LnFeatures') -> None:
"""Raises IncompatibleOrInsaneFeatures.
note: these checks are not done by the parser (in lndecode), as then when we started requiring a new feature,
old saved already paid invoices could no longer be parsed.
"""
from .lnutil import validate_features, ln_compare_features
invoice_features = self.get_features()
validate_features(invoice_features)
ln_compare_features(myfeatures.for_invoice(), invoice_features)

def __str__(self):
return "LnAddr[{}, amount={}{} tags=[{}]]".format(
hexlify(self.pubkey.serialize()).decode('utf-8') if self.pubkey else None,
Expand Down Expand Up @@ -381,6 +393,9 @@ def serialize(self):
return self.pubkey.get_public_key_bytes(True)

def lndecode(invoice: str, *, verbose=False, net=None) -> LnAddr:
"""Parses a string into an LnAddr object.
Can raise LnDecodeException or IncompatibleOrInsaneFeatures.
"""
if net is None:
net = constants.net
decoded_bech32 = bech32_decode(invoice, ignore_long_length=True)
Expand Down
18 changes: 9 additions & 9 deletions electrum/lnonion.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def from_fd(cls, fd: io.BytesIO) -> 'OnionHopsDataSingle':
elif first_byte == b'\x01':
# reserved for future use
raise Exception("unsupported hop payload: length==1")
else:
else: # tlv format
hop_payload_length = read_bigsize_int(fd)
hop_payload = fd.read(hop_payload_length)
if hop_payload_length != len(hop_payload):
Expand Down Expand Up @@ -266,8 +266,9 @@ def calc_hops_data_for_payment(
route: 'LNPaymentRoute',
amount_msat: int,
final_cltv: int, *,
total_msat=None,
payment_secret: bytes = None) -> Tuple[List[OnionHopsDataSingle], int, int]:
total_msat: int,
payment_secret: bytes,
) -> Tuple[List[OnionHopsDataSingle], int, int]:

"""Returns the hops_data to be used for constructing an onion packet,
and the amount_msat and cltv to be used on our immediate channel.
Expand All @@ -283,12 +284,11 @@ def calc_hops_data_for_payment(
}
# for multipart payments we need to tell the receiver about the total and
# partial amounts
if payment_secret is not None:
hop_payload["payment_data"] = {
"payment_secret": payment_secret,
"total_msat": total_msat,
"amount_msat": amt
}
hop_payload["payment_data"] = {
"payment_secret": payment_secret,
"total_msat": total_msat,
"amount_msat": amt
}
hops_data = [OnionHopsDataSingle(
is_tlv_payload=route[-1].has_feature_varonion(), payload=hop_payload)]
# payloads, backwards from last hop (but excluding the first edge):
Expand Down
30 changes: 13 additions & 17 deletions electrum/lnpeer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1441,7 +1441,7 @@ def pay(self, *,
total_msat: int,
payment_hash: bytes,
min_final_cltv_expiry: int,
payment_secret: bytes = None,
payment_secret: bytes,
trampoline_onion=None) -> UpdateAddHtlc:

assert amount_msat > 0, "amount_msat is not greater zero"
Expand Down Expand Up @@ -1786,7 +1786,8 @@ def log_fail_reason(reason: str):
try:
total_msat = processed_onion.hop_data.payload["payment_data"]["total_msat"]
except Exception:
total_msat = amt_to_forward # fall back to "amt_to_forward"
log_fail_reason(f"'total_msat' missing from onion")
raise exc_incorrect_or_unknown_pd

if not is_trampoline and amt_to_forward != htlc.amount_msat:
log_fail_reason(f"amt_to_forward != htlc.amount_msat")
Expand All @@ -1795,14 +1796,10 @@ def log_fail_reason(reason: str):
data=htlc.amount_msat.to_bytes(8, byteorder="big"))

try:
payment_secret_from_onion = processed_onion.hop_data.payload["payment_data"]["payment_secret"]
payment_secret_from_onion = processed_onion.hop_data.payload["payment_data"]["payment_secret"] # type: bytes
except Exception:
if total_msat > amt_to_forward:
# payment_secret is required for MPP
log_fail_reason(f"'payment_secret' missing from onion")
raise exc_incorrect_or_unknown_pd
# TODO fail here if invoice has set PAYMENT_SECRET_REQ
payment_secret_from_onion = None
log_fail_reason(f"'payment_secret' missing from onion")
raise exc_incorrect_or_unknown_pd

payment_status = self.lnworker.check_received_htlc(payment_secret_from_onion, chan.short_channel_id, htlc, total_msat)
if payment_status is None:
Expand All @@ -1825,14 +1822,13 @@ def log_fail_reason(reason: str):
log_fail_reason(f"no payment_info found for RHASH {htlc.payment_hash.hex()}")
raise exc_incorrect_or_unknown_pd
preimage = self.lnworker.get_preimage(htlc.payment_hash)
if payment_secret_from_onion:
expected_payment_secrets = [self.lnworker.get_payment_secret(htlc.payment_hash)]
if preimage:
# legacy secret for old invoices
expected_payment_secrets.append(derive_payment_secret_from_payment_preimage(preimage))
if payment_secret_from_onion not in expected_payment_secrets:
log_fail_reason(f'incorrect payment secret {payment_secret_from_onion.hex()} != {expected_payment_secrets[0].hex()}')
raise exc_incorrect_or_unknown_pd
expected_payment_secrets = [self.lnworker.get_payment_secret(htlc.payment_hash)]
if preimage:
# legacy secret for old invoices
expected_payment_secrets.append(derive_payment_secret_from_payment_preimage(preimage))
if payment_secret_from_onion not in expected_payment_secrets:
log_fail_reason(f'incorrect payment secret {payment_secret_from_onion.hex()} != {expected_payment_secrets[0].hex()}')
raise exc_incorrect_or_unknown_pd
invoice_msat = info.amount_msat
if not (invoice_msat is None or invoice_msat <= total_msat <= 2 * invoice_msat):
log_fail_reason(f"total_msat={total_msat} too different from invoice_msat={invoice_msat}")
Expand Down
14 changes: 8 additions & 6 deletions electrum/lnworker.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ class ErrorAddingPeer(Exception): pass
| LnFeatures.OPTION_DATA_LOSS_PROTECT_REQ
| LnFeatures.OPTION_STATIC_REMOTEKEY_REQ
| LnFeatures.GOSSIP_QUERIES_REQ
| LnFeatures.VAR_ONION_REQ
| LnFeatures.PAYMENT_SECRET_REQ
| LnFeatures.BASIC_MPP_OPT
| LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT_ELECTRUM
| LnFeatures.OPTION_SHUTDOWN_ANYSEGWIT_OPT
Expand Down Expand Up @@ -1238,7 +1240,7 @@ async def pay_to_node(
self, *,
node_pubkey: bytes,
payment_hash: bytes,
payment_secret: Optional[bytes],
payment_secret: bytes,
amount_to_pay: int, # in msat
min_cltv_expiry: int,
r_tags,
Expand Down Expand Up @@ -1389,7 +1391,7 @@ async def pay_to_route(
total_msat: int,
amount_receiver_msat:int,
payment_hash: bytes,
payment_secret: Optional[bytes],
payment_secret: bytes,
min_cltv_expiry: int,
trampoline_onion: bytes = None,
trampoline_fee_level: int,
Expand Down Expand Up @@ -1546,8 +1548,7 @@ def _decode_channel_update_msg(cls, chan_upd_msg: bytes) -> Optional[Dict[str, A
except Exception:
return None

@staticmethod
def _check_invoice(invoice: str, *, amount_msat: int = None) -> LnAddr:
def _check_invoice(self, invoice: str, *, amount_msat: int = None) -> LnAddr:
addr = lndecode(invoice)
if addr.is_expired():
raise InvoiceError(_("This invoice has expired"))
Expand All @@ -1562,6 +1563,7 @@ def _check_invoice(invoice: str, *, amount_msat: int = None) -> LnAddr:
raise InvoiceError("{}\n{}".format(
_("Invoice wants us to risk locking funds for unreasonably long."),
f"min_final_cltv_expiry: {addr.get_min_final_cltv_expiry()}"))
addr.validate_and_compare_features(self.features)
return addr

def is_trampoline_peer(self, node_id: bytes) -> bool:
Expand Down Expand Up @@ -1615,8 +1617,8 @@ async def create_routes_for_payment(
min_cltv_expiry,
r_tags,
invoice_features: int,
payment_hash,
payment_secret,
payment_hash: bytes,
payment_secret: bytes,
trampoline_fee_level: int,
use_two_trampolines: bool,
fwd_trampoline_onion=None,
Expand Down
Loading

0 comments on commit fc6486e

Please sign in to comment.