pay: delete HTLC when we delete payment.

This fixes the root cause of https://github.com/ElementsProject/lightning/issues/1212
where we deleted the payment because we wanted to retry, then retry failed
so we had an (old) HTLC without a matching payment.  We then fed that
HTLC to onchaind, which tells us it's missing, and we try to fail the
payment and deref a NULL pointer.

Fixes: #1212
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2018-03-16 10:39:37 +10:30
parent d8cecf03dd
commit 1b9791f0f5
4 changed files with 39 additions and 2 deletions

View file

@ -705,7 +705,6 @@ send_payment(const tal_t *ctx,
cb(result, cbarg);
return false;
}
wallet_payment_delete(ld->wallet, rhash);
log_add(ld->log, "... retrying");
}
@ -757,6 +756,16 @@ send_payment(const tal_t *ctx,
for (i = 0; i < n_hops; ++i)
channels[i] = route[i].channel_id;
/* If we're retrying, delete all trace of previous one. We delete
* outgoing HTLC, too, otherwise it gets reported to onchaind as
* a possibility, and we end up in handle_missing_htlc_output->
* onchain_failed_our_htlc->payment_failed with no payment.
*/
if (payment) {
wallet_payment_delete(ld->wallet, rhash);
wallet_local_htlc_out_delete(ld->wallet, channel, rhash);
}
/* If hout fails, payment should be freed too. */
payment = tal(hout, struct wallet_payment);
payment->id = 0;

View file

@ -1366,7 +1366,6 @@ class LightningDTests(BaseLightningDTests):
# Note: for this test we leave onchaind running, so we can detect
# any leaks!
@unittest.skip("Expected failure")
@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
def test_onchain_dust_out(self):
"""Onchain handling of outgoing dust htlcs (they should fail)"""

View file

@ -1468,6 +1468,24 @@ struct htlc_stub *wallet_htlc_stubs(const tal_t *ctx, struct wallet *wallet,
return stubs;
}
void wallet_local_htlc_out_delete(struct wallet *wallet,
struct channel *chan,
const struct sha256 *payment_hash)
{
sqlite3_stmt *stmt;
stmt = db_prepare(wallet->db,
"DELETE FROM channel_htlcs"
" WHERE direction = ?"
" AND origin_htlc = ?"
" AND payment_hash = ?");
sqlite3_bind_int(stmt, 1, DIRECTION_OUTGOING);
sqlite3_bind_int(stmt, 2, 0);
sqlite3_bind_sha256(stmt, 3, payment_hash);
db_exec_prepared(wallet->db, stmt);
}
static struct wallet_payment *
find_unstored_payment(struct wallet *wallet, const struct sha256 *payment_hash)
{

View file

@ -650,6 +650,17 @@ void wallet_payment_store(struct wallet *wallet,
void wallet_payment_delete(struct wallet *wallet,
const struct sha256 *payment_hash);
/**
* wallet_local_htlc_out_delete - Remove a local outgoing failed HTLC
*
* This is not a generic HTLC cleanup! This is specifically for the
* narrow (and simple!) case of removing the HTLC associated with a
* local outgoing payment.
*/
void wallet_local_htlc_out_delete(struct wallet *wallet,
struct channel *chan,
const struct sha256 *payment_hash);
/**
* wallet_payment_by_hash - Retrieve a specific payment
*