jsonrpc: Arm the minconf=1 parameter and deal with the fallout

We want to disallow using unconfirmed outputs by default, so making the
default 1 confirmation seems a good idea. This also matches `bitcoind`s
minimum confirmation requirement.

Arming however breaks some of our tests, so I used `minconf=0` for the
breaking tests and added a new test specifically for the `minconf` parameter
for `fundchannel`.

Signed-off-by: Christian Decker <decker.christian@gmail.com>

lightningd/opening_control.c

@@ -845,7 +845,7 @@ static struct command_result *json_fund_channel(struct command *cmd,
 		   p_req("satoshi", param_wtx, &fc->wtx),
 		   p_opt("feerate", param_feerate, &feerate_per_kw),
 		   p_opt_def("announce", param_bool, &announce_channel, true),
-		   p_opt_def("minconf", param_number, &minconf, 0),
+		   p_opt_def("minconf", param_number, &minconf, 1),
 		   NULL))
 		return command_param_failed();
 

tests/test_closing.py

@@ -215,7 +215,7 @@ def test_closing_different_fees(node_factory, bitcoind, executor):
             peers.append(p)
 
     for p in peers:
-        p.channel = l1.rpc.fundchannel(p.info['id'], 10**6)['channel_id']
+        p.channel = l1.rpc.fundchannel(p.info['id'], 10**6, minconf=0)['channel_id']
         # Technically, this is async to fundchannel returning.
         l1.daemon.wait_for_log('sendrawtx exit 0')
 

tests/test_connection.py

@@ -1169,7 +1169,7 @@ def test_no_fee_estimate(node_factory, bitcoind, executor):
 
     # Can with manual feerate.
     l1.rpc.withdraw(l2.rpc.newaddr()['address'], 10000, '1500perkb')
-    l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw')
+    l1.rpc.fundchannel(l2.info['id'], 10**6, '2000perkw', minconf=0)
 
     # Make sure we clean up cahnnel for later attempt.
     l1.daemon.wait_for_log('sendrawtx exit 0')
@@ -1554,3 +1554,28 @@ def test_fail_unconfirmed(node_factory, bitcoind, executor):
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     l1.fund_channel(l2, 200000, wait_for_active=True)
+
+
+def test_change_chaining(node_factory, bitcoind):
+    """Test change chaining of unconfirmed fundings
+
+    Change chaining is the case where one transaction is broadcast but not
+    confirmed yet and we already build a followup on top of the change. If the
+    first transaction doesn't confirm we may end up creating a series of
+    unconfirmable transactions. This is why we generally disallow chaining.
+
+    """
+    l1, l2, l3 = node_factory.get_nodes(3)
+    l1.fundwallet(10**8)  # This will create an output with 1 confirmation
+
+    # Now fund a channel from l1 to l2, that should succeed, with minconf=1 but not before
+    l1.connect(l2)
+    with pytest.raises(RpcError):
+        l1.rpc.fundchannel(l2.info['id'], 10**7, minconf=2)
+    l1.rpc.fundchannel(l2.info['id'], 10**7)  # Defaults to minconf=1
+
+    # We don't have confirmed outputs anymore, so this should fail without minconf=0
+    l1.connect(l3)
+    with pytest.raises(RpcError):
+        l1.rpc.fundchannel(l3.info['id'], 10**7)  # Defaults to minconf=1
+    l1.rpc.fundchannel(l3.info['id'], 10**7, minconf=0)

tests/test_misc.py

@@ -471,11 +471,11 @@ def test_withdraw(node_factory, bitcoind):
     assert l1.db_query('SELECT COUNT(*) as c FROM outputs WHERE status=0')[0]['c'] == 6
 
     # Test withdrawal to self.
-    l1.rpc.withdraw(l1.rpc.newaddr('bech32')['address'], 'all')
+    l1.rpc.withdraw(l1.rpc.newaddr('bech32')['address'], 'all', minconf=0)
     bitcoind.generate_block(1)
     assert l1.db_query('SELECT COUNT(*) as c FROM outputs WHERE status=0')[0]['c'] == 1
 
-    l1.rpc.withdraw(waddr, 'all')
+    l1.rpc.withdraw(waddr, 'all', minconf=0)
     assert l1.db_query('SELECT COUNT(*) as c FROM outputs WHERE status=0')[0]['c'] == 0
 
     # This should fail, can't even afford fee.

wallet/walletrpc.c

@@ -110,7 +110,7 @@ static struct command_result *json_withdraw(struct command *cmd,
 		   p_req("destination", param_tok, &desttok),
 		   p_req("satoshi", param_wtx, &withdraw->wtx),
 		   p_opt("feerate", param_feerate, &feerate_per_kw),
-		   p_opt_def("minconf", param_number, &minconf, 0),
+		   p_opt_def("minconf", param_number, &minconf, 1),
 		   NULL))
 		return command_param_failed();