From 5aed0e12f864cf27db2ed26f0eb9dbacbe210401 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 26 May 2016 15:25:24 +0930 Subject: [PATCH] daemon: remove closing states from state machine. We already removed the on-chain states, now we remove the "clearing" state (which wasn't fully implemented anyway). This turns into two smaller state machines: one for clearing, which still allows HTLCs to be failed and fulfilled, and one for mutual close negotiation which only allows close_signature messages. Signed-off-by: Rusty Russell --- daemon/packets.c | 124 ++-------- daemon/peer.c | 540 +++++++++++++++++++++++++++++++++++--------- daemon/peer.h | 3 - daemon/test/test.sh | 6 +- state.c | 157 ++----------- state.h | 21 +- state_types.h | 25 +- 7 files changed, 488 insertions(+), 388 deletions(-) diff --git a/daemon/packets.c b/daemon/packets.c index d0da82d31..2611c56dd 100644 --- a/daemon/packets.c +++ b/daemon/packets.c @@ -70,6 +70,8 @@ static void queue_raw_pkt(struct peer *peer, Pkt *pkt) tal_resize(&peer->outpkt, n+1); peer->outpkt[n] = pkt; + log_debug(peer->log, "Queued pkt %s", pkt_name(pkt->pkt_case)); + /* In case it was waiting for output. */ io_wake(peer); } @@ -202,17 +204,15 @@ void queue_pkt_htlc_add(struct peer *peer, queue_pkt(peer, PKT__PKT_UPDATE_ADD_HTLC, u); } -void queue_pkt_htlc_fulfill(struct peer *peer, - const struct htlc_progress *htlc_prog) +void queue_pkt_htlc_fulfill(struct peer *peer, u64 id, const struct sha256 *r) { UpdateFulfillHtlc *f = tal(peer, UpdateFulfillHtlc); size_t n; + union htlc_staging stage; update_fulfill_htlc__init(f); - assert(htlc_prog->stage.type == HTLC_FULFILL); - - f->id = htlc_prog->stage.fulfill.id; - f->r = sha256_to_proto(f, &htlc_prog->stage.fulfill.r); + f->id = id; + f->r = sha256_to_proto(f, r); /* BOLT #2: * @@ -222,23 +222,26 @@ void queue_pkt_htlc_fulfill(struct peer *peer, n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, THEIRS); assert(n != -1); funding_fulfill_htlc(peer->remote.staging_cstate, n, THEIRS); - add_unacked(&peer->remote, &htlc_prog->stage); + + stage.fulfill.fulfill = HTLC_FULFILL; + stage.fulfill.id = f->id; + stage.fulfill.r = *r; + add_unacked(&peer->remote, &stage); remote_changes_pending(peer); queue_pkt(peer, PKT__PKT_UPDATE_FULFILL_HTLC, f); } -void queue_pkt_htlc_fail(struct peer *peer, - const struct htlc_progress *htlc_prog) +void queue_pkt_htlc_fail(struct peer *peer, u64 id) { UpdateFailHtlc *f = tal(peer, UpdateFailHtlc); size_t n; + union htlc_staging stage; update_fail_htlc__init(f); - assert(htlc_prog->stage.type == HTLC_FAIL); + f->id = id; - f->id = htlc_prog->stage.fail.id; /* FIXME: reason! */ f->reason = tal(f, FailReason); fail_reason__init(f->reason); @@ -251,7 +254,10 @@ void queue_pkt_htlc_fail(struct peer *peer, n = funding_htlc_by_id(peer->remote.staging_cstate, f->id, THEIRS); assert(n != -1); funding_fail_htlc(peer->remote.staging_cstate, n, THEIRS); - add_unacked(&peer->remote, &htlc_prog->stage); + + stage.fail.fail = HTLC_FAIL; + stage.fail.id = f->id; + add_unacked(&peer->remote, &stage); remote_changes_pending(peer); queue_pkt(peer, PKT__PKT_UPDATE_FAIL_HTLC, f); @@ -306,8 +312,6 @@ void queue_pkt_commit(struct peer *peer) /* Switch to the new commitment. */ peer->remote.commit = ci; - peer_check_if_cleared(peer); - /* Now send message */ update_commit__init(u); u->sig = signature_to_proto(u, &ci->sig->sig); @@ -381,7 +385,7 @@ void queue_pkt_revocation(struct peer *peer) = tal(peer->local.commit->prev, struct sha256); peer_get_revocation_preimage(peer, peer->local.commit->prev->commit_num, peer->local.commit->prev->revocation_preimage); - peer_check_if_cleared(peer); + u->revocation_preimage = sha256_to_proto(u, peer->local.commit->prev->revocation_preimage); @@ -783,7 +787,6 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt) peer_get_revocation_hash(peer, ci->commit_num + 1, &peer->local.next_revocation_hash); - peer_check_if_cleared(peer); return NULL; } @@ -818,7 +821,6 @@ Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt) proto_to_sha256(r->revocation_preimage, peer->remote.commit->prev->revocation_preimage); - peer_check_if_cleared(peer); /* Save next revocation hash. */ proto_to_sha256(r->next_revocation_hash, @@ -851,91 +853,3 @@ Pkt *accept_pkt_close_clearing(struct peer *peer, const Pkt *pkt) return NULL; } - -Pkt *accept_pkt_close_sig(struct peer *peer, const Pkt *pkt, bool *acked, - bool *we_agree) -{ - const CloseSignature *c = pkt->close_signature; - struct bitcoin_tx *close_tx; - struct bitcoin_signature theirsig; - - log_info(peer->log, "accept_pkt_close_sig: they offered close fee %" - PRIu64, c->close_fee); - *acked = *we_agree = false; - - /* BOLT #2: - * - * The sender MUST set `close_fee` lower than or equal to the fee of the - * final commitment transaction, and MUST set `close_fee` to an even - * number of satoshis. - */ - if ((c->close_fee & 1) - || c->close_fee > commit_tx_fee(peer->remote.commit->tx, - peer->anchor.satoshis)) { - return pkt_err(peer, "Invalid close fee"); - } - - /* FIXME: Don't accept tiny fee at all? */ - - /* BOLT #2: - ... otherwise it SHOULD propose a - value strictly between the received `close_fee` and its - previously-sent `close_fee`. - */ - if (peer->closing.their_sig) { - /* We want more, they should give more. */ - if (peer->closing.our_fee > peer->closing.their_fee) { - if (c->close_fee <= peer->closing.their_fee) - return pkt_err(peer, "Didn't increase close fee"); - } else { - if (c->close_fee >= peer->closing.their_fee) - return pkt_err(peer, "Didn't decrease close fee"); - } - } - - /* BOLT #2: - * - * The receiver MUST check `sig` is valid for the close - * transaction with the given `close_fee`, and MUST fail the - * connection if it is not. */ - theirsig.stype = SIGHASH_ALL; - if (!proto_to_signature(c->sig, &theirsig.sig)) - return pkt_err(peer, "Invalid signature format"); - - close_tx = peer_create_close_tx(peer, c->close_fee); - if (!check_tx_sig(peer->dstate->secpctx, close_tx, 0, - NULL, 0, - peer->anchor.witnessscript, - &peer->remote.commitkey, &theirsig)) - return pkt_err(peer, "Invalid signature"); - - tal_free(peer->closing.their_sig); - peer->closing.their_sig = tal_dup(peer, - struct bitcoin_signature, &theirsig); - peer->closing.their_fee = c->close_fee; - - if (peer->closing.our_fee == peer->closing.their_fee) { - log_info(peer->log, "accept_pkt_close_sig: That's an ack"); - *acked = true; - } else { - /* Adjust our fee to close on their fee. */ - u64 sum; - - /* Beware overflow! */ - sum = (u64)peer->closing.our_fee + peer->closing.their_fee; - - peer->closing.our_fee = sum / 2; - if (peer->closing.our_fee & 1) - peer->closing.our_fee++; - - log_info(peer->log, "accept_pkt_close_sig: we change to %"PRIu64, - peer->closing.our_fee); - - /* Corner case: we may now agree with them. */ - if (peer->closing.our_fee == peer->closing.their_fee) - *we_agree = true; - } - - /* FIXME: Dynamic fee! */ - return NULL; -} diff --git a/daemon/peer.c b/daemon/peer.c index 802fbb368..dba1d9c0a 100644 --- a/daemon/peer.c +++ b/daemon/peer.c @@ -12,6 +12,7 @@ #include "names.h" #include "peer.h" #include "permute_tx.h" +#include "protobuf_convert.h" #include "pseudorand.h" #include "secrets.h" #include "state.h" @@ -88,22 +89,25 @@ static struct json_result *null_response(const tal_t *ctx) json_object_end(response); return response; } - -static void peer_cmd_complete(struct peer *peer, enum command_status status) -{ - assert(peer->curr_cmd.cmd != INPUT_NONE); - /* If it's a json command, complete that now. */ - if (peer->curr_cmd.jsoncmd) { +static void complete_json_command(struct command *jsoncmd, + enum command_status status) +{ + if (jsoncmd) { if (status == CMD_FAIL) /* FIXME: y'know, details. */ - command_fail(peer->curr_cmd.jsoncmd, "Failed"); + command_fail(jsoncmd, "Failed"); else { assert(status == CMD_SUCCESS); - command_success(peer->curr_cmd.jsoncmd, - null_response(peer->curr_cmd.jsoncmd)); + command_success(jsoncmd, null_response(jsoncmd)); } } +} + +static void peer_cmd_complete(struct peer *peer, enum command_status status) +{ + complete_json_command(peer->curr_cmd.jsoncmd, status); + peer->curr_cmd.jsoncmd = NULL; peer->curr_cmd.cmd = INPUT_NONE; } @@ -128,6 +132,86 @@ void set_peer_state(struct peer *peer, enum state newstate, const char *caller) peer->state = newstate; } +static void command_send_commit(struct peer *peer, struct command *jsoncmd) +{ + assert(peer->curr_cmd.cmd == INPUT_NONE); + + /* Commands should still be blocked during this! */ + assert(peer->state != STATE_CLEARING_COMMITTING); + + if (peer->state == STATE_CLEARING) { + peer->cond = PEER_BUSY; + peer->curr_cmd.jsoncmd = jsoncmd; + queue_pkt_commit(peer); + set_peer_state(peer, STATE_CLEARING_COMMITTING, __func__); + } else { + /* You can send commands if closing; peer->cond == CLOSING */ + assert(!state_is_closing(peer->state)); + set_current_command(peer, CMD_SEND_COMMIT, NULL, jsoncmd); + } +} + +static void set_htlc_command(struct peer *peer, + struct command *jsoncmd, + enum state_input cmd, + const union htlc_staging *stage) +{ + /* FIXME: memleak! */ + /* FIXME: Get rid of struct htlc_progress */ + struct htlc_progress *progress = tal(peer, struct htlc_progress); + + progress->stage = *stage; + set_current_command(peer, cmd, progress, jsoncmd); +} + +static void command_htlc_fail(struct peer *peer, u64 id, struct command *jsoncmd) +{ + assert(peer->curr_cmd.cmd == INPUT_NONE); + + /* Commands should still be blocked during this! */ + assert(peer->state != STATE_CLEARING_COMMITTING); + + if (peer->state == STATE_CLEARING) { + queue_pkt_htlc_fail(peer, id); + complete_json_command(jsoncmd, CMD_SUCCESS); + } else { + union htlc_staging stage; + + /* You can't send commands if closing; peer->cond == CLOSING */ + assert(!state_is_closing(peer->state)); + + stage.fail.fail = HTLC_FAIL; + stage.fail.id = id; + set_htlc_command(peer, jsoncmd, CMD_SEND_HTLC_FAIL, &stage); + } +} + +static void command_htlc_fulfill(struct peer *peer, + u64 id, + const struct sha256 *r, + struct command *jsoncmd) +{ + assert(peer->curr_cmd.cmd == INPUT_NONE); + + /* Commands should still be blocked during this! */ + assert(peer->state != STATE_CLEARING_COMMITTING); + + if (peer->state == STATE_CLEARING) { + queue_pkt_htlc_fulfill(peer, id, r); + complete_json_command(jsoncmd, CMD_SUCCESS); + } else { + union htlc_staging stage; + + /* You can't send commands if closing; peer->cond == CLOSING */ + assert(!state_is_closing(peer->state)); + + stage.fulfill.fulfill = HTLC_FULFILL; + stage.fulfill.r = *r; + stage.fulfill.id = id; + set_htlc_command(peer, jsoncmd, CMD_SEND_HTLC_FULFILL, &stage); + } +} + static void peer_breakdown(struct peer *peer) { peer->cond = PEER_CLOSED; @@ -158,6 +242,296 @@ static bool peer_uncommitted_changes(const struct peer *peer) != peer->remote.commit->cstate->changes; } +/* All unrevoked commit txs must have no HTLCs in them. */ +static bool committed_to_htlcs(const struct peer *peer) +{ + const struct commit_info *i; + + /* Before anchor exchange, we don't even have cstate. */ + if (!peer->local.commit || !peer->local.commit->cstate) + return false; + + i = peer->local.commit; + while (i && !i->revocation_preimage) { + if (tal_count(i->cstate->side[OURS].htlcs)) + return true; + if (tal_count(i->cstate->side[THEIRS].htlcs)) + return true; + i = i->prev; + } + + i = peer->remote.commit; + while (i && !i->revocation_preimage) { + if (tal_count(i->cstate->side[OURS].htlcs)) + return true; + if (tal_count(i->cstate->side[THEIRS].htlcs)) + return true; + i = i->prev; + } + + return false; +} + +static struct io_plan *peer_close(struct io_conn *conn, struct peer *peer) +{ + /* Tell writer to wrap it up (may have to xmit first) */ + peer->cond = PEER_CLOSED; + io_wake(peer); + /* We do nothing more. */ + return io_wait(conn, NULL, io_never, NULL); +} + +/* Communication failed: send err (if non-NULL), then dump to chain and close. */ +static struct io_plan *peer_comms_err(struct io_conn *conn, struct peer *peer, + Pkt *err) +{ + if (err) + queue_pkt_err(peer, err); + + set_peer_state(peer, STATE_ERR_BREAKDOWN, __func__); + peer_breakdown(peer); + return peer_close(conn, peer); +} + +/* Unexpected packet received: stop listening, start breakdown procedure. */ +static struct io_plan *peer_received_unexpected_pkt(struct io_conn *conn, + struct peer *peer, + const Pkt *pkt) +{ + peer_unexpected_pkt(peer, pkt); + return peer_comms_err(conn, peer, pkt_err_unexpected(peer, pkt)); +} + +/* This is the io loop while we're negotiating closing tx. */ +static struct io_plan *closing_pkt_in(struct io_conn *conn, struct peer *peer) +{ + const CloseSignature *c = peer->inpkt->close_signature; + struct bitcoin_tx *close_tx; + struct bitcoin_signature theirsig; + + assert(peer->state == STATE_MUTUAL_CLOSING); + + if (peer->inpkt->pkt_case != PKT__PKT_CLOSE_SIGNATURE) + return peer_received_unexpected_pkt(conn, peer, peer->inpkt); + + log_info(peer->log, "closing_pkt_in: they offered close fee %"PRIu64, + c->close_fee); + + /* BOLT #2: + * + * The sender MUST set `close_fee` lower than or equal to the fee of the + * final commitment transaction, and MUST set `close_fee` to an even + * number of satoshis. + */ + if ((c->close_fee & 1) + || c->close_fee > commit_tx_fee(peer->remote.commit->tx, + peer->anchor.satoshis)) { + return peer_comms_err(conn, peer, + pkt_err(peer, "Invalid close fee")); + } + + /* FIXME: Don't accept tiny fee at all? */ + + /* BOLT #2: + ... otherwise it SHOULD propose a + value strictly between the received `close_fee` and its + previously-sent `close_fee`. + */ + if (peer->closing.their_sig) { + /* We want more, they should give more. */ + if (peer->closing.our_fee > peer->closing.their_fee) { + if (c->close_fee <= peer->closing.their_fee) + return peer_comms_err(conn, peer, + pkt_err(peer, "Didn't increase close fee")); + } else { + if (c->close_fee >= peer->closing.their_fee) + return peer_comms_err(conn, peer, + pkt_err(peer, "Didn't decrease close fee")); + } + } + + /* BOLT #2: + * + * The receiver MUST check `sig` is valid for the close + * transaction with the given `close_fee`, and MUST fail the + * connection if it is not. */ + theirsig.stype = SIGHASH_ALL; + if (!proto_to_signature(c->sig, &theirsig.sig)) + return peer_comms_err(conn, peer, + pkt_err(peer, "Invalid signature format")); + + close_tx = peer_create_close_tx(peer, c->close_fee); + if (!check_tx_sig(peer->dstate->secpctx, close_tx, 0, + NULL, 0, + peer->anchor.witnessscript, + &peer->remote.commitkey, &theirsig)) + return peer_comms_err(conn, peer, + pkt_err(peer, "Invalid signature")); + + tal_free(peer->closing.their_sig); + peer->closing.their_sig = tal_dup(peer, + struct bitcoin_signature, &theirsig); + peer->closing.their_fee = c->close_fee; + + if (peer->closing.our_fee != peer->closing.their_fee) { + /* BOLT #2: + * + * If the receiver agrees with the fee, it SHOULD reply with a + * `close_signature` with the same `close_fee` value, + * otherwise it SHOULD propose a value strictly between the + * received `close_fee` and its previously-sent `close_fee`. + */ + + /* Adjust our fee to close on their fee. */ + u64 sum; + + /* Beware overflow! */ + sum = (u64)peer->closing.our_fee + peer->closing.their_fee; + + peer->closing.our_fee = sum / 2; + if (peer->closing.our_fee & 1) + peer->closing.our_fee++; + + log_info(peer->log, "accept_pkt_close_sig: we change to %"PRIu64, + peer->closing.our_fee); + + queue_pkt_close_signature(peer); + } + + /* Note corner case: we may *now* agree with them! */ + if (peer->closing.our_fee == peer->closing.their_fee) { + log_info(peer->log, "accept_pkt_close_sig: we agree"); + /* BOLT #2: + * + * Once a node has sent or received a `close_signature` with + * matching `close_fee` it SHOULD close the connection and + * SHOULD sign and broadcast the final closing transaction. + */ + broadcast_tx(peer, bitcoin_close(peer)); + return peer_close(conn, peer); + } + + /* FIXME: Dynamic fee! */ + return peer_read_packet(conn, peer, closing_pkt_in); +} + +/* FIXME: Predecls are bad, move up! */ +static void try_command(struct peer *peer); + +/* This is the io loop while we're clearing. */ +static struct io_plan *clearing_pkt_in(struct io_conn *conn, struct peer *peer) +{ + Pkt *err = NULL, *pkt = peer->inpkt; + + assert(peer->state == STATE_CLEARING + || peer->state == STATE_CLEARING_COMMITTING); + + switch (pkt->pkt_case) { + case PKT__PKT_UPDATE_REVOCATION: + if (peer->state == STATE_CLEARING) + err = pkt_err_unexpected(peer, pkt); + else { + err = accept_pkt_revocation(peer, pkt); + if (!err) { + set_peer_state(peer, STATE_CLEARING, __func__); + peer_cmd_complete(peer, CMD_SUCCESS); + peer->cond = PEER_CMD_OK; + /* In case command is queued. */ + try_command(peer); + } + } + break; + + case PKT__PKT_UPDATE_ADD_HTLC: + /* BOLT #2: + * + * A node MUST NOT send a `update_add_htlc` after a + * `close_clearing` */ + if (peer->closing.their_script) + err = pkt_err(peer, "Update during clearing"); + else + err = accept_pkt_htlc_add(peer, pkt); + break; + + case PKT__PKT_CLOSE_CLEARING: + /* BOLT #2: + * + * A node... MUST NOT send more than one `close_clearing`. */ + if (peer->closing.their_script) + err = pkt_err_unexpected(peer, pkt); + else + err = accept_pkt_close_clearing(peer, pkt); + break; + + case PKT__PKT_UPDATE_FULFILL_HTLC: + err = accept_pkt_htlc_fulfill(peer, pkt); + break; + case PKT__PKT_UPDATE_FAIL_HTLC: + err = accept_pkt_htlc_fail(peer, pkt); + break; + case PKT__PKT_UPDATE_COMMIT: + err = accept_pkt_commit(peer, pkt); + if (!err) + queue_pkt_revocation(peer); + break; + case PKT__PKT_ERROR: + peer_unexpected_pkt(peer, pkt); + return peer_comms_err(conn, peer, NULL); + + case PKT__PKT_AUTH: + case PKT__PKT_OPEN: + case PKT__PKT_OPEN_ANCHOR: + case PKT__PKT_OPEN_COMMIT_SIG: + case PKT__PKT_OPEN_COMPLETE: + case PKT__PKT_CLOSE_SIGNATURE: + default: + peer_unexpected_pkt(peer, pkt); + err = pkt_err_unexpected(peer, pkt); + break; + } + + if (err) + return peer_comms_err(conn, peer, err); + + if (!committed_to_htlcs(peer)) { + set_peer_state(peer, STATE_MUTUAL_CLOSING, __func__); + peer_calculate_close_fee(peer); + queue_pkt_close_signature(peer); + return peer_read_packet(conn, peer, closing_pkt_in); + } + + return peer_read_packet(conn, peer, clearing_pkt_in); +} + +static void peer_start_clearing(struct peer *peer) +{ + assert(peer->state == STATE_CLEARING + || peer->state == STATE_CLEARING_COMMITTING); + + /* If they started close, we might not have sent ours. */ + if (!peer->closing.our_script) { + u8 *redeemscript = bitcoin_redeem_single(peer, + &peer->local.finalkey); + + peer->closing.our_script = scriptpubkey_p2sh(peer, redeemscript); + tal_free(redeemscript); + /* BOLT #2: + * + * A node SHOULD send a `close_clearing` (if it has + * not already) after receiving `close_clearing`. + */ + queue_pkt_close_clearing(peer); + } + + /* Catch case where we've exchanged and had no HTLCs anyway. */ + if (peer->closing.our_script && peer->closing.their_script + && !committed_to_htlcs(peer)) { + set_peer_state(peer, STATE_MUTUAL_CLOSING, __func__); + peer_calculate_close_fee(peer); + queue_pkt_close_signature(peer); + } +} + static void state_single(struct peer *peer, const enum state_input input, const union input *idata) @@ -199,6 +573,10 @@ static void state_single(struct peer *peer, if (peer->cond == PEER_CLOSED) io_wake(peer); + else if (peer->state == STATE_CLEARING + || peer->state == STATE_CLEARING_COMMITTING) + peer_start_clearing(peer); + /* FIXME: Some of these should just result in this peer being killed? */ else if (state_is_error(peer->state)) { log_broken(peer->log, "Entered error state %s", @@ -245,7 +623,7 @@ static bool queue_cmd_(struct peer *peer, { struct pending_cmd *pend; - if (peer->cond == PEER_CLOSING || peer->cond == PEER_CLOSED) + if (peer->cond == PEER_CLOSED) return false; pend = tal(peer, struct pending_cmd); @@ -257,7 +635,7 @@ static bool queue_cmd_(struct peer *peer, return true; }; -static void queue_input(struct peer *peer, +static void UNNEEDED queue_input(struct peer *peer, enum state_input input, const union input *idata) { @@ -269,43 +647,19 @@ static void queue_input(struct peer *peer, list_add_tail(&peer->pending_input, &pend->list); } -/* All unrevoked commit txs must have no HTLCs in them. */ -static bool committed_to_htlcs(const struct peer *peer) -{ - const struct commit_info *i; - - /* Before anchor exchange, we don't even have cstate. */ - if (!peer->local.commit || !peer->local.commit->cstate) - return false; - - i = peer->local.commit; - while (i && !i->revocation_preimage) { - if (tal_count(i->cstate->side[OURS].htlcs)) - return true; - if (tal_count(i->cstate->side[THEIRS].htlcs)) - return true; - i = i->prev; - } - - i = peer->remote.commit; - while (i && !i->revocation_preimage) { - if (tal_count(i->cstate->side[OURS].htlcs)) - return true; - if (tal_count(i->cstate->side[THEIRS].htlcs)) - return true; - i = i->prev; - } - - return false; -} - static void state_event(struct peer *peer, const enum state_input input, const union input *idata) { struct pending_input *pend; - state_single(peer, input, idata); + if (state_is_closing(peer->state)) { + log_unusual(peer->log, + "Unexpected input %s while state %s", + input_name(input), state_name(peer->state)); + } else { + state_single(peer, input, idata); + } pend = list_pop(&peer->pending_input, struct pending_input, list); if (pend) { @@ -316,18 +670,6 @@ static void state_event(struct peer *peer, try_command(peer); } -void peer_check_if_cleared(struct peer *peer) -{ - if (peer->cleared == INPUT_NONE) - return; - - if (committed_to_htlcs(peer)) - return; - - queue_input(peer, peer->cleared, NULL); - peer->cleared = INPUT_NONE; -} - static struct io_plan *pkt_out(struct io_conn *conn, struct peer *peer) { Pkt *out; @@ -352,8 +694,16 @@ static struct io_plan *pkt_out(struct io_conn *conn, struct peer *peer) static struct io_plan *pkt_in(struct io_conn *conn, struct peer *peer) { union input idata; - const tal_t *ctx = tal(peer, char); + const tal_t *ctx; + /* Did something move us into STATE_CLEARING? */ + if (peer->state == STATE_CLEARING + || peer->state == STATE_CLEARING_COMMITTING) + return clearing_pkt_in(conn, peer); + else if (peer->state == STATE_MUTUAL_CLOSING) + return closing_pkt_in(conn, peer); + + ctx = tal(peer, char); idata.pkt = tal_steal(ctx, peer->inpkt); /* We ignore packets if they tell us to. */ @@ -435,7 +785,7 @@ static void do_commit(struct peer *peer, struct command *jsoncmd) && peer->remote.staging_cstate->changes != peer->remote.commit->cstate->changes) { log_debug(peer->log, "do_commit: sending commit command"); - set_current_command(peer, CMD_SEND_COMMIT, NULL, jsoncmd); + command_send_commit(peer, jsoncmd); return; } log_debug(peer->log, "do_commit: no changes to commit"); @@ -1724,16 +2074,6 @@ void peer_unexpected_pkt(struct peer *peer, const Pkt *pkt) log_unusual(peer->log, "Error pkt '%s'", pkt->error->problem); } -void peer_watch_htlcs_cleared(struct peer *peer, - enum state_input all_done) -{ - assert(peer->cleared == INPUT_NONE); - assert(all_done != INPUT_NONE); - peer->cleared = all_done; - - peer_check_if_cleared(peer); -} - /* Create a bitcoin close tx, using last signature they sent. */ const struct bitcoin_tx *bitcoin_close(struct peer *peer) { @@ -2060,19 +2400,6 @@ const struct json_command getpeers_command = { "Returns a 'peers' array" }; -static void set_htlc_command(struct peer *peer, - struct command *jsoncmd, - enum state_input cmd, - const union htlc_staging *stage) -{ - /* FIXME: memleak! */ - /* FIXME: Get rid of struct htlc_progress */ - struct htlc_progress *progress = tal(peer, struct htlc_progress); - - progress->stage = *stage; - set_current_command(peer, cmd, progress, jsoncmd); -} - /* FIXME: Keep a timeout for each peer, in case they're unresponsive. */ /* FIXME: Make sure no HTLCs in any unrevoked commit tx are live. */ @@ -2080,9 +2407,6 @@ static void set_htlc_command(struct peer *peer, static void check_htlc_expiry(struct peer *peer, void *unused) { size_t i; - union htlc_staging stage; - - stage.fail.fail = HTLC_FAIL; /* Check their currently still-existing htlcs for expiry: * We eliminate them from staging as we go. */ @@ -2098,8 +2422,7 @@ static void check_htlc_expiry(struct peer *peer, void *unused) < abs_locktime_to_seconds(&htlc->expiry)) continue; - stage.fail.id = htlc->id; - set_htlc_command(peer, NULL, CMD_SEND_HTLC_FAIL, &stage); + command_htlc_fail(peer, htlc->id, NULL); return; } } @@ -2147,9 +2470,19 @@ static void do_newhtlc(struct peer *peer, struct newhtlc *newhtlc) if (tal_count(peer->local.staging_cstate->side[OURS].htlcs) == 300 || tal_count(peer->remote.staging_cstate->side[OURS].htlcs) == 300) { command_fail(newhtlc->jsoncmd, "Too many HTLCs"); + return; } - + /* BOLT #2: + * + * A node MUST NOT send a `update_add_htlc` after a `close_clearing` + */ + if (state_is_closing(peer->state)) { + command_fail(newhtlc->jsoncmd, "Channel closing, state %s", + state_name(peer->state)); + return; + } + /* BOLT #2: * * A node MUST NOT offer `amount_msat` it cannot pay for in @@ -2292,10 +2625,7 @@ static void do_fullfill(struct peer *peer, { struct sha256 rhash; size_t i; - union htlc_staging stage; - - stage.fulfill.fulfill = HTLC_FULFILL; - stage.fulfill.r = fulfillhtlc->r; + u64 id; sha256(&rhash, &fulfillhtlc->r, sizeof(fulfillhtlc->r)); @@ -2304,9 +2634,8 @@ static void do_fullfill(struct peer *peer, command_fail(fulfillhtlc->jsoncmd, "preimage htlc not found"); return; } - stage.fulfill.id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id; - set_htlc_command(peer, fulfillhtlc->jsoncmd, - CMD_SEND_HTLC_FULFILL, &stage); + id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id; + command_htlc_fulfill(peer, id, &fulfillhtlc->r, fulfillhtlc->jsoncmd); } static void json_fulfillhtlc(struct command *cmd, @@ -2367,9 +2696,7 @@ static void do_failhtlc(struct peer *peer, struct failhtlc *failhtlc) { size_t i; - union htlc_staging stage; - - stage.fail.fail = HTLC_FAIL; + u64 id; /* Look in peer->remote.staging_cstate->a, as that's where we'll * immediately remove it from: avoids double-handling. */ @@ -2378,9 +2705,8 @@ static void do_failhtlc(struct peer *peer, command_fail(failhtlc->jsoncmd, "htlc not found"); return; } - stage.fail.id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id; - - set_htlc_command(peer, failhtlc->jsoncmd, CMD_SEND_HTLC_FAIL, &stage); + id = peer->remote.staging_cstate->side[THEIRS].htlcs[i].id; + command_htlc_fail(peer, id, failhtlc->jsoncmd); } static void json_failhtlc(struct command *cmd, @@ -2485,14 +2811,22 @@ static void json_close(struct command *cmd, command_fail(cmd, "Could not find peer with that peerid"); return; } - if (peer->cond == PEER_CLOSING) { - command_fail(cmd, "Peer is already closing"); + + if (state_is_closing(peer->state)) { + command_fail(cmd, "Peer is already closing: state %s", + state_name(peer->state)); return; } - /* Unlike other things, CMD_CLOSE is always valid. */ - log_debug(peer->log, "Sending CMD_CLOSE"); - state_event(peer, CMD_CLOSE, NULL); + /* Closing causes any current command to fail. */ + if (peer->curr_cmd.cmd != INPUT_NONE) + peer_cmd_complete(peer, CMD_FAIL); + + if (peer->state == STATE_NORMAL_COMMITTING) + set_peer_state(peer, STATE_CLEARING_COMMITTING, __func__); + else + set_peer_state(peer, STATE_CLEARING, __func__); + peer_start_clearing(peer); command_success(cmd, null_response(cmd)); } diff --git a/daemon/peer.h b/daemon/peer.h index ecc6820a5..8fb0fbc17 100644 --- a/daemon/peer.h +++ b/daemon/peer.h @@ -235,9 +235,6 @@ bool setup_first_commit(struct peer *peer); void set_peer_state(struct peer *peer, enum state newstate, const char *why); -/* Call this after commit changes, or revocation accepted/sent. */ -void peer_check_if_cleared(struct peer *peer); - /* Set up timer: we have something we can commit. */ void remote_changes_pending(struct peer *peer); diff --git a/daemon/test/test.sh b/daemon/test/test.sh index 51fdce1d9..83fd34816 100755 --- a/daemon/test/test.sh +++ b/daemon/test/test.sh @@ -557,9 +557,9 @@ check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE "" lcli1 close $ID2 -# They should be waiting for close. -check_peerstate lcli1 STATE_CLOSE_WAIT_CLOSE -check_peerstate lcli2 STATE_CLOSE_WAIT_CLOSE +# They should be negotiating the close. +check_peerstate lcli1 STATE_MUTUAL_CLOSING +check_peerstate lcli2 STATE_MUTUAL_CLOSING $CLI generate 1 diff --git a/state.c b/state.c index 0068916fe..0cff7d8b5 100644 --- a/state.c +++ b/state.c @@ -34,12 +34,6 @@ static enum command_status unchanged_state(const struct peer *peer, return cstatus; } -static void set_peer_cond(struct peer *peer, enum state_peercond cond) -{ - assert(peer->cond != cond); - peer->cond = cond; -} - static void change_peer_cond(struct peer *peer, enum state_peercond old, enum state_peercond new) @@ -110,9 +104,6 @@ enum command_status state(struct peer *peer, goto err_breakdown; } return next_state(peer, input, cstatus, STATE_OPEN_WAIT_FOR_ANCHOR); - } else if (input_is(input, CMD_CLOSE)) { - complete_cmd(peer, &cstatus, CMD_FAIL); - goto breakdown; } else if (input_is_pkt(input)) { complete_cmd(peer, &cstatus, CMD_FAIL); goto unexpected_pkt; @@ -128,9 +119,6 @@ enum command_status state(struct peer *peer, bitcoin_create_anchor(peer, BITCOIN_ANCHOR_CREATED); return next_state(peer, input, cstatus, STATE_OPEN_WAIT_FOR_ANCHOR_CREATE); - } else if (input_is(input, CMD_CLOSE)) { - complete_cmd(peer, &cstatus, CMD_FAIL); - goto breakdown; } else if (input_is_pkt(input)) { complete_cmd(peer, &cstatus, CMD_FAIL); goto unexpected_pkt; @@ -141,10 +129,6 @@ enum command_status state(struct peer *peer, queue_pkt_anchor(peer); return next_state(peer, input, cstatus, STATE_OPEN_WAIT_FOR_COMMIT_SIG); - } else if (input_is(input, CMD_CLOSE)) { - bitcoin_release_anchor(peer, BITCOIN_ANCHOR_CREATED); - complete_cmd(peer, &cstatus, CMD_FAIL); - goto breakdown; } else if (input_is_pkt(input)) { bitcoin_release_anchor(peer, BITCOIN_ANCHOR_CREATED); complete_cmd(peer, &cstatus, CMD_FAIL); @@ -165,9 +149,6 @@ enum command_status state(struct peer *peer, return next_state(peer, input, cstatus, STATE_OPEN_WAITING_THEIRANCHOR); - } else if (input_is(input, CMD_CLOSE)) { - complete_cmd(peer, &cstatus, CMD_FAIL); - goto breakdown; } else if (input_is_pkt(input)) { complete_cmd(peer, &cstatus, CMD_FAIL); goto unexpected_pkt; @@ -187,10 +168,6 @@ enum command_status state(struct peer *peer, INPUT_NONE); return next_state(peer, input, cstatus, STATE_OPEN_WAITING_OURANCHOR); - } else if (input_is(input, CMD_CLOSE)) { - bitcoin_release_anchor(peer, INPUT_NONE); - complete_cmd(peer, &cstatus, CMD_FAIL); - goto breakdown; } else if (input_is_pkt(input)) { bitcoin_release_anchor(peer, INPUT_NONE); complete_cmd(peer, &cstatus, CMD_FAIL); @@ -221,13 +198,6 @@ enum command_status state(struct peer *peer, } return next_state(peer, input, cstatus, STATE_OPEN_WAIT_FOR_COMPLETE_OURANCHOR); - } else if (input_is(input, CMD_CLOSE)) { - /* We no longer care about anchor depth. */ - peer_unwatch_anchor_depth(peer, - BITCOIN_ANCHOR_DEPTHOK, - INPUT_NONE); - complete_cmd(peer, &cstatus, CMD_FAIL); - goto start_clearing; } else if (input_is(input, PKT_CLOSE_CLEARING)) { /* We no longer care about anchor depth. */ peer_unwatch_anchor_depth(peer, @@ -272,13 +242,6 @@ enum command_status state(struct peer *peer, } return next_state(peer, input, cstatus, STATE_OPEN_WAIT_FOR_COMPLETE_THEIRANCHOR); - } else if (input_is(input, CMD_CLOSE)) { - /* We no longer care about anchor depth. */ - peer_unwatch_anchor_depth(peer, - BITCOIN_ANCHOR_DEPTHOK, - BITCOIN_ANCHOR_TIMEOUT); - complete_cmd(peer, &cstatus, CMD_FAIL); - goto start_clearing; } else if (input_is(input, PKT_CLOSE_CLEARING)) { /* We no longer care about anchor depth. */ peer_unwatch_anchor_depth(peer, @@ -306,9 +269,6 @@ enum command_status state(struct peer *peer, complete_cmd(peer, &cstatus, CMD_SUCCESS); return next_state(peer, input, cstatus, STATE_NORMAL); } - } else if (input_is(input, CMD_CLOSE)) { - complete_cmd(peer, &cstatus, CMD_FAIL); - goto start_clearing; } else if (input_is(input, PKT_CLOSE_CLEARING)) { complete_cmd(peer, &cstatus, CMD_FAIL); goto accept_clearing; @@ -337,12 +297,16 @@ enum command_status state(struct peer *peer, queue_pkt_htlc_add(peer, idata->htlc_prog); return instant_cmd_success(peer, cstatus); } else if (input_is(input, CMD_SEND_HTLC_FULFILL)) { + assert(idata->htlc_prog->stage.type == HTLC_FULFILL); /* We are to send an HTLC fulfill. */ - queue_pkt_htlc_fulfill(peer, idata->htlc_prog); + queue_pkt_htlc_fulfill(peer, + idata->htlc_prog->stage.fulfill.id, + &idata->htlc_prog->stage.fulfill.r); return instant_cmd_success(peer, cstatus); } else if (input_is(input, CMD_SEND_HTLC_FAIL)) { + assert(idata->htlc_prog->stage.type == HTLC_FAIL); /* We are to send an HTLC fail. */ - queue_pkt_htlc_fail(peer, idata->htlc_prog); + queue_pkt_htlc_fail(peer, idata->htlc_prog->stage.fail.id); return instant_cmd_success(peer, cstatus); } /* Fall through... */ @@ -359,9 +323,7 @@ enum command_status state(struct peer *peer, return next_state(peer, input, cstatus, STATE_NORMAL); } - if (input_is(input, CMD_CLOSE)) { - goto start_clearing; - } else if (input_is(input, PKT_UPDATE_ADD_HTLC)) { + if (input_is(input, PKT_UPDATE_ADD_HTLC)) { err = accept_pkt_htlc_add(peer, idata->pkt); if (err) goto err_breakdown; @@ -388,85 +350,17 @@ enum command_status state(struct peer *peer, goto unexpected_pkt; } break; - case STATE_US_CLEARING: - /* This is their reply once they're clearing too. */ - if (input_is(input, PKT_CLOSE_CLEARING)) { - err = accept_pkt_close_clearing(peer, idata->pkt); - if (err) - goto err_breakdown; - - /* Notify us when there are no more htlcs in - * either commit tx */ - peer_watch_htlcs_cleared(peer, INPUT_HTLCS_CLEARED); - - return next_state(peer, input, cstatus, STATE_BOTH_CLEARING); - /* FIXME: We must continue to allow fulfill & fail! */ - } else if (input_is(input, CMD_SEND_HTLC_FAIL) - || input_is(input, CMD_SEND_HTLC_FULFILL)) { - err = pkt_err(peer, "FIXME: cmd during clearing."); - goto err_breakdown; - } else if (input_is_pkt(input)) { - /* FIXME: We must continue to allow add, fulfill & fail packets */ - goto unexpected_pkt; - } - break; - case STATE_BOTH_CLEARING: - if (input_is(input, INPUT_HTLCS_CLEARED)) { - goto start_closing_cleared; - } else if (input_is(input, CMD_SEND_HTLC_FAIL) - || input_is(input, CMD_SEND_HTLC_FULFILL)) { - err = pkt_err(peer, "FIXME: cmd during clearing."); - goto err_breakdown; - } else if (input_is_pkt(input)) { - /* FIXME: We must continue to allow fulfill & fail packets */ - goto unexpected_pkt; - } - break; - case STATE_WAIT_FOR_CLOSE_SIG: - if (input_is(input, PKT_CLOSE_SIGNATURE)) { - bool acked, we_agree; - err = accept_pkt_close_sig(peer, idata->pkt, - &acked, &we_agree); - if (err) - goto err_breakdown; - - /* Are we about to offer the same fee they did? */ - if (we_agree) { - /* Offer the new fee. */ - queue_pkt_close_signature(peer); - acked = true; - } - - /* Do fees now match? */ - if (acked) { - /* Send close TX. */ - queue_tx_broadcast(broadcast, - bitcoin_close(peer)); - change_peer_cond(peer, - PEER_CLOSING, PEER_CLOSED); - return next_state(peer, input, cstatus, - STATE_CLOSE_WAIT_CLOSE); - } - - /* Offer the new fee. */ - queue_pkt_close_signature(peer); - return unchanged_state(peer, input, cstatus); - } else if (input_is(input, INPUT_CLOSE_COMPLETE_TIMEOUT)) { - err = pkt_err(peer, "Close timed out"); - goto err_breakdown; - } else if (input_is_pkt(input)) { - goto unexpected_pkt; - } - break; /* Should never happen. */ case STATE_ERR_INTERNAL: case STATE_ERR_ANCHOR_TIMEOUT: case STATE_ERR_INFORMATION_LEAK: case STATE_ERR_BREAKDOWN: - case STATE_CLOSE_WAIT_CLOSE: case STATE_CLOSED: case STATE_MAX: + case STATE_CLEARING: + case STATE_CLEARING_COMMITTING: + case STATE_MUTUAL_CLOSING: case STATE_CLOSE_ONCHAIN_CHEATED: case STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL: case STATE_CLOSE_ONCHAIN_OUR_UNILATERAL: @@ -491,36 +385,13 @@ err_breakdown: breakdown: return next_state(peer, input, cstatus, STATE_ERR_BREAKDOWN); -start_clearing: - /* - * Start a mutual close: tell them we want to clear. - */ - queue_pkt_close_clearing(peer); - - /* No more commands, we're already closing. */ - set_peer_cond(peer, PEER_CLOSING); - - return next_state(peer, input, cstatus, STATE_US_CLEARING); - -start_closing_cleared: - /* As soon as we send packet, they could close. */ - peer_calculate_close_fee(peer); - queue_pkt_close_signature(peer); - return next_state(peer, input, cstatus, STATE_WAIT_FOR_CLOSE_SIG); - accept_clearing: err = accept_pkt_close_clearing(peer, idata->pkt); if (err) goto err_breakdown; - /* Notify us when there are no more htlcs in either commit tx */ - peer_watch_htlcs_cleared(peer, INPUT_HTLCS_CLEARED); - - /* No more commands, we're already closing. */ - set_peer_cond(peer, PEER_CLOSING); - - /* Tell them we're clearing too. */ - queue_pkt_close_clearing(peer); - - return next_state(peer, input, cstatus, STATE_BOTH_CLEARING); + /* If we've sent commit, we're still waiting for it when clearing. */ + if (peer->state == STATE_NORMAL_COMMITTING) + return next_state(peer, input, cstatus, STATE_CLEARING_COMMITTING); + return next_state(peer, input, cstatus, STATE_CLEARING); } diff --git a/state.h b/state.h index c3d81c3af..cdf50ddb7 100644 --- a/state.h +++ b/state.h @@ -19,6 +19,11 @@ static inline bool state_is_error(enum state s) return s >= STATE_ERR_ANCHOR_TIMEOUT && s <= STATE_ERR_INTERNAL; } +static inline bool state_is_closing(enum state s) +{ + return s >= STATE_CLEARING; +} + struct peer; struct bitcoin_tx; @@ -84,10 +89,8 @@ void queue_pkt_open_commit_sig(struct peer *peer); void queue_pkt_open_complete(struct peer *peer); void queue_pkt_htlc_add(struct peer *peer, const struct htlc_progress *htlc_prog); -void queue_pkt_htlc_fulfill(struct peer *peer, - const struct htlc_progress *htlc_prog); -void queue_pkt_htlc_fail(struct peer *peer, - const struct htlc_progress *htlc_prog); +void queue_pkt_htlc_fulfill(struct peer *peer, u64 id, const struct sha256 *r); +void queue_pkt_htlc_fail(struct peer *peer, u64 id); void queue_pkt_commit(struct peer *peer); void queue_pkt_revocation(struct peer *peer); void queue_pkt_close_clearing(struct peer *peer); @@ -118,8 +121,6 @@ Pkt *accept_pkt_commit(struct peer *peer, const Pkt *pkt); Pkt *accept_pkt_revocation(struct peer *peer, const Pkt *pkt); Pkt *accept_pkt_close_clearing(struct peer *peer, const Pkt *pkt); -Pkt *accept_pkt_close_sig(struct peer *peer, const Pkt *pkt, - bool *acked, bool *we_agree); /** * peer_watch_anchor: create a watch for the anchor transaction. @@ -145,14 +146,6 @@ void peer_unwatch_anchor_depth(struct peer *peer, enum state_input depthok, enum state_input timeout); -/** - * peer_watch_htlcs_cleared: tell us when no HTLCs are in commit txs. - * @peer: the state data for this peer. - * @all_done: input to give when all HTLCs are done. - */ -void peer_watch_htlcs_cleared(struct peer *peer, - enum state_input all_done); - /** * peer_calculate_close_fee: figure out what the fee for closing is. * @peer: the state data for this peer. diff --git a/state_types.h b/state_types.h index bb4acc11f..b63706032 100644 --- a/state_types.h +++ b/state_types.h @@ -29,26 +29,21 @@ enum state { STATE_NORMAL_COMMITTING, /* - * Closing. + * Closing (handled outside state machine). */ - /* We told them to clear. */ - STATE_US_CLEARING, - /* They told us to clear, or acked our CLEARING. */ - STATE_BOTH_CLEARING, - /* We're cleared, waiting for close signature / negotiation */ - STATE_WAIT_FOR_CLOSE_SIG, - /* We've broadcast the mutual close, waiting for onchain. */ - STATE_CLOSE_WAIT_CLOSE, + STATE_CLEARING, + STATE_CLEARING_COMMITTING, + STATE_MUTUAL_CLOSING, - /* All closed. */ - STATE_CLOSED, - /* Four states to represent closing onchain (for getpeers) */ STATE_CLOSE_ONCHAIN_CHEATED, STATE_CLOSE_ONCHAIN_THEIR_UNILATERAL, STATE_CLOSE_ONCHAIN_OUR_UNILATERAL, STATE_CLOSE_ONCHAIN_MUTUAL, + /* All closed. */ + STATE_CLOSED, + /* * Where angels fear to tread. */ @@ -84,9 +79,8 @@ enum state_input { PKT_UPDATE_COMMIT = PKT__PKT_UPDATE_COMMIT, PKT_UPDATE_REVOCATION = PKT__PKT_UPDATE_REVOCATION, - /* Mutual close sequence. */ + /* If they want to close. */ PKT_CLOSE_CLEARING = PKT__PKT_CLOSE_CLEARING, - PKT_CLOSE_SIGNATURE = PKT__PKT_CLOSE_SIGNATURE, /* Something unexpected went wrong. */ PKT_ERROR = PKT__PKT_ERROR, @@ -120,7 +114,6 @@ enum state_input { CMD_SEND_HTLC_FULFILL, CMD_SEND_HTLC_FAIL, CMD_SEND_COMMIT, - CMD_CLOSE, INPUT_MAX }; @@ -130,8 +123,6 @@ enum state_peercond { PEER_CMD_OK, /* Don't send me commands, I'm busy. */ PEER_BUSY, - /* No more commands, I'm closing. */ - PEER_CLOSING, /* No more packets, I'm closed. */ PEER_CLOSED };