From cb9e0268a76e475f32e21a6c9f4e1dc64e8db930 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 21 Jul 2021 15:07:49 +0930 Subject: [PATCH] offer / offerout: return existing if its still active. As requested by @shesek: it's weird to fail if they ask for the exact same thing (which is quite possible, since offers don't expire by default). And add a new "created" field so they can tell if they have an old one. Signed-off-by: Rusty Russell --- doc/lightning-offer.7 | 19 +++++++++++++------ doc/lightning-offer.7.md | 17 +++++++++++------ doc/lightning-offerout.7 | 4 +++- doc/lightning-offerout.7.md | 3 ++- doc/schemas/offer.schema.json | 7 +++++-- doc/schemas/offerout.schema.json | 6 +++++- lightningd/offer.c | 25 +++++++++++++++++++------ plugins/offers_offer.c | 30 +++++++++++++++++++++++++++--- tests/test_pay.py | 10 ++++++++++ 9 files changed, 95 insertions(+), 26 deletions(-) diff --git a/doc/lightning-offer.7 b/doc/lightning-offer.7 index ee94a4194..54d4cf800 100644 --- a/doc/lightning-offer.7 +++ b/doc/lightning-offer.7 @@ -10,9 +10,10 @@ lightning-offer - Command for accepting payments .SH DESCRIPTION -The \fBoffer\fR RPC command creates an offer, which is a precursor to -creating one or more invoices\. It automatically enables the processing of -an incoming invoice_request, and issuing of invoices\. +The \fBoffer\fR RPC command creates an offer (or returns an existing +one), which is a precursor to creating one or more invoices\. It +automatically enables the processing of an incoming invoice_request, +and issuing of invoices\. Note that it creates two variants of the offer: a signed and an @@ -119,7 +120,9 @@ On success, an object is returned, containing: .IP \[bu] \fBbolt12_unsigned\fR (string): the bolt12 encoding of the offer, without a signature .IP \[bu] -\fBused\fR (boolean): True if an associated invoice has been paid (always \fIfalse\fR) +\fBused\fR (boolean): True if an associated invoice has been paid +.IP \[bu] +\fBcreated\fR (boolean): false if the offer already existed .IP \[bu] \fBlabel\fR (string, optional): the (optional) user-specified label @@ -131,13 +134,17 @@ lightning process fails before responding, the caller should use not\. +If the offer already existed, and is still active, that is returned; +if it's not active then this call fails\. + + The following error codes may occur: .RS .IP \[bu] -1: Catchall nonspecific error\. .IP \[bu] -1000: Offer with this offer_id already exists\. +1000: Offer with this offer_id already exists (but is not active)\. .RE .SH AUTHOR @@ -152,4 +159,4 @@ Rusty Russell \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:f7faf86c7cb052200c3b4d6e3d6209afa54600cc7a33e1082583a2766d02a275 +\" SHA256STAMP:bd1bae6f6b56d4efa95e17b7aff042b897acacdd783cef7df0dd729f6c9b5d8f diff --git a/doc/lightning-offer.7.md b/doc/lightning-offer.7.md index d739ad02a..d7c0acf6a 100644 --- a/doc/lightning-offer.7.md +++ b/doc/lightning-offer.7.md @@ -11,9 +11,10 @@ SYNOPSIS DESCRIPTION ----------- -The **offer** RPC command creates an offer, which is a precursor to -creating one or more invoices. It automatically enables the processing of -an incoming invoice_request, and issuing of invoices. +The **offer** RPC command creates an offer (or returns an existing +one), which is a precursor to creating one or more invoices. It +automatically enables the processing of an incoming invoice_request, +and issuing of invoices. Note that it creates two variants of the offer: a signed and an unsigned one (which is smaller). Wallets should accept both: the @@ -100,7 +101,8 @@ On success, an object is returned, containing: - **single_use** (boolean): whether this expires as soon as it's paid (reflects the *single_use* parameter) - **bolt12** (string): the bolt12 encoding of the offer - **bolt12_unsigned** (string): the bolt12 encoding of the offer, without a signature -- **used** (boolean): True if an associated invoice has been paid (always *false*) +- **used** (boolean): True if an associated invoice has been paid +- **created** (boolean): false if the offer already existed - **label** (string, optional): the (optional) user-specified label [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -109,9 +111,12 @@ lightning process fails before responding, the caller should use lightning-listoffers(7) to query whether this offer was created or not. +If the offer already existed, and is still active, that is returned; +if it's not active then this call fails. + The following error codes may occur: - -1: Catchall nonspecific error. -- 1000: Offer with this offer_id already exists. +- 1000: Offer with this offer_id already exists (but is not active). AUTHOR ------ @@ -128,4 +133,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:0f9cfd3cc68aaba20af0eee763c93b475016619d960e3f5bbc0b762a809f0fef) +[comment]: # ( SHA256STAMP:53f1460e28d45129b2c00b7116c87eb47a5b717f7912f499e864f4aa28b320fa) diff --git a/doc/lightning-offerout.7 b/doc/lightning-offerout.7 index a0317f5da..d5f5edcc8 100644 --- a/doc/lightning-offerout.7 +++ b/doc/lightning-offerout.7 @@ -73,6 +73,8 @@ On success, an object is returned, containing: .IP \[bu] \fBused\fR (boolean): True if an incoming invoice has been paid (always \fIfalse\fR) .IP \[bu] +\fBcreated\fR (boolean): false if the offer already existed +.IP \[bu] \fBlabel\fR (string, optional): the (optional) user-specified label .RE @@ -113,4 +115,4 @@ Rusty Russell \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:3de11c6de7905322d9ef748981fc1d4f9ca91f4be46d76af6e9124572853047d +\" SHA256STAMP:4998ef3e06f37823c969a524fc3deb96b58eb2babee560df27e3f006fc535186 diff --git a/doc/lightning-offerout.7.md b/doc/lightning-offerout.7.md index 5fa92266e..4eec757e4 100644 --- a/doc/lightning-offerout.7.md +++ b/doc/lightning-offerout.7.md @@ -61,6 +61,7 @@ On success, an object is returned, containing: - **bolt12** (string): the bolt12 encoding of the offer - **bolt12_unsigned** (string): the bolt12 encoding of the offer, without a signature - **used** (boolean): True if an incoming invoice has been paid (always *false*) +- **created** (boolean): false if the offer already existed - **label** (string, optional): the (optional) user-specified label [comment]: # (GENERATE-FROM-SCHEMA-END) @@ -97,4 +98,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:2b7e7b543a88a10dbfbca2508e034af79f43ed0845abdb9df1fdf7e28ee33c26) +[comment]: # ( SHA256STAMP:14fada9336956a08b6d55c4ce01fcb62726cbdef9a065f1966335f61c4e91ce5) diff --git a/doc/schemas/offer.schema.json b/doc/schemas/offer.schema.json index 83f7c4e5b..45e0e7872 100644 --- a/doc/schemas/offer.schema.json +++ b/doc/schemas/offer.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "additionalProperties": false, - "required": [ "offer_id", "active", "single_use", "bolt12", "bolt12_unsigned", "used" ], + "required": [ "offer_id", "active", "single_use", "bolt12", "bolt12_unsigned", "used", "created" ], "properties": { "offer_id": { "type": "hex", @@ -29,9 +29,12 @@ }, "used": { "type": "boolean", - "enum": [ false ], "description": "True if an associated invoice has been paid" }, + "created": { + "type": "boolean", + "description": "false if the offer already existed" + }, "label": { "type": "string", "description": "the (optional) user-specified label" diff --git a/doc/schemas/offerout.schema.json b/doc/schemas/offerout.schema.json index 03d98aeeb..531d9dad8 100644 --- a/doc/schemas/offerout.schema.json +++ b/doc/schemas/offerout.schema.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "additionalProperties": false, - "required": [ "offer_id", "active", "single_use", "bolt12", "bolt12_unsigned", "used" ], + "required": [ "offer_id", "active", "single_use", "bolt12", "bolt12_unsigned", "used", "created" ], "properties": { "offer_id": { "type": "hex", @@ -33,6 +33,10 @@ "enum": [ false ], "description": "True if an incoming invoice has been paid" }, + "created": { + "type": "boolean", + "description": "false if the offer already existed" + }, "label": { "type": "string", "description": "the (optional) user-specified label" diff --git a/lightningd/offer.c b/lightningd/offer.c index ef5dd9053..0330e4bf8 100644 --- a/lightningd/offer.c +++ b/lightningd/offer.c @@ -1,3 +1,4 @@ +#include #include #include #include @@ -91,6 +92,7 @@ static struct command_result *json_createoffer(struct command *cmd, bool *single_use; enum offer_status status; struct pubkey32 key; + bool created; if (!param(cmd, buffer, params, p_req("bolt12", param_b12_offer, &offer), @@ -110,17 +112,28 @@ static struct command_result *json_createoffer(struct command *cmd, hsm_sign_b12(cmd->ld, "offer", "signature", &merkle, NULL, &key, offer->signature); b12str = offer_encode(cmd, offer); - if (!wallet_offer_create(cmd->ld->wallet, &merkle, b12str, label, - status)) { - return command_fail(cmd, - OFFER_ALREADY_EXISTS, - "Duplicate offer"); - } + + /* If it already exists, we use that one instead (and then + * the offer plugin will complain if it's inactive or expired) */ + if (!wallet_offer_create(cmd->ld->wallet, &merkle, + b12str, label, status)) { + if (!wallet_offer_find(cmd, cmd->ld->wallet, &merkle, + cast_const2(const struct json_escape **, + &label), + &status)) { + return command_fail(cmd, LIGHTNINGD, + "Could not create, nor find offer"); + } + created = false; + } else + created = true; + offer->signature = tal_free(offer->signature); b12str_nosig = offer_encode(cmd, offer); response = json_stream_success(cmd); json_populate_offer(response, &merkle, b12str, b12str_nosig, label, status); + json_add_bool(response, "created", created); return command_success(cmd, response); } diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c index 9d270ec5a..8fa29ae71 100644 --- a/plugins/offers_offer.c +++ b/plugins/offers_offer.c @@ -255,6 +255,31 @@ struct offer_info { bool *single_use; }; +static struct command_result *check_result(struct command *cmd, + const char *buf, + const jsmntok_t *result, + void *arg UNNEEDED) +{ + bool active; + + /* If it's inactive, we can't return it, */ + if (!json_to_bool(buf, json_get_member(buf, result, "active"), + &active)) { + return command_fail(cmd, + LIGHTNINGD, + "Bad creaoffer status reply %.*s", + json_tok_full_len(result), + json_tok_full(buf, result)); + } + if (!active) + return command_fail(cmd, + OFFER_ALREADY_EXISTS, + "Offer already exists, but isn't active"); + + /* Otherwise, push through the result. */ + return forward_result(cmd, buf, result, arg); +} + static struct command_result *create_offer(struct command *cmd, struct offer_info *offinfo) { @@ -262,7 +287,7 @@ static struct command_result *create_offer(struct command *cmd, /* We simply pass this through. */ req = jsonrpc_request_start(cmd->plugin, cmd, "createoffer", - forward_result, forward_error, + check_result, forward_error, offinfo); json_add_string(req->js, "bolt12", offer_encode(tmpctx, offinfo->offer)); @@ -436,9 +461,8 @@ struct command_result *json_offerout(struct command *cmd, offer->node_id = tal_dup(offer, struct pubkey32, &id); - /* We simply pass this through. */ req = jsonrpc_request_start(cmd->plugin, cmd, "createoffer", - forward_result, forward_error, + check_result, forward_error, offer); json_add_string(req->js, "bolt12", offer_encode(tmpctx, offer)); if (label) diff --git a/tests/test_pay.py b/tests/test_pay.py index 433e35e1b..678810885 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -4122,6 +4122,7 @@ def test_fetchinvoice(node_factory, bitcoind): # Simple offer first. offer1 = l3.rpc.call('offer', {'amount': '2msat', 'description': 'simple test'}) + assert offer1['created'] is True inv1 = l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12']}) inv2 = l1.rpc.call('fetchinvoice', {'offer': offer1['bolt12_unsigned'], @@ -4274,6 +4275,15 @@ def test_fetchinvoice(node_factory, bitcoind): # But we can still pay the (already-converted) invoice. l1.rpc.pay(inv['invoice']) + # Identical creation gives it again, just with created false. + offer1 = l3.rpc.call('offer', {'amount': '2msat', + 'description': 'simple test'}) + assert offer1['created'] is False + l3.rpc.call('disableoffer', {'offer_id': offer1['offer_id']}) + with pytest.raises(RpcError, match="1000.*Offer already exists, but isn't active"): + l3.rpc.call('offer', {'amount': '2msat', + 'description': 'simple test'}) + # Test timeout. l3.stop() with pytest.raises(RpcError, match='Timeout waiting for response'):