API: txprepare for multiple outputs

[trueptolemy]

Fix #2679

• Change the paramaters of withdraw and txprepare :

withdraw 'output' ['feerate'] ['minconf']
txprepare 'output' ['feerate'] ['minconf']

The 'output' is the array of outputs that must specify 'destination' and 'satoshi'.
Its format is like:
[{'destination': address1, 'satoshi': satoshi1}, {'destination': address2, 'satoshi': satoshi2}, ...]
or
[{'destination': address, 'satoshi': "all"}]).

There's also 2 related questions:
1. Do you think this output array is suitable?
2. About the test test_mutiple_withdraw, it looks a bit duplicative with test_withdraw. Should I merge these two tests?

EDIT: Remove the change of withdraw, only make txprepare support mutiple outputs.

---

• Add CHANGELOG

[ZmnSCPxj]

withdraw is an old, user-facing command, whereas txprepare is a relatively new and arguably low-level command. I would be fine with changing txprepare interface but not withdraw interface. Others might have different opinions.

[trueptolemy]

I would be fine with changing txprepare interface but not withdraw interface. Others might have different opinions.

Thank you, I'll restore withdraw to its original.

[niftynei]

This is great! A few nits about naming conventions, otherwise it's a nice improvement.

[niftynei]

we should do this before the loop, right after we initialize all_funds = false

[trueptolemy]

Yes, thank you. Corrected.

[trueptolemy]

Here shouldn't be changed:
At the beginning, we initial the (*utx)->wtx->amount as AMOUNT_SAT(-1ULL).
This is the max permissible for wallet_select_all.

	if (amount_sat_eq(*amount, AMOUNT_SAT(-1ULL))) {
		if (outputstok->size > 1)
			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
					    "outputs[%zi]: this destination wants"
					    " all satoshi. The count of outputs"
					    " can't be more than 1. ", i);
		(*utx)->wtx->all_funds = true;
		break;
	}

	if (i == 0)
		(*utx)->wtx->amount = AMOUNT_SAT(0);

After handling the all case, we can initial (*utx)->wtx->amount again.

[niftynei]

then we're in a split mode, because you've initialized all_funds to be false, but left the wtx->amount pegged at what basically signifies all_funds. this is not ideal.

the wtx amount should be set to zero at the loop start along with the all_funds flag. in the case that the *amount is the max, then set (utx)->wtx->amount to be *amount before breaking. this keeps the state fields of the wtx in sync.

[trueptolemy]

Yes, I got your idea. It's better 👍

[niftynei]

Few minor nits, otherwise lgtm

[niftynei]

Travis shows that test_txprepare and test_txprepare_restart in test_wallet.py are failing with this error

E           lightning.lightning.RpcError: RPC call failed: method: txprepare, payload: {'outputs': [{'bcrt1qeyyk6sl5pr49ycpqyckvmttus5ttj25pd0zpvg': 'all'}]}, error: {'code': 302, 'message': 'Output 0sat would be dust'}

[trueptolemy]

Rebased and solved the bug.(The reason is here ).
Wait for Travis-CI to pass.

[niftynei]

ACK 08882c7