Skip to content

xpay notify attempts #8354

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rustyrussell
Copy link
Contributor

Closes: #8036

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…eam.

And also slightly generalize: plugin_notification_start() can take any
tal ptr.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell added this to the v25.09 milestone Jun 13, 2025
this particular attempt to pay part of that.

`total_payment_msat` is the total amount (usually the invoice amount),
which will be the same across all parts, adn `attempt_msat` is the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
which will be the same across all parts, adn `attempt_msat` is the
which will be the same across all parts, and `attempt_msat` is the

Comment on lines 584 to 585
to the other end (`amount_reaching_next_node_msat`). The final
`amount_reaching_next_node_msat` will be equal to the `attempt_msat`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example below has "attempt_msat": 5000, which differs from the final "amount_reaching_next_node_msat": 100000. Would be great to get the example in line with the description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I am not happy with the names, changed them several times. Maybe "channel_in_msat" and "channel_out_msat"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need both values? Are there scenarios in which the amount_reaching_next_node_msat is not the amount_with_fees_msat of the next channel? Actually, I am asking myself the same question right now https://github.com/ElementsProject/lightning/pull/8354/files/d2a9afd284f537570705ab627320864e5e66bcb7#r2146077543

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is redundant. But it's also explicit, because there are N entries for N+1 nodes (though we don't generally pay ourselves fees, so you can known the 0'th input if you know the output).

Emitted by `xpay` when part of a payment ends. `payment_hash`, `groupid` and `partid`
will match a previous `xpay_attempt_start`.

`status` will be "success" or "failure". `duration` will be an integer (with 9 decimal places)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duration is not in the example below

Emitted by `xpay` when part of a payment ends. `payment_hash`, `groupid` and `partid`
will match a previous `xpay_attempt_start`.

`status` will be "success" or "failure". `duration` will be an integer (with 9 decimal places)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integer with decimals?

### `xpay_attempt_start` (v25.09 onward)

Emitted by `xpay` when part of a payment begins. `payment_hash` and
`groupid` uniquely identify this xpay invocation, and `partid` then identifes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
`groupid` uniquely identify this xpay invocation, and `partid` then identifes
`groupid` uniquely identify this xpay invocation, and `partid` then identifies

Comment on lines 604 to 608
"next_node": "022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59",
"short_channel_id": "2x3x4",
"direction": 0,
"amount_with_fees_msat": 100030,
"amount_reaching_next_node_msat": 100000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly that in this hop
022d22 is the destination?
2x3x4 is a channel between 035d2b and 022d22
amount_with_fees_msat is the amount coming into 022d22
amount_reaching_next_node_msat is the amount 'going out of' 022d22 (i.e. that's the amount 022d22 receives here?)

"failed_node_id": "035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d",
"failed_short_channel_id": "1x2x3",
"failed_direction": 1,
"error_code": 4103,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The htlc_accepted hook also takes failure messages as a parameter, and they're hex encoded. Perhaps it's good to align with that and hex encode this one as well?

Can we include the full failure message here somewhere too? To extract things like the channel update?

Requested-by: Michael at Boltz
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `xpay` now publishes `xpay_attempt_start` and `xpay_attempt_end` notifications on every payment send attempt.
The custom_notifications handler produces really ugly results, and I
was lazy, but it works!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/xpay-notify-attempts branch from d2a9afd to 3562641 Compare June 15, 2025 06:20
@rustyrussell
Copy link
Contributor Author

OK, unf. I edited in place rather than adding a fixup commit. But here's the diff in the docs:

diff --git a/doc/developers-guide/plugin-development/event-notifications.md b/doc/developers-guide/plugin-development/event-notifications.md
index 3ee755ee9..49d5742d5 100644
--- a/doc/developers-guide/plugin-development/event-notifications.md
+++ b/doc/developers-guide/plugin-development/event-notifications.md
@@ -572,7 +572,7 @@ Where:
 ### `xpay_attempt_start` (v25.09 onward) 
 
 Emitted by `xpay` when part of a payment begins.  `payment_hash` and
-`groupid` uniquely identify this xpay invocation, and `partid` then identifes
+`groupid` uniquely identify this xpay invocation, and `partid` then identifies
 this particular attempt to pay part of that.
 
 `total_payment_msat` is the total amount (usually the invoice amount),
@@ -580,9 +580,12 @@ which will be the same across all parts, adn `attempt_msat` is the
 amount being delivered to the destination by this part.
 
 Each element in `hops` shows the amount going into the node (i.e. with
-fees, `amount_with_fees_msat`) and the amount we're telling it to send
-to the other end (`amount_reaching_next_node_msat`).  The final
-`amount_reaching_next_node_msat` will be equal to the `attempt_msat`.
+fees, `channel_in_msat`) and the amount we're telling it to send
+to the other end (`channel_out_msat`).  The `channel_out_msat` will
+be equal to the next `channel_in_msat.  The final
+`channel_out_msat` will be equal to the `attempt_msat`.
+
+The example shows a payment from this node via 1x2x3 (direction 1) to 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d, then via 2x3x4 (direction 0) to 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59.
 
 ```json
 {
@@ -590,22 +593,22 @@ to the other end (`amount_reaching_next_node_msat`).  The final
     "payment_hash": "f5a6a059a25d1e329d9b094aeeec8c2191ca037d3f5b0662e21ae850debe8ea2",
     "groupid": 1,
     "partid": 1,
-    "total_payment_msat": 100000,
-    "attempt_msat": 5000,
+    "total_payment_msat": 200000,
+    "attempt_msat": 100000,
     "hops": [
       {
         "next_node": "035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d",
         "short_channel_id": "1x2x3",
         "direction": 1,
-        "amount_with_fees_msat": 100030,
-        "amount_reaching_next_node_msat": 100030
+        "channel_in_msat": 100030,
+        "channel_out_msat": 100030
       },
       {
         "next_node": "022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59",
         "short_channel_id": "2x3x4",
         "direction": 0,
-        "amount_with_fees_msat": 100030,
-        "amount_reaching_next_node_msat": 100000
+        "channel_in_msat": 100030,
+        "channel_out_msat": 100000
       }
     ]
   }
@@ -617,9 +620,7 @@ to the other end (`amount_reaching_next_node_msat`).  The final
 Emitted by `xpay` when part of a payment ends.  `payment_hash`, `groupid` and `partid`
 will match a previous `xpay_attempt_start`.
 
-`status` will be "success" or "failure".  `duration` will be an integer (with 9 decimal places)
-in seconds, between the time `xpay` tells lightningd to send the onion, to when `xpay` processes
-the response.
+`status` will be "success" or "failure".  `duration` will be a number of seconds, with 9 decimal places.  This is the time between `xpay` telling lightningd to send the onion, to when `xpay` processes the response.
 
 If `status` is "failure", there will always be an `error_message`: the other fields below
 will be missing in the unusual case where the error onion is corrupted.
@@ -627,6 +628,7 @@ will be missing in the unusual case where the error onion is corrupted.
 `failed_node_id`: If it's a non-local error, the source of the error.
 `failed_short_channel_id`: if it's not the final node, the channel it's complaining about.
 `failed_direction`: if it's not the final node, the channel direction.
+`failed_msg`: the decrypted onion message, in hex, if it was valid.
 `error_code`: the error code returned (present unless onion was corrupted).
 `error_message`: always present: if `failed_node_id` is present it's just the name of the `error_code`, but otherwise it can be a more informative error from our own node.
 
@@ -636,12 +638,14 @@ will be missing in the unusual case where the error onion is corrupted.
     "payment_hash": "f5a6a059a25d1e329d9b094aeeec8c2191ca037d3f5b0662e21ae850debe8ea2",
     "groupid": 12345677890,
     "partid": 1,
+    "duration": 1.123456789,
     "status": "failure",
     "failed_node_id": "035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d",
+    "failed_msg": "1007008a01024eb43f5212a864e19c426ec0278fb1c506eb043a1cdfde88bd1747080f711dbb472ecce9b1c44f2df7dbbc501a78451fe3ac93b6b9a2aac1bddc9dbb86e81b1b06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f0000670000010000684e64ea010000060000000000000000000000010000000a000000003b023380",
     "failed_short_channel_id": "1x2x3",
     "failed_direction": 1,
     "error_code": 4103,
-	"error_message": "temporary_channel_failure"
+    "error_message": "temporary_channel_failure"
   }
 }

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xpay: htlc_(added/removed) notifications
3 participants