Skip to content

Commit

Permalink
lightningd: save the fee_states into the database.
Browse files Browse the repository at this point in the history
This is the final step: we pass the complete fee_states to and from
channeld.

Changelog-Fixed: "Bad commitment signature" closing channels when we sent back-to-back update_fee messages across multiple reconnects.
Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
rustyrussell committed Dec 12, 2019
1 parent 64f198e commit 27828ff
Show file tree
Hide file tree
Showing 19 changed files with 191 additions and 90 deletions.
10 changes: 5 additions & 5 deletions channeld/channel_wire.csv
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <common/cryptomsg.h>
#include <common/channel_config.h>
#include <common/derive_basepoints.h>
#include <common/fee_states.h>
#include <common/per_peer_state.h>

# Begin! (passes gossipd-client fd)
Expand All @@ -12,8 +13,7 @@ msgdata,channel_init,funding_satoshi,amount_sat,
msgdata,channel_init,minimum_depth,u32,
msgdata,channel_init,our_config,channel_config,
msgdata,channel_init,their_config,channel_config,
# FIXME: Fix generate-wire.py to allow NUM_SIDES*u32 here.,
msgdata,channel_init,feerate_per_kw,u32,2
msgdata,channel_init,fee_states,fee_states,
msgdata,channel_init,feerate_min,u32,
msgdata,channel_init,feerate_max,u32,
msgdata,channel_init,first_commit_sig,bitcoin_signature,
Expand Down Expand Up @@ -108,7 +108,7 @@ msgdata,channel_got_funding_locked,next_per_commit_point,pubkey,
# When we send a commitment_signed message, tell master.
msgtype,channel_sending_commitsig,1020
msgdata,channel_sending_commitsig,commitnum,u64,
msgdata,channel_sending_commitsig,feerate,u32,
msgdata,channel_sending_commitsig,fee_states,fee_states,
# SENT_ADD_COMMIT, SENT_REMOVE_ACK_COMMIT, SENT_ADD_ACK_COMMIT, SENT_REMOVE_COMMIT
msgdata,channel_sending_commitsig,num_changed,u16,
msgdata,channel_sending_commitsig,changed,changed_htlc,num_changed
Expand All @@ -122,7 +122,7 @@ msgtype,channel_sending_commitsig_reply,1120
# When we have a commitment_signed message, tell master to remember.
msgtype,channel_got_commitsig,1021
msgdata,channel_got_commitsig,commitnum,u64,
msgdata,channel_got_commitsig,feerate,u32,
msgdata,channel_got_commitsig,fee_states,fee_states,
msgdata,channel_got_commitsig,signature,bitcoin_signature,
msgdata,channel_got_commitsig,num_htlcs,u16,
msgdata,channel_got_commitsig,htlc_signature,secp256k1_ecdsa_signature,num_htlcs
Expand Down Expand Up @@ -150,7 +150,7 @@ msgdata,channel_got_revoke,revokenum,u64,
msgdata,channel_got_revoke,per_commitment_secret,secret,
msgdata,channel_got_revoke,next_per_commit_point,pubkey,
# RCVD_ADD_ACK_REVOCATION, RCVD_REMOVE_ACK_REVOCATION, RCVD_ADD_REVOCATION, RCVD_REMOVE_REVOCATION
msgdata,channel_got_revoke,feerate,u32,
msgdata,channel_got_revoke,fee_states,fee_states,
msgdata,channel_got_revoke,num_changed,u16,
msgdata,channel_got_revoke,changed,changed_htlc,num_changed
# Wait for reply, to make sure it's on disk before we continue
Expand Down
28 changes: 15 additions & 13 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ static struct changed_htlc *changed_htlc_arr(const tal_t *ctx,

static u8 *sending_commitsig_msg(const tal_t *ctx,
u64 remote_commit_index,
u32 remote_feerate,
const struct fee_states *fee_states,
const struct htlc **changed_htlcs,
const struct bitcoin_signature *commit_sig,
const secp256k1_ecdsa_signature *htlc_sigs)
Expand All @@ -734,7 +734,7 @@ static u8 *sending_commitsig_msg(const tal_t *ctx,
* committed to. */
changed = changed_htlc_arr(tmpctx, changed_htlcs);
msg = towire_channel_sending_commitsig(ctx, remote_commit_index,
remote_feerate,
fee_states,
changed, commit_sig, htlc_sigs);
return msg;
}
Expand Down Expand Up @@ -1181,7 +1181,7 @@ static void send_commit(struct peer *peer)
status_debug("Telling master we're about to commit...");
/* Tell master to save this next commit to database, then wait. */
msg = sending_commitsig_msg(NULL, peer->next_index[REMOTE],
channel_feerate(peer->channel, REMOTE),
peer->channel->fee_states,
changed_htlcs,
&commit_sig,
htlc_sigs);
Expand Down Expand Up @@ -1364,10 +1364,12 @@ static void send_revocation(struct peer *peer,

/* Tell master daemon about commitsig (and by implication, that we're
* sending revoke_and_ack), then wait for it to ack. */
/* We had to do this after channel_sending_revoke_and_ack, since we
* want it to save the fee_states produced there. */
msg_for_master
= towire_channel_got_commitsig(NULL,
peer->next_index[LOCAL] - 1,
channel_feerate(peer->channel, LOCAL),
peer->channel->fee_states,
commit_sig, htlc_sigs,
added,
shared_secret,
Expand Down Expand Up @@ -1517,7 +1519,7 @@ static u8 *got_revoke_msg(const tal_t *ctx, u64 revoke_num,
const struct secret *per_commitment_secret,
const struct pubkey *next_per_commit_point,
const struct htlc **changed_htlcs,
u32 feerate)
const struct fee_states *fee_states)
{
u8 *msg;
struct changed_htlc *changed = tal_arr(tmpctx, struct changed_htlc, 0);
Expand All @@ -1536,7 +1538,7 @@ static u8 *got_revoke_msg(const tal_t *ctx, u64 revoke_num,
}

msg = towire_channel_got_revoke(ctx, revoke_num, per_commitment_secret,
next_per_commit_point, feerate, changed);
next_per_commit_point, fee_states, changed);
return msg;
}

Expand Down Expand Up @@ -1596,7 +1598,7 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg)
msg = got_revoke_msg(NULL, peer->revocations_received++,
&old_commit_secret, &next_per_commit,
changed_htlcs,
channel_feerate(peer->channel, LOCAL));
peer->channel->fee_states);
master_wait_sync_reply(tmpctx, peer, take(msg),
WIRE_CHANNEL_GOT_REVOKE_REPLY);

Expand Down Expand Up @@ -2946,7 +2948,7 @@ static void init_channel(struct peer *peer)
bool reconnected;
u8 *funding_signed;
const u8 *msg;
u32 feerate_per_kw[NUM_SIDES];
struct fee_states *fee_states;
u32 minimum_depth, failheight;
struct secret last_remote_per_commit_secret;
secp256k1_ecdsa_signature *remote_ann_node_sig;
Expand All @@ -2964,7 +2966,7 @@ static void init_channel(struct peer *peer)
&funding,
&minimum_depth,
&conf[LOCAL], &conf[REMOTE],
feerate_per_kw,
&fee_states,
&peer->feerate_min, &peer->feerate_max,
&peer->their_commit_sig,
&peer->pps,
Expand Down Expand Up @@ -3021,15 +3023,15 @@ static void init_channel(struct peer *peer)
" next_idx_local = %"PRIu64
" next_idx_remote = %"PRIu64
" revocations_received = %"PRIu64
" feerates %u/%u (range %u-%u)",
" feerates %s range %u-%u",
side_to_str(funder),
type_to_string(tmpctx, struct pubkey,
&peer->remote_per_commit),
type_to_string(tmpctx, struct pubkey,
&peer->old_remote_per_commit),
peer->next_index[LOCAL], peer->next_index[REMOTE],
peer->revocations_received,
feerate_per_kw[LOCAL], feerate_per_kw[REMOTE],
type_to_string(tmpctx, struct fee_states, fee_states),
peer->feerate_min, peer->feerate_max);

status_debug("option_static_remotekey = %u", option_static_remotekey);
Expand Down Expand Up @@ -3063,7 +3065,7 @@ static void init_channel(struct peer *peer)
minimum_depth,
funding,
local_msat,
feerate_per_kw,
take(fee_states),
&conf[LOCAL], &conf[REMOTE],
&points[LOCAL], &points[REMOTE],
&funding_pubkey[LOCAL],
Expand Down Expand Up @@ -3098,7 +3100,7 @@ static void init_channel(struct peer *peer)

/* Default desired feerate is the feerate we set for them last. */
if (peer->channel->funder == LOCAL)
peer->desired_feerate = feerate_per_kw[REMOTE];
peer->desired_feerate = channel_feerate(peer->channel, REMOTE);

/* from now we need keep watch over WIRE_CHANNEL_FUNDING_DEPTH */
peer->depth_togo = minimum_depth;
Expand Down
7 changes: 2 additions & 5 deletions channeld/full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ struct channel *new_full_channel(const tal_t *ctx,
u32 minimum_depth,
struct amount_sat funding,
struct amount_msat local_msat,
const u32 feerate_per_kw[NUM_SIDES],
const struct fee_states *fee_states,
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
Expand All @@ -102,16 +102,13 @@ struct channel *new_full_channel(const tal_t *ctx,
bool option_static_remotekey,
enum side funder)
{
/* FIXME: Support full feestates! */
struct fee_states *fee_states = new_fee_states(NULL, funder,
&feerate_per_kw[funder]);
struct channel *channel = new_initial_channel(ctx,
funding_txid,
funding_txout,
minimum_depth,
funding,
local_msat,
take(fee_states),
fee_states,
local, remote,
local_basepoints,
remote_basepoints,
Expand Down
5 changes: 2 additions & 3 deletions channeld/full_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
* @minimum_depth: The minimum confirmations needed for funding transaction.
* @funding: The commitment transaction amount.
* @local_msat: The amount for the local side (remainder goes to remote)
* @feerate_per_kw: feerate per kiloweight (satoshis) for the commitment
* transaction and HTLCS for each side.
* @fee_states: The fee update states.
* @local: local channel configuration
* @remote: remote channel configuration
* @local_basepoints: local basepoints.
Expand All @@ -34,7 +33,7 @@ struct channel *new_full_channel(const tal_t *ctx,
u32 minimum_depth,
struct amount_sat funding,
struct amount_msat local_msat,
const u32 feerate_per_kw[NUM_SIDES],
const struct fee_states *fee_states,
const struct channel_config *local,
const struct channel_config *remote,
const struct basepoints *local_basepoints,
Expand Down
7 changes: 5 additions & 2 deletions channeld/test/run-full_channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ int main(void)
lchannel = new_full_channel(tmpctx,
&funding_txid, funding_output_index, 0,
funding_amount, to_local,
feerate_per_kw,
take(new_fee_states(NULL, LOCAL,
&feerate_per_kw[LOCAL])),
local_config,
remote_config,
&localbase, &remotebase,
Expand All @@ -479,7 +480,8 @@ int main(void)
rchannel = new_full_channel(tmpctx,
&funding_txid, funding_output_index, 0,
funding_amount, to_remote,
feerate_per_kw,
take(new_fee_states(NULL, REMOTE,
&feerate_per_kw[REMOTE])),
remote_config,
local_config,
&remotebase, &localbase,
Expand Down Expand Up @@ -655,6 +657,7 @@ int main(void)

/* No memory leaks please */
wally_cleanup(0);
take_cleanup();
tal_free(tmpctx);

/* FIXME: Do BOLT comparison! */
Expand Down
8 changes: 5 additions & 3 deletions devtools/mkcommit.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <channeld/full_channel.h>
#include <common/amount.h>
#include <common/derive_basepoints.h>
#include <common/fee_states.h>
#include <common/htlc_wire.h>
#include <common/key_derive.h>
#include <common/keyset.h>
Expand Down Expand Up @@ -251,7 +252,7 @@ int main(int argc, char *argv[])
struct amount_sat funding_amount;
struct bitcoin_txid funding_txid;
unsigned int funding_outnum;
u32 feerate_per_kw[NUM_SIDES];
u32 feerate_per_kw;
struct pubkey local_per_commit_point, remote_per_commit_point;
struct bitcoin_signature local_sig, remote_sig;
struct channel_config localconfig, remoteconfig;
Expand Down Expand Up @@ -318,7 +319,7 @@ int main(int argc, char *argv[])
if (!parse_amount_sat(&funding_amount, argv[argnum], strlen(argv[argnum])))
errx(1, "Bad funding-amount");
argnum++;
feerate_per_kw[LOCAL] = feerate_per_kw[REMOTE] = atoi(argv[argnum++]);
feerate_per_kw = atoi(argv[argnum++]);
if (!parse_amount_msat(&local_msat,
argv[argnum], strlen(argv[argnum])))
errx(1, "Bad local-msat");
Expand Down Expand Up @@ -384,7 +385,8 @@ int main(int argc, char *argv[])
&funding_txid, funding_outnum, 1,
funding_amount,
local_msat,
feerate_per_kw,
take(new_fee_states(NULL, fee_payer,
&feerate_per_kw)),
&localconfig, &remoteconfig,
&localbase, &remotebase,
&funding_localkey, &funding_remotekey,
Expand Down
1 change: 1 addition & 0 deletions lightningd/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ LIGHTNINGD_COMMON_OBJS := \
common/daemon.o \
common/derive_basepoints.o \
common/features.o \
common/fee_states.o \
common/funding_tx.o \
common/gen_peer_status_wire.o \
common/gen_status_wire.o \
Expand Down
3 changes: 3 additions & 0 deletions lightningd/channel.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <bitcoin/script.h>
#include <ccan/crypto/hkdf_sha256/hkdf_sha256.h>
#include <ccan/tal/str/str.h>
#include <common/fee_states.h>
#include <common/json_command.h>
#include <common/jsonrpc_errors.h>
#include <common/utils.h>
Expand Down Expand Up @@ -236,6 +237,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
channel->last_sig = *last_sig;
channel->last_htlc_sigs = tal_steal(channel, last_htlc_sigs);
channel->channel_info = *channel_info;
channel->channel_info.fee_states
= dup_fee_states(channel, channel_info->fee_states);
channel->shutdown_scriptpubkey[REMOTE]
= tal_steal(channel, remote_shutdown_scriptpubkey);
channel->final_key_idx = final_key_idx;
Expand Down
2 changes: 1 addition & 1 deletion lightningd/channel_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ void peer_start_channeld(struct channel *channel,
channel->minimum_depth,
&channel->our_config,
&channel->channel_info.their_config,
channel->channel_info.feerate_per_kw,
channel->channel_info.fee_states,
feerate_min(ld, NULL),
feerate_max(ld, NULL),
&channel->last_sig,
Expand Down
9 changes: 6 additions & 3 deletions lightningd/closing_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <bitcoin/script.h>
#include <closingd/gen_closing_wire.h>
#include <common/close_tx.h>
#include <common/fee_states.h>
#include <common/initial_commit_tx.h>
#include <common/per_peer_state.h>
#include <common/utils.h>
Expand Down Expand Up @@ -181,6 +182,7 @@ void peer_start_closingd(struct channel *channel,
int hsmfd;
struct secret last_remote_per_commit_secret;
struct lightningd *ld = channel->peer->ld;
u32 final_commit_feerate;

if (!channel->shutdown_scriptpubkey[REMOTE]) {
channel_internal_error(channel,
Expand Down Expand Up @@ -221,16 +223,17 @@ void peer_start_closingd(struct channel *channel,
* fee of the final commitment transaction, as calculated in
* [BOLT #3](03-transactions.md#fee-calculation).
*/
feelimit = commit_tx_base_fee(channel->channel_info.feerate_per_kw[LOCAL],
0);
final_commit_feerate = get_feerate(channel->channel_info.fee_states,
channel->funder, LOCAL);
feelimit = commit_tx_base_fee(final_commit_feerate, 0);

/* Pick some value above slow feerate (or min possible if unknown) */
minfee = commit_tx_base_fee(feerate_min(ld, NULL), 0);

/* If we can't determine feerate, start at half unilateral feerate. */
feerate = mutual_close_feerate(ld->topology);
if (!feerate) {
feerate = channel->channel_info.feerate_per_kw[LOCAL] / 2;
feerate = final_commit_feerate / 2;
if (feerate < feerate_floor())
feerate = feerate_floor();
}
Expand Down
7 changes: 3 additions & 4 deletions lightningd/opening_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <common/addr.h>
#include <common/channel_config.h>
#include <common/features.h>
#include <common/fee_states.h>
#include <common/funding_tx.h>
#include <common/json_command.h>
#include <common/jsonrpc_errors.h>
Expand Down Expand Up @@ -194,10 +195,8 @@ wallet_commit_channel(struct lightningd *ld,
} else
our_msat = push;

/* Feerates begin identical. */
channel_info->feerate_per_kw[LOCAL]
= channel_info->feerate_per_kw[REMOTE]
= feerate;
channel_info->fee_states = new_fee_states(uc, uc->fc ? LOCAL : REMOTE,
&feerate);

/* old_remote_per_commit not valid yet, copy valid one. */
channel_info->old_remote_per_commit = channel_info->remote_per_commit;
Expand Down
6 changes: 4 additions & 2 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,8 @@ static void json_add_htlcs(struct lightningd *ld,
struct htlc_in_map_iter ini;
const struct htlc_out *hout;
struct htlc_out_map_iter outi;
u32 local_feerate = channel->channel_info.feerate_per_kw[LOCAL];
u32 local_feerate = get_feerate(channel->channel_info.fee_states,
channel->funder, LOCAL);

/* FIXME: Add more fields. */
json_array_start(response, "htlcs");
Expand Down Expand Up @@ -519,7 +520,8 @@ static struct amount_sat commit_txfee(const struct channel *channel,
const struct htlc_out *hout;
struct htlc_out_map_iter outi;
struct lightningd *ld = channel->peer->ld;
u32 local_feerate = channel->channel_info.feerate_per_kw[LOCAL];
u32 local_feerate = get_feerate(channel->channel_info.fee_states,
channel->funder, LOCAL);
size_t num_untrimmed_htlcs = 0;

/* Assume we tried to spend "spendable" */
Expand Down
Loading

0 comments on commit 27828ff

Please sign in to comment.