From a1f1f1eda82efa447f26a66683b2d3ebec7ae2d3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 7 Oct 2016 14:00:17 +1030 Subject: [PATCH] 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 --- daemon/db.c | 16 ++++++++++++++++ daemon/db.h | 2 ++ daemon/feechange.c | 3 +++ daemon/peer.c | 5 +++++ daemon/test/test.sh | 9 +++++++++ 5 files changed, 35 insertions(+) diff --git a/daemon/db.c b/daemon/db.c index a80dc7a24..94ca4907f 100644 --- a/daemon/db.c +++ b/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); diff --git a/daemon/db.h b/daemon/db.h index ce85c1435..41a957581 100644 --- a/daemon/db.h +++ b/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); diff --git a/daemon/feechange.c b/daemon/feechange.c index c3645a779..2991c6c90 100644 --- a/daemon/feechange.c +++ b/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); } diff --git a/daemon/peer.c b/daemon/peer.c index 331dc9e78..97a2c3a90 100644 --- a/daemon/peer.c +++ b/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)); diff --git a/daemon/test/test.sh b/daemon/test/test.sh index 1df721273..586397d72 100755 --- a/daemon/test/test.sh +++ b/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