plugin: Do not return multiple times from pay

While we were unsetting the `payment->cmd` in case of a success to signal that
we should not return to the JSON-RPC command twice, we were not doing that in
the case of failures. This was causing multiple responses to a single incoming
command, and `lightningd` was correctly killing the plugin. This issue was
introduced through early returns (anything setting `payment->abort=true`) and
was caused in Rusty's case through an MPP timeout.

Fixes #3847
Reported-by: Rusty Russell <@rustyrussell>
Signed-off-by: Christian Decker <@cdecker>
This commit is contained in:
Christian Decker 2020-07-18 08:37:28 +02:00
parent 734f292695
commit 2146a548bd
2 changed files with 10 additions and 12 deletions

View file

@ -1063,18 +1063,20 @@ static void payment_finished(struct payment *p)
assert((result.leafstates & PAYMENT_STEP_SUCCESS) == 0 ||
result.preimage != NULL);
if (p->parent == NULL && cmd == NULL) {
/* This is the tree root, but we already reported success or
* failure, so noop. */
return;
} else if (p->parent == NULL) {
if (payment_is_success(p)) {
if (p->parent == NULL) {
/* We are about to reply, unset the pointer to the cmd so we
* don't attempt to return a response twice. */
p->cmd = NULL;
if (cmd == NULL) {
/* This is the tree root, but we already reported
* success or failure, so noop. */
return;
} else if (payment_is_success(p)) {
assert(result.treestates & PAYMENT_STEP_SUCCESS);
assert(result.leafstates & PAYMENT_STEP_SUCCESS);
assert(result.preimage != NULL);
ret = jsonrpc_stream_success(p->cmd);
ret = jsonrpc_stream_success(cmd);
json_add_node_id(ret, "destination", p->destination);
json_add_sha256(ret, "payment_hash", p->payment_hash);
json_add_timeabs(ret, "created_at", p->start_time);
@ -1096,9 +1098,6 @@ static void payment_finished(struct payment *p)
json_add_string(ret, "status", "complete");
/* Unset the pointer to the cmd so we don't attempt to
* return a response twice. */
p->cmd = NULL;
if (command_finished(cmd, ret)) {/* Ignore result. */}
return;
} else if (result.failure == NULL || result.failure->failcode < NODE) {

View file

@ -1499,7 +1499,6 @@ def test_coin_movement_notices(node_factory, bitcoind, chainparams):
check_coin_moves_idx(l2)
@pytest.mark.xfail(strict=True)
def test_3847_repro(node_factory, bitcoind):
"""Reproduces the issue in #3847: duplicate response from plugin