lnrpc: clarify payment request expiry semantics

[starius]

Change Description

The DecodePayReq response exposes an absolute invoice timestamp and a relative expiry duration, but the expiry field did not document that distinction. Clarify that expiry is measured in seconds from timestamp to avoid it being interpreted as an absolute Unix timestamp.

This is a documentation-only change and does not alter RPC behavior.

Pull Request Checklist

Testing

• Your PR passes all CI checks.
• Tests covering the positive and negative (error paths) are included.
• Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

• The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
• The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• Commits follow the Ideal Git Commit Structure.
• Any new logging statements use an appropriate subsystem and logging level.
• Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
• There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[GustavoStingelin]

LGTM

[Lrifton92]

Verified against the implementation: DecodePayReq fills this field from payReq.Expiry().Seconds() (rpcserver.go), where zpay32.Invoice.Expiry() returns the relative duration from the BOLT-11 x tagged field, while timestamp is set separately from payReq.Timestamp.Unix(). So the clarification matches the actual semantics, and the .pb.go / swagger regeneration is consistent (the pb.go churn is just gofmt realignment from the inserted comment).

One optional addition while you're touching this comment: when the payment request carries no expiry tag, zpay32.Invoice.Expiry() returns the BOLT-11 default of 3600 seconds, so this field is never 0 in practice. Spelling that out would preempt the next likely misreading (0 = "no expiry"), e.g.:

// The expiry duration in seconds relative to the `timestamp` field. If
// the payment request does not encode an expiry, the BOLT-11 default of
// 3600 seconds is returned.

Fine as-is too — the current wording is accurate.

[starius]

@Lrifton92 Great proposal! Thanks! Applied.

[github-actions]

🟠 PR Severity: HIGH

gh pr view | 3 files | 41 lines changed

🟠 High (2 files)

• lnrpc/lightning.proto - RPC/API definition change under lnrpc/*
• lnrpc/lightning.swagger.json - generated swagger spec for lnrpc/* API change

🟡 Medium (1 file)

• lnrpc/lightning.pb.go - auto-generated protobuf bindings for the proto change above

Analysis

This PR only touches lnrpc/*: a small addition to lightning.proto (5 lines) plus the corresponding generated lightning.pb.go and lightning.swagger.json updates. Per the severity rules, lnrpc/* (RPC/API definitions) is classified HIGH, requiring a knowledgeable engineer to review the API surface change. File/line counts are well below the bump thresholds, and no other packages are touched, so no severity bump applies.

---

_{To override, add a severity-override-{critical,high,medium,low} label.}

[ziggie1984]

LGTM

[ziggie1984]

let's maybe add proper description to all of them ?

[Lrifton92]

+1 — since this PR is about making the PayReq semantics explicit, here is a draft for the remaining fields, checked against what DecodePayReq in rpcserver.go actually fills in from zpay32.Invoice (in case it saves @starius a round trip; feel free to reword):

message PayReq {
    // The hex-encoded compressed public key of the payee (the node that
    // created the payment request).
    string destination = 1;

    // The hex-encoded payment hash (BOLT-11 `p` field).
    string payment_hash = 2;

    // The amount in satoshis. 0 if the payment request does not specify
    // an amount. Any millisatoshi remainder is truncated; see num_msat for
    // the exact value.
    int64 num_satoshis = 3;

    // The creation time of the payment request as a Unix timestamp
    // (seconds).
    int64 timestamp = 4;

    // The expiry duration in seconds relative to the `timestamp` field. If
    // the payment request omits the BOLT-11 `x` expiry field, the BOLT-11
    // default of 3600 seconds is returned.
    int64 expiry = 5;

    // The description (memo) of the payment (BOLT-11 `d` field). Empty if
    // the payment request carries a description_hash instead.
    string description = 6;

    // The hex-encoded SHA-256 hash of the payment description (BOLT-11 `h`
    // field). Empty if not present.
    string description_hash = 7;

    // The on-chain fallback address (BOLT-11 `f` field), encoded for the
    // active network. Empty if not present.
    string fallback_addr = 8;

    // The minimum CLTV expiry delta to use for the final hop (BOLT-11 `c`
    // field). If the payment request omits it, lnd's assumed default
    // (zpay32.DefaultAssumedFinalCLTVDelta) is returned.
    int64 cltv_expiry = 9;

    // Route hints that can each be individually used to assist in reaching
    // the invoice's destination (BOLT-11 `r` field).
    repeated RouteHint route_hints = 10;

    // The payment address / payment secret (BOLT-11 `s` field), used for MPP
    // and required by newer invoices. All-zero (32 bytes) if not present.
    // When using REST, this field is base64 encoded.
    bytes payment_addr = 11;

    // The amount in millisatoshis. 0 if the payment request does not
    // specify an amount.
    int64 num_msat = 12;

    // The feature bits advertised in the payment request (BOLT-11 `9`
    // field).
    map<uint32, Feature> features = 13;

    // The blinded payment paths included in the payment request, if any.
    repeated BlindedPaymentPath blinded_paths = 14;
}

Notes on the non-obvious ones, from the implementation:

• num_satoshis / num_msat: both come from payReq.MilliSat, which is nil for amountless invoices → both are 0; num_satoshis is MilliSat.ToSatoshis() so it truncates sub-sat amounts.
• payment_addr: payReq.PaymentAddr.UnwrapOr([32]byte{}) → 32 zero bytes when the s field is absent, not an empty slice.
• description_hash: hex.EncodeToString of an empty slice → "" when absent.
• cltv_expiry: payReq.MinFinalCLTVExpiry() falls back to DefaultAssumedFinalCLTVDelta; I referenced the constant rather than the number so the comment doesn't rot.

[starius]

Fixed