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 <rusty@rustcorp.com.au>

state.c

@@ -50,24 +50,28 @@ struct state_effect *state(const tal_t *ctx,
 	/*
 	 * Initial channel opening states.
 	 */
-	case STATE_INIT_NOANCHOR:
+	case STATE_INIT:
-		assert(input == INPUT_NONE);
+		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);
-	case STATE_INIT_WITHANCHOR:
+		}
-		assert(input == INPUT_NONE);
+		break;
-		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_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;

state_types.h

@@ -12,8 +12,7 @@
 #define STATE_CLOSE_SPENDOURS_BIT 32
 
 enum state {
-	STATE_INIT_NOANCHOR,
+	STATE_INIT,
-	STATE_INIT_WITHANCHOR,
 
 	/*
 	 * 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,

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.state = STATE_INIT;
-	peer->core.num_outputs = 1;
+	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(&copy, &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, 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
+	if (peer->core.state != STATE_INIT
-	    && peer->core.state != STATE_INIT_NOANCHOR
 	    && 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
+	/* Try sending commands (unless closed or already doing one). */
-	 * already doing one). */
+	if (peer->core.cmd_inputs
-	if (peer->core.state != STATE_INIT_WITHANCHOR
-	    && peer->core.state != STATE_INIT_NOANCHOR
-	    && peer->core.cmd_inputs
 	    && peer->core.current_command == INPUT_NONE
 	    && !peer->core.closing_cmd) {
 		unsigned int i;
@@ -2437,8 +2443,8 @@ int main(int argc, char *argv[])
 #endif
 
 	/* Initialize universe. */
-	peer_init(&a, &b, STATE_INIT_WITHANCHOR, "A");
+	peer_init(&a, &b, "A");
-	peer_init(&b, &a, STATE_INIT_NOANCHOR, "B");
+	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
+		if (i == STATE_OPEN_WAIT_FOR_OPEN_NOANCHOR
-		    || 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
+		if (i == STATE_OPEN_WAIT_FOR_OPEN_WITHANCHOR
-		    || 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)