Drop user-provided payer_id from InvoiceRequestBuilder

[TheBlueMatt]

We shouldn't have supported this to begin with, and we should drop it before people start using it.

[jkczyz]

Wouldn't this prevent anyone from using the offers module in a standalone way? They'd be required to use ExpandedKey and PaymentId, which are more ChannelManager-specific.

[TheBlueMatt]

You could use ExpandedKey and PaymentId without a ChannelManager? PaymentId still retains its meaning here - we use it to derive the per-payment metadata, which users still need to make constant per payment. Similar for ExpandedKey, even if a bit less so - its just some key material, which users still need to deal with but there's just some extra key material there that's not used.

[tnull]

I understand that allowing to override payer_id is a potentially dangerous API, but wonder if we should/need to keep payer_id exposed for inbound payments, e.g., if other implementations choose to use it differently than us?

[TheBlueMatt]

Honestly we should probably kill off payer_id entirely, but mostly I think we want to very, very strongly discourage anyone from using payer_id for the purpose of identifying payers. Its really dangerous to use that way and causes social costs if people do. IMO we should remove it if only so that people who want it come ask us and we can tell them to stop breaking lightning.

[TheBlueMatt]

Slipping. We should still do this so users don't rely on this API, but there's no super strong reason to do it now.

[jkczyz]

Was just looking at LNDK's usage. It no longer uses a fixed key to sign in order to fix the caching issue -- the spec was changed to cache based on payer_metadata instead of payer_id, so that should no longer be an issue anyway. However, LNDK does use derived keys from LND for signing. I'm not sure if this is a hard requirement though (cc: @orbitalturtle). Just want to raise that before going forward with this change.

See lndk-org/lndk#133.

[orbitalturtle]

@jkczyz Yeah I'm pretty sure it's not a hard requirement for LNDK so moving in that direction should be fine for us! So if I'm understanding right, in the future we'd want to use request_invoice_deriving_payer_id and let LDK derive the payer_id instead of using request_invoice_deriving_metadata?

[jkczyz]

Yes, and doing so means you would call build_and_sign instead of build and then sign as attempting to use the latter methods will fail to compile.