From d93be58bd01211febee662f77b9c3842bcfa576b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Aug 2018 08:57:24 +0930 Subject: [PATCH] pytest: remove use dev-override-feerates. Manipulate fees via fake-bitcoin-cli. It's not quite the same, as these are pre-smoothing, so we need a restart to override that where we really need an exact change. Or we can wait until it reaches a certain value in cases we don't care about exact amounts. Signed-off-by: Rusty Russell --- contrib/pylightning/lightning/lightning.py | 13 ---- lightningd/chaintopology.c | 76 ---------------------- lightningd/chaintopology.h | 5 -- lightningd/options.c | 26 -------- tests/test_closing.py | 11 ++-- tests/test_connection.py | 63 ++++++------------ tests/test_pay.py | 5 +- 7 files changed, 27 insertions(+), 172 deletions(-) diff --git a/contrib/pylightning/lightning/lightning.py b/contrib/pylightning/lightning/lightning.py index 2513b4bd6..985b92027 100644 --- a/contrib/pylightning/lightning/lightning.py +++ b/contrib/pylightning/lightning/lightning.py @@ -108,19 +108,6 @@ class LightningRpc(UnixDomainSocketRpc): res = self.call("listpeers", payload) return res.get("peers") and res["peers"][0] or None - def dev_setfees(self, immediate=None, normal=None, slow=None): - """ - Set feerate in satoshi-per-kw for {immediate}, {normal} and {slow} - (each is optional, when set, separate by spaces) and show the value - of those three feerates - """ - payload = { - "immediate": immediate, - "normal": normal, - "slow": slow - } - return self.call("dev-setfees", payload) - def listnodes(self, node_id=None): """ Show all nodes in our local network view, filter on node {id} diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 3b418cfbc..3fa083ee3 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -648,12 +648,6 @@ static u32 guess_feerate(const struct chain_topology *topo, enum feerate feerate u32 get_feerate(const struct chain_topology *topo, enum feerate feerate) { -#if DEVELOPER - if (topo->dev_override_fee_rate) { - log_debug(topo->log, "Forcing fee rate, ignoring estimate"); - return topo->dev_override_fee_rate[feerate]; - } -#endif if (topo->feerate[feerate] == 0) { return guess_feerate(topo, feerate); } @@ -661,62 +655,6 @@ u32 get_feerate(const struct chain_topology *topo, enum feerate feerate) } #if DEVELOPER -static void json_dev_setfees(struct command *cmd, - const char *buffer, const jsmntok_t *params) -{ - struct chain_topology *topo = cmd->ld->topology; - struct json_result *response; - u32 *imm, *norm, *slow; - - if (!topo->dev_override_fee_rate) { - u32 fees[NUM_FEERATES]; - for (size_t i = 0; i < ARRAY_SIZE(fees); i++) - fees[i] = get_feerate(topo, i); - topo->dev_override_fee_rate = tal_dup_arr(topo, u32, fees, - ARRAY_SIZE(fees), 0); - } - - if (!param(cmd, buffer, params, - p_opt("immediate", json_tok_number, &imm), - p_opt("normal", json_tok_number, &norm), - p_opt("slow", json_tok_number, &slow), - NULL)) - return; - - if (imm) - topo->dev_override_fee_rate[FEERATE_IMMEDIATE] = *imm; - if (norm) - topo->dev_override_fee_rate[FEERATE_NORMAL] = *norm; - if (slow) - topo->dev_override_fee_rate[FEERATE_SLOW] = *slow; - - log_debug(topo->log, - "dev-setfees: fees now %u/%u/%u", - topo->dev_override_fee_rate[FEERATE_IMMEDIATE], - topo->dev_override_fee_rate[FEERATE_NORMAL], - topo->dev_override_fee_rate[FEERATE_SLOW]); - - notify_feerate_change(cmd->ld); - - response = new_json_result(cmd); - json_object_start(response, NULL); - json_add_num(response, "immediate", - topo->dev_override_fee_rate[FEERATE_IMMEDIATE]); - json_add_num(response, "normal", - topo->dev_override_fee_rate[FEERATE_NORMAL]); - json_add_num(response, "slow", - topo->dev_override_fee_rate[FEERATE_SLOW]); - json_object_end(response); - command_success(cmd, response); -} - -static const struct json_command dev_setfees_command = { - "dev-setfees", - json_dev_setfees, - "Set feerate in satoshi-per-kw for {immediate}, {normal} and {slow} (each is optional, when set, separate by spaces) and show the value of those three feerates" -}; -AUTODATA(json_command, &dev_setfees_command); - void chaintopology_mark_pointers_used(struct htable *memtable, const struct chain_topology *topo) { @@ -764,9 +702,6 @@ struct chain_topology *new_topology(struct lightningd *ld, struct log *log) topo->poll_seconds = 30; topo->feerate_uninitialized = true; topo->root = NULL; -#if DEVELOPER - topo->dev_override_fee_rate = NULL; -#endif return topo; } @@ -787,18 +722,7 @@ void setup_topology(struct chain_topology *topo, tal_add_destructor(topo, destroy_chain_topology); -#if DEVELOPER - if (topo->dev_override_fee_rate) { - log_info(topo->log, "Fee estimation disabled because: " - "--dev-override-fee-rates"); - topo->feerate_uninitialized = false; - } else { - /* Begin fee estimation. */ - start_fee_estimate(topo); - } -#else start_fee_estimate(topo); -#endif /* Once it gets initial block, it calls io_break() and we return. */ io_loop(NULL, NULL); diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 268d65857..32e91cace 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -111,11 +111,6 @@ struct chain_topology { /* Transactions/txos we are watching. */ struct txwatch_hash txwatches; struct txowatch_hash txowatches; - -#if DEVELOPER - /* Force a particular fee rate regardless of estimatefee (satoshis/kw) */ - u32 *dev_override_fee_rate; -#endif }; /* Information relevant to locating a TX in a blockchain. */ diff --git a/lightningd/options.c b/lightningd/options.c index 055fb30bc..b1ef871af 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -401,29 +401,6 @@ static void config_register_opts(struct lightningd *ld) } #if DEVELOPER -static char *opt_set_fee_rates(const char *arg, struct chain_topology *topo) -{ - tal_free(topo->dev_override_fee_rate); - topo->dev_override_fee_rate = tal_arr(topo, u32, 3); - - for (size_t i = 0; i < tal_count(topo->dev_override_fee_rate); i++) { - char *endp; - char term; - - if (i == tal_count(topo->dev_override_fee_rate)-1) - term = '\0'; - else - term = '/'; - topo->dev_override_fee_rate[i] = strtol(arg, &endp, 10); - if (endp == arg || *endp != term) - return tal_fmt(NULL, - "Feerates must be //"); - - arg = endp + 1; - } - return NULL; -} - static void dev_register_opts(struct lightningd *ld) { opt_register_noarg("--dev-no-reconnect", opt_set_invbool, @@ -451,9 +428,6 @@ static void dev_register_opts(struct lightningd *ld) "will cause channels to be closed more often due to " "fee fluctuations, large values may result in large " "fees."); - opt_register_arg("--dev-override-fee-rates", opt_set_fee_rates, NULL, - ld->topology, - "Force a specific rates (immediate/normal/slow) in satoshis per kw regardless of estimated fees"); opt_register_arg( "--dev-channel-update-interval=", opt_set_u32, opt_show_u32, diff --git a/tests/test_closing.py b/tests/test_closing.py index ca2526cd1..5af1a914a 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -200,11 +200,7 @@ def test_closing_different_fees(node_factory, bitcoind, executor): peers = [] for feerate in feerates: for amount in amounts: - p = node_factory.get_node(options={ - 'dev-override-fee-rates': '{}/{}/{}'.format(feerate[0], - feerate[1], - feerate[2]) - }) + p = node_factory.get_node(feerates=feerate) p.feerate = feerate p.amount = amount l1.rpc.connect(p.info['id'], 'localhost', p.port) @@ -896,8 +892,9 @@ def test_onchain_all_dust(node_factory, bitcoind, executor): l2.daemon.wait_for_log('permfail') l2.daemon.wait_for_log('sendrawtx exit 0') - # Make l1's fees really high. - l1.rpc.dev_setfees('100000', '100000', '100000') + # Make l1's fees really high (and wait for it to exceed 50000) + l1.set_feerates((100000, 100000, 100000)) + l1.daemon.wait_for_log('Feerate estimate for Normal set to [56789][0-9]{4}') bitcoind.generate_block(1) l1.daemon.wait_for_log(' to ONCHAIN') diff --git a/tests/test_connection.py b/tests/test_connection.py index 6664025fb..47b40ed47 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -816,7 +816,7 @@ def test_update_fee(node_factory, bitcoind): chanid = l1.get_channel_scid(l2) # Make l1 send out feechange. - l1.rpc.dev_setfees('14000') + l1.set_feerates((14000, 7500, 3750)) l2.daemon.wait_for_log('peer updated fee to 14000') # Now make sure an HTLC works. @@ -848,62 +848,36 @@ def test_update_fee(node_factory, bitcoind): l2.daemon.wait_for_log('onchaind complete, forgetting peer') -@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") -def test_update_all_fees(node_factory): - l1, l2 = node_factory.line_graph(2, fundchannel=True) - - # Set all fees as positional parameters. - l1.rpc.dev_setfees('12345', '6789', '123') - l1.daemon.wait_for_log('dev-setfees: fees now 12345/6789/123') - l2.daemon.wait_for_log('peer updated fee to 12345') - - # Call setfees with fees passed as named parameters in different order. - l1.rpc.dev_setfees(slow='123', normal='4567', immediate='8901') - l1.daemon.wait_for_log('dev-setfees: fees now 8901/4567/123') - l2.daemon.wait_for_log('peer updated fee to 8901') - - # Set one value at a time. - l1.rpc.dev_setfees(slow='321') - l1.daemon.wait_for_log('dev-setfees: fees now 8901/4567/321') - l1.rpc.dev_setfees(normal='7654') - l1.daemon.wait_for_log('dev-setfees: fees now 8901/7654/321') - l1.rpc.dev_setfees(immediate='21098') - l1.daemon.wait_for_log('dev-setfees: fees now 21098/7654/321') - l2.daemon.wait_for_log('peer updated fee to 21098') - - # Verify that all fees are indeed optional in setfees call. - l1.rpc.dev_setfees() - l1.daemon.wait_for_log('dev-setfees: fees now 21098/7654/321') - - # This should return finish closing. - l1.rpc.close(l1.get_channel_scid(l2)) - - @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1") def test_fee_limits(node_factory): # FIXME: Test case where opening denied. - l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5}, fundchannel=True) + l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5, 'may_reconnect': True}, fundchannel=True) - # L1 asks for stupid low fees - l1.rpc.dev_setfees(15) + # L1 asks for stupid low fee (will actually hit the floor of 253) + l1.stop() + l1.set_feerates((15, 15, 15), False) + l1.start() - l1.daemon.wait_for_log('Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 15 outside range 1875-75000') + l1.daemon.wait_for_log('Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 253 outside range 1875-75000') # Make sure the resolution of this one doesn't interfere with the next! # Note: may succeed, may fail with insufficient fee, depending on how # bitcoind feels! l1.daemon.wait_for_log('sendrawtx exit') # Restore to normal. - l1.rpc.dev_setfees(15000) + l1.stop() + l1.set_feerates((15000, 7500, 3750), False) + l1.start() # Try with node which sets --ignore-fee-limits - l3 = node_factory.get_node(options={'ignore-fee-limits': 'true'}) + l3 = node_factory.get_node(options={'ignore-fee-limits': 'true'}, may_reconnect=True) l1.rpc.connect(l3.info['id'], 'localhost', l3.port) - chan = l1.fund_channel(l3, 10**6) # Try stupid high fees - l1.rpc.dev_setfees(15000 * 10) + l1.stop() + l1.set_feerates((15000 * 10, 7500, 3750), False) + l1.start() l3.daemon.wait_for_log('peer_in WIRE_UPDATE_FEE') l3.daemon.wait_for_log('peer_in WIRE_COMMITMENT_SIGNED') @@ -923,13 +897,16 @@ def test_update_fee_reconnect(node_factory, bitcoind): disconnects = ['+WIRE_COMMITMENT_SIGNED'] # Feerates identical so we don't get gratuitous commit to update them l1 = node_factory.get_node(disconnect=disconnects, may_reconnect=True, - feerates=(7500, 7500, 7500)) - l2 = node_factory.get_node(may_reconnect=True) + feerates=(15000, 15000, 3750)) + # We match l2's later feerate, so we agree on same closing tx for simplicity. + l2 = node_factory.get_node(may_reconnect=True, + feerates=(14000, 14000, 3750)) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) chan = l1.fund_channel(l2, 10**6) # Make l1 send out feechange; triggers disconnect/reconnect. - l1.rpc.dev_setfees('14000') + # (Note: < 10% change, so no smoothing here!) + l1.set_feerates((14000, 14000, 3750)) l1.daemon.wait_for_log('Setting REMOTE feerate to 14000') l2.daemon.wait_for_log('Setting LOCAL feerate to 14000') l1.daemon.wait_for_log('dev_disconnect: \+WIRE_COMMITMENT_SIGNED') diff --git a/tests/test_pay.py b/tests/test_pay.py index e43e5e2c7..d5ea50223 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -106,9 +106,10 @@ def test_pay_disconnect(node_factory, bitcoind): l1.daemon.wait_for_log('peer_out WIRE_CHANNEL_REESTABLISH') # Make l2 upset by asking for crazy fee. - l1.rpc.dev_setfees('150000') + l1.set_feerates((10**6, 1000**6, 1000**6), False) + # Wait for l1 notice - l1.daemon.wait_for_log(r'Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee 150000 outside range 1875-75000') + l1.daemon.wait_for_log(r'Peer permanent failure in CHANNELD_NORMAL: lightning_channeld: received ERROR channel .*: update_fee \d+ outside range 1875-75000') # Should fail due to permenant channel fail with pytest.raises(RpcError):