Add log when sending BOLT 12 invoice #684
Conversation
There was a problem hiding this comment.
There's something else that we should fix in this PR: at the beginning of processEvent in Peer.kt, in the handler for MessageReceived, we log the whole incoming message (msg.let { if (it !is Ping && it !is Pong) logger.info { "received $it" } }). This is an issue for onion messages which can be up to 65kB and fill the logs very quickly.
Can you update this to avoid logging the onion itself? It's not useful as we can't easily decrypt it for support anyway. I don't know if simply overriding the toString() on OnionMessage is enough or if we need a specific case in Peer.kt, I'll let you test that.
| @@ -183,7 +183,10 @@ class OfferManager(val nodeParams: NodeParams, val walletParams: WalletParams, v | |||
| logger.warning { "offerId:${offer.offerId} pathId:${decrypted.pathId} ignoring invoice request, could not build onion message: ${invoiceMessage.value}" } | |||
| sendInvoiceError("failed to build onion message", decrypted.content.replyPath) | |||
| } | |||
| is Right -> OnionMessageAction.SendMessage(invoiceMessage.value) | |||
| is Right -> { | |||
| logger.info { "sending BOLT 12 invoice for ${invoice.amount} to introduction node ${destination.route.introductionNodeId}" } | |||
There was a problem hiding this comment.
Can we also log the whole invoice? This way we have everything we need to troubleshoot potential issues.
It shouldn't be too spammy.
| logger.info { "sending BOLT 12 invoice for ${invoice.amount} to introduction node ${destination.route.introductionNodeId}" } | |
| logger.info { "sending BOLT 12 invoice for ${invoice.amount} to introduction node ${destination.route.introductionNodeId} (invoice=${invoice.toString()})" } |
There was a problem hiding this comment.
I thought about it but the invoice can be very big as we copy all fields from the invoice request. An attacker could make the invoice as large as the largest onion message. Do you still think we should log it?
I would be more comfortable adding specific fields.
There was a problem hiding this comment.
I hesitated exactly because of that...it's true that the invoice_request can be arbitrarily big because of the padding TLV. Let me check what other fields may be useful for debugging.
There was a problem hiding this comment.
You're right, there are a lot of things that aren't very useful for debugging, let's just also include paymentHash and payerId and we should have the most useful fields!
There was a problem hiding this comment.
Maybe we can just truncate the invoice at a fixed length? That should be enough for debugging normal users
There was a problem hiding this comment.
That wouldn't be useful, if we truncate it we can't decode it...
Log relevant fields when we send an invoice as a reply to an invoice request.