Skip to content
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Lightningd: add support for `signet` networks using the `--network=signet` or `--signet` startup option
- JSON API: `listfunds` now returns also `funding_output` for `channels`
- plugins: plugins can now suggest `lightning-cli` default to -H for responses.
- Plugin: new notification `forward_event` offered/settled/failed/local_failed.

### Changed

Expand All @@ -23,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- plugins: a new boolean field can be added to a plugin manifest, `dynamic`. It allows a plugin to tell if it can be started or stopped "on-the-fly".
- lightningd: check bitcoind version when setup topology and confirm the version not older than v0.15.0.
- startup: space out reconnections on startup if we have more than 5 peers.
- JSON API: `listforwards` includes the 'payment_hash' field.

### Deprecated

Expand Down
83 changes: 83 additions & 0 deletions doc/PLUGINS.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,89 @@ forms:
`jcon fd <error_fd_to_jsonrpc>:`, `plugin-manager`;
4. `log` is the context of the original log entry.

#### `forward_event`

A notification for topic `forward_event` is sent every time the status
of a forward payment is set. The json format is same as the API
`listforwards`.

```json
{
"forward_event": {
Comment thread
trueptolemy marked this conversation as resolved.
"payment_hash": "f5a6a059a25d1e329d9b094aeeec8c2191ca037d3f5b0662e21ae850debe8ea2",
"in_channel": "103x2x1",
"out_channel": "103x1x1",
"in_msatoshi": 100001001,
"in_msat": "100001001msat",
"out_msatoshi": 100000000,
"out_msat": "100000000msat",
"fee": 1001,
"fee_msat": "1001msat",
"status": "settled",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The rest of your document refers to FORWARD_OFFERED, FORWARD_SETTLED, etc. Which one is actually used on the actual plugin interface? Please use what is actually visible to plugins, as this is PLUGINS.md, and avoid reference to internal defines that plugins need not know about.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I'll change FORWARD_SETTLED to "settled", etc.

"received_time": 1560696342.368,
"resolved_time": 1560696342.556
}
}
```
or

```json
{
"forward_event": {
"payment_hash": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"in_channel": "103x2x1",
"out_channel": "110x1x0",
"in_msatoshi": 100001001,
"in_msat": "100001001msat",
"out_msatoshi": 100000000,
"out_msat": "100000000msat",
"fee": 1001,
"fee_msat": "1001msat",
"status": "local_failed",
"failcode": 16392,
"failreason": "WIRE_PERMANENT_CHANNEL_FAILURE",
"received_time": 1560696343.052
}
}

```
- The status includes `offered`, `settled`, `failed` and `local_failed`,
and they are all string type in json.
- When the forward payment is valid for us, we'll set `offered`
and send the forward payment to next hop to resolve;
- When the payment forwarded by us gets paid eventually, the forward
payment will change the status from `offered` to `settled`;
- If payment fails locally(like failing to resolve locally) or the
corresponding htlc with next hop fails(like htlc timeout), we will
set the status as `local_failed`. `local_failed` may be set before
setting `offered` or after setting `offered`. In fact, from the
time we receive the htlc of the previous hop, all we can know the
cause of the failure is treated as `local_failed`. `local_failed`
only occuors locally or happens in the htlc between us and next hop;
- If `local_failed` is set before `offered`, this
means we just received htlc from the previous hop and haven't
generate htlc for next hop. In this case, the json of `forward_event`
sets the fields of `out_msatoshi`, `out_msat`,`fee` and `out_channel`
as 0;
- Note: In fact, for this case we may be not sure if this incoming
htlc represents a pay to us or a payment we need to forward.
We just simply treat all incoming failed to resolve as
`local_failed`.
- Only in `local_failed` case, json includes `failcode` and
`failreason` fields;
- `failed` means the payment forwarded by us fails in the
latter hops, and the failure isn't related to us, so we aren't
accessed to the fail reason. `failed` must be set after
`offered`.
- `failed` case doesn't include `failcode` and `failreason`
fields;
- `received_time` means when we received the htlc of this payment from
the previous peer. It will be contained into all status case;
- `resolved_time` means when the htlc of this payment between us and the
next peer was resolved. The resolved result may success or fail, so
only `settled` and `failed` case contain `resolved_time`;
- The `failcode` and `failreason` are defined in [BOLT 4][bolt4-failure-codes].


## Hooks

Expand Down
2 changes: 1 addition & 1 deletion lightningd/invoice.c
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd,
details = wallet_invoice_details(info, wallet, invoice);

response = json_stream_success(info->cmd);
json_add_hex(response, "payment_hash", details->rhash.u.u8,
json_add_hex(response, "payment_hash", &details->rhash,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this is common enough, maybe a botique json_add_sha256 could be added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! What about adding it in anothor PR?
It seems need change many places.

sizeof(details->rhash));
json_add_u64(response, "expires_at", details->expiry_time);
json_add_string(response, "bolt11", details->bolt11);
Expand Down
41 changes: 40 additions & 1 deletion lightningd/notification.c
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
#include <ccan/array_size/array_size.h>
#include <lightningd/channel.h>
#include <lightningd/json.h>
#include <lightningd/notification.h>
#include <lightningd/peer_htlcs.h>

const char *notification_topics[] = {
"connect",
"disconnect",
"warning",
"invoice_payment",
"channel_opened"
"channel_opened",
"forward_event"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: For future reference, I prefer , on the trailing element, to avoid the extra line change like this. It's legal since C99 and it's been allowed by compilers for even longer, too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess you prefer autodata to register notification. :)

};

bool notifications_have_topic(const char *topic)
Expand Down Expand Up @@ -93,3 +96,39 @@ void notify_channel_opened(struct lightningd *ld, struct node_id *node_id,
jsonrpc_notification_end(n);
plugins_notify(ld->plugins, take(n));
}

void notify_forward_event(struct lightningd *ld,
const struct htlc_in *in,
const struct htlc_out *out,
enum forward_status state,
enum onion_type failcode,
struct timeabs *resolved_time)
{
struct jsonrpc_notification *n =
jsonrpc_notification_start(NULL, "forward_event");
/* Here is more neat to initial a forwarding structure than
* to pass in a bunch of parameters directly*/
struct forwarding *cur = tal(tmpctx, struct forwarding);
cur->channel_in = *in->key.channel->scid;
cur->msat_in = in->msat;
if (out) {
cur->channel_out = *out->key.channel->scid;
cur->msat_out = out->msat;
assert(amount_msat_sub(&cur->fee, in->msat, out->msat));
} else {
cur->channel_out.u64 = 0;
cur->msat_out = AMOUNT_MSAT(0);
cur->fee = AMOUNT_MSAT(0);
}
cur->payment_hash = tal(cur, struct sha256_double);
cur->payment_hash->sha = in->payment_hash;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ZmnSCPxj My thoughts about struct sha256 and struct sha256_double are mainly reflected here.
The same payment hash data, the different structures we used.
Before storing forward payment information into DB, we used struct sha256. And after storing, we used struct sha256_double, which is involved in struct forwarding:

lightning/wallet/wallet.h

Lines 189 to 198 in 2945b25

struct forwarding {
struct short_channel_id channel_in, channel_out;
struct amount_msat msat_in, msat_out, fee;
struct sha256_double *payment_hash;
enum forward_status status;
enum onion_type failcode;
struct timeabs received_time;
/* May not be present if the HTLC was not resolved yet. */
struct timeabs *resolved_time;
};

For another example, we used struct sha256 for payment_hash in struct wallet_payment:

lightning/wallet/wallet.h

Lines 241 to 262 in 2945b25

struct wallet_payment {
/* If it's in unstored_payments */
struct list_node list;
u64 id;
u32 timestamp;
struct sha256 payment_hash;
enum wallet_payment_status status;
struct node_id destination;
struct amount_msat msatoshi;
struct amount_msat msatoshi_sent;
/* If and only if PAYMENT_COMPLETE */
struct preimage *payment_preimage;
/* Needed for recovering from routing failures. */
struct secret *path_secrets;
struct node_id *route_nodes;
struct short_channel_id *route_channels;
/* bolt11 string; NULL for old payments. */
const char *bolt11;
/* The label of the payment. Must support `tal_len` */
const char *label;
};

Like BOLT#11 said,

set payment_hash to the SHA2 256-bit hash of the payment_preimage that will be given in return for payment.

Should we change the structure of payment_hash in struct forwarding to struct sha256?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It does look quite wrong... yes, we should probably fix this in a new PR.

cur->status = state;
cur->failcode = failcode;
cur->received_time = in->received_time;
cur->resolved_time = tal_steal(cur, resolved_time);

json_format_forwarding_object(n->stream, "forward_event", cur);

jsonrpc_notification_end(n);
plugins_notify(ld->plugins, take(n));
}
8 changes: 8 additions & 0 deletions lightningd/notification.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <lightningd/lightningd.h>
#include <lightningd/log.h>
#include <lightningd/plugin.h>
#include <wallet/wallet.h>

bool notifications_have_topic(const char *topic);

Expand All @@ -26,4 +27,11 @@ void notify_channel_opened(struct lightningd *ld, struct node_id *node_id,
struct amount_sat *funding_sat, struct bitcoin_txid *funding_txid,
bool *funding_locked);

void notify_forward_event(struct lightningd *ld,
const struct htlc_in *in,
const struct htlc_out *out,
enum forward_status state,
enum onion_type failcode,
struct timeabs *resolved_time);

#endif /* LIGHTNING_LIGHTNINGD_NOTIFICATION_H */
81 changes: 47 additions & 34 deletions lightningd/peer_htlcs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2104,49 +2104,62 @@ static const struct json_command dev_ignore_htlcs = {
AUTODATA(json_command, &dev_ignore_htlcs);
#endif /* DEVELOPER */

static void listforwardings_add_forwardings(struct json_stream *response, struct wallet *wallet)
/* Warp this process to ensure the consistent json object structure
* between 'listforwards' API and 'forward_event' notification. */
void json_format_forwarding_object(struct json_stream *response,
const char *fieldname,
const struct forwarding *cur)
{
const struct forwarding *forwardings;
forwardings = wallet_forwarded_payments_get(wallet, tmpctx);

json_array_start(response, "forwards");
for (size_t i=0; i<tal_count(forwardings); i++) {
const struct forwarding *cur = &forwardings[i];
json_object_start(response, NULL);

json_add_short_channel_id(response, "in_channel", &cur->channel_in);
json_add_short_channel_id(response, "out_channel", &cur->channel_out);
json_add_amount_msat_compat(response,
cur->msat_in,
"in_msatoshi", "in_msat");
json_add_amount_msat_compat(response,
cur->msat_out,
"out_msatoshi", "out_msat");
json_add_amount_msat_compat(response,
cur->fee,
"fee", "fee_msat");
json_add_string(response, "status", forward_status_name(cur->status));

if(cur->failcode != 0) {
json_add_num(response, "failcode", cur->failcode);
json_add_string(response, "failreason",
onion_type_name(cur->failcode));
}
json_object_start(response, fieldname);

json_add_hex(response, "payment_hash",
Comment thread
trueptolemy marked this conversation as resolved.
cur->payment_hash,
sizeof(*cur->payment_hash));
json_add_short_channel_id(response, "in_channel", &cur->channel_in);
json_add_short_channel_id(response, "out_channel", &cur->channel_out);
json_add_amount_msat_compat(response,
cur->msat_in,
"in_msatoshi", "in_msat");
json_add_amount_msat_compat(response,
cur->msat_out,
"out_msatoshi", "out_msat");
json_add_amount_msat_compat(response,
cur->fee,
"fee", "fee_msat");
json_add_string(response, "status", forward_status_name(cur->status));

if(cur->failcode != 0) {
json_add_num(response, "failcode", cur->failcode);
json_add_string(response, "failreason",
onion_type_name(cur->failcode));
}

#ifdef COMPAT_V070
/* If a forwarding doesn't have received_time it was created
* before we added the tracking, do not include it here. */
if (cur->received_time.ts.tv_sec) {
json_add_timeabs(response, "received_time", cur->received_time);
if (cur->resolved_time)
json_add_timeabs(response, "resolved_time", *cur->resolved_time);
}
#else
if (cur->received_time.ts.tv_sec) {
json_add_timeabs(response, "received_time", cur->received_time);
if (cur->resolved_time)
json_add_timeabs(response, "resolved_time", *cur->resolved_time);
}
#else
json_add_timeabs(response, "received_time", cur->received_time);
if (cur->resolved_time)
json_add_timeabs(response, "resolved_time", *cur->resolved_time);
#endif
json_object_end(response);
json_object_end(response);
}


static void listforwardings_add_forwardings(struct json_stream *response, struct wallet *wallet)
{
const struct forwarding *forwardings;
forwardings = wallet_forwarded_payments_get(wallet, tmpctx);

json_array_start(response, "forwards");
for (size_t i=0; i<tal_count(forwardings); i++) {
const struct forwarding *cur = &forwardings[i];
json_format_forwarding_object(response, NULL, cur);
}
json_array_end(response);

Expand Down
6 changes: 6 additions & 0 deletions lightningd/peer_htlcs.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ struct htlc_out;
struct htlc_out_map;
struct htlc_stub;
struct lightningd;
struct forwarding;
struct json_stream;

/* FIXME: Define serialization primitive for this? */
struct channel_info {
Expand Down Expand Up @@ -69,4 +71,8 @@ void htlcs_reconnect(struct lightningd *ld,
void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage);
void fail_htlc(struct htlc_in *hin, enum onion_type failcode);

/* This json process will be both used in 'notify_forward_event()'
* and 'listforwardings_add_forwardings()'*/
void json_format_forwarding_object(struct json_stream *response, const char *fieldname,
const struct forwarding *cur);
#endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */
54 changes: 54 additions & 0 deletions tests/plugins/forward_payment_status.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#!/usr/bin/env python3
"""This plugin is used to check that forward_event calls are working correctly.
"""
from lightning import Plugin

plugin = Plugin()


def check(forward, dbforward):
# After finding the corresponding notification record, this function will
# make some changes on mutative fields of this record to make this record
# same as the ideal format with given status.
record = forward
if record['status'] == 'offered':
if dbforward['status'] == 'local_failed':
record['failcode'] = dbforward['failcode']
record['failreason'] = dbforward['failreason']
elif dbforward['status'] != 'offered':
record['resolved_time'] = dbforward['resolved_time']
record['status'] = dbforward['status']
if record == dbforward:
return True
else:
return False


@plugin.init()
def init(configuration, options, plugin):
plugin.forward_list = []


@plugin.subscribe("forward_event")
def notify_warning(plugin, forward_event):
# One forward payment may have many notification records for different status,
# but one forward payment has only one record in 'listforwards' eventrually.
plugin.log("receive a forward recored, status: {}, payment_hash: {}".format(forward_event['status'], forward_event['payment_hash']))
plugin.forward_list.append(forward_event)


@plugin.method('recordcheck')
def record_lookup(payment_hash, status, dbforward, plugin):
# Check if we received all notifications when forward changed.
# This check is based on the records of 'listforwards'
plugin.log("recordcheck: payment_hash: {}, status: {}".format(payment_hash, status))
for forward in plugin.forward_list:
if forward['payment_hash'] == payment_hash and forward['status'] == status:
plugin.log("record exists")
check_result = check(forward, dbforward)
return check_result
plugin.log("no record")
return False


plugin.run()
Loading