diff --git a/state.c b/state.c index 26f493fad..4f39691c3 100644 --- a/state.c +++ b/state.c @@ -19,6 +19,7 @@ static inline bool high_priority(enum state state) #define INIT_EFFECT_defer INPUT_NONE #define INIT_EFFECT_complete INPUT_NONE #define INIT_EFFECT_status CMD_STATUS_ONGOING +#define INIT_EFFECT_close_status CMD_STATUS_ONGOING #define INIT_EFFECT_faildata NULL #define INIT_EFFECT_stop_packets false #define INIT_EFFECT_stop_commands false @@ -44,6 +45,7 @@ void state_effect_init(struct state_effect *effect) effect->complete = INIT_EFFECT_complete; effect->status = INIT_EFFECT_status; effect->faildata = INIT_EFFECT_faildata; + effect->close_status = INIT_EFFECT_close_status; effect->stop_packets = INIT_EFFECT_stop_packets; effect->stop_commands = INIT_EFFECT_stop_commands; effect->close_timeout = INIT_EFFECT_close_timeout; @@ -468,6 +470,10 @@ enum state state(const enum state state, const struct state_data *sdata, fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); set_effect(effect, htlc_abandon, true); goto old_commit_spotted; + } else if (input_is(input, CMD_CLOSE)) { + fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); + set_effect(effect, htlc_abandon, true); + goto start_closing; } else if (input_is_pkt(input)) { fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); set_effect(effect, htlc_abandon, true); @@ -495,6 +501,9 @@ enum state state(const enum state state, const struct state_data *sdata, } else if (input_is(input, PKT_CLOSE)) { fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); goto accept_closing; + } else if (input_is(input, CMD_CLOSE)) { + fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); + goto start_closing; } else if (input_is_pkt(input)) { fail_cmd(effect, CMD_SEND_UPDATE_ANY, NULL); goto unexpected_pkt; @@ -529,6 +538,9 @@ enum state state(const enum state state, const struct state_data *sdata, } else if (input_is(input, CMD_CLOSE)) { set_effect(effect, htlc_abandon, true); goto start_closing; + } else if (input_is(input, PKT_CLOSE)) { + set_effect(effect, htlc_abandon, true); + goto accept_closing; } else if (input_is_pkt(input)) { set_effect(effect, htlc_abandon, true); goto unexpected_pkt; @@ -541,7 +553,7 @@ enum state state(const enum state state, const struct state_data *sdata, idata->pkt); if (err) goto err_start_unilateral_close; - complete_cmd(effect, CMD_CLOSE); + set_effect(effect, close_status, CMD_STATUS_SUCCESS); set_effect(effect, send, pkt_close_ack(effect, sdata)); set_effect(effect, broadcast, bitcoin_close(effect, sdata)); @@ -554,7 +566,7 @@ enum state state(const enum state state, const struct state_data *sdata, idata->pkt); if (err) goto err_start_unilateral_close; - complete_cmd(effect, CMD_CLOSE); + set_effect(effect, close_status, CMD_STATUS_SUCCESS); set_effect(effect, send, pkt_close_ack(effect, sdata)); set_effect(effect, broadcast, bitcoin_close(effect, sdata)); @@ -568,7 +580,7 @@ enum state state(const enum state state, const struct state_data *sdata, /* They didn't respond in time. Unilateral close. */ set_effect(effect, send, pkt_err(effect, "Close timed out")); - fail_cmd(effect, CMD_CLOSE, effect->send); + set_effect(effect, close_status, CMD_STATUS_FAILED); set_effect(effect, stop_commands, true); set_effect(effect, stop_packets, true); set_effect(effect, broadcast, @@ -584,7 +596,7 @@ enum state state(const enum state state, const struct state_data *sdata, return STATE_CLOSE_WAIT_CLOSE_OURCOMMIT; } - fail_cmd(effect, CMD_CLOSE, NULL); + set_effect(effect, close_status, CMD_STATUS_FAILED); set_effect(effect, stop_commands, true); goto fail_during_close; @@ -1005,7 +1017,7 @@ start_closing: */ /* Protocol doesn't (currently?) allow closing with HTLCs. */ if (committed_to_htlcs(sdata)) { - fail_cmd(effect, CMD_CLOSE, NULL); + set_effect(effect, close_status, CMD_STATUS_FAILED); err = pkt_err(effect, "Close forced due to HTLCs"); goto err_start_unilateral_close; } @@ -1035,7 +1047,7 @@ instant_close: * Closing, but we haven't sent anything to the blockchain so * there's nothing to clean up. */ - complete_cmd(effect, CMD_CLOSE); + set_effect(effect, close_status, CMD_STATUS_SUCCESS); /* FIXME: Should we tell other side we're going? */ set_effect(effect, stop_packets, true); set_effect(effect, stop_commands, true); diff --git a/state.h b/state.h index 8b6f3c061..fdcc053dd 100644 --- a/state.h +++ b/state.h @@ -38,6 +38,9 @@ struct state_effect { enum cmd_complete_status status; void *faildata; + /* Completing a CMD_CLOSE */ + enum cmd_complete_status close_status; + /* Stop taking packets? commands? */ bool stop_packets, stop_commands; diff --git a/test/test_state_coverage.c b/test/test_state_coverage.c index 0c9c0d3a6..96ea4a455 100644 --- a/test/test_state_coverage.c +++ b/test/test_state_coverage.c @@ -76,9 +76,10 @@ struct core_state { uint8_t capped_live_htlcs_to_them; uint8_t capped_live_htlcs_to_us; - + bool closing_cmd; bool valid; - uint8_t pad[5]; + + uint8_t pad[4]; }; struct state_data { @@ -165,6 +166,7 @@ static bool situation_eq(const struct situation *a, const struct situation *b) + sizeof(a->a.s.capped_htlc_spends_to_them) + sizeof(a->a.s.capped_live_htlcs_to_us) + sizeof(a->a.s.capped_live_htlcs_to_them) + + sizeof(a->a.s.closing_cmd) + sizeof(a->a.s.valid) + sizeof(a->a.s.pad))); return structeq(&a->a.s, &b->a.s) && structeq(&a->b.s, &b->b.s); @@ -939,6 +941,7 @@ static void sdata_init(struct state_data *sdata, sdata->core.event_notifies = 0; sdata->core.pkt_inputs = true; sdata->core.cmd_inputs = true; + sdata->core.closing_cmd = false; sdata->name = name; sdata->peer = other; } @@ -1180,12 +1183,20 @@ static const char *apply_effects(struct state_data *sdata, } } if (effect->complete != INPUT_NONE) { - if (!is_current_command(sdata, effect->complete)) + if (!is_current_command(sdata, effect->complete)) { return tal_fmt(NULL, "Completed %s not %s", input_name(effect->complete), input_name(sdata->core.current_command)); + } sdata->core.current_command = INPUT_NONE; } + if (effect->close_status != CMD_STATUS_ONGOING) { + if (!sdata->core.closing_cmd) + return tal_fmt(NULL, "%s but not closing", + effect->close_status == CMD_STATUS_SUCCESS ? "Success" + : "Failure"); + sdata->core.closing_cmd = false; + } if (effect->stop_packets) { if (!sdata->core.pkt_inputs) return "stop_packets twice"; @@ -1201,6 +1212,8 @@ static const char *apply_effects(struct state_data *sdata, if (sdata->core.current_command != INPUT_NONE) return tal_fmt(NULL, "stop_commands with pending command %s", input_name(sdata->core.current_command)); + if (sdata->core.closing_cmd) + return "stop_commands with pending CMD_CLOSE"; sdata->core.cmd_inputs = false; } if (effect->close_timeout != INPUT_NONE) { @@ -1792,21 +1805,29 @@ static struct trail *run_peer(const struct state_data *sdata, copy.core.event_notifies = old_notifies; } - /* Try sending commands (unless in init state, closed or - * already doing one). */ + /* We can send a close command even if already sending a + * (different) command. */ if (sdata->core.state != STATE_INIT_WITHANCHOR && sdata->core.state != STATE_INIT_NOANCHOR && sdata->core.cmd_inputs - && sdata->core.current_command == INPUT_NONE) { - unsigned int i; - - /* We can always send a close. */ - copy.core.current_command = CMD_CLOSE; - t = try_input(©, copy.core.current_command, idata, + && !sdata->core.closing_cmd) { + copy.core.closing_cmd = true; + t = try_input(©, CMD_CLOSE, idata, normalpath, errorpath, depth, hist); if (t) return t; + copy.core.closing_cmd = false; + } + + /* Try sending commands (unless in init state, closed or + * already doing one). */ + if (sdata->core.state != STATE_INIT_WITHANCHOR + && sdata->core.state != STATE_INIT_NOANCHOR + && sdata->core.cmd_inputs + && sdata->core.current_command == INPUT_NONE + && !sdata->core.closing_cmd) { + unsigned int i; /* Add a new HTLC if not at max. */ if (copy.num_htlcs_to_them < MAX_HTLCS) {