Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump htlc resolution transactions feerate #3592

Merged
merged 1 commit into from
Apr 2, 2020
Merged

Bump htlc resolution transactions feerate #3592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions contrib/pyln-testing/pyln/testing/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,10 +860,12 @@ def mock_estimatesmartfee(r):
params = r['params']
if params == [2, 'CONSERVATIVE']:
feerate = feerates[0] * 4
elif params == [4, 'ECONOMICAL']:
elif params == [3, 'CONSERVATIVE']:
feerate = feerates[1] * 4
elif params == [100, 'ECONOMICAL']:
elif params == [4, 'ECONOMICAL']:
feerate = feerates[2] * 4
elif params == [100, 'ECONOMICAL']:
feerate = feerates[3] * 4
else:
raise ValueError()
return {
Expand All @@ -878,13 +880,14 @@ def mock_estimatesmartfee(r):
# Technically, this waits until it's called, not until it's processed.
# We wait until all three levels have been called.
if wait_for_effect:
wait_for(lambda: self.daemon.rpcproxy.mock_counts['estimatesmartfee'] >= 3)
wait_for(lambda:
self.daemon.rpcproxy.mock_counts['estimatesmartfee'] >= 4)

# force new feerates by restarting and thus skipping slow smoothed process
# Note: testnode must be created with: opts={'may_reconnect': True}
def force_feerates(self, rate):
assert(self.may_reconnect)
self.set_feerates([rate] * 3, False)
self.set_feerates([rate] * 4, False)
self.restart()
self.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE')
assert(self.rpc.feerates('perkw')['perkw']['opening'] == rate)
Expand Down Expand Up @@ -1005,7 +1008,7 @@ def get_nodes(self, num_nodes, opts=None):
return [j.result() for j in jobs]

def get_node(self, node_id=None, options=None, dbfile=None,
feerates=(15000, 7500, 3750), start=True,
feerates=(15000, 11000, 7500, 3750), start=True,
wait_for_bitcoind_sync=True, expect_fail=False, **kwargs):

node_id = self.get_node_id() if not node_id else node_id
Expand Down
42 changes: 33 additions & 9 deletions plugins/bcli.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ static struct command_result *process_getblockchaininfo(struct bitcoin_cli *bcli

struct estimatefees_stash {
/* FIXME: We use u64 but lightningd will store them as u32. */
u64 urgent, normal, slow;
u64 very_urgent, urgent, normal, slow;
};

static struct command_result *
Expand Down Expand Up @@ -532,10 +532,10 @@ static struct command_result *estimatefees_final_step(struct bitcoin_cli *bcli)
response = jsonrpc_stream_success(bcli->cmd);
json_add_u64(response, "opening", stash->normal);
json_add_u64(response, "mutual_close", stash->normal);
json_add_u64(response, "unilateral_close", stash->urgent);
json_add_u64(response, "unilateral_close", stash->very_urgent);
json_add_u64(response, "delayed_to_us", stash->normal);
json_add_u64(response, "htlc_resolution", stash->normal);
json_add_u64(response, "penalty", stash->normal);
json_add_u64(response, "htlc_resolution", stash->urgent);
json_add_u64(response, "penalty", stash->urgent);
/* We divide the slow feerate for the minimum acceptable, lightningd
* will use floor if it's hit, though. */
json_add_u64(response, "min_acceptable", stash->slow / 2);
Expand All @@ -546,13 +546,13 @@ static struct command_result *estimatefees_final_step(struct bitcoin_cli *bcli)
* margin (say 5x the expected fee requirement)
*/
json_add_u64(response, "max_acceptable",
stash->urgent * bitcoind->max_fee_multiplier);
stash->very_urgent * bitcoind->max_fee_multiplier);

return command_finished(bcli->cmd, response);
}

/* We got the response for the normal feerate, now treat the slow one. */
static struct command_result *estimatefees_third_step(struct bitcoin_cli *bcli)
static struct command_result *estimatefees_fourth_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
Expand All @@ -575,7 +575,7 @@ static struct command_result *estimatefees_third_step(struct bitcoin_cli *bcli)
}

/* We got the response for the urgent feerate, now treat the normal one. */
static struct command_result *estimatefees_second_step(struct bitcoin_cli *bcli)
static struct command_result *estimatefees_third_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
Expand All @@ -591,6 +591,29 @@ static struct command_result *estimatefees_second_step(struct bitcoin_cli *bcli)

params[0] = "4";
params[1] = "ECONOMICAL";
start_bitcoin_cli(NULL, bcli->cmd, estimatefees_fourth_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

return command_still_pending(bcli->cmd);
}

/* We got the response for the very urgent feerate, now treat the urgent one. */
static struct command_result *estimatefees_second_step(struct bitcoin_cli *bcli)
{
struct command_result *err;
struct estimatefees_stash *stash = bcli->stash;
const char **params = tal_arr(bcli->cmd, const char *, 2);

/* If we cannot estimatefees, no need to continue bothering bitcoind. */
if (*bcli->exitstatus != 0)
return estimatefees_null_response(bcli);

err = estimatefees_parse_feerate(bcli, &stash->very_urgent);
if (err)
return err;

params[0] = "3";
params[1] = "CONSERVATIVE";
start_bitcoin_cli(NULL, bcli->cmd, estimatefees_third_step, true,
BITCOIND_LOW_PRIO, "estimatesmartfee", params, stash);

Expand Down Expand Up @@ -726,8 +749,9 @@ static struct command_result *getchaininfo(struct command *cmd,
return command_still_pending(cmd);
}

/* Get the current feerates. We use an urgent feerate for unilateral_close and max,
* a slow feerate for min, and a normal for all others.
/* Get the current feerates. We us an urgent feerate for unilateral_close and max,
* a slightly less urgent feerate for htlc_resolution and penalty transactions,
* a slow feerate for min, and a normal one for all others.
*
* Calls `estimatesmartfee` with targets 2/CONSERVATIVE (urgent),
* 4/ECONOMICAL (normal), and 100/ECONOMICAL (slow) then returns the
Expand Down
48 changes: 30 additions & 18 deletions tests/test_closing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,9 @@ def test_closing_torture(node_factory, executor, bitcoind):
def test_closing_different_fees(node_factory, bitcoind, executor):
l1 = node_factory.get_node()

# Default feerate = 15000/7500/1000
# Default feerate = 15000/11000/7500/1000
# It will start at the second number, accepting anything above the first.
feerates = [[20000, 15000, 7400], [8000, 1001, 100]]
feerates = [[20000, 11000, 15000, 7400], [8000, 6000, 1001, 100]]
amounts = [0, 545999, 546000]
num_peers = len(feerates) * len(amounts)

Expand Down Expand Up @@ -362,10 +362,10 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams):

def closing_fee(node_factory, bitcoind, chainparams, opts):
rate = opts['funder_feerate_per_kw']
funder = node_factory.get_node(feerates=(rate, rate, rate))
funder = node_factory.get_node(feerates=(rate, rate, rate, rate))

rate = opts['fundee_feerate_per_kw']
fundee = node_factory.get_node(feerates=(rate, rate, rate))
fundee = node_factory.get_node(feerates=(rate, rate, rate, rate))

funder_id = funder.info['id']
fundee_id = fundee.info['id']
Expand Down Expand Up @@ -446,7 +446,9 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams):
"""Test penalty transaction with an incoming HTLC"""
# We suppress each one after first commit; HTLC gets added not fulfilled.
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'], may_fail=True, feerates=(7500, 7500, 7500), allow_broken_log=True)
l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'],
may_fail=True, feerates=(7500, 7500, 7500, 7500),
allow_broken_log=True)
l2 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED-nocommit'])

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -487,6 +489,7 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams):
bitcoind.generate_block(1)

l2.daemon.wait_for_log(' to ONCHAIN')

# FIXME: l1 should try to stumble along!
wait_for(lambda: len(l2.getactivechannels()) == 0)

Expand All @@ -509,7 +512,7 @@ def test_penalty_inhtlc(node_factory, bitcoind, executor, chainparams):
outputs = l2.rpc.listfunds()['outputs']
assert [o['status'] for o in outputs] == ['confirmed'] * 2
# Allow some lossage for fees.
slack = 27000 if chainparams['elements'] else 15000
slack = 30000 if chainparams['elements'] else 20000
assert sum(o['value'] for o in outputs) < 10**6
assert sum(o['value'] for o in outputs) > 10**6 - slack

Expand All @@ -519,7 +522,9 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams):
"""Test penalty transaction with an outgoing HTLC"""
# First we need to get funds to l2, so suppress after second.
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'], may_fail=True, feerates=(7500, 7500, 7500), allow_broken_log=True)
l1 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'],
may_fail=True, feerates=(7500, 7500, 7500, 7500),
allow_broken_log=True)
l2 = node_factory.get_node(disconnect=['=WIRE_COMMITMENT_SIGNED*3-nocommit'])

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -589,7 +594,7 @@ def test_penalty_outhtlc(node_factory, bitcoind, executor, chainparams):
outputs = l2.rpc.listfunds()['outputs']
assert [o['status'] for o in outputs] == ['confirmed'] * 3
# Allow some lossage for fees.
slack = 27000 if chainparams['elements'] else 15000
slack = 30000 if chainparams['elements'] else 20000
assert sum(o['value'] for o in outputs) < 10**6
assert sum(o['value'] for o in outputs) > 10**6 - slack

Expand Down Expand Up @@ -687,7 +692,8 @@ def test_onchaind_replay(node_factory, bitcoind):
disconnects = ['+WIRE_REVOKE_AND_ACK', 'permfail']
options = {'watchtime-blocks': 201, 'cltv-delta': 101}
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(options=options, disconnect=disconnects, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(options=options, disconnect=disconnects,
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node(options=options)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -740,7 +746,8 @@ def test_onchain_dust_out(node_factory, bitcoind, executor):
# HTLC 1->2, 1 fails after it's irrevocably committed
disconnects = ['@WIRE_REVOKE_AND_ACK', 'permfail']
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(disconnect=disconnects, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(disconnect=disconnects,
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node()

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -803,7 +810,8 @@ def test_onchain_timeout(node_factory, bitcoind, executor):
# HTLC 1->2, 1 fails just after it's irrevocably committed
disconnects = ['+WIRE_REVOKE_AND_ACK*3', 'permfail']
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(disconnect=disconnects, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(disconnect=disconnects,
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node()

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -1038,7 +1046,8 @@ def test_onchain_all_dust(node_factory, bitcoind, executor):
# is generated on-the-fly, and is thus feerate sensitive.
disconnects = ['-WIRE_UPDATE_FAIL_HTLC', 'permfail']
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(options={'dev-no-reconnect': None}, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(options={'dev-no-reconnect': None},
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node(disconnect=disconnects)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand All @@ -1060,7 +1069,7 @@ def test_onchain_all_dust(node_factory, bitcoind, executor):
l2.wait_for_channel_onchain(l1.info['id'])

# Make l1's fees really high (and wait for it to exceed 50000)
l1.set_feerates((100000, 100000, 100000))
l1.set_feerates((100000, 100000, 100000, 100000))
l1.daemon.wait_for_log('Feerate estimate for unilateral_close set to [56789][0-9]{4}')

bitcoind.generate_block(1)
Expand Down Expand Up @@ -1097,12 +1106,12 @@ def test_onchain_different_fees(node_factory, bitcoind, executor):
p1 = executor.submit(l1.pay, l2, 1000000000)
l1.daemon.wait_for_log('htlc 0: RCVD_ADD_ACK_COMMIT->SENT_ADD_ACK_REVOCATION')

l1.set_feerates((16000, 7500, 3750))
l1.set_feerates((16000, 11000, 7500, 3750))
p2 = executor.submit(l1.pay, l2, 900000000)
l1.daemon.wait_for_log('htlc 1: RCVD_ADD_ACK_COMMIT->SENT_ADD_ACK_REVOCATION')

# Restart with different feerate for second HTLC.
l1.set_feerates((5000, 5000, 3750))
l1.set_feerates((5000, 5000, 5000, 3750))
l1.restart()
l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE')

Expand Down Expand Up @@ -1156,7 +1165,8 @@ def test_permfail_new_commit(node_factory, bitcoind, executor):
# Test case where we have two possible commits: it will use new one.
disconnects = ['-WIRE_REVOKE_AND_ACK', 'permfail']
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(options={'dev-no-reconnect': None}, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(options={'dev-no-reconnect': None},
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node(disconnect=disconnects)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -1457,7 +1467,8 @@ def test_permfail_htlc_in(node_factory, bitcoind, executor):
# Test case where we fail with unsettled incoming HTLC.
disconnects = ['-WIRE_UPDATE_FULFILL_HTLC', 'permfail']
# Feerates identical so we don't get gratuitous commit to update them
l1 = node_factory.get_node(options={'dev-no-reconnect': None}, feerates=(7500, 7500, 7500))
l1 = node_factory.get_node(options={'dev-no-reconnect': None},
feerates=(7500, 7500, 7500, 7500))
l2 = node_factory.get_node(disconnect=disconnects)

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
Expand Down Expand Up @@ -1503,7 +1514,8 @@ def test_permfail_htlc_out(node_factory, bitcoind, executor):
disconnects = ['+WIRE_REVOKE_AND_ACK', 'permfail']
l1 = node_factory.get_node(options={'dev-no-reconnect': None})
# Feerates identical so we don't get gratuitous commit to update them
l2 = node_factory.get_node(disconnect=disconnects, feerates=(7500, 7500, 7500))
l2 = node_factory.get_node(disconnect=disconnects,
feerates=(7500, 7500, 7500, 7500))

l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
l2.daemon.wait_for_log('openingd-chan#1: Handed peer, entering loop'.format(l1.info['id']))
Expand Down
Loading