From 24d54f98ad72ca2f286b4b66d1e588f963a77fd2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 13 Dec 2019 00:37:53 +1030 Subject: [PATCH] channeld: use fee_states internally. This is an intermediary step: we still don't save it to the database, but we do use the fee_states struct to track it internally. Signed-off-by: Rusty Russell --- channeld/full_channel.c | 71 ++++++++++++++++++++------------ channeld/test/run-full_channel.c | 7 +++- common/initial_channel.c | 17 ++++---- common/initial_channel.h | 11 +++-- devtools/Makefile | 2 + openingd/openingd.c | 7 +++- tests/test_connection.py | 2 +- 7 files changed, 71 insertions(+), 46 deletions(-) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 350b6e6f7..bfb3c9b13 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -101,13 +102,16 @@ 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, - feerate_per_kw[LOCAL], + take(fee_states), local, remote, local_basepoints, remote_basepoints, @@ -117,8 +121,6 @@ struct channel *new_full_channel(const tal_t *ctx, funder); if (channel) { - /* Feerates can be different. */ - channel->view[REMOTE].feerate_per_kw = feerate_per_kw[REMOTE]; channel->htlcs = tal(channel, struct htlc_map); htlc_map_init(channel->htlcs); memleak_add_helper(channel->htlcs, memleak_help_htlcmap); @@ -812,6 +814,37 @@ static void htlc_incstate(struct channel *channel, } } +/* Returns true if a change was made. */ +static bool fee_incstate(struct channel *channel, + enum side sidechanged, + enum htlc_state hstate) +{ + int preflags, postflags; + const int committed_f = HTLC_FLAG(sidechanged, HTLC_F_COMMITTED); + + preflags = htlc_state_flags(hstate); + postflags = htlc_state_flags(hstate + 1); + + /* You can't change sides. */ + assert((preflags & (HTLC_LOCAL_F_OWNER|HTLC_REMOTE_F_OWNER)) + == (postflags & (HTLC_LOCAL_F_OWNER|HTLC_REMOTE_F_OWNER))); + + /* These only advance through ADDING states. */ + if (!(htlc_state_flags(hstate) & HTLC_ADDING)) + return false; + + if (!inc_fee_state(channel->fee_states, hstate)) + return false; + + if (!(preflags & committed_f) && (postflags & committed_f)) + status_debug("Feerate: %s->%s %s now %u", + htlc_state_name(hstate), + htlc_state_name(hstate+1), + side_to_str(sidechanged), + *channel->fee_states->feerate[hstate+1]); + return true; +} + /* Returns flags which were changed. */ static int change_htlcs(struct channel *channel, enum side sidechanged, @@ -855,6 +888,13 @@ static int change_htlcs(struct channel *channel, } } + /* Update fees. */ + for (i = 0; i < n_hstates; i++) { + if (fee_incstate(channel, sidechanged, htlc_states[i])) + cflags |= (htlc_state_flags(htlc_states[i]) + ^ htlc_state_flags(htlc_states[i]+1)); + } + return cflags; } @@ -967,7 +1007,8 @@ bool channel_update_feerate(struct channel *channel, u32 feerate_per_kw) status_debug("Setting %s feerate to %u", side_to_str(!channel->funder), feerate_per_kw); - channel->view[!channel->funder].feerate_per_kw = feerate_per_kw; + start_fee_update(channel->fee_states, channel->funder, feerate_per_kw); + channel->changes_pending[!channel->funder] = true; return true; } @@ -1011,17 +1052,6 @@ bool channel_rcvd_revoke_and_ack(struct channel *channel, if (change & HTLC_LOCAL_F_PENDING) channel->changes_pending[LOCAL] = true; - /* For funder, ack also means time to apply new feerate locally. */ - if (channel->funder == LOCAL && - (channel->view[LOCAL].feerate_per_kw - != channel->view[REMOTE].feerate_per_kw)) { - status_debug("Applying feerate %u to LOCAL", - channel->view[REMOTE].feerate_per_kw); - channel->view[LOCAL].feerate_per_kw - = channel->view[REMOTE].feerate_per_kw; - channel->changes_pending[LOCAL] = true; - } - return channel->changes_pending[LOCAL]; } @@ -1062,17 +1092,6 @@ bool channel_sending_revoke_and_ack(struct channel *channel) if (change & HTLC_REMOTE_F_PENDING) channel->changes_pending[REMOTE] = true; - /* For non-funder, sending ack means we apply any fund changes to them */ - if (channel->funder == REMOTE - && (channel->view[LOCAL].feerate_per_kw - != channel->view[REMOTE].feerate_per_kw)) { - status_debug("Applying feerate %u to REMOTE", - channel->view[LOCAL].feerate_per_kw); - channel->view[REMOTE].feerate_per_kw - = channel->view[LOCAL].feerate_per_kw; - channel->changes_pending[REMOTE] = true; - } - return channel->changes_pending[REMOTE]; } diff --git a/channeld/test/run-full_channel.c b/channeld/test/run-full_channel.c index 6e9ba032c..1ff9be67e 100644 --- a/channeld/test/run-full_channel.c +++ b/channeld/test/run-full_channel.c @@ -1,3 +1,4 @@ +#include "../../common/fee_states.c" #include "../../common/initial_channel.c" #include "../../common/keyset.c" #include "../full_channel.c" @@ -629,8 +630,10 @@ int main(void) for (i = 0; i < ARRAY_SIZE(feerates); i++) { feerate_per_kw[LOCAL] = feerate_per_kw[REMOTE] = feerates[i]; - lchannel->view[LOCAL].feerate_per_kw = feerate_per_kw[LOCAL]; - rchannel->view[REMOTE].feerate_per_kw = feerate_per_kw[REMOTE]; + *lchannel->fee_states->feerate[SENT_ADD_ACK_REVOCATION] + = feerate_per_kw[LOCAL]; + *rchannel->fee_states->feerate[RCVD_ADD_ACK_REVOCATION] + = feerate_per_kw[REMOTE]; raw_tx = commit_tx( tmpctx, &funding_txid, funding_output_index, diff --git a/common/initial_channel.c b/common/initial_channel.c index e542b54be..e6a943468 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -1,7 +1,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -14,7 +16,7 @@ struct channel *new_initial_channel(const tal_t *ctx, u32 minimum_depth, struct amount_sat funding, struct amount_msat local_msatoshi, - u32 feerate_per_kw, + const struct fee_states *fee_states TAKES, const struct channel_config *local, const struct channel_config *remote, const struct basepoints *local_basepoints, @@ -44,9 +46,8 @@ struct channel *new_initial_channel(const tal_t *ctx, channel->changes_pending[LOCAL] = channel->changes_pending[REMOTE] = false; - channel->view[LOCAL].feerate_per_kw - = channel->view[REMOTE].feerate_per_kw - = feerate_per_kw; + /* takes() if necessary */ + channel->fee_states = dup_fee_states(channel, fee_states); channel->view[LOCAL].owed[LOCAL] = channel->view[REMOTE].owed[LOCAL] @@ -112,22 +113,20 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, u32 channel_feerate(const struct channel *channel, enum side side) { - return channel->view[side].feerate_per_kw; + return get_feerate(channel->fee_states, channel->funder, side); } static char *fmt_channel_view(const tal_t *ctx, const struct channel_view *view) { - return tal_fmt(ctx, "{ feerate_per_kw=%"PRIu32"," - " owed_local=%s," + return tal_fmt(ctx, "{ owed_local=%s," " owed_remote=%s }", - view->feerate_per_kw, type_to_string(tmpctx, struct amount_msat, &view->owed[LOCAL]), type_to_string(tmpctx, struct amount_msat, &view->owed[REMOTE])); } -/* FIXME: This should reference HTLCs somehow. */ +/* FIXME: This should reference HTLCs somehow, and feerates! */ static char *fmt_channel(const tal_t *ctx, const struct channel *channel) { return tal_fmt(ctx, "{ funding=%s," diff --git a/common/initial_channel.h b/common/initial_channel.h index b7e5dca95..e0741d231 100644 --- a/common/initial_channel.h +++ b/common/initial_channel.h @@ -20,9 +20,6 @@ struct fulfilled_htlc; /* View from each side */ struct channel_view { - /* Current feerate in satoshis per 1000 weight. */ - u32 feerate_per_kw; - /* How much is owed to each side (includes pending changes) */ struct amount_msat owed[NUM_SIDES]; }; @@ -59,6 +56,9 @@ struct channel { /* Do we have changes pending for ourselves/other? */ bool changes_pending[NUM_SIDES]; + /* Fee changes, some which may be in transit */ + struct fee_states *fee_states; + /* What it looks like to each side. */ struct channel_view view[NUM_SIDES]; @@ -74,8 +74,7 @@ struct channel { * @minimum_depth: The minimum confirmations needed for funding transaction. * @funding_satoshis: The commitment transaction amount. * @local_msatoshi: The amount for the local side (remainder goes to remote) - * @feerate_per_kw: feerate per kiloweight (satoshis) for the commitment - * transaction and HTLCS (at this stage, same for both sides) + * @fee_states: The fee update states. * @local: local channel configuration * @remote: remote channel configuration * @local_basepoints: local basepoints. @@ -92,7 +91,7 @@ struct channel *new_initial_channel(const tal_t *ctx, u32 minimum_depth, struct amount_sat funding, struct amount_msat local_msatoshi, - u32 feerate_per_kw, + const struct fee_states *fee_states TAKES, const struct channel_config *local, const struct channel_config *remote, const struct basepoints *local_basepoints, diff --git a/devtools/Makefile b/devtools/Makefile index e6e04c7b8..176345ad7 100644 --- a/devtools/Makefile +++ b/devtools/Makefile @@ -16,8 +16,10 @@ DEVTOOLS_COMMON_OBJS := \ common/crypto_state.o \ common/decode_array.o \ common/features.o \ + common/fee_states.o \ common/gossip_rcvd_filter.o \ common/hash_u5.o \ + common/htlc_state.o \ common/memleak.o \ common/node_id.o \ common/onion.o \ diff --git a/openingd/openingd.c b/openingd/openingd.c index 58f45e7a7..d4d34e16e 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -670,7 +671,8 @@ static bool funder_finalize_channel_setup(struct state *state, state->minimum_depth, state->funding, local_msat, - state->feerate_per_kw, + take(new_fee_states(NULL, LOCAL, + &state->feerate_per_kw)), &state->localconf, &state->remoteconf, &state->our_points, @@ -1136,7 +1138,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) state->minimum_depth, state->funding, state->push_msat, - state->feerate_per_kw, + take(new_fee_states(NULL, REMOTE, + &state->feerate_per_kw)), &state->localconf, &state->remoteconf, &state->our_points, &theirs, diff --git a/tests/test_connection.py b/tests/test_connection.py index 277e49cde..3cd056c8d 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1499,7 +1499,7 @@ def test_update_fee_reconnect(node_factory, bitcoind): l1.daemon.wait_for_log(r'dev_disconnect: \+WIRE_COMMITMENT_SIGNED') # Wait for reconnect.... - l1.daemon.wait_for_log('Applying feerate 14000 to LOCAL') + l1.daemon.wait_for_log('Feerate:.*LOCAL now 14000') l1.pay(l2, 200000000) l2.pay(l1, 100000000)