dual-funding: introduce racy state

It's unlikely but possible that a race condition will result in us not
being at the 'secured' state yet here.

Crashlogs. All required msgs are received (in order)
from peers, but the crash suggests they weren't relayed/processed by the
spender plugin in the order received.

WIRE_TX_SIGNATURES is passed the the plugin via a notification;
WIRE_COMMITMENT_SIGNED is returned as the result of an RPC call.

```
021-03-25T12:12:33.5213247Z lightningd-1: 2021-03-25T11:50:13.351Z DEBUG   035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-dualopend-chan#3: peer_in WIRE_COMMITMENT_SIGNED
2021-03-25T12:12:33.5221140Z lightningd-1: 2021-03-25T11:50:13.659Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-dualopend-chan#1: peer_in WIRE_COMMITMENT_SIGNED
2021-03-25T12:12:33.5228462Z lightningd-1: 2021-03-25T11:50:14.169Z DEBUG   035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-dualopend-chan#3: peer_in WIRE_TX_SIGNATURES
2021-03-25T12:12:33.5230957Z lightningd-1: 2021-03-25T11:50:14.375Z DEBUG   plugin-spenderp: mfc 275, dest 1: openchannel_update 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d returned.
2021-03-25T12:12:33.5233307Z lightningd-1: 2021-03-25T11:50:14.539Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-dualopend-chan#1: peer_in WIRE_TX_SIGNATURES
2021-03-25T12:12:33.5235120Z lightningd-1: 2021-03-25T11:50:17.240Z INFO    plugin-spenderp: Killing plugin: exited during normal operation
2021-03-25T12:12:33.5236707Z lightningd-1: 2021-03-25T11:50:17.260Z **BROKEN** plugin-spenderp: Plugin marked as important, shutting down lightningd!
```

Fixes #4455
This commit is contained in:
niftynei 2021-03-26 17:50:56 -05:00 committed by Rusty Russell
parent f891c7096b
commit 6db6ba6c03
3 changed files with 22 additions and 4 deletions

View file

@ -122,6 +122,7 @@ has_commitments_secured(const struct multifundchannel_destination *dest)
return false; return false;
case MULTIFUNDCHANNEL_COMPLETED: case MULTIFUNDCHANNEL_COMPLETED:
case MULTIFUNDCHANNEL_SECURED: case MULTIFUNDCHANNEL_SECURED:
case MULTIFUNDCHANNEL_SIGNED_NOT_SECURED:
case MULTIFUNDCHANNEL_SIGNED: case MULTIFUNDCHANNEL_SIGNED:
case MULTIFUNDCHANNEL_DONE: case MULTIFUNDCHANNEL_DONE:
return true; return true;
@ -311,6 +312,7 @@ mfc_cleanup_(struct multifundchannel_command *mfc,
continue; continue;
case MULTIFUNDCHANNEL_SECURED: case MULTIFUNDCHANNEL_SECURED:
case MULTIFUNDCHANNEL_SIGNED: case MULTIFUNDCHANNEL_SIGNED:
case MULTIFUNDCHANNEL_SIGNED_NOT_SECURED:
/* We don't actually *send* the /* We don't actually *send* the
* transaction until here, * transaction until here,
* but peer isnt going to forget. This * but peer isnt going to forget. This

View file

@ -38,6 +38,8 @@ enum multifundchannel_state {
MULTIFUNDCHANNEL_SECURED, MULTIFUNDCHANNEL_SECURED,
/* We've recieved the peer sigs for this destination */ /* We've recieved the peer sigs for this destination */
MULTIFUNDCHANNEL_SIGNED, MULTIFUNDCHANNEL_SIGNED,
/* We've gotten their sigs, but still waiting for their commit sigs */
MULTIFUNDCHANNEL_SIGNED_NOT_SECURED,
/* The transaction might now be broadcasted. */ /* The transaction might now be broadcasted. */
MULTIFUNDCHANNEL_DONE, MULTIFUNDCHANNEL_DONE,

View file

@ -564,8 +564,6 @@ static void json_peer_sigs(struct command *cmd,
dest->mfc->id, dest->mfc->id,
tal_hexstr(tmpctx, &cid, sizeof(cid))); tal_hexstr(tmpctx, &cid, sizeof(cid)));
assert(dest->state == MULTIFUNDCHANNEL_SECURED);
/* Combine with the parent. Unknown map dupes are ignored, /* Combine with the parent. Unknown map dupes are ignored,
* so the updated serial_id should persist on the parent */ * so the updated serial_id should persist on the parent */
tal_wally_start(); tal_wally_start();
@ -579,7 +577,19 @@ static void json_peer_sigs(struct command *cmd,
dest->mfc->psbt)); dest->mfc->psbt));
tal_wally_end(dest->mfc->psbt); tal_wally_end(dest->mfc->psbt);
/* Bit of a race is possible here. If we're still waiting for
* their commitment sigs to come back, we'll be in
* "UPDATED" still. We check that SIGNED is hit before
* we mark ourselves as ready to send the sigs, so it's ok
* to relax this check */
if (dest->state == MULTIFUNDCHANNEL_UPDATED)
dest->state = MULTIFUNDCHANNEL_SIGNED_NOT_SECURED;
else {
assert(dest->state == MULTIFUNDCHANNEL_SECURED);
dest->state = MULTIFUNDCHANNEL_SIGNED; dest->state = MULTIFUNDCHANNEL_SIGNED;
}
check_sigs_ready(dest->mfc); check_sigs_ready(dest->mfc);
} }
@ -727,8 +737,12 @@ openchannel_update_ok(struct command *cmd,
/* It's possible they beat us to the SIGNED flag, /* It's possible they beat us to the SIGNED flag,
* in which case we just let that be the more senior * in which case we just let that be the more senior
* state position */ * state position */
if (dest->state != MULTIFUNDCHANNEL_SIGNED) if (dest->state == MULTIFUNDCHANNEL_SIGNED_NOT_SECURED)
dest->state = MULTIFUNDCHANNEL_SIGNED;
else {
assert(dest->state == MULTIFUNDCHANNEL_UPDATED);
dest->state = MULTIFUNDCHANNEL_SECURED; dest->state = MULTIFUNDCHANNEL_SECURED;
}
} else } else
dest->state = MULTIFUNDCHANNEL_UPDATED; dest->state = MULTIFUNDCHANNEL_UPDATED;