Skip to content

Commit

Permalink
closingd: use a more accurate fee for closing fee negotiation.
Browse files Browse the repository at this point in the history 
We were actually using the last commit tx's size, since we were
setting it in lightningd.  Instead, hand the min and desired feerates
to closingd, and (as it knows the weight of the closing tx), and have
it start negotiation from there.

This can be significantly less when anchor outputs are enabled: for
example in test_closing.py, the commit tx weight is 1124 Sipa, the
close is 672 Sipa!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: Use a more accurate fee for mutual close negotiation.
  • Loading branch information
@rustyrussell
rustyrussell committed Jun 28, 2021
1 parent 2be82a5 commit 88616c7
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 53 deletions.
98 changes: 92 additions & 6 deletions closingd/closingd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,84 @@ static void closing_dev_memleak(const tal_t *ctx,
}
#endif /* DEVELOPER */

/* Figure out what weight we actually expect for this closing tx (using zero fees
* gives the largest possible tx: larger values might omit outputs). */
static size_t closing_tx_weight_estimate(u8 *scriptpubkey[NUM_SIDES],
const u8 *funding_wscript,
const struct amount_sat *out,
struct amount_sat funding,
struct amount_sat dust_limit)
{
/* We create a dummy close */
struct bitcoin_tx *tx;
struct bitcoin_txid dummy_txid;
struct bitcoin_signature dummy_sig;
struct privkey dummy_privkey;
struct pubkey dummy_pubkey;
u8 **witness;

memset(&dummy_txid, 0, sizeof(dummy_txid));
tx = create_close_tx(tmpctx, chainparams,
scriptpubkey[LOCAL], scriptpubkey[REMOTE],
funding_wscript,
&dummy_txid, 0,
funding,
out[LOCAL],
out[REMOTE],
dust_limit);

/* Create a signature, any signature, so we can weigh fully "signed"
* tx. */
dummy_sig.sighash_type = SIGHASH_ALL;
memset(&dummy_privkey, 1, sizeof(dummy_privkey));
sign_hash(&dummy_privkey, &dummy_txid.shad, &dummy_sig.s);
pubkey_from_privkey(&dummy_privkey, &dummy_pubkey);
witness = bitcoin_witness_2of2(NULL, &dummy_sig, &dummy_sig,
&dummy_pubkey, &dummy_pubkey);
bitcoin_tx_input_set_witness(tx, 0, take(witness));

return bitcoin_tx_weight(tx);
}

/* Get the minimum and desired fees */
static void calc_fee_bounds(size_t expected_weight,
u32 min_feerate,
u32 desired_feerate,
struct amount_sat maxfee,
struct amount_sat *minfee,
struct amount_sat *desiredfee)
{
*minfee = amount_tx_fee(min_feerate, expected_weight);
*desiredfee = amount_tx_fee(desired_feerate, expected_weight);

/* Can't exceed maxfee. */
if (amount_sat_greater(*minfee, maxfee))
*minfee = maxfee;

if (amount_sat_less(*desiredfee, *minfee)) {
status_unusual("Our ideal fee is %s (%u sats/perkw),"
" but our minimum is %s: using that",
type_to_string(tmpctx, struct amount_sat, desiredfee),
desired_feerate,
type_to_string(tmpctx, struct amount_sat, minfee));
*desiredfee = *minfee;
}
if (amount_sat_greater(*desiredfee, maxfee)) {
status_unusual("Our ideal fee is %s (%u sats/perkw),"
" but our maximum is %s: using that",
type_to_string(tmpctx, struct amount_sat, desiredfee),
desired_feerate,
type_to_string(tmpctx, struct amount_sat, &maxfee));
*desiredfee = maxfee;
}

status_debug("Expected closing weight = %zu, fee %s (min %s, max %s)",
expected_weight,
type_to_string(tmpctx, struct amount_sat, desiredfee),
type_to_string(tmpctx, struct amount_sat, minfee),
type_to_string(tmpctx, struct amount_sat, &maxfee));
}

int main(int argc, char *argv[])
{
setup_locale();
Expand All @@ -504,6 +582,7 @@ int main(int argc, char *argv[])
struct amount_sat funding, out[NUM_SIDES];
struct amount_sat our_dust_limit;
struct amount_sat min_fee_to_accept, commitment_fee, offer[NUM_SIDES];
u32 min_feerate, initial_feerate;
struct feerange feerange;
enum side opener;
u8 *scriptpubkey[NUM_SIDES], *funding_wscript;
Expand Down Expand Up @@ -531,8 +610,8 @@ int main(int argc, char *argv[])
&out[LOCAL],
&out[REMOTE],
&our_dust_limit,
&min_fee_to_accept, &commitment_fee,
&offer[LOCAL],
&min_feerate, &initial_feerate,
&commitment_fee,
&scriptpubkey[LOCAL],
&scriptpubkey[REMOTE],
&fee_negotiation_step,
Expand All @@ -544,6 +623,17 @@ int main(int argc, char *argv[])
/* stdin == requests, 3 == peer, 4 = gossip, 5 = gossip_store, 6 = hsmd */
per_peer_state_set_fds(notleak(pps), 3, 4, 5);

funding_wscript = bitcoin_redeem_2of2(ctx,
&funding_pubkey[LOCAL],
&funding_pubkey[REMOTE]);

/* Start at what we consider a reasonable feerate for this tx. */
calc_fee_bounds(closing_tx_weight_estimate(scriptpubkey,
funding_wscript,
out, funding, our_dust_limit),
min_feerate, initial_feerate, commitment_fee,
&min_fee_to_accept, &offer[LOCAL]);

snprintf(fee_negotiation_step_str, sizeof(fee_negotiation_step_str),
"%" PRIu64 "%s", fee_negotiation_step,
fee_negotiation_step_unit ==
Expand All @@ -565,10 +655,6 @@ int main(int argc, char *argv[])
&wrong_funding->txid),
wrong_funding->n);

funding_wscript = bitcoin_redeem_2of2(ctx,
&funding_pubkey[LOCAL],
&funding_pubkey[REMOTE]);

peer_billboard(
true,
"Negotiating closing fee between %s and %s satoshi (ideal %s) "
Expand Down
4 changes: 2 additions & 2 deletions closingd/closingd_wire.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ msgdata,closingd_init,opener,enum side,
msgdata,closingd_init,local_sat,amount_sat,
msgdata,closingd_init,remote_sat,amount_sat,
msgdata,closingd_init,our_dust_limit,amount_sat,
msgdata,closingd_init,min_fee_satoshi,amount_sat,
msgdata,closingd_init,min_feerate_perksipa,u32,
msgdata,closingd_init,preferred_feerate_perksipa,u32,
msgdata,closingd_init,fee_limit_satoshi,amount_sat,
msgdata,closingd_init,initial_fee_satoshi,amount_sat,
msgdata,closingd_init,local_scriptpubkey_len,u16,
msgdata,closingd_init,local_scriptpubkey,u8,local_scriptpubkey_len
msgdata,closingd_init,remote_scriptpubkey_len,u16,
Expand Down
14 changes: 7 additions & 7 deletions closingd/closingd_wiregen.c
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions closingd/closingd_wiregen.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 2 additions & 13 deletions lightningd/closing_control.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ void peer_start_closingd(struct channel *channel,
{
u8 *initmsg;
u32 feerate;
struct amount_sat minfee, startfee, feelimit;
struct amount_msat their_msat;
struct amount_sat feelimit;
int hsmfd;
struct lightningd *ld = channel->peer->ld;
u32 final_commit_feerate;
Expand Down Expand Up @@ -247,24 +247,13 @@ void peer_start_closingd(struct channel *channel,
feelimit = commit_tx_base_fee(final_commit_feerate, 0,
channel->option_anchor_outputs);

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

/* If we can't determine feerate, start at half unilateral feerate. */
feerate = mutual_close_feerate(ld->topology);
if (!feerate) {
feerate = final_commit_feerate / 2;
if (feerate < feerate_floor())
feerate = feerate_floor();
}
startfee = commit_tx_base_fee(feerate, 0,
channel->option_anchor_outputs);

if (amount_sat_greater(startfee, feelimit))
startfee = feelimit;
if (amount_sat_greater(minfee, feelimit))
minfee = feelimit;

/* BOLT #3:
*
Expand Down Expand Up @@ -298,7 +287,7 @@ void peer_start_closingd(struct channel *channel,
amount_msat_to_sat_round_down(channel->our_msat),
amount_msat_to_sat_round_down(their_msat),
channel->our_config.dust_limit,
minfee, feelimit, startfee,
feerate_min(ld, NULL), feerate, feelimit,
channel->shutdown_scriptpubkey[LOCAL],
channel->shutdown_scriptpubkey[REMOTE],
channel->closing_fee_negotiation_step,
Expand Down
26 changes: 13 additions & 13 deletions tests/test_closing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pyln.testing.utils import SLOW_MACHINE
from utils import (
only_one, sync_blockheight, wait_for, TIMEOUT,
account_balance, first_channel_id, basic_fee, TEST_NETWORK,
account_balance, first_channel_id, closing_fee, TEST_NETWORK,
scriptpubkey_addr
)

Expand All @@ -22,7 +22,7 @@
def test_closing(node_factory, bitcoind, chainparams):
l1, l2 = node_factory.line_graph(2)
chan = l1.get_channel_scid(l2)
fee = basic_fee(3750) if not chainparams['elements'] else 4477
fee = closing_fee(3750, 2) if not chainparams['elements'] else 3603

l1.pay(l2, 200000000)

Expand Down Expand Up @@ -377,7 +377,7 @@ def feerate_for(target, minimum=0, maximum=10000000):
"""Binary search to find feerate"""
assert minimum != maximum
mid = (minimum + maximum) // 2
mid_fee = basic_fee(mid)
mid_fee = closing_fee(mid, 1)
if mid_fee > target:
return feerate_for(target, minimum, mid)
elif mid_fee < target:
Expand Down Expand Up @@ -452,11 +452,11 @@ def test_closing_negotiation_step_30pct(node_factory, bitcoind, chainparams):
opts['fee_negotiation_step'] = '30%'

opts['close_initiated_by'] = 'opener'
opts['expected_close_fee'] = 20537 if not chainparams['elements'] else 33870
opts['expected_close_fee'] = 20537 if not chainparams['elements'] else 26046
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)

opts['close_initiated_by'] = 'peer'
opts['expected_close_fee'] = 20233 if not chainparams['elements'] else 33366
opts['expected_close_fee'] = 20233 if not chainparams['elements'] else 25657
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)


Expand All @@ -466,11 +466,11 @@ def test_closing_negotiation_step_50pct(node_factory, bitcoind, chainparams):
opts['fee_negotiation_step'] = '50%'

opts['close_initiated_by'] = 'opener'
opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 33533
opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 25789
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)

opts['close_initiated_by'] = 'peer'
opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 33533
opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 25789
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)


Expand All @@ -480,7 +480,7 @@ def test_closing_negotiation_step_100pct(node_factory, bitcoind, chainparams):
opts['fee_negotiation_step'] = '100%'

opts['close_initiated_by'] = 'opener'
opts['expected_close_fee'] = 20001 if not chainparams['elements'] else 32985
opts['expected_close_fee'] = 20001 if not chainparams['elements'] else 25366
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)

# The close fee of 20499 looks strange in this case - one would expect
Expand All @@ -489,7 +489,7 @@ def test_closing_negotiation_step_100pct(node_factory, bitcoind, chainparams):
# * the opener is always first to propose, he uses 50% step, so he proposes 20500
# * the range is narrowed to [20001, 20499] and the peer proposes 20499
opts['close_initiated_by'] = 'peer'
opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 33808
opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 25998
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)


Expand All @@ -499,11 +499,11 @@ def test_closing_negotiation_step_1sat(node_factory, bitcoind, chainparams):
opts['fee_negotiation_step'] = '1'

opts['close_initiated_by'] = 'opener'
opts['expected_close_fee'] = 20989 if not chainparams['elements'] else 34621
opts['expected_close_fee'] = 20989 if not chainparams['elements'] else 26624
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)

opts['close_initiated_by'] = 'peer'
opts['expected_close_fee'] = 20010 if not chainparams['elements'] else 32995
opts['expected_close_fee'] = 20010 if not chainparams['elements'] else 25373
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)


Expand All @@ -513,11 +513,11 @@ def test_closing_negotiation_step_700sat(node_factory, bitcoind, chainparams):
opts['fee_negotiation_step'] = '700'

opts['close_initiated_by'] = 'opener'
opts['expected_close_fee'] = 20151 if not chainparams['elements'] else 33459
opts['expected_close_fee'] = 20151 if not chainparams['elements'] else 25650
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)

opts['close_initiated_by'] = 'peer'
opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 33746
opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 25998
closing_negotiation_step(node_factory, bitcoind, chainparams, opts)


Expand Down
Loading

0 comments on commit 88616c7

Please sign in to comment.