In a couple of places we accept arrays of strings and don't validate
them. If we forward them, e.g., call a JSON-RPC method from the
plugin, we end up embedding the unverified string in the JSON-RPC
call without escaping, which then leads to invalid JSON being passed
on.
This at least partially causes #4238
Includes:
psbt: Use renamed functions for new wally version
psbt: Set the transaction directly to avoid script workarounds
psbt: Use low-S grinding when computing signatures
tx: Use wally_tx_clone from libwally now that its exported
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
Technically, they could do this themselves, but it's much nicer to have one
place to do it (and it makes sure we get the required information into the
PSBT, which is actually not entirely accessible through listfunds, as that
doesn't want to consult with the HSM for close outputs).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON RPC: new low-level coin selection `fundpsbt` routine.
These are pulled from wallet/wallet.c, with the fix now that we grind sigs.
This reduces the fees we pay slightly, as you can see in the coinmoves changes.
I now print out all the coin moves in suitable format before we match:
you only see this if the test fails, but it's really helpful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
the bitcoin_tx version is basically a wrapper for the wally_tx script
extraction -- here we pull it apart so we can easily get a tal'd script
for a wally_tx_output
We're not using the change_outnum for withdraw tx's (and the way
we were calculating it was broken as of the addition of 'multiple
outputs'). This removes the change output knowhow from withdraw_tx
entirely, and pushes the responsibility up to the caller to
include the change output in the output set if desired.
Consequently, we also remove the change output knowhow from hsmd.
Update the `bitcoin_tx_add_input` interface to accept a witness script
and or scriptPubkey.
We save the amount + witness script + witness program (if known) to
the PSBT object for a transaction when creating an input.
These are useful for tx_parts:
1. wally_txid.
2. linearize_wtx.
3. wally_tx_input_spends
4. wally_tx_output_get_amount
5. wally_tx_input_get_txid
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
now that witness script data is saved into the tx/psbt which is
serialized across the wire, there's no reason to use witscript to do
this. good bye witscript!
We did this originally because these types are referred to in the bolts, and we
had no way of injecting the correct include lines into those. Now we do, so
there's less excuse for this.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Transactions that we 'get' from bitciond don't have the input values
available on the transaction; for these cases we'll sum up the inputs
amounts using a different data source than the transaction's
`input_amounts`. So we need to expose it here.
We roll the `elements_add_fee_output` function and the cropping of
overallocated arrays into the `bitcoin_tx_finalize` function. This is supposed
to be the final cleanup and compaction step before a tx can be sent to bitcoin
or passed off to other daemons.
This is the cleanup promised in #3491
This sets the nLockTime to the tip (and accordingly each input's nSequence to
0xfffffffe) for withdrawal transactions.
Even if the anti fee-sniping argument might not be valid until some time yet,
this makes our regular wallet transactions far less distinguishable from
bitcoind's ones since it now defaults to using native Segwit transactions
(like us). Moreover other wallets are likely to implement this (if they
haven't already).
Changelog-Added: wallet: withdrawal transactions now sets nlocktime to the current tip.
Currently the only source for amount_asset is the value getter on a tx output,
and we don't hand it too far around (mainly ignoring it if it isn't the
chain's main currency). Eventually we could bubble them up to the wallet, use
them to select outputs or actually support assets in the channels.
Since we don't hand them around too widely I thought it was ok for them to be
pass-by-value rather than having to allocate them and pass them around by
reference. They're just 41 bytes currently so the overhead should be ok.
Signed-off-by: Christian Decker <@cdecker>
Especially when we grind fees we may end up setting the fees several times, so
instead of always adding a new fee output look for an existing one and set its
value.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the normal convention for this type; it makes using converters
a little easier. See next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The way we build transactions, serialize them, and compute fees depends on the
chain we are working on, so let's add some context to the transactions.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is what all of this has been working towards: ripping out the handwoven
transaction handling. By removing the custom parsing we can finally switch
over to using `wally_tx` as sole representation of transactions in
memory. The commit is a bit larger but it's mostly removing setters and old
references to the input and output fields.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These are handled internally in the `wally_tx` and do not conform to our usual
tallocated strings that can by inspected using `tal_bytelen`, and we don't
really want to litter our code with whitelisting comments for the
`amount_sat.satoshis` access, so these just do read-only on the fly conversions.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The `wally_tx_input`s do not keep track of their input value, which means we
need to track them ourselves if we try to sign these transactions at a later
point in time.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
