From 83adb94583f0adcf4ef15bd62b994ea2533565b4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 21 Feb 2019 13:09:21 +1030 Subject: [PATCH] lightningd and routing: use struct amount_msat. We use it in route_hop, and paper over it in the JSON APIs. Signed-off-by: Rusty Russell --- gossipd/routing.c | 2 +- gossipd/routing.h | 3 +- lightningd/gossip_control.c | 11 ++++--- lightningd/gossip_msg.c | 4 +-- lightningd/json.c | 2 +- lightningd/pay.c | 57 +++++++++++++++++++++++++------------ lightningd/peer_control.c | 8 ++++-- plugins/pay.c | 34 ++++++++++++---------- tests/test_pay.py | 4 ++- wallet/test/run-wallet.c | 7 +++++ 10 files changed, 87 insertions(+), 45 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index 1ec935a99..7939f5a1d 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1559,7 +1559,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, c = &route[i]->half[idx]; hops[i].channel_id = route[i]->scid; hops[i].nodeid = n->id; - hops[i].amount = total_amount; + hops[i].amount.millisatoshis = total_amount; hops[i].delay = total_delay; hops[i].direction = idx; total_amount += connection_fee(c, total_amount); diff --git a/gossipd/routing.h b/gossipd/routing.h index 038a9ea5a..05744d702 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -205,7 +206,7 @@ struct route_hop { struct short_channel_id channel_id; int direction; struct pubkey nodeid; - u64 amount; + struct amount_msat amount; u32 delay; }; diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 9fc6e7be3..657627891 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -300,7 +301,7 @@ static struct command_result *json_getroute(struct command *cmd, struct pubkey *destination; struct pubkey *source; const jsmntok_t *excludetok; - u64 *msatoshi; + struct amount_msat *msat; unsigned *cltv; double *riskfactor; struct short_channel_id_dir *excluded; @@ -315,7 +316,7 @@ static struct command_result *json_getroute(struct command *cmd, if (!param(cmd, buffer, params, p_req("id", param_pubkey, &destination), - p_req("msatoshi", param_u64, &msatoshi), + p_req("msatoshi", param_msat, &msat), p_req("riskfactor", param_double, &riskfactor), p_opt_def("cltv", param_number, &cltv, 9), p_opt_def("fromid", param_pubkey, &source, ld->id), @@ -353,7 +354,7 @@ static struct command_result *json_getroute(struct command *cmd, } u8 *req = towire_gossip_getroute_request(cmd, source, destination, - *msatoshi, + msat->millisatoshis, *riskfactor * 1000000.0, *cltv, fuzz, excluded, @@ -398,7 +399,9 @@ static void json_listchannels_reply(struct subd *gossip UNUSED, const u8 *reply, type_to_string(reply, struct short_channel_id, &entries[i].short_channel_id)); json_add_bool(response, "public", entries[i].public); - json_add_u64(response, "satoshis", entries[i].satoshis); + json_add_amount_sat(response, + (struct amount_sat){ entries[i].satoshis }, + "satoshis", "amount_msat"); json_add_num(response, "message_flags", entries[i].message_flags); json_add_num(response, "channel_flags", entries[i].channel_flags); /* Prior to spec v0891374d47ddffa64c5a2e6ad151247e3d6b7a59, these two were a single u16 field */ diff --git a/lightningd/gossip_msg.c b/lightningd/gossip_msg.c index 39155fbea..2d5ab6dec 100644 --- a/lightningd/gossip_msg.c +++ b/lightningd/gossip_msg.c @@ -65,7 +65,7 @@ void fromwire_route_hop(const u8 **pptr, size_t *max, struct route_hop *entry) fromwire_pubkey(pptr, max, &entry->nodeid); fromwire_short_channel_id(pptr, max, &entry->channel_id); entry->direction = fromwire_u8(pptr, max); - entry->amount = fromwire_u64(pptr, max); + entry->amount = fromwire_amount_msat(pptr, max); entry->delay = fromwire_u32(pptr, max); } @@ -74,7 +74,7 @@ void towire_route_hop(u8 **pptr, const struct route_hop *entry) towire_pubkey(pptr, &entry->nodeid); towire_short_channel_id(pptr, &entry->channel_id); towire_u8(pptr, entry->direction); - towire_u64(pptr, entry->amount); + towire_amount_msat(pptr, entry->amount); towire_u32(pptr, entry->delay); } diff --git a/lightningd/json.c b/lightningd/json.c index 501213556..8fae0c9ce 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -32,7 +32,7 @@ json_add_route_hop(struct json_stream *r, char const *n, json_add_short_channel_id(r, "channel", &h->channel_id); json_add_num(r, "direction", h->direction); - json_add_u64(r, "msatoshi", h->amount); + json_add_amount_msat(r, h->amount, "msatoshi", "amount_msat"); json_add_num(r, "delay", h->delay); json_object_end(r); } diff --git a/lightningd/pay.c b/lightningd/pay.c index 3f58a0cb8..c1b4b98f8 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -78,8 +78,12 @@ json_add_payment_fields(struct json_stream *response, json_add_u64(response, "id", t->id); json_add_hex(response, "payment_hash", &t->payment_hash, sizeof(t->payment_hash)); json_add_pubkey(response, "destination", &t->destination); - json_add_u64(response, "msatoshi", t->msatoshi); - json_add_u64(response, "msatoshi_sent", t->msatoshi_sent); + json_add_amount_msat(response, + ((struct amount_msat){ t->msatoshi }), + "msatoshi", "amount_msat"); + json_add_amount_msat(response, + ((struct amount_msat){ t->msatoshi_sent }), + "msatoshi_sent", "amount_sent_msat"); json_add_u64(response, "created_at", t->timestamp); switch (t->status) { @@ -606,7 +610,7 @@ send_payment(struct lightningd *ld, for (i = 0; i < n_hops - 1; i++) { hop_data[i].realm = 0; hop_data[i].channel_id = route[i+1].channel_id; - hop_data[i].amt_forward = route[i+1].amount; + hop_data[i].amt_forward = route[i+1].amount.millisatoshis; hop_data[i].outgoing_cltv = base_expiry + route[i+1].delay; } @@ -615,7 +619,7 @@ send_payment(struct lightningd *ld, hop_data[i].realm = 0; hop_data[i].outgoing_cltv = base_expiry + route[i].delay; memset(&hop_data[i].channel_id, 0, sizeof(struct short_channel_id)); - hop_data[i].amt_forward = route[i].amount; + hop_data[i].amt_forward = route[i].amount.millisatoshis; /* Now, do we already have a payment? */ payment = wallet_payment_by_hash(tmpctx, ld->wallet, rhash); @@ -667,10 +671,11 @@ send_payment(struct lightningd *ld, sizeof(struct sha256), &path_secrets); onion = serialize_onionpacket(tmpctx, packet); - log_info(ld->log, "Sending %"PRIu64" over %zu hops to deliver %"PRIu64"", - route[0].amount, n_hops, msatoshi); + log_info(ld->log, "Sending %s over %zu hops to deliver %"PRIu64"", + type_to_string(tmpctx, struct amount_msat, &route[0].amount), + n_hops, msatoshi); - failcode = send_htlc_out(channel, route[0].amount, + failcode = send_htlc_out(channel, route[0].amount.millisatoshis, base_expiry + route[0].delay, rhash, onion, NULL, &hout); if (failcode) { @@ -705,7 +710,7 @@ send_payment(struct lightningd *ld, payment->destination = ids[n_hops - 1]; payment->status = PAYMENT_PENDING; payment->msatoshi = msatoshi; - payment->msatoshi_sent = route[0].amount; + payment->msatoshi_sent = route[0].amount.millisatoshis; payment->timestamp = time_now().ts.tv_sec; payment->payment_preimage = NULL; payment->path_secrets = tal_steal(payment, path_secrets); @@ -737,7 +742,7 @@ static struct command_result *json_sendpay(struct command *cmd, size_t i; struct sha256 *rhash; struct route_hop *route; - u64 *msatoshi; + struct amount_msat *msat; const char *description; struct command_result *res; @@ -745,7 +750,7 @@ static struct command_result *json_sendpay(struct command *cmd, p_req("route", param_array, &routetok), p_req("payment_hash", param_sha256, &rhash), p_opt("description", param_escaped_string, &description), - p_opt("msatoshi", param_u64, &msatoshi), + p_opt("msatoshi", param_msat, &msat), NULL)) return command_param_failed(); @@ -792,7 +797,7 @@ static struct command_result *json_sendpay(struct command *cmd, if (!msat) msat = amount_msat; - route[i].amount = msat->millisatoshis; + route[i].amount = *msat; route[i].nodeid = *id; route[i].delay = *delay; route[i].channel_id = *channel; @@ -805,17 +810,33 @@ static struct command_result *json_sendpay(struct command *cmd, * be from the msatoshi to twice msatoshi. */ /* if not: msatoshi <= finalhop.amount <= 2 * msatoshi, fail. */ - if (msatoshi) { - if (!(*msatoshi <= route[routetok->size-1].amount && - route[routetok->size-1].amount <= 2 * *msatoshi)) { + if (msat) { + struct amount_msat two_times_final; + + two_times_final.millisatoshis + = route[routetok->size-1].amount.millisatoshis * 2; + if (amount_msat_less(*msat, route[routetok->size-1].amount)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "msatoshi %"PRIu64" out of range", - *msatoshi); - } + "msatoshi %s less than final %s", + type_to_string(tmpctx, + struct amount_msat, + msat), + type_to_string(tmpctx, + struct amount_msat, + &route[routetok->size-1].amount)); + if (amount_msat_greater(*msat, two_times_final)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "msatoshi %s more than twice final %s", + type_to_string(tmpctx, + struct amount_msat, + msat), + type_to_string(tmpctx, + struct amount_msat, + &route[routetok->size-1].amount)); } res = send_payment(cmd->ld, cmd, rhash, route, - msatoshi ? *msatoshi : route[routetok->size-1].amount, + msat ? msat->millisatoshis : route[routetok->size-1].amount.millisatoshis, description); if (res) return res; diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 8a29b7040..5bd16fbc4 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -453,7 +453,9 @@ static void json_add_htlcs(struct lightningd *ld, json_object_start(response, NULL); json_add_string(response, "direction", "in"); json_add_u64(response, "id", hin->key.id); - json_add_u64(response, "msatoshi", hin->msatoshi); + json_add_amount_msat(response, + (struct amount_msat){ hin->msatoshi }, + "msatoshi", "amount_msat"); json_add_u64(response, "expiry", hin->cltv_expiry); json_add_hex(response, "payment_hash", &hin->payment_hash, sizeof(hin->payment_hash)); @@ -471,7 +473,9 @@ static void json_add_htlcs(struct lightningd *ld, json_object_start(response, NULL); json_add_string(response, "direction", "out"); json_add_u64(response, "id", hout->key.id); - json_add_u64(response, "msatoshi", hout->msatoshi); + json_add_amount_msat(response, + (struct amount_msat){ hout->msatoshi }, + "msatoshi", "amount_msat"); json_add_u64(response, "expiry", hout->cltv_expiry); json_add_hex(response, "payment_hash", &hout->payment_hash, sizeof(hout->payment_hash)); diff --git a/plugins/pay.c b/plugins/pay.c index e746520e7..e1fdb06fd 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -404,7 +404,7 @@ static struct command_result *getroute_done(struct command *cmd, struct pay_attempt *attempt = current_attempt(pc); const jsmntok_t *t = json_get_member(buf, result, "route"); char *json_desc; - u64 fee; + struct amount_msat fee; u32 delay; double feepercent; @@ -418,32 +418,36 @@ static struct command_result *getroute_done(struct command *cmd, else attempt->route = json_strdup(pc->ps->attempts, buf, t); - if (!json_to_u64(buf, json_delve(buf, t, "[0].msatoshi"), &fee)) - plugin_err("getroute with invalid msatoshi? '%.*s'", + if (!json_to_msat(buf, json_delve(buf, t, "[0].msatoshi"), &fee)) + plugin_err("getroute with invalid msatoshi? %.*s", result->end - result->start, buf); - fee -= pc->msatoshi; + fee.millisatoshis -= pc->msatoshi; if (!json_to_number(buf, json_delve(buf, t, "[0].delay"), &delay)) - plugin_err("getroute with invalid delay? '%.*s'", + plugin_err("getroute with invalid delay? %.*s", result->end - result->start, buf); /* Casting u64 to double will lose some precision. The loss of precision * in feepercent will be like 3.0000..(some dots)..1 % - 3.0 %. * That loss will not be representable in double. So, it's Okay to * cast u64 to double for feepercent calculation. */ - feepercent = ((double)fee) * 100.0 / ((double) pc->msatoshi); + feepercent = ((double)fee.millisatoshis) * 100.0 / ((double) pc->msatoshi); - if (fee > pc->exemptfee && feepercent > pc->maxfeepercent) { + if (fee.millisatoshis > pc->exemptfee && feepercent > pc->maxfeepercent) { const jsmntok_t *charger; - attempt_failed_fmt(pc, "{ 'message': 'Route wanted fee of %"PRIu64" msatoshis' }", fee); + attempt_failed_fmt(pc, "{ 'message': 'Route wanted fee of %s' }", + type_to_string(tmpctx, struct amount_msat, + &fee)); /* Remember this if we eliminating this causes us to have no * routes at all! */ if (!pc->expensive_route) pc->expensive_route - = tal_fmt(pc, "Route wanted fee of %"PRIu64 - " msatoshis", fee); + = tal_fmt(pc, "Route wanted fee of %s", + type_to_string(tmpctx, + struct amount_msat, + &fee)); /* Try excluding most fee-charging channel (unless it's in * routeboost). */ @@ -817,7 +821,7 @@ static struct command_result *handle_pay(struct command *cmd, const char *buf, const jsmntok_t *params) { - u64 *msatoshi; + struct amount_msat *msat; struct bolt11 *b11; const char *b11str; char *fail; @@ -832,7 +836,7 @@ static struct command_result *handle_pay(struct command *cmd, if (!param(cmd, buf, params, p_req("bolt11", param_string, &b11str), - p_opt("msatoshi", param_u64, &msatoshi), + p_opt("msatoshi", param_msat, &msat), p_opt("description", param_string, &pc->desc), p_opt_def("riskfactor", param_double, &riskfactor, 10), p_opt_def("maxfeepercent", param_percent, &maxfeepercent, 0.5), @@ -854,17 +858,17 @@ static struct command_result *handle_pay(struct command *cmd, } if (b11->msat) { - if (msatoshi) { + if (msat) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi parameter unnecessary"); } pc->msatoshi = b11->msat->millisatoshis; } else { - if (!msatoshi) { + if (!msat) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi parameter required"); } - pc->msatoshi = *msatoshi; + pc->msatoshi = msat->millisatoshis; } pc->maxfeepercent = *maxfeepercent; diff --git a/tests/test_pay.py b/tests/test_pay.py index 422b9ed3a..fd0f6ac51 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -69,7 +69,7 @@ def test_pay_limits(node_factory): inv = l3.rpc.invoice("any", "any", 'description') # Fee too high. - with pytest.raises(RpcError, match=r'Route wanted fee of .* msatoshis') as err: + with pytest.raises(RpcError, match=r'Route wanted fee of .*msat') as err: l1.rpc.call('pay', {'bolt11': inv['bolt11'], 'msatoshi': 100000, 'maxfeepercent': 0.0001, 'exemptfee': 0}) assert err.value.error['code'] == PAY_ROUTE_TOO_EXPENSIVE @@ -1016,8 +1016,10 @@ def test_forward_pad_fees_and_cltv(node_factory, bitcoind): # Modify so we overpay, overdo the cltv. route[0]['msatoshi'] += 2000 + route[0]['amount_msat'] = str(route[0]['msatoshi']) + 'msat' route[0]['delay'] += 20 route[1]['msatoshi'] += 1000 + route[1]['amount_msat'] = str(route[1]['msatoshi']) + 'msat' route[1]['delay'] += 10 # This should work. diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 35586a742..27d121176 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -199,6 +199,13 @@ void json_add_address_internal(struct json_stream *response UNNEEDED, const char *fieldname UNNEEDED, const struct wireaddr_internal *addr UNNEEDED) { fprintf(stderr, "json_add_address_internal called!\n"); abort(); } +/* Generated stub for json_add_amount_msat */ +void json_add_amount_msat(struct json_stream *result UNNEEDED, + struct amount_msat msat UNNEEDED, + const char *rawfieldname UNNEEDED, + const char *msatfieldname) + +{ fprintf(stderr, "json_add_amount_msat called!\n"); abort(); } /* Generated stub for json_add_bool */ void json_add_bool(struct json_stream *result UNNEEDED, const char *fieldname UNNEEDED, bool value UNNEEDED)