state: return status of current command.

We temporarily move effect to the end of the arg list: we'll get rid
of it eventually.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

state.h

@@ -24,12 +24,6 @@ enum state_effect_type {
 	STATE_EFFECT_send_pkt,
 	STATE_EFFECT_watch,
 	STATE_EFFECT_unwatch,
-	STATE_EFFECT_cmd_defer,
-	STATE_EFFECT_cmd_requeue,
-	STATE_EFFECT_cmd_success,
-	/* (never applies to CMD_CLOSE) */
-	STATE_EFFECT_cmd_fail,
-	STATE_EFFECT_cmd_close_done,
 	/* FIXME: Use a watch for this?. */
 	STATE_EFFECT_close_timeout,
 	STATE_EFFECT_htlc_in_progress,
@@ -70,17 +64,6 @@ struct state_effect {
 		/* Events to no longer watch for. */
 		struct watch *unwatch;
 
-		/* Defer an input. */
-		enum state_input cmd_defer;
-
-		/* Requeue/succeed/fail command. */
-		enum state_input cmd_requeue;
-		enum state_input cmd_success;
-		void *cmd_fail;
-
-		/* CMD_CLOSE is complete (true if successful mutual close). */
-		bool cmd_close_done;
-
 		/* Set a timeout for close tx. */
 		enum state_input close_timeout;
 
@@ -136,11 +119,23 @@ union input {
 	struct htlc_progress *htlc_prog;
 };
 
-struct state_effect *state(const tal_t *ctx,
+enum command_status {
+	/* Nothing changed. */
+	CMD_NONE,
+	/* Command succeeded. */
+	CMD_SUCCESS,
+	/* HTLC-command needs re-issuing (theirs takes preference) */
+	CMD_REQUEUE,
+	/* Failed. */
+	CMD_FAIL
+};
+	
+enum command_status state(const tal_t *ctx,
 			  const enum state state,
 			  struct peer *peer,
 			  const enum state_input input,
-			   const union input *idata);
+			  const union input *idata,
+			  struct state_effect **effect);
 
 /* Any CMD_SEND_HTLC_* */
 #define CMD_SEND_UPDATE_ANY INPUT_MAX

test/test_state_coverage.c

@@ -104,9 +104,8 @@ struct core_state {
 
 	uint8_t capped_live_htlcs_to_them;
 	uint8_t capped_live_htlcs_to_us;
-	bool closing_cmd;
 	bool valid;
-	bool pad[1];
+	bool pad[2];
 };
 
 struct peer {
@@ -201,7 +200,6 @@ 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);
@@ -1188,7 +1186,6 @@ static void peer_init(struct peer *peer,
 	peer->pkt_data[0] = -1;
 	peer->core.current_command = INPUT_NONE;
 	peer->core.event_notifies = 0;
-	peer->core.closing_cmd = false;
 	peer->name = name;
 	peer->other = other;
 	peer->trail = NULL;
@@ -1308,18 +1305,6 @@ static void report_trail(const struct trail *t, const char *problem)
 	exit(1);
 }
 
-static bool is_current_command(const struct peer *peer,
-			       enum state_input cmd)
-{
-	if (cmd == CMD_SEND_UPDATE_ANY) {
-		return is_current_command(peer, CMD_SEND_HTLC_UPDATE)
-			|| is_current_command(peer, CMD_SEND_HTLC_FULFILL)
-			|| is_current_command(peer, CMD_SEND_HTLC_TIMEDOUT)
-			|| is_current_command(peer, CMD_SEND_HTLC_ROUTEFAIL);
-	}
-	return peer->core.current_command == cmd;
-}
-
 static void add_htlc(struct htlc *to_us, unsigned int *num_to_us,
 		     struct htlc *to_them, unsigned int *num_to_them,
 		     size_t arrsize,
@@ -1475,32 +1460,6 @@ static const char *apply_effects(struct peer *peer,
 			return "unset event unwatched";
 		peer->core.event_notifies &= ~effect->u.unwatch->events;
 		break;
-	case STATE_EFFECT_cmd_defer:
-		/* If it was current command, it is no longer. */
-		assert(is_current_command(peer, effect->u.cmd_defer));
-		/* We will resubmit this later anyway. */
-		peer->core.current_command = INPUT_NONE;
-		break;
-	case STATE_EFFECT_cmd_requeue:
-		assert(is_current_command(peer, effect->u.cmd_requeue));
-		peer->core.current_command = INPUT_NONE;
-		break;
-	case STATE_EFFECT_cmd_success:
-		assert(is_current_command(peer, effect->u.cmd_success));
-		peer->core.current_command = INPUT_NONE;
-		break;
-	case STATE_EFFECT_cmd_fail:
-		if (peer->core.current_command == INPUT_NONE)
-			return "Failed command with none current";
-		peer->core.current_command = INPUT_NONE;
-		break;
-	case STATE_EFFECT_cmd_close_done:
-		if (!peer->core.closing_cmd)
-			return tal_fmt(NULL, "%s but not closing",
-				       effect->u.cmd_close_done
-				       ? "Success" : "Failure");
-		peer->core.closing_cmd = false;
-		break;
 	case STATE_EFFECT_close_timeout:
 		add_event(&peer->core.event_notifies,
 			  effect->u.close_timeout);
@@ -1678,8 +1637,6 @@ static const char *check_changes(const struct peer *old, struct peer *new)
 				       input_name(new->core.current_command));
 	}
 	if (new->cond == PEER_CLOSED) {
-		if (new->core.closing_cmd)
-			return "cond CLOSED with pending CMD_CLOSE";
 		/* FIXME: Move to state core */
 		/* Can no longer receive packet timeouts, either. */
 		remove_event_(&new->core.event_notifies,
@@ -1690,6 +1647,7 @@ static const char *check_changes(const struct peer *old, struct peer *new)
 }
 
 static const char *apply_all_effects(const struct peer *old,
+				     enum command_status cstatus,
 				     struct peer *peer,
 				     const struct state_effect *effect,
 				     Pkt **output)
@@ -1697,6 +1655,17 @@ static const char *apply_all_effects(const struct peer *old,
 	const char *problem;
 	uint64_t effects = 0;
 	*output = NULL;
+
+	if (cstatus != CMD_NONE) {
+		assert(peer->core.current_command != INPUT_NONE);
+		/* We should only requeue HTLCs if we're lowprio */
+		if (cstatus == CMD_REQUEUE)
+			assert(!high_priority(old->core.state)
+			       && input_is(peer->core.current_command,
+					   CMD_SEND_UPDATE_ANY));
+		peer->core.current_command = INPUT_NONE;
+	}
+
 	problem = apply_effects(peer, effect, &effects, output);
 	if (!problem)
 		problem = check_effects(peer, effect);
@@ -1934,6 +1903,7 @@ static void try_input(const struct peer *peer,
 	const char *problem;
 	Pkt *output;
 	const tal_t *ctx = tal(NULL, char);
+	enum command_status cstatus;
 
 	copy_peers(&copy, &other, peer);
 
@@ -1943,7 +1913,7 @@ static void try_input(const struct peer *peer,
 	copy.trail = &t;
 
 	eliminate_input(&hist->inputs_per_state[copy.core.state], i);
-	effect = state(ctx, copy.core.state, &copy, i, idata);
+	cstatus = state(ctx, copy.core.state, &copy, i, idata, &effect);
 
 	newstate = get_state_effect(effect, peer->core.state);
 
@@ -1969,7 +1939,7 @@ static void try_input(const struct peer *peer,
 				get_send_pkt(effect));
 	}
 
-	problem = apply_all_effects(peer, &copy, effect, &output);
+	problem = apply_all_effects(peer, cstatus, &copy, effect, &output);
 	update_trail(&t, &copy, output);
 	if (problem)
 		report_trail(&t, problem);
@@ -2002,7 +1972,7 @@ static void try_input(const struct peer *peer,
 		 * And if we're being quick, always stop.
 		 */
 		if (quick
-		    || get_effect(effect, STATE_EFFECT_cmd_defer)
+		    || cstatus == CMD_REQUEUE
 		    || newstate == STATE_NORMAL_LOWPRIO
 		    || newstate == STATE_NORMAL_HIGHPRIO
 		    || i == BITCOIN_ANCHOR_OTHERSPEND
@@ -2181,10 +2151,8 @@ static void run_peer(const struct peer *peer,
 	if (peer->core.state != STATE_INIT
 	    && (peer->cond == PEER_CMD_OK
 		|| peer->cond == PEER_BUSY)) {
-		copy.core.closing_cmd = true;
 		try_input(&copy, CMD_CLOSE, idata,
 			  normalpath, errorpath, prev_trail, hist);
-		copy.core.closing_cmd = false;
 	}
 
 	/* Try sending commands if allowed. */
@@ -2334,8 +2302,9 @@ static enum state_input **map_inputs(void)
 		/* This adds to mapping_inputs every input_is() call */
 		if (!state_is_error(i)) {
 			struct peer dummy;
+			struct state_effect *effect;
 			memset(&dummy, 0, sizeof(dummy));
-			state(ctx, i, &dummy, INPUT_NONE, NULL);
+			state(ctx, i, &dummy, INPUT_NONE, NULL, &effect);
 		}
 		inps[i] = mapping_inputs;
 	}