utxopsbt command #3845
utxopsbt command #3845
Conversation
rustyrussell
force-pushed
the
utxopsbt-command
branch
from
eb331e3 to
a985185
Compare
doc/lightning-fundchannel.7.md
Outdated
| @@ -52,7 +52,7 @@ private. | |||
| *minconf* specifies the minimum number of confirmations that used | |||
| outputs should have. Default is 1. | |||
|
|
|||
| *utxos* specifies the utxos to be used to fund the channel, as an array | |||
| *utxos* specifies the utxos to be used to fund the channel, as an array | |||
| compulsory list of outputs to use. | ||
|
|
||
| *utxos* must be an array of "txid:vout", each of which must be | ||
| reserved or available: the total amount must be sufficient to pay for |
There was a problem hiding this comment.
I do not see a use-case for allowing reserved state? If a UTXO is reserved for a different spend, we should not allow spending it: xref how fundchannel today works, if you fundchannel in parallel with the same UTXO, one will error due to being unable to reserve.
There was a problem hiding this comment.
If you're attempting an RBF, you'll very likely be spending a still reserved UTXO. You could require a user to 'unreserve' it first but that seems like a roundabout way to achieve the same result.
There was a problem hiding this comment.
If you are attempting an RBF, you could just mutate the PSBT without messing with the reservations, right? Like what we are planning with fundchannel/multifundchannel. We get a partial transaction pre-fundchannel_start, then once we have the addresses we mutate that PSBT without changing reservations. That works, right? Because if it does not work, we would have to reopen #3415. But if that works, we can mutate a PSBT for RBF just fine as well. Keep track of the change output, delete it, replace with smaller change output (or if the smaller amount is below dust, just delete it). Then if on Elements, update the fee output.
Edit: Or do not even track a change output, just scan for any output that pays to us, we have dev-listaddrs for that.
There was a problem hiding this comment.
Contrasting with fundpsbt, fundpsbt does not select UTXOs that are reserved. So I do not think it is safe to allow selection of reserved UTXOs either.
Consider the case where we have a fundchannel. The user invokes it twice with two different peers but with the same UTXO set. To protect against this, fundchannel has to go check the reservations by checking by listfunds, but this runs into atomicity problems (it could get reserved between when it queries listfunds and it performs utxopsbt by the other fundchannel --- or other operations like withdraw or novel plugins --- running in parallel). Then the two parallel funchannel processes proceed, but at sendpsbt time only one of those will confirm, and the other channel is negotiated with the peer and useless. The user has to manually fundchannel_cancel the channel that did not get in.
Now maybe you can argue that the user should not do that, but if the user is running multiple plugins on top of their lightningd (and we should want them to do that, what would be the point of implementing and maintaining the plugin support if we did not want them to do that?) then such conflicts are going to be inevitable, and much harder to handle.
See, reservations are locks. If you violate a lock, parallel operations will screw with you. Basic multithreading. So we should really be a lot more serious about respecting locks. At least we do not block when a resource (UTXO) is currently locked by somebody else, but instead fail immediately, so that we can handle this case.
There was a problem hiding this comment.
Agreed it's great to alter the PSBT if you have it, but if you only have the tx, you want to get the PSBT, and utxopsbt will do that for you.
There's a good reason that reserved-only is the default: otherwise it's up to the caller to ensure that they are entitled to extend the reservation. See last commit.
Consider the case where we have a fundchannel. The user invokes it twice with two different peers but with the same UTXO set. To protect against this, fundchannel has to go check the reservations by checking by listfunds, but this runs into atomicity problems (it could get reserved between when it queries listfunds and it performs utxopsbt by the other fundchannel --- or other operations like withdraw or novel plugins --- running in parallel). Then the two parallel funchannel processes proceed, but at sendpsbt time only one of those will confirm, and the other channel is negotiated with the peer and useless. The user has to manually fundchannel_cancel the channel that did not get in.
No, I wrote this code (before I wrote utxopsbt): it did listfunds and then tried reservepsbt and then could try again if it failed. However, this was quite complex, and utxopsbt made it much simpler.
See, reservations are locks. If you violate a lock, parallel operations will screw with you. Basic multithreading. So we should really be a lot more serious about respecting locks. At least we do not block when a resource (UTXO) is currently locked by somebody else, but instead fail immediately, so that we can handle this case.
No, they are probabolistic datastructures. Reservations are "likely to work" inputs for creating new transactions.
This leaves significant other uses, including deliberately conflicting transactions (such as dual funding where you may prefer to do that than to be DoS out of inputs), combining transactions, RBF and cancelling transactions by deliberately double-spending.
| the resulting transaction plus *startweight* at the given *feerate*, | ||
| with at least *satoshi* left over (unless *satoshi* is **all**, which | ||
| is equivalent to setting it to zero). | ||
|
|
There was a problem hiding this comment.
*reserve* parameter exists but is not described here.
There was a problem hiding this comment.
It's described below...
| *excess_msat* containing the amount above *satoshi* which is | ||
| available. This could be zero, or dust. If *satoshi* was "all", | ||
| then *excess_msat* is the entire amount once fees are subtracted | ||
| for the weights of the inputs and *startweight*. |
There was a problem hiding this comment.
On Elements, do either fundpsbt or utxopsbt add a fee output? Or does the caller have to do that?
There was a problem hiding this comment.
If caller has to do that, I suggest adding this explicitly to the documentation, and recommending that if the excess_msat is below a dust limit (which should be documented as well) then it should be added to the fee output as well.
There was a problem hiding this comment.
bitcoin_tx_finalize does this automatically, but third parties outside of lightningd should probably know.
rustyrussell
force-pushed
the
utxopsbt-command
branch
from
22e8459 to
884a728
Compare
rustyrussell
force-pushed
the
utxopsbt-command
branch
2 times, most recently
from
ae0a233 to
aafa79b
Compare
wire/Makefile
Outdated
| @@ -57,7 +57,8 @@ wire/gen_peer_wire_csv: wire/extracted_peer_wire_csv $(EXPERIMENTAL_PEER_PATCHES | |||
| @set -e; trap "rm -f $@.$$$$" 0; cp $< $@.$$$$; for exp in $(EXPERIMENTAL_PEER_PATCHES); do patch $@.$$$$ $$exp >/dev/null ; done; mv $@.$$$$ $@ | |||
|
|
|||
| wire/gen_onion_wire_csv: wire/extracted_onion_wire_csv $(EXPERIMENTAL_ONION_PATCHES) | |||
| @set -e; trap "rm -f $@.$$$$" 0; cp $< $@.$$$$; for exp in $(EXPERIMENTAL_ONION_PATCHES); do patch $@.$$$$ $$exp; done >/dev/null ; mv $@.$$$$ $@ | |||
| echo $(EXPERIMENTAL_ONION_PATCHES) | |||
There was a problem hiding this comment.
debugging print statement?
It's *possible* to do this using various RPC calls, but it's unfriendly: 1. Call getinfo to get the current block height. 2. Call listfunds to map the UTXOs. 3. Create the PSBT and hope you get all the fields correct. Instead, this presents an interface just like `fundpsbt`, with identical returns. I think it's different enough to justify a new command (though it shares much internally, of course). In particular, it's now quite simple to create a command which uses specified utxos, and then adds more to meet any shortfall. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This more closely mirrors fundpsbt (which will only select unreserved ones) but you can turn it off if you want to e.g. rbf in future. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: New low-level command `utxopsbt` to create PSBT from existing utxos.
rustyrussell
force-pushed
the
utxopsbt-command
branch
from
aafa79b to
4d2457d
Compare
|
Ack 4d2457d Due to trivial rebase where I removed Makefile noise accidentally committed. |
OK, after pondering @ZmnSCPxj 's feedback, I came up with a new helper to make life easier.
It's on top of #3844 so see last commit...Now ready to go!