Browse Source

wallet: abstract away delayed entry of wallet_payment.

For performance, we delay entering the 'wallet_payment' into the db
until we actually commit to the HTLC (when we have to touch the DB
anyway).

This opens a race where we can try to pay twice, and since it's not in
the database yet, we don't notice the duplicate.

So remove the temporary payment field from htlc_out, which was always
an uncomfortable hack, and make the wallet code abstract over the
deferred entry a little by maintaining a 'unstored_payments' list
and incorporating that in results.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ppa-0.6.1
Rusty Russell 7 years ago
committed by Christian Decker
parent
commit
38e8601cf6
  1. 2
      lightningd/htlc_end.c
  2. 6
      lightningd/htlc_end.h
  3. 28
      lightningd/pay.c
  4. 20
      lightningd/peer_htlcs.c
  5. 1
      lightningd/peer_htlcs.h
  6. 51
      wallet/test/run-wallet.c
  7. 67
      wallet/wallet.c
  8. 20
      wallet/wallet.h

2
lightningd/htlc_end.c

@ -140,7 +140,6 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
struct htlc_in *in,
struct wallet_payment *payment,
struct command *cmd)
{
struct htlc_out *hout = tal(ctx, struct htlc_out);
@ -153,7 +152,6 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
hout->msatoshi = msatoshi;
hout->cltv_expiry = cltv_expiry;
hout->payment_hash = *payment_hash;
hout->payment = tal_steal(hout, payment);
hout->cmd = cmd;
memcpy(hout->onion_routing_packet, onion_routing_packet,
sizeof(hout->onion_routing_packet));

6
lightningd/htlc_end.h

@ -78,9 +78,6 @@ struct htlc_out {
/* Otherwise, this MAY be non-null if there's a pay command waiting */
struct command *cmd;
/* Temporary payment store, so we can save everything in one go */
struct wallet_payment *payment;
};
static inline const struct htlc_key *keyof_htlc_in(const struct htlc_in *in)
@ -129,14 +126,13 @@ struct htlc_in *new_htlc_in(const tal_t *ctx,
const struct secret *shared_secret,
const u8 *onion_routing_packet);
/* You need to set the ID, then connect_htlc_out this! Steals @payment. */
/* You need to set the ID, then connect_htlc_out this! */
struct htlc_out *new_htlc_out(const tal_t *ctx,
struct peer *peer,
u64 msatoshi, u32 cltv_expiry,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
struct htlc_in *in,
struct wallet_payment *payment,
struct command *cmd);
void connect_htlc_in(struct htlc_in_map *map, struct htlc_in *hin);

28
lightningd/pay.c

@ -199,24 +199,12 @@ static bool send_payment(struct command *cmd,
sizeof(struct sha256), &path_secrets);
onion = serialize_onionpacket(cmd, packet);
/* We write this into db when HTLC is actually sent. */
payment = tal(tmpctx, struct wallet_payment);
payment->id = 0;
payment->payment_hash = *rhash;
payment->destination = ids[n_hops - 1];
payment->status = PAYMENT_PENDING;
payment->msatoshi = route[n_hops-1].amount;
payment->timestamp = time_now().ts.tv_sec;
payment->payment_preimage = NULL;
payment->path_secrets = tal_steal(payment, path_secrets);
log_info(cmd->ld->log, "Sending %u over %zu hops to deliver %u",
route[0].amount, n_hops, route[n_hops-1].amount);
/* Steals payment into hout */
failcode = send_htlc_out(peer, route[0].amount,
base_expiry + route[0].delay,
rhash, onion, NULL, payment, cmd,
rhash, onion, NULL, cmd,
&hout);
if (failcode) {
command_fail(cmd, "first peer not ready: %s",
@ -224,6 +212,20 @@ static bool send_payment(struct command *cmd,
return false;
}
/* If hout fails, payment should be freed too. */
payment = tal(hout, struct wallet_payment);
payment->id = 0;
payment->payment_hash = *rhash;
payment->destination = ids[n_hops - 1];
payment->status = PAYMENT_PENDING;
payment->msatoshi = route[n_hops-1].amount;
payment->timestamp = time_now().ts.tv_sec;
payment->payment_preimage = NULL;
payment->path_secrets = tal_steal(payment, path_secrets);
/* We write this into db when HTLC is actually sent. */
wallet_payment_setup(cmd->ld->wallet, payment);
/* If we fail, remove cmd ptr from htlc_out. */
tal_add_destructor2(cmd, remove_cmd_from_hout, hout);

20
lightningd/peer_htlcs.c

@ -372,7 +372,6 @@ enum onion_type send_htlc_out(struct peer *out, u64 amount, u32 cltv,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
struct htlc_in *in,
struct wallet_payment *payment,
struct command *cmd,
struct htlc_out **houtp)
{
@ -394,7 +393,7 @@ enum onion_type send_htlc_out(struct peer *out, u64 amount, u32 cltv,
/* Make peer's daemon own it, catch if it dies. */
hout = new_htlc_out(out->owner, out, amount, cltv,
payment_hash, onion_routing_packet, in,
payment, cmd);
cmd);
tal_add_destructor(hout, hout_subd_died);
msg = towire_channel_offer_htlc(out, amount, cltv, payment_hash,
@ -486,7 +485,7 @@ static void forward_htlc(struct htlc_in *hin,
failcode = send_htlc_out(next, amt_to_forward,
outgoing_cltv_value, &hin->payment_hash,
next_onion, hin, NULL, NULL, NULL);
next_onion, hin, NULL, NULL);
if (!failcode)
return;
@ -864,18 +863,11 @@ static bool update_out_htlc(struct peer *peer, u64 id, enum htlc_state newstate)
if (!hout->dbid) {
wallet_htlc_save_out(peer->ld->wallet, peer->channel, hout);
}
/* We only have a payment if we initiated the payment. */
if (hout->payment) {
/* Now that we are committed, and inside the
* transaction context of the update, add the payment
* to the history. */
wallet_payment_add(peer->ld->wallet, hout->payment);
/* No need to carry the payment info around anymore,
* we'll update in the database directly */
hout->payment = tal_free(hout->payment);
/* For our own HTLCs, we commit payment to db lazily */
if (hout->origin_htlc_id == 0)
wallet_payment_store(peer->ld->wallet,
&hout->payment_hash);
}
if (!htlc_out_update_state(peer, hout, newstate))

1
lightningd/peer_htlcs.h

@ -39,7 +39,6 @@ enum onion_type send_htlc_out(struct peer *out, u64 amount, u32 cltv,
const struct sha256 *payment_hash,
const u8 *onion_routing_packet,
struct htlc_in *in,
struct wallet_payment *payment,
struct command *cmd,
struct htlc_out **houtp);

51
wallet/test/run-wallet.c

@ -72,6 +72,7 @@ static struct wallet *create_test_wallet(const tal_t *ctx)
ltmp = tal_tmpctx(ctx);
log_book = new_log_book(w, 20*1024*1024, LOG_DBG);
w->log = new_log(w, log_book, "wallet_tests(%u):", (int)getpid());
list_head_init(&w->unstored_payments);
CHECK_MSG(w->db, "Failed opening the db");
db_migrate(w->db, w->log);
@ -549,41 +550,39 @@ static bool test_htlc_crud(const tal_t *ctx)
static bool test_payment_crud(const tal_t *ctx)
{
struct wallet_payment t, *t2;
struct wallet_payment *t = tal(ctx, struct wallet_payment), *t2;
struct wallet *w = create_test_wallet(ctx);
mempat(&t, sizeof(t));
memset(&t.destination, 1, sizeof(t.destination));
mempat(t, sizeof(*t));
memset(&t->destination, 1, sizeof(t->destination));
t.id = 0;
t.msatoshi = 100;
t.status = PAYMENT_PENDING;
t.payment_preimage = NULL;
memset(&t.payment_hash, 1, sizeof(t.payment_hash));
t->id = 0;
t->msatoshi = 100;
t->status = PAYMENT_PENDING;
t->payment_preimage = NULL;
memset(&t->payment_hash, 1, sizeof(t->payment_hash));
db_begin_transaction(w->db);
CHECK(wallet_payment_add(w, &t));
CHECK(t.id != 0);
t2 = wallet_payment_by_hash(ctx, w, &t.payment_hash);
wallet_payment_setup(w, tal_dup(NULL, struct wallet_payment, t));
wallet_payment_store(w, &t->payment_hash);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash);
CHECK(t2 != NULL);
CHECK(t2->id == t.id);
CHECK(t2->status == t.status);
CHECK(pubkey_cmp(&t2->destination, &t.destination) == 0);
CHECK(t2->msatoshi == t.msatoshi);
CHECK(t2->status == t->status);
CHECK(pubkey_cmp(&t2->destination, &t->destination) == 0);
CHECK(t2->msatoshi == t->msatoshi);
CHECK(!t2->payment_preimage);
t.status = PAYMENT_COMPLETE;
t.payment_preimage = tal(w, struct preimage);
memset(t.payment_preimage, 2, sizeof(*t.payment_preimage));
wallet_payment_set_status(w, &t.payment_hash, t.status,
t.payment_preimage);
t2 = wallet_payment_by_hash(ctx, w, &t.payment_hash);
t->status = PAYMENT_COMPLETE;
t->payment_preimage = tal(w, struct preimage);
memset(t->payment_preimage, 2, sizeof(*t->payment_preimage));
wallet_payment_set_status(w, &t->payment_hash, t->status,
t->payment_preimage);
t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash);
CHECK(t2 != NULL);
CHECK(t2->id == t.id);
CHECK(t2->status == t.status);
CHECK(pubkey_cmp(&t2->destination, &t.destination) == 0);
CHECK(t2->msatoshi == t.msatoshi);
CHECK(structeq(t.payment_preimage, t2->payment_preimage));
CHECK(t2->status == t->status);
CHECK(pubkey_cmp(&t2->destination, &t->destination) == 0);
CHECK(t2->msatoshi == t->msatoshi);
CHECK(structeq(t->payment_preimage, t2->payment_preimage));
db_commit_transaction(w->db);
return true;

67
wallet/wallet.c

@ -2,6 +2,7 @@
#include "wallet.h"
#include <bitcoin/script.h>
#include <ccan/structeq/structeq.h>
#include <ccan/tal/str/str.h>
#include <inttypes.h>
#include <lightningd/invoice.h>
@ -22,6 +23,7 @@ struct wallet *wallet_new(const tal_t *ctx, struct log *log)
wallet->log = log;
wallet->bip32_base = NULL;
wallet->invoices = invoices_new(wallet, wallet->db, log);
list_head_init(&wallet->unstored_payments);
return wallet;
}
@ -1056,8 +1058,6 @@ static bool wallet_stmt2htlc_out(const struct wallet_channel *channel,
* htlcs, will wire using origin_htlc_id */
out->in = NULL;
out->payment = NULL;
return ok;
}
@ -1243,10 +1243,38 @@ struct htlc_stub *wallet_htlc_stubs(const tal_t *ctx, struct wallet *wallet,
return stubs;
}
bool wallet_payment_add(struct wallet *wallet,
struct wallet_payment *payment)
static struct wallet_payment *
find_unstored_payment(struct wallet *wallet, const struct sha256 *payment_hash)
{
struct wallet_payment *i;
list_for_each(&wallet->unstored_payments, i, list) {
if (structeq(payment_hash, &i->payment_hash))
return i;
}
return NULL;
}
static void remove_unstored_payment(struct wallet_payment *payment)
{
list_del(&payment->list);
}
void wallet_payment_setup(struct wallet *wallet, struct wallet_payment *payment)
{
assert(!find_unstored_payment(wallet, &payment->payment_hash));
list_add_tail(&wallet->unstored_payments, &payment->list);
tal_add_destructor(payment, remove_unstored_payment);
}
void wallet_payment_store(struct wallet *wallet,
const struct sha256 *payment_hash)
{
sqlite3_stmt *stmt;
struct wallet_payment *payment;
payment = find_unstored_payment(wallet, payment_hash);
/* Don't attempt to add the same payment twice */
assert(!payment->id);
@ -1268,14 +1296,21 @@ bool wallet_payment_add(struct wallet *wallet,
sqlite3_bind_int(stmt, 5, payment->timestamp);
db_exec_prepared(wallet->db, stmt);
payment->id = sqlite3_last_insert_rowid(wallet->db->sql);
return true;
tal_free(payment);
}
void wallet_payment_delete(struct wallet *wallet,
const struct sha256 *payment_hash)
{
sqlite3_stmt *stmt;
struct wallet_payment *payment;
payment = find_unstored_payment(wallet, payment_hash);
if (payment) {
tal_free(payment);
return;
}
stmt = db_prepare(
wallet->db,
@ -1314,7 +1349,12 @@ wallet_payment_by_hash(const tal_t *ctx, struct wallet *wallet,
const struct sha256 *payment_hash)
{
sqlite3_stmt *stmt;
struct wallet_payment *payment = NULL;
struct wallet_payment *payment;
/* Present the illusion that it's in the db... */
payment = find_unstored_payment(wallet, payment_hash);
if (payment)
return payment;
stmt = db_prepare(wallet->db,
"SELECT id, status, destination,"
@ -1358,6 +1398,9 @@ void wallet_payment_set_status(struct wallet *wallet,
{
sqlite3_stmt *stmt;
/* We should never try this on an unstored payment! */
assert(!find_unstored_payment(wallet, payment_hash));
stmt = db_prepare(wallet->db,
"UPDATE payments SET status=? "
"WHERE payment_hash=?");
@ -1382,6 +1425,8 @@ const struct wallet_payment **wallet_payment_list(const tal_t *ctx,
{
const struct wallet_payment **payments;
sqlite3_stmt *stmt;
struct wallet_payment *p;
size_t i;
payments = tal_arr(ctx, const struct wallet_payment *, 0);
stmt = db_prepare(
@ -1391,12 +1436,18 @@ const struct wallet_payment **wallet_payment_list(const tal_t *ctx,
"path_secrets "
"FROM payments;");
for (int i = 0; sqlite3_step(stmt) == SQLITE_ROW; i++) {
for (i = 0; sqlite3_step(stmt) == SQLITE_ROW; i++) {
tal_resize(&payments, i+1);
payments[i] = wallet_stmt2payment(payments, stmt);
}
sqlite3_finalize(stmt);
/* Now attach payments not yet in db. */
list_for_each(&wallet->unstored_payments, p, list) {
tal_resize(&payments, i+1);
payments[i++] = p;
}
return payments;
}

20
wallet/wallet.h

@ -23,6 +23,7 @@ struct wallet {
struct log *log;
struct ext_key *bip32_base;
struct invoices *invoices;
struct list_head unstored_payments;
};
/* Possible states for tracked outputs in the database. Not sure yet
@ -82,6 +83,8 @@ enum wallet_payment_status {
* a UI (alongside invoices) to display the balance history.
*/
struct wallet_payment {
/* If it's in unstored_payments */
struct list_node list;
u64 id;
u32 timestamp;
struct sha256 payment_hash;
@ -530,12 +533,23 @@ struct htlc_stub *wallet_htlc_stubs(const tal_t *ctx, struct wallet *wallet,
struct wallet_channel *chan);
/**
* wallet_payment_add - Record a new incoming/outgoing payment
* wallet_payment_setup - Remember this payment for later committing.
*
* Either wallet_payment_store() gets called to put in db once hout
* is ready to go (and frees @payment), or @payment is tal_free'd.
*
* @wallet: wallet we're going to store it in.
* @payment: the payment for later committing.
*/
void wallet_payment_setup(struct wallet *wallet, struct wallet_payment *payment);
/**
* wallet_payment_store - Record a new incoming/outgoing payment
*
* Stores the payment in the database.
*/
bool wallet_payment_add(struct wallet *wallet,
struct wallet_payment *payment);
void wallet_payment_store(struct wallet *wallet,
const struct sha256 *payment_hash);
/**
* wallet_payment_delete - Remove a payment

Loading…
Cancel
Save