diff --git a/lightningd/pay.c b/lightningd/pay.c index c1b277185810..a0a1ec91e160 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -2076,14 +2076,6 @@ static struct command_result *json_injectpaymentonion(struct command *cmd, if (command_check_only(cmd)) return command_check_done(cmd); - register_payment_and_waiter(cmd, - payment_hash, - *partid, *groupid, - *destination_msat, *msat, AMOUNT_MSAT(0), - label, invstring, local_invreq_id, - &shared_secret, - destination); - /* If unknown, we set this equal (so accounting logs 0 fees) */ if (amount_msat_eq(*destination_msat, AMOUNT_MSAT(0))) *destination_msat = *msat; @@ -2098,6 +2090,16 @@ static struct command_result *json_injectpaymentonion(struct command *cmd, "Could not send to first peer: %s", onion_wire_name(fromwire_peektype(failmsg))); } + + /* Now HTLC is created, we can add the payment as pending */ + register_payment_and_waiter(cmd, + payment_hash, + *partid, *groupid, + *destination_msat, *msat, AMOUNT_MSAT(0), + label, invstring, local_invreq_id, + &shared_secret, + destination); + return command_still_pending(cmd); } diff --git a/tests/data/l1-pending-sendpays-with-no-htlc.sqlite3.xz b/tests/data/l1-pending-sendpays-with-no-htlc.sqlite3.xz new file mode 100644 index 000000000000..4a32fba44792 Binary files /dev/null and b/tests/data/l1-pending-sendpays-with-no-htlc.sqlite3.xz differ diff --git a/tests/test_pay.py b/tests/test_pay.py index 1afddd56c0fe..9f97ab82820e 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -6755,6 +6755,49 @@ def test_injectpaymentonion_failures(node_factory, executor): assert 'onionreply' in err.value.error['data'] +def test_injectpaymentonion_peerfail(node_factory, executor): + l1, l2 = node_factory.line_graph(2, + opts=[{'may_reconnect': True, + 'dev-no-reconnect': None, + 'disconnect': ['=WIRE_UPDATE_ADD_HTLC', '-WIRE_COMMITMENT_SIGNED']}, + {'may_reconnect': True, + 'dev-no-reconnect': None}]) + blockheight = l1.rpc.getinfo()['blockheight'] + + inv1 = l2.rpc.invoice(1000, "test_injectpaymentonion_peerfail", "test_injectpaymentonion_peerfail") + + # First hop for injectpaymentonion is self. + hops = [{'pubkey': l1.info['id'], + 'payload': serialize_payload_tlv(1000, 18 + 6, first_scid(l1, l2), blockheight).hex()}, + {'pubkey': l2.info['id'], + 'payload': serialize_payload_final_tlv(1000, 18, 1000, blockheight, inv1['payment_secret']).hex()}] + onion = l1.rpc.createonion(hops=hops, assocdata=inv1['payment_hash']) + + l1.rpc.disconnect(l2.info['id'], force=True) + with pytest.raises(RpcError, match='WIRE_TEMPORARY_CHANNEL_FAILURE'): + l1.rpc.injectpaymentonion(onion=onion['onion'], + payment_hash=inv1['payment_hash'], + amount_msat=1000, + cltv_expiry=blockheight + 18 + 6, + partid=1, + groupid=0) + # In fact, it won't create any sendpays entry, since it fails too early. + assert l1.rpc.listsendpays() == {'payments': []} + + # This will hang, since we disconnect once committed. But provides another + # (legitimately) pending payment for our migration code to test. + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + executor.submit(l1.rpc.injectpaymentonion, + onion=onion['onion'], + payment_hash=inv1['payment_hash'], + amount_msat=1000, + cltv_expiry=blockheight + 18 + 6, + partid=2, + groupid=0) + l1.daemon.wait_for_log("dev_disconnect: =WIRE_UPDATE_ADD_HTLC") + assert [p['status'] for p in l1.rpc.listsendpays()['payments']] == ['pending'] + + def test_parallel_channels_reserve(node_factory, bitcoind): """Tests wether we are able to pay through parallel channels concurrently. To do that we need to enable strict-forwarding.""" diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 8edd8963c4b6..074e01ebf0ec 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -2467,3 +2467,12 @@ def test_old_htlcs_cleanup(node_factory, bitcoind): # Now they're not assert l1.db_query('SELECT COUNT(*) as c FROM channel_htlcs')[0]['c'] == 0 assert l1.rpc.listhtlcs() == {'htlcs': []} + + +@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db") +@unittest.skipIf(TEST_NETWORK != 'regtest', "sqlite3 snapshot is regtest") +def test_pending_payments_cleanup(node_factory, bitcoind): + bitcoind.generate_block(1) + l1 = node_factory.get_node(dbfile='l1-pending-sendpays-with-no-htlc.sqlite3.xz', options={'database-upgrade': True}) + assert [p['status'] for p in l1.rpc.listsendpays()['payments']] == ['failed', 'pending'] + assert [p['status'] for p in l1.rpc.listpays()['pays']] == ['pending'] diff --git a/wallet/db.c b/wallet/db.c index 2457349613f5..26e6af23f95d 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -84,6 +84,8 @@ static void migrate_convert_old_channel_keyidx(struct lightningd *ld, struct db *db); static void migrate_initialize_channel_htlcs_wait_indexes_and_fixup_forwards(struct lightningd *ld, struct db *db); +static void migrate_fail_pending_payments_without_htlcs(struct lightningd *ld, + struct db *db); /* Do not reorder or remove elements from this array, it is used to * migrate existing databases from a previous state, based on the @@ -1087,7 +1089,8 @@ static struct migration dbmigrations[] = { {SQL("CREATE INDEX chain_moves_utxo_idx ON chain_moves (utxo)"), NULL}, {NULL, migrate_from_account_db}, /* We accidentally allowed duplicate entries */ - {NULL, migrate_remove_chain_moves_duplicates} + {NULL, migrate_remove_chain_moves_duplicates}, + {NULL, migrate_fail_pending_payments_without_htlcs}, }; /** @@ -2117,3 +2120,25 @@ static void migrate_convert_old_channel_keyidx(struct lightningd *ld, db_bind_int(stmt, channel_state_in_db(CLOSED)); db_exec_prepared_v2(take(stmt)); } + +static void migrate_fail_pending_payments_without_htlcs(struct lightningd *ld, + struct db *db) +{ + /* If channeld died or was offline at the right moment, we + * could register a payment as pending, but then not create an + * HTLC. Clean those up. */ + struct db_stmt *stmt; + + stmt = db_prepare_v2(db, SQL("UPDATE payments AS p" + " SET status = ?" + " WHERE p.status = ?" + " AND NOT EXISTS (" + " SELECT 1" + " FROM channel_htlcs AS h" + " WHERE h.payment_hash = p.payment_hash" + " AND h.groupid = p.groupid" + " AND h.partid = p.partid);")); + db_bind_int(stmt, payment_status_in_db(PAYMENT_FAILED)); + db_bind_int(stmt, payment_status_in_db(PAYMENT_PENDING)); + db_exec_prepared_v2(take(stmt)); +}