lukechilds
/
lightning
mirror of https://github.com/lukechilds/lightning.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

cleanups: feedback from Christian Decker review. 

1. Hoist 7200 constant into the bolt12 heade2.
2. Make preimage the last createinvoice arg, so we could make it optional.
3. Check the validity of the preimage in createinvoice.
4. Always output used flag in listoffers.
5. Rename wallet offer iterators to offer_id iterators.
6. Fix paramter typos.
7. Rename `local_offer_id` parameter to `localofferid`.
8. Add reference constraints on local_offer_id db fields.
9. Remove cut/paste comment.
10. Clarify source of fatal() messages in wallet.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ppa
Rusty Russell 4 years ago
committed by Christian Decker
parent
0fb0b94e8d
commit
723c16072a
17 changed files with 84 additions and 63 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 10
      common/bolt12.h
  2. 5
      devtools/bolt12-cli.c
  3. 12
      doc/lightning-createinvoice.7
  4. 7
      doc/lightning-createinvoice.7.md
  5. 15
      lightningd/invoice.c
  6. 7
      lightningd/offer.c
  7. 8
      lightningd/pay.c
  8. 2
      plugins/libplugin-pay.c
  9. 4
      plugins/pay.c
  10. 2
      tests/test_pay.py
  11. 2
      tests/test_plugin.py
  12. 4
      wallet/db.c
  13. 10
      wallet/db_postgres_sqlgen.c
  14. 10
      wallet/db_sqlite3_sqlgen.c
  15. 16
      wallet/statements_gettextgen.po
  16. 17
      wallet/wallet.c
  17. 16
      wallet/wallet.h

10
common/bolt12.h
View File

@ -5,6 +5,16 @@
struct feature_set;
/* BOLT-offers #12:
* - if `relative_expiry` is present:
* - MUST reject the invoice if the current time since 1970-01-01 UTC
* is greater than `timestamp` plus `seconds_from_timestamp`.
* - otherwise:
* - MUST reject the invoice if the current time since 1970-01-01 UTC
* is greater than `timestamp` plus 7200.
*/
#define BOLT12_DEFAULT_REL_EXPIRY 7200
/**
* offer_encode - encode this complete bolt12 offer TLV into text.
*/

5
devtools/bolt12-cli.c
View File

@ -405,8 +405,9 @@ static void print_relative_expiry(u64 *timestamp, u32 *relative)
* is greater than `timestamp` plus 7200.
*/
if (!relative)
printf("relative_expiry: %u (%s) (default)\n", 7200,
fmt_time(tmpctx, *timestamp + 7200));
printf("relative_expiry: %u (%s) (default)\n",
BOLT12_DEFAULT_REL_EXPIRY,
fmt_time(tmpctx, *timestamp + BOLT12_DEFAULT_REL_EXPIRY));
else
printf("relative_expiry: %u (%s)\n", *relative,
fmt_time(tmpctx, *timestamp + *relative));

12
doc/lightning-createinvoice.7
View File

@ -3,7 +3,7 @@
lightning-createinvoice - Low-level invoice creation
.SH SYNOPSIS
\fBcreateinvoice\fR \fIinvstring\fR \fIpreimage\fR \fIlabel\fR
\fBcreateinvoice\fR \fIinvstring\fR \fIlabel\fR \fIpreimage\fR
.SH DESCRIPTION
@ -15,15 +15,15 @@ The \fIinvstring\fR parameter is of bolt11 form, but without the final
signature appended\. Minimal sanity checks are done\.
The \fIpreimage\fR is the preimage to supply upon successful payment of
the invoice\.
The \fIlabel\fR must be a unique string or number (which is treated as a
string, so "01" is different from "1"); it is never revealed to other
nodes on the lightning network, but it can be used to query the status
of this invoice\.
The \fIpreimage\fR is the preimage to supply upon successful payment of
the invoice\.
.SH RETURN VALUE
On success, an invoice object is returned, as per \fBlistinvoices\fR(7)\.
@ -57,4 +57,4 @@ Rusty Russell \fI<rusty@rustcorp.com.au\fR> is mainly responsible\.
Main web site: \fIhttps://github.com/ElementsProject/lightning\fR
\" SHA256STAMP:1b6598e430d416615867dc09604574c62a8ecb47c51a370711da75359ff80ef3
\" SHA256STAMP:e9abfb1d376a1a5103cb4da2ce7d9d79d0ad6e23683ab1d68423ea4ea4a99f2f

7
doc/lightning-createinvoice.7.md
View File

@ -4,7 +4,7 @@ lightning-createinvoice -- Low-level invoice creation
SYNOPSIS
--------
**createinvoice** *invstring* *preimage* *label*
**createinvoice** *invstring* *label* *preimage*
DESCRIPTION
-----------
@ -15,14 +15,13 @@ database.
The *invstring* parameter is of bolt11 form, but without the final
signature appended. Minimal sanity checks are done.
The *preimage* is the preimage to supply upon successful payment of
the invoice.
The *label* must be a unique string or number (which is treated as a
string, so "01" is different from "1"); it is never revealed to other
nodes on the lightning network, but it can be used to query the status
of this invoice.
The *preimage* is the preimage to supply upon successful payment of
the invoice.
RETURN VALUE
------------

15
lightningd/invoice.c
View File

@ -1515,8 +1515,8 @@ static struct command_result *json_createinvoice(struct command *cmd,
if (!param(cmd, buffer, params,
p_req("invstring", param_string, &invstring),
p_req("preimage", param_preimage, &preimage),
p_req("label", param_label, &label),
p_req("preimage", param_preimage, &preimage),
NULL))
return command_param_failed();
@ -1537,6 +1537,10 @@ static struct command_result *json_createinvoice(struct command *cmd,
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Missing expiry in invoice");
if (!sha256_eq(&payment_hash, &b11->payment_hash))
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Incorrect preimage");
if (!wallet_invoice_create(cmd->ld->wallet,
&invoice,
b11->msat,
@ -1588,7 +1592,14 @@ static struct command_result *json_createinvoice(struct command *cmd,
if (inv->relative_expiry)
expiry = *inv->relative_expiry;
else
expiry = 7200;
expiry = BOLT12_DEFAULT_REL_EXPIRY;
if (!inv->payment_hash)
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Missing payment_hash in invoice");
if (!sha256_eq(&payment_hash, inv->payment_hash))
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Incorrect preimage");
if (!inv->description)
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,

7
lightningd/offer.c
View File

@ -20,8 +20,7 @@ static void json_populate_offer(struct json_stream *response,
json_add_bool(response, "active", offer_status_active(status));
json_add_bool(response, "single_use", offer_status_single(status));
json_add_string(response, "bolt12", b12);
if (status == OFFER_USED)
json_add_bool(response, "used", true);
json_add_bool(response, "used", status == OFFER_USED);
if (label)
json_add_escaped_string(response, "label", label);
}
@ -143,9 +142,9 @@ static struct command_result *json_listoffers(struct command *cmd,
struct db_stmt *stmt;
struct sha256 id;
for (stmt = wallet_offer_first(cmd->ld->wallet, &id);
for (stmt = wallet_offer_id_first(cmd->ld->wallet, &id);
stmt;
stmt = wallet_offer_next(cmd->ld->wallet, stmt, &id)) {
stmt = wallet_offer_id_next(cmd->ld->wallet, stmt, &id)) {
b12 = wallet_offer_find(tmpctx, wallet, &id,
&label, &status);
if (offer_status_active(status) >= *active_only) {

8
lightningd/pay.c
View File

@ -1292,7 +1292,7 @@ static struct command_result *json_sendonion(struct command *cmd,
p_opt("label", param_escaped_string, &label),
p_opt("shared_secrets", param_secrets_array, &path_secrets),
p_opt_def("partid", param_u64, &partid, 0),
/* FIXME: paramter should be invstring now */
/* FIXME: parameter should be invstring now */
p_opt("bolt11", param_string, &invstring),
p_opt_def("msatoshi", param_msat, &msat, AMOUNT_MSAT(0)),
p_opt("destination", param_node_id, &destination),
@ -1447,12 +1447,12 @@ static struct command_result *json_sendpay(struct command *cmd,
p_req("payment_hash", param_sha256, &rhash),
p_opt("label", param_escaped_string, &label),
p_opt("msatoshi", param_msat, &msat),
/* FIXME: paramter should be invstring now */
/* FIXME: parameter should be invstring now */
p_opt("bolt11", param_string, &invstring),
p_opt("payment_secret", param_secret, &payment_secret),
p_opt_def("partid", param_u64, &partid, 0),
#if EXPERIMENTAL_FEATURES
p_opt("local_offer_id", param_sha256, &local_offer_id),
p_opt("localofferid", param_sha256, &local_offer_id),
#endif
NULL))
return command_param_failed();
@ -1561,7 +1561,7 @@ static struct command_result *json_listsendpays(struct command *cmd,
const char *invstring;
if (!param(cmd, buffer, params,
/* FIXME: paramter should be invstring now */
/* FIXME: parameter should be invstring now */
p_opt("bolt11", param_string, &invstring),
p_opt("payment_hash", param_sha256, &rhash),
NULL))

2
plugins/libplugin-pay.c
View File

@ -1453,7 +1453,7 @@ static struct command_result *payment_createonion_success(struct command *cmd,
json_add_node_id(req->js, "destination", p->destination);
if (p->local_offer_id)
json_add_sha256(req->js, "local_offer_id", p->local_offer_id);
json_add_sha256(req->js, "localofferid", p->local_offer_id);
send_outreq(p->plugin, req);
return command_still_pending(cmd);

4
plugins/pay.c
View File

@ -1994,7 +1994,7 @@ static struct command_result *json_paymod(struct command *cmd,
maxdelay_default),
p_opt_def("exemptfee", param_msat, &exemptfee, AMOUNT_MSAT(5000)),
#if EXPERIMENTAL_FEATURES
p_opt("local_offer_id", param_sha256, &local_offer_id),
p_opt("localofferid", param_sha256, &local_offer_id),
#endif
#if DEVELOPER
p_opt_def("use_shadow", param_bool, &use_shadow, true),
@ -2085,7 +2085,7 @@ static struct command_result *json_paymod(struct command *cmd,
if (b12->relative_expiry)
invexpiry = *b12->timestamp + *b12->relative_expiry;
else
invexpiry = *b12->timestamp + 7200;
invexpiry = *b12->timestamp + BOLT12_DEFAULT_REL_EXPIRY;
p->local_offer_id = tal_steal(p, local_offer_id);
#endif /* EXPERIMENTAL_FEATURES */
} else

2
tests/test_pay.py
View File

@ -1883,7 +1883,7 @@ def test_setchannelfee_usage(node_factory, bitcoind):
wait_for(lambda: [c['base_fee_millisatoshi'] for c in l1.rpc.listchannels(scid)['channels']] == [DEF_BASE, 1337])
wait_for(lambda: [c['fee_per_millionth'] for c in l1.rpc.listchannels(scid)['channels']] == [DEF_PPM, 137])
# also test with named and missing paramters
# also test with named and missing parameters
result = l1.rpc.setchannelfee(ppm=42, id=scid)
assert(result['base'] == DEF_BASE)
assert(result['ppm'] == 42)

2
tests/test_plugin.py
View File

@ -388,7 +388,7 @@ def test_pay_plugin(node_factory):
msg = 'pay bolt11 [msatoshi] [label] [riskfactor] [maxfeepercent] '\
'[retry_for] [maxdelay] [exemptfee]'
if EXPERIMENTAL_FEATURES:
msg += ' [local_offer_id]'
msg += ' [localofferid]'
if DEVELOPER:
msg += ' [use_shadow]'
assert only_one(l1.rpc.help('pay')['help'])['command'] == msg

4
wallet/db.c
View File

@ -665,9 +665,9 @@ static struct migration dbmigrations[] = {
", PRIMARY KEY (offer_id)"
");"), NULL},
/* A reference into our own offers table, if it was made from one */
{SQL("ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL;"), NULL},
{SQL("ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);"), NULL},
/* A reference into our own offers table, if it was made from one */
{SQL("ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL;"), NULL},
{SQL("ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);"), NULL},
};
/* Leak tracking. */

10
wallet/db_postgres_sqlgen.c
View File

@ -855,14 +855,14 @@ struct db_query db_postgres_queries[] = {
.readonly = false,
},
{
.name = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.query = "ALTER TABLE invoices ADD COLUMN local_offer_id BYTEA DEFAULT NULL;",
.name = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.query = "ALTER TABLE invoices ADD COLUMN local_offer_id BYTEA DEFAULT NULL REFERENCES offers(offer_id);",
.placeholders = 0,
.readonly = false,
},
{
.name = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.query = "ALTER TABLE payments ADD COLUMN local_offer_id BYTEA DEFAULT NULL;",
.name = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.query = "ALTER TABLE payments ADD COLUMN local_offer_id BYTEA DEFAULT NULL REFERENCES offers(offer_id);",
.placeholders = 0,
.readonly = false,
},
@ -1762,4 +1762,4 @@ struct db_query db_postgres_queries[] = {
#endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */
// SHA256STAMP:774d5116e102d98351730072b4aa0b48d27b05c364680fc406f8b12bf4c7e293
// SHA256STAMP:26bbd985dc45de765f06b8c2644ecd76c098532ff71a33c8f71608890631d726

10
wallet/db_sqlite3_sqlgen.c
View File

@ -855,14 +855,14 @@ struct db_query db_sqlite3_queries[] = {
.readonly = false,
},
{
.name = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.query = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.name = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.query = "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.placeholders = 0,
.readonly = false,
},
{
.name = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.query = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL;",
.name = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.query = "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);",
.placeholders = 0,
.readonly = false,
},
@ -1762,4 +1762,4 @@ struct db_query db_sqlite3_queries[] = {
#endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */
// SHA256STAMP:774d5116e102d98351730072b4aa0b48d27b05c364680fc406f8b12bf4c7e293
// SHA256STAMP:26bbd985dc45de765f06b8c2644ecd76c098532ff71a33c8f71608890631d726

16
wallet/statements_gettextgen.po
View File

@ -563,11 +563,11 @@ msgid "CREATE TABLE offers ( offer_id BLOB, bolt12 TEXT, label TEXT, status INT
msgstr ""
#: wallet/db.c:668
msgid "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL;"
msgid "ALTER TABLE invoices ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);"
msgstr ""
#: wallet/db.c:670
msgid "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL;"
msgid "ALTER TABLE payments ADD COLUMN local_offer_id BLOB DEFAULT NULL REFERENCES offers(offer_id);"
msgstr ""
#: wallet/db.c:897
@ -1130,23 +1130,23 @@ msgstr ""
msgid "INSERT INTO offers ( offer_id, bolt12, label, status) VALUES (?, ?, ?, ?);"
msgstr ""
#: wallet/wallet.c:4084
#: wallet/wallet.c:4083
msgid "SELECT bolt12, label, status FROM offers WHERE offer_id = ?;"
msgstr ""
#: wallet/wallet.c:4112
#: wallet/wallet.c:4111
msgid "SELECT offer_id FROM offers;"
msgstr ""
#: wallet/wallet.c:4138
#: wallet/wallet.c:4137
msgid "UPDATE offers SET status=? WHERE offer_id = ?;"
msgstr ""
#: wallet/wallet.c:4149
#: wallet/wallet.c:4148
msgid "UPDATE invoices SET state=? WHERE state=? AND local_offer_id = ?;"
msgstr ""
#: wallet/wallet.c:4177
#: wallet/wallet.c:4176
msgid "SELECT status FROM offers WHERE offer_id = ?;"
msgstr ""
@ -1161,4 +1161,4 @@ msgstr ""
#: wallet/test/run-wallet.c:1378
msgid "INSERT INTO channels (id) VALUES (1);"
msgstr ""
# SHA256STAMP:50622dd1a8b0f2fe71efa1c1d175f7ad130f3042db84e0c9547f849a328e8f5d
# SHA256STAMP:1e8fd669b4322c65584b19aa1613b74ac1daa8d80a5c02e0dc1381e2b9e08f9e

17
wallet/wallet.c
View File

@ -4080,7 +4080,6 @@ char *wallet_offer_find(const tal_t *ctx,
struct db_stmt *stmt;
char *bolt12;
/* Test if already exists. */
stmt = db_prepare_v2(w->db, SQL("SELECT bolt12, label, status"
" FROM offers"
" WHERE offer_id = ?;"));
@ -4105,19 +4104,19 @@ char *wallet_offer_find(const tal_t *ctx,
return bolt12;
}
struct db_stmt *wallet_offer_first(struct wallet *w, struct sha256 *offer_id)
struct db_stmt *wallet_offer_id_first(struct wallet *w, struct sha256 *offer_id)
{
struct db_stmt *stmt;
stmt = db_prepare_v2(w->db, SQL("SELECT offer_id FROM offers;"));
db_query_prepared(stmt);
return wallet_offer_next(w, stmt, offer_id);
return wallet_offer_id_next(w, stmt, offer_id);
}
struct db_stmt *wallet_offer_next(struct wallet *w,
struct db_stmt *stmt,
struct sha256 *offer_id)
struct db_stmt *wallet_offer_id_next(struct wallet *w,
struct db_stmt *stmt,
struct sha256 *offer_id)
{
if (!db_step(stmt))
return tal_free(stmt);
@ -4180,14 +4179,16 @@ void wallet_offer_mark_used(struct db *db, const struct sha256 *offer_id)
db_bind_sha256(stmt, 0, offer_id);
db_query_prepared(stmt);
if (!db_step(stmt))
fatal("Unknown stats offer_id %s",
fatal("%s: unknown offer_id %s",
__func__,
type_to_string(tmpctx, struct sha256, offer_id));
status = offer_status_in_db(db_column_int(stmt, 0));
tal_free(stmt);
if (!offer_status_active(status))
fatal("offer_id %s status %i",
fatal("%s: offer_id %s not active: status %i",
__func__,
type_to_string(tmpctx, struct sha256, offer_id),
status);

16
wallet/wallet.h
View File

@ -1428,23 +1428,23 @@ char *wallet_offer_find(const tal_t *ctx,
* @offer_id: the first offer id (if returns non-NULL)
*
* Returns pointer to hand as @stmt to wallet_offer_next(), or NULL.
* If you choose not to call wallet_offer_next() you must free it!
* If you choose not to call wallet_offer_id_next() you must free it!
*/
struct db_stmt *wallet_offer_first(struct wallet *w,
struct sha256 *offer_id);
struct db_stmt *wallet_offer_id_first(struct wallet *w,
struct sha256 *offer_id);
/**
* Iterate through all the offers.
* @w: the wallet
* @stmt: return from wallet_offer_first() or previous wallet_offer_next()
* @stmt: return from wallet_offer_id_first() or previous wallet_offer_id_next()
* @offer_id: the next offer id (if returns non-NULL)
*
* Returns NULL once we're out of offers. If you choose not to call
* wallet_offer_next() again you must free return.
* wallet_offer_id_next() again you must free return.
*/
struct db_stmt *wallet_offer_next(struct wallet *w,
struct db_stmt *stmt,
struct sha256 *offer_id);
struct db_stmt *wallet_offer_id_next(struct wallet *w,
struct db_stmt *stmt,
struct sha256 *offer_id);
/**
* Disable an offer in the database.

Write Preview
Loading…
Cancel
Save