From 874ca99c32083daca71e8b8c840ef1d474b5bc46 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 18 Feb 2021 14:45:54 +1030 Subject: [PATCH] offers: make 'used' flag more useful. We used to only set it for single-use offers (where it's required), but it's still interesting for multi-use offers, so let's keep it there. We also put this field in the documentation. Signed-off-by: Rusty Russell --- doc/lightning-listoffers.7 | 28 ++++++++++++--- doc/lightning-listoffers.7.md | 15 ++++++-- doc/lightning-offer.7 | 23 ++----------- doc/lightning-offer.7.md | 10 +----- lightningd/channel_control.c | 7 ++++ lightningd/offer.c | 6 ++-- tests/test_pay.py | 61 ++++++++++++++++++++------------- wallet/db_postgres_sqlgen.c | 2 +- wallet/db_sqlite3_sqlgen.c | 2 +- wallet/statements_gettextgen.po | 2 +- wallet/wallet.c | 11 ++++-- wallet/wallet.h | 27 ++++++++++----- 12 files changed, 117 insertions(+), 77 deletions(-) diff --git a/doc/lightning-listoffers.7 b/doc/lightning-listoffers.7 index 86c94f95e..8f9d2da34 100644 --- a/doc/lightning-listoffers.7 +++ b/doc/lightning-listoffers.7 @@ -29,9 +29,29 @@ set and is true, only offers with \fBactive\fR true are returned\. .fi .SH RETURN VALUE -On success, an array \fIoffers\fR of objects is returned\. Each object contains -\fIoffer_id\fR, \fIactive\fR, \fIsingle_use\fR, \fIbolt12\fR and \fIused\fR\. +On success, an array \fIoffers\fR of objects is returned\. Each object contains: +.RS +.IP \[bu] +\fIoffer_id\fR: the hash of the offer\. +.IP \[bu] +\fIactive\fR: true +.IP \[bu] +\fIsingle_use\fR: true if \fIsingle_use\fR was specified\. +.IP \[bu] +\fIbolt12\fR: the bolt12 offer, starting with "lno1" +.IP \[bu] +\fIused\fR: true if an associated invoice has been paid\. + +.RE + +Optionally: + +.RS +.IP \[bu] +\fIlabel\fR: the user-specified label\. + +.RE .SH EXAMPLE JSON RESPONSE .nf .RS @@ -49,7 +69,7 @@ On success, an array \fIoffers\fR of objects is returned\. Each object contains "active": true, "single_use": false, "bolt12": "lno1qgsqvgnwgcg35z6ee2h3yczraddm72xrfua9uve2rlrm9deu7xyfzrcxqd24x3qgqgqlgzs3gdhkven9v5sxvmmjype82um50ys3ug9kxsmqdvj3c6ut2cuu2s4nrf8k2dulccgaqcdzxgp583utjlu49rcyqt8hc3s797umxn3r9367rdqc577rma7key58fywkajxnuzyapge86hj2pg80rjrma40xdqrxnsnva5l3ce7hz4ua8wf755dees4y9vnq", - "used": false + "used": true } ] } @@ -68,4 +88,4 @@ Rusty Russell \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:f654c090c24a1cae9a3947b4ba0bfd051949502a5de6d78d899a7f6cf73a8fe6 +\" SHA256STAMP:bc4abe90e5475a272c61af45e4f00be9f3fab89d3adca767b577b46d70d33726 diff --git a/doc/lightning-listoffers.7.md b/doc/lightning-listoffers.7.md index d25b548c0..c04536228 100644 --- a/doc/lightning-listoffers.7.md +++ b/doc/lightning-listoffers.7.md @@ -29,8 +29,17 @@ EXAMPLE JSON REQUEST RETURN VALUE ------------ -On success, an array *offers* of objects is returned. Each object contains -*offer_id*, *active*, *single_use*, *bolt12* and *used*. +On success, an array *offers* of objects is returned. Each object contains: + +* *offer_id*: the hash of the offer. +* *active*: true +* *single_use*: true if *single_use* was specified. +* *bolt12*: the bolt12 offer, starting with "lno1" +* *used*: true if an associated invoice has been paid. + +Optionally: +* *label*: the user-specified label. + EXAMPLE JSON RESPONSE ----- @@ -49,7 +58,7 @@ EXAMPLE JSON RESPONSE "active": true, "single_use": false, "bolt12": "lno1qgsqvgnwgcg35z6ee2h3yczraddm72xrfua9uve2rlrm9deu7xyfzrcxqd24x3qgqgqlgzs3gdhkven9v5sxvmmjype82um50ys3ug9kxsmqdvj3c6ut2cuu2s4nrf8k2dulccgaqcdzxgp583utjlu49rcyqt8hc3s797umxn3r9367rdqc577rma7key58fywkajxnuzyapge86hj2pg80rjrma40xdqrxnsnva5l3ce7hz4ua8wf755dees4y9vnq", - "used": false + "used": true } ] } diff --git a/doc/lightning-offer.7 b/doc/lightning-offer.7 index f9a87203f..a09f6d0aa 100644 --- a/doc/lightning-offer.7 +++ b/doc/lightning-offer.7 @@ -100,27 +100,8 @@ invoices will be expired (i\.e\. only one person can pay this offer)\. .SH RETURN VALUE -On success, an object as follows is returned: +On success, an object as follows is returned as per \fBlightning-listoffers\fR(7)\. -.RS -.IP \[bu] -\fIoffer_id\fR: the hash of the offer\. -.IP \[bu] -\fIactive\fR: true -.IP \[bu] -\fIsingle_use\fR: true if \fIsingle_use\fR was specified\. -.IP \[bu] -\fIbolt12\fR: the bolt12 offer, starting with "lno1" - -.RE - -Optionally: - -.RS -.IP \[bu] -\fIlabel\fR: the user-specified label\. - -.RE On failure, an error is returned and no offer is created\. If the lightning process fails before responding, the caller should use @@ -149,4 +130,4 @@ Rusty Russell \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:4ab9701b01ad474482f7aa8d31ea2c1170904402c2f222f6d5a9ba826e963457 +\" SHA256STAMP:32c3f313b94ca968683060c9b8ba81c76b610b3b0f0c604a24a2f88bb8130c2c diff --git a/doc/lightning-offer.7.md b/doc/lightning-offer.7.md index d9de6c6f8..1de0fb6ad 100644 --- a/doc/lightning-offer.7.md +++ b/doc/lightning-offer.7.md @@ -89,15 +89,7 @@ invoices will be expired (i.e. only one person can pay this offer). RETURN VALUE ------------ -On success, an object as follows is returned: - -* *offer_id*: the hash of the offer. -* *active*: true -* *single_use*: true if *single_use* was specified. -* *bolt12*: the bolt12 offer, starting with "lno1" - -Optionally: -* *label*: the user-specified label. +On success, an object as follows is returned as per lightning-listoffers(7). On failure, an error is returned and no offer is created. If the lightning process fails before responding, the caller should use diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index e871ef7d4..6775b645f 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -44,6 +44,13 @@ static void update_feerates(struct lightningd *ld, struct channel *channel) if (!feerate) return; + log_debug(ld->log, + "update_feerates: feerate = %u, min=%u, max=%u, penalty=%u", + feerate, + feerate_min(ld, NULL), + feerate_max(ld, NULL), + try_get_feerate(ld->topology, FEERATE_PENALTY)); + msg = towire_channeld_feerates(NULL, feerate, feerate_min(ld, NULL), feerate_max(ld, NULL), diff --git a/lightningd/offer.c b/lightningd/offer.c index 9b34dd5d7..4cb993707 100644 --- a/lightningd/offer.c +++ b/lightningd/offer.c @@ -21,7 +21,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); - json_add_bool(response, "used", status == OFFER_USED); + json_add_bool(response, "used", offer_status_used(status)); if (label) json_add_escaped_string(response, "label", label); } @@ -96,9 +96,9 @@ static struct command_result *json_createoffer(struct command *cmd, return command_param_failed(); if (*single_use) - status = OFFER_SINGLE_USE; + status = OFFER_SINGLE_USE_UNUSED; else - status = OFFER_MULTIPLE_USE; + status = OFFER_MULTIPLE_USE_UNUSED; merkle_tlv(offer->fields, &merkle); offer->signature = tal(offer, struct bip340sig); if (!pubkey32_from_node_id(&key, &cmd->ld->id)) diff --git a/tests/test_pay.py b/tests/test_pay.py index c7a110bdd..27ddc1301 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3915,29 +3915,32 @@ def test_fetchinvoice(node_factory, bitcoind): # Simple offer first. offer1 = l3.rpc.call('offer', {'amount': '2msat', - 'description': 'simple test'})['bolt12'] + 'description': 'simple test'}) - inv1 = l1.rpc.call('fetchinvoice', {'offer': offer1}) - inv2 = l1.rpc.call('fetchinvoice', {'offer': offer1}) + inv1 = l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12']}) + inv2 = l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12']}) assert inv1 != inv2 assert 'next_period' not in inv1 assert 'next_period' not in inv2 + assert only_one(l3.rpc.call('listoffers', [offer1['offer_id']])['offers'])['used'] is False l1.rpc.pay(inv1['invoice']) + assert only_one(l3.rpc.call('listoffers', [offer1['offer_id']])['offers'])['used'] is True l1.rpc.pay(inv2['invoice']) + assert only_one(l3.rpc.call('listoffers', [offer1['offer_id']])['offers'])['used'] is True # We can also set the amount explicitly, to tip. - inv1 = l1.rpc.call('fetchinvoice', {'offer': offer1, 'msatoshi': 3}) + inv1 = l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12'], 'msatoshi': 3}) assert l1.rpc.call('decode', [inv1['invoice']])['amount_msat'] == 3 l1.rpc.pay(inv1['invoice']) # More than ~5x expected is rejected as absurd (it's actually a divide test, # which means we need 15 here, not 11). with pytest.raises(RpcError, match="Remote node sent failure message.*Amount vastly exceeds 2msat"): - l1.rpc.call('fetchinvoice', {'offer': offer1, 'msatoshi': 15}) + l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12'], 'msatoshi': 15}) # Underpay is rejected. with pytest.raises(RpcError, match="Remote node sent failure message.*Amount must be at least 2msat"): - l1.rpc.call('fetchinvoice', {'offer': offer1, 'msatoshi': 1}) + l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12'], 'msatoshi': 1}) # Single-use invoice can be fetched multiple times, only paid once. offer2 = l3.rpc.call('offer', {'amount': '1msat', @@ -3963,9 +3966,10 @@ def test_fetchinvoice(node_factory, bitcoind): # Recurring offer. offer3 = l2.rpc.call('offer', {'amount': '1msat', 'description': 'recurring test', - 'recurrence': '1minutes'})['bolt12'] + 'recurrence': '1minutes'}) + assert only_one(l2.rpc.call('listoffers', [offer3['offer_id']])['offers'])['used'] is False - ret = l1.rpc.call('fetchinvoice', {'offer': offer3, + ret = l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 0, 'recurrence_label': 'test recurrence'}) period1 = ret['next_period'] @@ -3973,10 +3977,12 @@ def test_fetchinvoice(node_factory, bitcoind): assert period1['endtime'] == period1['starttime'] + 59 assert period1['paywindow_start'] == period1['starttime'] - 60 assert period1['paywindow_end'] == period1['endtime'] + assert only_one(l2.rpc.call('listoffers', [offer3['offer_id']])['offers'])['used'] is False l1.rpc.pay(ret['invoice'], label='test recurrence') + assert only_one(l2.rpc.call('listoffers', [offer3['offer_id']])['offers'])['used'] is True - ret = l1.rpc.call('fetchinvoice', {'offer': offer3, + ret = l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 1, 'recurrence_label': 'test recurrence'}) period2 = ret['next_period'] @@ -3988,7 +3994,7 @@ def test_fetchinvoice(node_factory, bitcoind): # Can't request 2 before paying 1. with pytest.raises(RpcError, match='previous invoice has not been paid'): - l1.rpc.call('fetchinvoice', {'offer': offer3, + l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 2, 'recurrence_label': 'test recurrence'}) @@ -3996,7 +4002,7 @@ def test_fetchinvoice(node_factory, bitcoind): # Now we can, but it's too early: with pytest.raises(RpcError, match="Too early: can't send until time {}".format(period1['starttime'])): - l1.rpc.call('fetchinvoice', {'offer': offer3, + l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 2, 'recurrence_label': 'test recurrence'}) @@ -4004,14 +4010,14 @@ def test_fetchinvoice(node_factory, bitcoind): while time.time() < period1['starttime']: time.sleep(1) - l1.rpc.call('fetchinvoice', {'offer': offer3, + l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 2, 'recurrence_label': 'test recurrence'}) # Check we can request invoice without a channel. l4 = node_factory.get_node(options={'experimental-offers': None}) l4.rpc.connect(l2.info['id'], 'localhost', l2.port) - ret = l4.rpc.call('fetchinvoice', {'offer': offer3, + ret = l4.rpc.call('fetchinvoice', {'offer': offer3['bolt12'], 'recurrence_counter': 0, 'recurrence_label': 'test nochannel'}) @@ -4037,7 +4043,7 @@ def test_fetchinvoice(node_factory, bitcoind): # Test timeout. l3.stop() with pytest.raises(RpcError, match='Timeout waiting for response'): - l1.rpc.call('fetchinvoice', {'offer': offer1, 'timeout': 10}) + l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12'], 'timeout': 10}) # Now try an offer with a more complex paywindow (only 10 seconds before) offer = l2.rpc.call('offer', {'amount': '1msat', @@ -4089,15 +4095,17 @@ def test_sendinvoice(node_factory, bitcoind): # Simple offer to send money (balances channel a little) offer = l1.rpc.call('offerout', {'amount': '100000sat', - 'description': 'simple test'})['bolt12'] - print(offer) + 'description': 'simple test'}) # Fetchinvoice will refuse, since you're supposed to send an invoice. with pytest.raises(RpcError, match='Offer wants an invoice, not invoice_request'): - l2.rpc.call('fetchinvoice', {'offer': offer}) + l2.rpc.call('fetchinvoice', {'offer': offer['bolt12']}) + + # used will be false + assert only_one(l1.rpc.call('listoffers', [offer['offer_id']])['offers'])['used'] is False # sendinvoice should work. - out = l2.rpc.call('sendinvoice', {'offer': offer, + out = l2.rpc.call('sendinvoice', {'offer': offer['bolt12'], 'label': 'test sendinvoice 1'}) print(out) assert out['label'] == 'test sendinvoice 1' @@ -4117,22 +4125,29 @@ def test_sendinvoice(node_factory, bitcoind): # *but* if it hasn't heard about payment success yet, l2 will fail # simply because payments are already pending. with pytest.raises(RpcError, match='Offer no longer available|pay attempt failed'): - l2.rpc.call('sendinvoice', {'offer': offer, + l2.rpc.call('sendinvoice', {'offer': offer['bolt12'], 'label': 'test sendinvoice 2'}) + # Technically, l1 may not have gotten payment success, so we need to wait. + wait_for(lambda: only_one(l1.rpc.call('listoffers', [offer['offer_id']])['offers'])['used'] is True) + # Now try a refund. offer = l2.rpc.call('offer', {'amount': '100msat', - 'description': 'simple test'})['bolt12'] + 'description': 'simple test'}) + assert only_one(l2.rpc.call('listoffers', [offer['offer_id']])['offers'])['used'] is False - inv = l1.rpc.call('fetchinvoice', {'offer': offer}) + inv = l1.rpc.call('fetchinvoice', {'offer': offer['bolt12']}) l1.rpc.pay(inv['invoice']) + assert only_one(l2.rpc.call('listoffers', [offer['offer_id']])['offers'])['used'] is True refund = l2.rpc.call('offerout', {'amount': '100msat', 'description': 'refund test', - 'refund_for': inv['invoice']})['bolt12'] + 'refund_for': inv['invoice']}) + assert only_one(l2.rpc.call('listoffers', [refund['offer_id']])['offers'])['used'] is False - l1.rpc.call('sendinvoice', {'offer': refund, + l1.rpc.call('sendinvoice', {'offer': refund['bolt12'], 'label': 'test sendinvoice refund'}) + wait_for(lambda: only_one(l2.rpc.call('listoffers', [refund['offer_id']])['offers'])['used'] is True) def test_self_pay(node_factory): diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c index 011aa5209..a805cdcb7 100644 --- a/wallet/db_postgres_sqlgen.c +++ b/wallet/db_postgres_sqlgen.c @@ -1792,4 +1792,4 @@ struct db_query db_postgres_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */ -// SHA256STAMP:12a32ce7169fd1200edf6a6361c65fa6bd94ff9cfee926bd0eb6a882f46aa917 +// SHA256STAMP:d5554774287ded802c96fc975565e5557e5f7e174fea04ad7ec080400a15992b diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c index 98ad9ed64..600791b57 100644 --- a/wallet/db_sqlite3_sqlgen.c +++ b/wallet/db_sqlite3_sqlgen.c @@ -1792,4 +1792,4 @@ struct db_query db_sqlite3_queries[] = { #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */ -// SHA256STAMP:12a32ce7169fd1200edf6a6361c65fa6bd94ff9cfee926bd0eb6a882f46aa917 +// SHA256STAMP:d5554774287ded802c96fc975565e5557e5f7e174fea04ad7ec080400a15992b diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po index 4c829f780..fbed496ec 100644 --- a/wallet/statements_gettextgen.po +++ b/wallet/statements_gettextgen.po @@ -1181,4 +1181,4 @@ msgstr "" #: wallet/test/run-wallet.c:1395 msgid "INSERT INTO channels (id) VALUES (1);" msgstr "" -# SHA256STAMP:ecf013adbde1360d2544fc6a84105d620a2ae3a5acc6394288ee24e6110548f7 +# SHA256STAMP:6aef0052df3b7141edbb7af83a1242a31af18b55aed8e14ca5ede7d783f86309 diff --git a/wallet/wallet.c b/wallet/wallet.c index 0c79e0c16..c6ecf5f27 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -4234,6 +4234,13 @@ void wallet_offer_mark_used(struct db *db, const struct sha256 *offer_id) type_to_string(tmpctx, struct sha256, offer_id), status); - if (status == OFFER_SINGLE_USE) - offer_status_update(db, offer_id, status, OFFER_USED); + if (!offer_status_used(status)) { + enum offer_status newstatus; + + if (offer_status_single(status)) + newstatus = OFFER_SINGLE_USE_USED; + else + newstatus = OFFER_MULTIPLE_USE_USED; + offer_status_update(db, offer_id, status, newstatus); + } } diff --git a/wallet/wallet.h b/wallet/wallet.h index acf5d828f..7ddf69ebe 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1354,9 +1354,10 @@ void wallet_penalty_base_delete(struct wallet *w, u64 chan_id, u64 commitnum); #define OFFER_STATUS_SINGLE_F 0x2 #define OFFER_STATUS_USED_F 0x4 enum offer_status { - OFFER_MULTIPLE_USE = OFFER_STATUS_ACTIVE_F, - OFFER_SINGLE_USE = OFFER_STATUS_ACTIVE_F|OFFER_STATUS_SINGLE_F, - OFFER_USED = OFFER_STATUS_SINGLE_F|OFFER_STATUS_USED_F, + OFFER_MULTIPLE_USE_UNUSED = OFFER_STATUS_ACTIVE_F, + OFFER_MULTIPLE_USE_USED = OFFER_STATUS_ACTIVE_F|OFFER_STATUS_USED_F, + OFFER_SINGLE_USE_UNUSED = OFFER_STATUS_ACTIVE_F|OFFER_STATUS_SINGLE_F, + OFFER_SINGLE_USE_USED = OFFER_STATUS_SINGLE_F|OFFER_STATUS_USED_F, OFFER_SINGLE_DISABLED = OFFER_STATUS_SINGLE_F, OFFER_MULTIPLE_DISABLED = 0, }; @@ -1364,14 +1365,17 @@ enum offer_status { static inline enum offer_status offer_status_in_db(enum offer_status s) { switch (s) { - case OFFER_MULTIPLE_USE: - BUILD_ASSERT(OFFER_MULTIPLE_USE == 1); + case OFFER_MULTIPLE_USE_UNUSED: + BUILD_ASSERT(OFFER_MULTIPLE_USE_UNUSED == 1); return s; - case OFFER_SINGLE_USE: - BUILD_ASSERT(OFFER_SINGLE_USE == 3); + case OFFER_MULTIPLE_USE_USED: + BUILD_ASSERT(OFFER_MULTIPLE_USE_USED == 5); return s; - case OFFER_USED: - BUILD_ASSERT(OFFER_USED == 6); + case OFFER_SINGLE_USE_UNUSED: + BUILD_ASSERT(OFFER_SINGLE_USE_UNUSED == 3); + return s; + case OFFER_SINGLE_USE_USED: + BUILD_ASSERT(OFFER_SINGLE_USE_USED == 6); return s; case OFFER_SINGLE_DISABLED: BUILD_ASSERT(OFFER_SINGLE_DISABLED == 2); @@ -1393,6 +1397,11 @@ static inline bool offer_status_single(enum offer_status s) return s & OFFER_STATUS_SINGLE_F; } +static inline bool offer_status_used(enum offer_status s) +{ + return s & OFFER_STATUS_USED_F; +} + /** * Store an offer in the database. * @w: the wallet