daemon: fix feechange logic.

Firstly, we need to update the staging fee amount when we queue a change.
Secondly we need to remove completed fee updates, otherwise we hit a
database constraint that peer & state are unique.

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

daemon/db.c

@@ -1515,6 +1515,22 @@ void db_update_feechange_state(struct peer *peer,
 	tal_free(ctx);
 }
 
+void db_remove_feechange(struct peer *peer, const struct feechange *feechange,
+			 enum htlc_state oldstate)
+{
+	const char *ctx = tal(peer, char);
+	const char *peerid = pubkey_to_hexstr(ctx, peer->dstate->secpctx, peer->id);
+
+	log_debug(peer->log, "%s(%s)", __func__, peerid);
+	assert(peer->dstate->db->in_transaction);
+
+	db_exec(__func__, peer->dstate, 
+		"DELETE FROM feechanges WHERE peer=x'%s' AND state='%s';",
+		peerid, feechange_state_name(oldstate));
+
+	tal_free(ctx);
+}
+
 void db_update_state(struct peer *peer)
 {
 	const char *ctx = tal(peer, char);

daemon/db.h

@@ -58,6 +58,8 @@ void db_resolve_invoice(struct lightningd_state *dstate,
 void db_update_feechange_state(struct peer *peer,
 			       const struct feechange *f,
 			       enum htlc_state oldstate);
+void db_remove_feechange(struct peer *peer, const struct feechange *feechange,
+			 enum htlc_state oldstate);
 void db_new_commit_info(struct peer *peer, enum side side,
 			const struct sha256 *prev_rhash);
 void db_remove_their_prev_revocation_hash(struct peer *peer);

daemon/feechange.c

@@ -134,6 +134,9 @@ void feechange_changestate(struct peer *peer,
 		if (newstate == RCVD_FEECHANGE_COMMIT
 		    || newstate == SENT_FEECHANGE_COMMIT)
 			db_new_feechange(peer, f);
+		else if (newstate == RCVD_FEECHANGE_ACK_REVOCATION
+			 || SENT_FEECHANGE_ACK_REVOCATION)
+			db_remove_feechange(peer, f, oldstate);
 		else
 			db_update_feechange_state(peer, f, oldstate);
 	}

daemon/peer.c

@@ -2056,6 +2056,9 @@ static bool want_feechange(const struct peer *peer)
 {
 	if (!state_is_normal(peer->state) && !state_is_shutdown(peer->state))
 		return false;
+	log_debug(peer->log, "Current fee_rate: %"PRIu64" want %"PRIu64,
+		  peer->local.staging_cstate->fee_rate,
+		  desired_commit_feerate(peer->dstate));
 	return peer->local.staging_cstate->fee_rate
 		!= desired_commit_feerate(peer->dstate);
 }
@@ -2165,6 +2168,7 @@ static void maybe_propose_new_feerate(struct peer *peer)
 
 	set_feechange(peer, rate, SENT_FEECHANGE);
 	queue_pkt_feechange(peer, rate);
+	peer->local.staging_cstate->fee_rate = rate;
 }
 
 static void do_commit(struct peer *peer, struct command *jsoncmd)
@@ -4729,6 +4733,7 @@ static void json_feerate(struct command *cmd,
 		command_fail(cmd, "Invalid feerate");
 		return;
 	}
+	log_debug(cmd->jcon->log, "Fee rate changed to %"PRIu64, feerate);
 	cmd->dstate->config.default_fee_rate = feerate;
 	
 	command_success(cmd, null_response(cmd));

daemon/test/test.sh

@@ -568,6 +568,15 @@ if [ -n "$DIFFERENT_FEES" ]; then
 
     check_status $(($AMOUNT - $HTLC_AMOUNT - $NO_HTLCS_FEE / 2)) $(($NO_HTLCS_FEE / 2)) "" $(($HTLC_AMOUNT - $NO_HTLCS_FEE / 2)) $(($NO_HTLCS_FEE / 2)) "" 
 
+    # Change back.
+    lcli2 dev-feerate 50000
+    $CLI generate 1
+    [ ! -n "$MANUALCOMMIT" ] || lcli2 dev-commit $ID1
+    [ ! -n "$MANUALCOMMIT" ] || lcli1 dev-commit $ID2
+
+    check_status_single lcli1 $(($AMOUNT - $HTLC_AMOUNT - $NO_HTLCS_FEE / 2)) $(($NO_HTLCS_FEE / 2)) "" $(($HTLC_AMOUNT - $NO_HTLCS_FEE / 2)) $(($NO_HTLCS_FEE / 2)) ""
+    check_status_single lcli2 $(($HTLC_AMOUNT - $NO_HTLCS_FEE2 / 2)) $(($NO_HTLCS_FEE2 / 2)) "" $(($AMOUNT - $HTLC_AMOUNT - $NO_HTLCS_FEE2 / 2)) $(($NO_HTLCS_FEE2 / 2)) ""
+
     lcli1 close $ID2
     # Make sure they notice it.
     check_peerstate lcli1 STATE_MUTUAL_CLOSING