From fd370075f253fbdb94ccd1962afc28054360b249 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 22 Jan 2016 06:41:46 +1030 Subject: [PATCH] state: use STATE_INIT and separate inputs to decide on anchor. This is conceptually cleaner, especially since it means we're running a command until we're set up (which prevents other commands, so no special case needed). Signed-off-by: Rusty Russell --- state.c | 79 +++++++++++++++++++++++++++----------- state_types.h | 5 ++- test/test_state_coverage.c | 40 ++++++++++--------- 3 files changed, 81 insertions(+), 43 deletions(-) diff --git a/state.c b/state.c index 6d98b2268..0dfd72764 100644 --- a/state.c +++ b/state.c @@ -50,24 +50,28 @@ struct state_effect *state(const tal_t *ctx, /* * Initial channel opening states. */ - case STATE_INIT_NOANCHOR: - assert(input == INPUT_NONE); - add_effect(&effect, send_pkt, - pkt_open(ctx, peer, - OPEN_CHANNEL__ANCHOR_OFFER__WONT_CREATE_ANCHOR)); - return next_state(ctx, effect, - STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR); - case STATE_INIT_WITHANCHOR: - assert(input == INPUT_NONE); - add_effect(&effect, send_pkt, - pkt_open(ctx, peer, - OPEN_CHANNEL__ANCHOR_OFFER__WILL_CREATE_ANCHOR)); - return next_state(ctx, effect, STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR); + case STATE_INIT: + if (input_is(input, CMD_OPEN_WITH_ANCHOR)) { + add_effect(&effect, send_pkt, + pkt_open(ctx, peer, + OPEN_CHANNEL__ANCHOR_OFFER__WILL_CREATE_ANCHOR)); + return next_state(ctx, effect, + STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR); + } else if (input_is(input, CMD_OPEN_WITHOUT_ANCHOR)) { + add_effect(&effect, send_pkt, + pkt_open(ctx, peer, + OPEN_CHANNEL__ANCHOR_OFFER__WONT_CREATE_ANCHOR)); + return next_state(ctx, effect, + STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR); + } + break; case STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR: if (input_is(input, PKT_OPEN)) { err = accept_pkt_open(ctx, peer, idata->pkt, &effect); - if (err) + if (err) { + add_effect(&effect, cmd_fail, NULL); goto err_close_nocleanup; + } return next_state(ctx, effect, STATE_OPEN_WAIT_FOR_ANCHOR); } else if (input_is(input, CMD_SEND_HTLC_UPDATE)) { /* Can't do this until we're open. */ @@ -75,16 +79,20 @@ struct state_effect *state(const tal_t *ctx, /* No state change. */ return effect; } else if (input_is(input, CMD_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto instant_close; } else if (input_is_pkt(input)) { + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt_nocleanup; } break; case STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR: if (input_is(input, PKT_OPEN)) { err = accept_pkt_open(ctx, peer, idata->pkt, &effect); - if (err) + if (err) { + add_effect(&effect, cmd_fail, NULL); goto err_close_nocleanup; + } add_effect(&effect, send_pkt, pkt_anchor(ctx, peer)); return next_state(ctx, effect, STATE_OPEN_WAIT_FOR_COMMIT_SIG); @@ -94,16 +102,20 @@ struct state_effect *state(const tal_t *ctx, /* No state change. */ return effect; } else if (input_is(input, CMD_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto instant_close; } else if (input_is_pkt(input)) { + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt_nocleanup; } break; case STATE_OPEN_WAIT_FOR_ANCHOR: if (input_is(input, PKT_OPEN_ANCHOR)) { err = accept_pkt_anchor(ctx, peer, idata->pkt, &effect); - if (err) + if (err) { + add_effect(&effect, cmd_fail, NULL); goto err_close_nocleanup; + } add_effect(&effect, send_pkt, pkt_open_commit_sig(ctx, peer)); add_effect(&effect, watch, @@ -122,8 +134,10 @@ struct state_effect *state(const tal_t *ctx, /* No state change. */ return effect; } else if (input_is(input, CMD_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto instant_close; } else if (input_is_pkt(input)) { + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt_nocleanup; } break; @@ -131,8 +145,10 @@ struct state_effect *state(const tal_t *ctx, if (input_is(input, PKT_OPEN_COMMIT_SIG)) { err = accept_pkt_open_commit_sig(ctx, peer, idata->pkt, &effect); - if (err) + if (err) { + add_effect(&effect, cmd_fail, NULL); goto err_start_unilateral_close; + } add_effect(&effect, broadcast_tx, bitcoin_anchor(ctx, peer)); add_effect(&effect, watch, @@ -149,8 +165,10 @@ struct state_effect *state(const tal_t *ctx, /* No state change. */ return effect; } else if (input_is(input, CMD_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto instant_close; } else if (input_is_pkt(input)) { + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt_nocleanup; } break; @@ -161,6 +179,7 @@ struct state_effect *state(const tal_t *ctx, return next_state(ctx, effect, STATE_OPEN_WAIT_FOR_COMPLETE_OURANCHOR); } else if (input_is(input, BITCOIN_ANCHOR_UNSPENT)) { + add_effect(&effect, cmd_fail, NULL); goto anchor_unspent; } else if (input_is(input, PKT_OPEN_COMPLETE)) { /* Ignore until we've hit depth ourselves. */ @@ -178,6 +197,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, INPUT_NONE)); + add_effect(&effect, cmd_fail, NULL); goto them_unilateral; } else if (input_is(input, BITCOIN_ANCHOR_OTHERSPEND)) { /* This should be impossible. */ @@ -188,6 +208,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, INPUT_NONE)); + add_effect(&effect, cmd_fail, NULL); goto start_closing; } else if (input_is(input, PKT_CLOSE)) { /* We no longer care about anchor depth. */ @@ -195,6 +216,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, INPUT_NONE)); + add_effect(&effect, cmd_fail, NULL); goto accept_closing; } else if (input_is_pkt(input)) { /* We no longer care about anchor depth. */ @@ -202,7 +224,8 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, INPUT_NONE)); - goto unexpected_pkt; + add_effect(&effect, cmd_fail, NULL); + goto unexpected_pkt_nocleanup; } break; case STATE_OPEN_WAITING_THEIRANCHOR: @@ -216,6 +239,7 @@ struct state_effect *state(const tal_t *ctx, pkt_open_complete(ctx, peer)); return next_state(ctx, effect, STATE_OPEN_WAIT_FOR_COMPLETE_THEIRANCHOR); } else if (input_is(input, BITCOIN_ANCHOR_UNSPENT)) { + add_effect(&effect, cmd_fail, NULL); goto anchor_unspent; } else if (input_is(input, BITCOIN_ANCHOR_OTHERSPEND)) { /* This should be impossible. */ @@ -226,6 +250,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, BITCOIN_ANCHOR_TIMEOUT)); + add_effect(&effect, cmd_fail, NULL); goto them_unilateral; } else if (input_is(input, PKT_OPEN_COMPLETE)) { /* Ignore until we've hit depth ourselves. */ @@ -243,6 +268,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, BITCOIN_ANCHOR_TIMEOUT)); + add_effect(&effect, cmd_fail, NULL); goto start_closing; } else if (input_is(input, PKT_CLOSE)) { /* We no longer care about anchor depth. */ @@ -250,6 +276,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, BITCOIN_ANCHOR_TIMEOUT)); + add_effect(&effect, cmd_fail, NULL); goto accept_closing; } else if (input_is_pkt(input)) { /* We no longer care about anchor depth. */ @@ -257,6 +284,7 @@ struct state_effect *state(const tal_t *ctx, bitcoin_unwatch_anchor_depth(ctx, peer, BITCOIN_ANCHOR_DEPTHOK, BITCOIN_ANCHOR_TIMEOUT)); + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt; } break; @@ -264,31 +292,36 @@ struct state_effect *state(const tal_t *ctx, case STATE_OPEN_WAIT_FOR_COMPLETE_THEIRANCHOR: if (input_is(input, PKT_OPEN_COMPLETE)) { /* Ready for business! Anchorer goes first. */ - if (state == STATE_OPEN_WAIT_FOR_COMPLETE_OURANCHOR) + if (state == STATE_OPEN_WAIT_FOR_COMPLETE_OURANCHOR) { + add_effect(&effect, cmd_success, CMD_OPEN_WITH_ANCHOR); return next_state(ctx, effect, STATE_NORMAL_HIGHPRIO); - else + } else { + add_effect(&effect, cmd_success, CMD_OPEN_WITHOUT_ANCHOR); return next_state(ctx, effect, STATE_NORMAL_LOWPRIO); + } } else if (input_is(input, BITCOIN_ANCHOR_UNSPENT)) { + add_effect(&effect, cmd_fail, NULL); goto anchor_unspent; /* Nobody should be able to spend anchor, except via the * commit txs. */ } else if (input_is(input, BITCOIN_ANCHOR_OTHERSPEND)) { return next_state(ctx, effect, STATE_ERR_INFORMATION_LEAK); } else if (input_is(input, BITCOIN_ANCHOR_THEIRSPEND)) { + add_effect(&effect, cmd_fail, NULL); goto them_unilateral; - } else if (input_is(input, PKT_OPEN_COMPLETE)) { - /* Ready for business! */ - return next_state(ctx, effect, STATE_NORMAL_HIGHPRIO); } else if (input_is(input, CMD_SEND_HTLC_UPDATE)) { /* Can't do this until we're open. */ add_effect(&effect, cmd_defer, input); /* No state change. */ return effect; } else if (input_is(input, CMD_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto start_closing; } else if (input_is(input, PKT_CLOSE)) { + add_effect(&effect, cmd_fail, NULL); goto accept_closing; } else if (input_is_pkt(input)) { + add_effect(&effect, cmd_fail, NULL); goto unexpected_pkt; } break; diff --git a/state_types.h b/state_types.h index 975d2fafb..7322eb4df 100644 --- a/state_types.h +++ b/state_types.h @@ -12,8 +12,7 @@ #define STATE_CLOSE_SPENDOURS_BIT 32 enum state { - STATE_INIT_NOANCHOR, - STATE_INIT_WITHANCHOR, + STATE_INIT, /* * Opening. @@ -268,6 +267,8 @@ enum state_input { INPUT_RVALUE, /* Commands */ + CMD_OPEN_WITH_ANCHOR, + CMD_OPEN_WITHOUT_ANCHOR, CMD_SEND_HTLC_UPDATE, CMD_SEND_HTLC_FULFILL, CMD_SEND_HTLC_TIMEDOUT, diff --git a/test/test_state_coverage.c b/test/test_state_coverage.c index b975ec58e..a1ff01404 100644 --- a/test/test_state_coverage.c +++ b/test/test_state_coverage.c @@ -1167,11 +1167,10 @@ struct htlc_rval *r_value_from_pkt(const tal_t *ctx, const Pkt *pkt) static void peer_init(struct peer *peer, struct peer *other, - enum state_input initstate, const char *name) { - peer->core.state = initstate; - peer->core.num_outputs = 1; + peer->core.state = STATE_INIT; + peer->core.num_outputs = 0; peer->current_htlc.htlc.id = -1; peer->num_htlcs_to_us = 0; peer->num_htlcs_to_them = 0; @@ -1184,7 +1183,6 @@ static void peer_init(struct peer *peer, memset(peer->core.outputs, 0, sizeof(peer->core.outputs)); peer->core.deferred_pkt = INPUT_NONE; peer->core.deferred_state = STATE_MAX; - peer->core.outputs[0] = INPUT_NONE; peer->pkt_data[0] = -1; peer->core.current_command = INPUT_NONE; peer->core.event_notifies = 0; @@ -2141,6 +2139,18 @@ static void run_peer(const struct peer *peer, /* We want to frob some things... */ copy_peers(©, &other, peer); + /* If in init state, we can only send start command. */ + if (peer->core.state == STATE_INIT) { + if (streq(peer->name, "A")) + copy.core.current_command = CMD_OPEN_WITH_ANCHOR; + else + copy.core.current_command = CMD_OPEN_WITHOUT_ANCHOR; + try_input(©, copy.core.current_command, idata, + normalpath, errorpath, + prev_trail, hist); + return; + } + /* Try the event notifiers */ old_notifies = copy.core.event_notifies; for (i = 0; i < 64; i++) { @@ -2158,8 +2168,7 @@ static void run_peer(const struct peer *peer, /* We can send a close command even if already sending a * (different) command. */ - if (peer->core.state != STATE_INIT_WITHANCHOR - && peer->core.state != STATE_INIT_NOANCHOR + if (peer->core.state != STATE_INIT && peer->core.cmd_inputs && !peer->core.closing_cmd) { copy.core.closing_cmd = true; @@ -2168,11 +2177,8 @@ static void run_peer(const struct peer *peer, copy.core.closing_cmd = false; } - /* Try sending commands (unless in init state, closed or - * already doing one). */ - if (peer->core.state != STATE_INIT_WITHANCHOR - && peer->core.state != STATE_INIT_NOANCHOR - && peer->core.cmd_inputs + /* Try sending commands (unless closed or already doing one). */ + if (peer->core.cmd_inputs && peer->core.current_command == INPUT_NONE && !peer->core.closing_cmd) { unsigned int i; @@ -2435,10 +2441,10 @@ int main(int argc, char *argv[]) #else quick = true; #endif - + /* Initialize universe. */ - peer_init(&a, &b, STATE_INIT_WITHANCHOR, "A"); - peer_init(&b, &a, STATE_INIT_NOANCHOR, "B"); + peer_init(&a, &b, "A"); + peer_init(&b, &a, "B"); if (!sithash_update(&hist.sithash, &a)) abort(); failhash_init(&failhash); @@ -2484,15 +2490,13 @@ int main(int argc, char *argv[]) continue; /* A supplied anchor, so doesn't enter NOANCHOR states. */ - if (i == STATE_INIT_NOANCHOR - || i == STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR + if (i == STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR || i == STATE_OPEN_WAIT_FOR_ANCHOR || i == STATE_OPEN_WAITING_THEIRANCHOR || i == STATE_OPEN_WAIT_FOR_COMPLETE_THEIRANCHOR || i == STATE_ERR_ANCHOR_TIMEOUT) a_expect = false; - if (i == STATE_INIT_WITHANCHOR - || i == STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR + if (i == STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR || i == STATE_OPEN_WAIT_FOR_COMMIT_SIG || i == STATE_OPEN_WAIT_FOR_COMPLETE_OURANCHOR || i == STATE_OPEN_WAITING_OURANCHOR)