Allow users to specify input weights when funding a transaction (wallet, rpc/rest/zmq)

https://github.com/bitcoin/bitcoin/pull/23201

Host: glozow  -  PR author: achow101

The PR branch HEAD was db4efdeb6d at the time of this review club meeting.

Notes

  • Funding a transaction involves selecting inputs with a total amount that covers both the payment(s) and the fees for the transaction itself. The target feerate is calculated at the beginning, and the fees depend on the total size of the transaction.

  • In some use cases, users may want to fund a transaction with external inputs, i.e., inputs that are not controlled by the user themselves, but some external wallet such as that of a LN channel counterparty. The challenge in this scenario is accounting for these inputs when funding the fees of the transaction - the wallet needs to know their size.

    • Given solving data such as public keys, scripts, or descriptors, the wallet can approximate the size of the input and corresponding witness data. For example, this stackexchange post breaks down the possible sizes of a P2PKH input.

    • PR #17211 added support for allowing users to provide solving data to help the wallet determine the size of external input(s).

    • Some sizes might differ depending on how the external wallet generates the signature. In these cases, the wallet uses the maximum possible size to avoid underestimating the fees needed to reach the target feerate.

    • Still, solving data might not be available and external inputs may be nonstandard.

  • PR #23201 adds support for specifying the weights of external inputs in the wallet RPCs send, walletcreatefundedpsbt, and fundrawtransaction. This allows the wallet to take these inputs into account when funding the transaction at a target feerate, even if it doesn’t have the solving data for calculating the size itself.

    • This is achieved by adding a map from the input’s outpoint to weight in CCoinControl.

    • If a user provides both solving data and input weights, the provided weight overrides the size calculated using solving data.

  • Raw transactions are serialized in a specific format which includes lengths and sizes (e.g. number of inputs in vin and number of bytes in the signature script) represented as Compact Size Unsigned Integers, which vary in length depending on the value.

    • The helper function, FillInputToWeight(), fills a CTxIn by adding to the witness stack until it reaches a target weight. It takes into account situations in which increasing the size of the witness stack also increases the size of the Compact Size Uint used to encode its length.

Questions

  1. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK?

  2. What are some use cases of allowing external inputs to be used in send, walletcreatefundedpsbt, fundrawtransaction, and bumpfee?

  3. Why might a user want to specify input weights instead of using the existing solving_data option?

  4. Why does FundTransaction() need to know the external inputs ahead of time at all?

  5. In the interface modified by this PR, how would a user call {send, walletcreatefundedpsbt, fundrawtransaction} to specify a maximum input weight?

  6. The RPCs will throw a JSONRPCError: RPC_INVALID_PARAMETER if the specified weight is below 165. Why can’t the weight of an input be less than 165? (Hint: see the definition of GetTransactionInputWeight() and witness serialization specified in BIP144).

  7. Quiz: Given that an external input is a {P2PKH, P2WPKH, P2WSH, P2TR}, can you calculate the maximum weight you need to add to the transaction you’re funding?

  8. What is the purpose of CCoinControl? What are the different purposes of CCoinControl versus CoinSelectionParams?

  9. What is a Compact Size Unsigned Integer? Where is it used in the Bitcoin protocol?

  10. How does the FillInputToWeight() function fill an input to the target weight? Why does it care when the weight to add is between 253 and 263?

Meeting Log

  117:00 <glozow> #startmeeting
  217:00 <glozow> Hello everyone! This is PR Review Club
  317:00 <ekzyis> hi
  417:00 <ziggie> hello
  517:00 <glozow> Feel free to say hi so we know you're here
  617:00 <svav> Hi
  717:00 <bitcoin_pleb_pau> hi
  817:00 <kouloumos> hi
  917:00 <glozow> and let us know if it's your first time :)
 1017:00 <btckid> hi
 1117:00 * docallag Heya
 1217:00 <stickies-v> hi
 1317:01 <tarun> hi
 1417:01 <btckid> first time here
 1517:01 <sanya> hi, first time here
 1617:01 <sipa> hi
 1717:01 <RRa> hi, first time here
 1817:01 <michaelfolkson> hi
 1917:01 <glozow> Today we're looking at #23201, "Allow users to specify input weights when funding a transaction", which seems to be a favorite among lightning devs
 2017:01 <glozow> notes here: https://bitcoincore.reviews/23201
 2117:01 <glozow> btckid: sanya: RRa: welcome!
 2217:02 <glozow> Did any of you get a chance to review today's PR? y/n, and what was your review approach?
 2317:02 <michaelfolkson> s/lightning devs/t-bast :)
 2417:02 <ekzyis> n
 2517:02 <tarun> reviewed it (but quickly)
 2617:02 <svav> n but read the notes
 2717:02 <stickies-v> n, quick gloss soo will be lurking mostly
 2817:02 <bitcoin_pleb_pau> n. same as svav
 2917:03 <ziggie> y (but not testing)
 3017:03 <glozow> that's good to know. since we're not all familiar, can anybody summarize what the PR does?
 3117:03 <btckid> y, but not testing
 3217:05 <bitcoin_pleb_pau> in my understanding, this PR creates a new way to find the input weight of an external input. prior, we used solving data, now we use wallet RPC's to get more accurate data.
 3317:05 <larryruane> hi .. built light code review
 3417:05 <stickies-v> it allows the (RPC) user to specify the exact weight (units) of external inputs they're using to construct a transaction
 3517:05 <glozow> bitcoin_pleb_pau: good answer, you're close :) we now allow the RPC caller to manually specify the input weight of an external input
 3617:05 <glozow> stickies-v: bingo!
 3717:05 <svav> It allows for accounting for any external inputs of a transaction in the fee estimation of the transaction, so that any transactions involving external inputs can be processed successfully
 3817:06 <tarun> is there a good source to understand external inputs?
 3917:06 <sipa> external input: just money received from someone else
 4017:06 <sipa> oh, no, i'm sorry
 4117:07 <glozow> external input = input that our wallet is unable to spend, because we don't control the keys, etc.
 4217:07 <glozow> i believe
 4317:07 <bitcoin_pleb_pau> I don't want to derail the conversation, but if ti doesn't, could glozow elaborate on what manually specifying entails?
 4417:07 <sipa> ignore me
 4517:07 <OliverOffing> So we're talking about a multi-sig tx, yes? Since it's an external input, I'm guessing it'd need at least 2 signatures… (sanity check)
 4617:07 <michaelfolkson> I think it is for wallets where we own the keys but it isn't the Core wallet right?
 4717:07 <glozow> bitcoin_pleb_pau: not derailing at all! good question. the weight is passed in as an argument to the RPC.
 4817:08 <michaelfolkson> Or at least we are able to spend the output. Like a Lightning channel close to our Bitcoin wallet?
 4917:08 <glozow> michaelfolkson: not sure what you're saying? this *is* the Core wallet code we're looking at
 5017:08 <bitcoin_pleb_pau> michaelfolkson The notes say it is useful for lightning channels, where the other party in the channel has a set of keys that we don't own
 5117:09 <michaelfolkson> Yeah I think the notes are wrong
 5217:09 <achow101> an external input is an input that the wallet is unable to solve for, where solving means that if it had private keys available, a spending transaction could be created
 5317:09 <achow101> this usually means that the wallet is unaware of those inputs as it is not watching for them, but there are some cases where the wallet may be watching for those inputs but lack solving data
 5417:09 <svav> I have a general question about GitHub. Where is there a description of the functionality of this (or any) PR, because when I look I just find the comments. Do you only get the code to look at, or does the PR submitter give a summary anywhere?
 5517:09 <stickies-v> this technically doesn't have to be multisig though, right? Could just be a single input transaction where the keys of that single input are stored somewhere outside of core?
 5617:10 <bitcoin_pleb_pau> ok glozow thanks. in my understanding the argument is passed 'manually' but that doesn't mean it requires user handling, it's still done 'automatically' but just on a case-by-case basis, and this case-by-case is what your aiming at when you say manually?
 5717:10 <achow101> stickies-v: yes
 5817:10 <tarun> I too don't want to derail the convo, but doesn't the external input need to be known (and hence its weight) immediately prior to publishing and so why would we need to estimate?
 5917:10 <michaelfolkson> It is more the Core wallet doesn't understand how to spend the output than we are unable to spend the output using external wallet (e.g. Lightning wallet)
 6017:11 <achow101> tarun: we need to be able to estimate fees when setting the inputs for a transaction
 6117:11 <larryruane> svav: some (but not most) PRs reference a GitHub "issue" (aka a ticket) that usually explains more background; in this case there is a ticket: https://github.com/bitcoin/bitcoin/issues/23187
 6217:11 <glozow> stickies-v: yes. OliverOffing: i usually think of multisig as a single input. perhaps a more clear example here is a transaction with multiple inputs, controlled by us and other parties
 6317:12 <glozow> tarun: when we're funding the transaction, we're trying to put fees on it to reach a target feerate. if we don't know what the weight will be, we won't know how much in fees to put on it
 6417:12 <tarun> ok thank you. it seems the estimate is needed in deciding which other inputs might be used.
 6517:12 <stickies-v> tarun: I think what you might be missing is that a known scriptpubkey can have an unknown scriptsig length. E.g. if you know a P2SH scriptpubkey, you don't know how long the scriptsig is going to be (script is unknown until input is spent)
 6617:13 <glozow> (So we've answered Question #4 in the notes)
 6717:13 <tarun> ok. thank you that is helpful.
 6817:13 <glozow> great. let's also answer the first question which is about motivation - What are some use cases of allowing external inputs to be used in send, walletcreatefundedpsbt, fundrawtransaction, and bumpfee?
 6917:14 <tarun> sorry for jumping the gun glozow :)
 7017:15 * docallag Spending from a LN channel to an onchain transaction (submarine swap)?
 7117:15 <larryruane> I was surprised `fundrawtransaction` is on this list, because doesn't that fund (only) from sources (UTXOs) that this wallet has the keys for? So wouldn't it know the size?
 7217:15 <glozow> tarun: no worries! it's a fundamental part of understanding the PR so thanks for bringing it up. the ordering of questions isn't that important
 7317:16 <achow101> larryruane: fundrawtransaction can take specified inputs which may be external
 7417:16 <glozow> larryruane: nope, fundrawtransaction doesn't need to only use internal inputs
 7517:16 <larryruane> ok thanks, in case anyone's wondering: https://bitcoinexplorer.org/rpc-browser?method=fundrawtransaction#Help-Content
 7617:17 * glozow doesn't know how submarine swaps work, but thanks docallag for providing an answer
 7717:17 <michaelfolkson> Basically any complex script that the Core wallet doesn't understand (at least until the Core wallet supports Miniscript)
 7817:17 <larryruane> i see now, `fundrawtransaction` can take an argument that already includes some inputs, got it
 7917:18 <achow101> michaelfolkson: miniscript does not cover all cases either. e.g. Lightniing HTLCs are not valid miniscript
 8017:18 <glozow> michaelfolkson: no, it's not about understanding, it's about knowing what the input will look like. miniscript doesn't help with this.
 8117:18 <achow101> glozow: miniscript does help if provided as solving_data
 8217:19 <glozow> ah because it tells you max spend size?
 8317:19 <btckid> sorry not clear for me the possible use cases
 8417:19 <achow101> yes
 8517:20 <michaelfolkson> A collaborative Lightning channel close is just a spend from a 2-of-2. If you want to construct a transaction from that 2-of-2 to a single key address in your Core wallet?
 8617:21 <ziggie> larryruane>Note that all existing inputs must have their previous output transaction be in the wallet> so I guess this PR introduces this possiblity to include inputs thats are external ?
 8717:21 <ziggie> concerning the fundrawtransaction command
 8817:22 <michaelfolkson> It gets a bit complicated because with Lightning you are dealing with two wallets, a Lightning wallet and the Core wallet. And the Core wallet doesn't understand what the Lightning wallet is doing
 8917:22 <achow101> michaelfolkson: no. the lightning use case here is to be able to spend HTLCs because the wallet does not recognize HTLCs, even if tracking those outputs
 9017:22 <achow101> s/spend/fund a transaction spending HTLC inputs
 9117:23 <achow101> and unilateral closes as those are a complex script
 9217:23 <docallag> cccccclvtngetjekdrftvirnijtrucvirluulckneggf
 9317:23 <glozow> presumably you can specify the input weight spending from the (as mentioned previously, non-standard HTLC) outputs in your fee-bumping child
 9417:23 <docallag> cccccclvtngevdchrkfejhgnccehrigbkeunjuvjutjf
 9517:23 <michaelfolkson> So spending from a commitment transaction (ie not a collaborative 2-of-2 close)
 9617:23 <docallag> sorry
 9717:24 <bitcoin_pleb_pau> poetry
 9817:24 <achow101> michaelfolkson: yes
 9917:24 <michaelfolkson> The Core wallet would understand a spend from a 2-of-2 but it wouldn't understand the hash lock, time lock etc. Ok yeah, thanks
10017:24 <svav> Why doesn't the original issue give more information about the use cases? It does not seem very detailed https://github.com/bitcoin/bitcoin/issues/23187
10117:25 <btckid> svav: agree
10217:26 <svav> Also, is the current status that the type of transactions referenced by this issue simply cannot happen?
10317:26 <kouloumos> svav it actually links to a comment on the previous PR which shows how the issue came to be
10417:26 <glozow> i believe so. https://github.com/bitcoin/bitcoin/pull/17211#issuecomment-933656171
10517:27 <glozow> yeah, #23187 links to the conversation between tbast and achow
10617:28 <michaelfolkson> svav: Just that you can't work out if you enough inputs to pay the estimated fee (I think)
10717:28 <glozow> it looks like we've answered the first 4 questions so i'll move on the next, if people are comfortable?
10817:29 <glozow> ok cool
10917:29 <btckid> ok
11017:29 <glozow> The RPCs will throw a `JSONRPCError: RPC_INVALID_PARAMETER` if the specified weight is below 165. Why can’t the weight of an input be less than 165?
11117:29 <michaelfolkson> svav: You might think you have enough in your inputs to pay the estimated fee but then later you realize you don't
11217:29 <bitcoin_pleb_pau> Question: Does using an RPC call to request the weight of an external input sacrifice some privacy because it conveys what kind of script-spending-conditions are associated with a public key?
11317:29 <achow101> michaelfolkson: for external inputs, if their size cannot be estimated, the funding will simply fail
11417:30 <glozow> bitcoin_pleb_pau: no, the resulting transaction should look the same regardless of what kinds of arguments you passed to the RPC
11517:31 <michaelfolkson> achow101: You mean the wallet won't try? In theory it could try and occasionally fail right?
11617:31 <docallag> glozow given the sig size it couldn't be smaller than 165?
11717:31 <achow101> bitcoin_pleb_pau: not anymore than broadcasting the final transaction would be
11817:31 <glozow> docallag: well, not all inputs need a signature
11917:32 <glozow> hint: how large is the smallest possible input? https://github.com/bitcoin/bips/blob/master/bip-0144.mediawiki#serialization
12017:32 <glozow> hint #2: how is weight calculated? https://github.com/bitcoin/bitcoin/blob/d0bf9bb6a539f151ec92725d20a2b6c22cb095a5/src/consensus/validation.h#L155-L159
12117:32 <achow101> michaelfolkson: if a pre-selected input is set and its size is unable to be estimated, coin selection will fail.
12217:32 <OliverOffing> bitcoin_pleb_pau when you say RPC call, do you mean from the wallet to the node?
12317:33 <bitcoin_pleb_pau> OliverOffing yes
12417:33 <glozow> OliverOffing: the wallet doesn't communicate with the node via RPC. this would be the user talking to the wallet via RPC.
12517:33 <bitcoin_pleb_pau> i mean...no.
12617:34 <glozow> ok let's break down the components of the smallest possible input (i.e. a completely empty input)
12717:35 <glozow> we can calculate it together
12817:35 <bitcoin_pleb_pau> I need to brush up on my understanding of RPCs...I always assumed it was only between nodes, but glozow points out, if i understand, that when you are manipulating your wallet, your wallet is communicating with Your OWN node via RPC?
12917:36 <glozow> if you see the link to GetTransactionInputWeight, you'll see that the weight = 4 * serialized size of nonwitness data + serialized size of witness data
13017:36 <achow101> bitcoin_pleb_pau: RPC is not used to communicate between nodes nor between any components of core. it is an external interface for users to interact with Core in a programmatic way
13117:36 <glozow> nodes communicate with each other via the P2P protocol, which is built on TCP/IP
13217:37 <ziggie> (size of outpoint + sequence + empty scriptSig) *4
13317:37 <glozow> ziggie: exactly! :)
13417:37 <ziggie> for the nonwitness part
13517:37 <glozow> the outpoint is txid + index in vout
13617:38 <stickies-v> I think we need (32 txhash + 4 txindex + 1 scriptlen +4 sequenco_no) * 4 bytes = 164?
13717:38 <glozow> txid size should be easy - can someone tell us how many bytes that is?
13817:38 <kouloumos> 32
13917:38 <glozow> stickies-v: very good!
14017:38 <stickies-v> sorry not bytes, WUs
14117:38 <ziggie> + 1 empty fitness stickies-v>
14217:38 <glozow> kouloumos: yes
14317:38 <ziggie> = 165
14417:38 <glozow> ziggie: bingo. empty witness requires 1 byte to encode its size, which is 0
14517:38 <stickies-v> yeah I was wondering where that last one went haha, thx
14617:39 <glozow> stickies-v: my hint was going to be that since it's 1 mod 4, it must be in the witness
14717:39 <glozow> awesome. thanks ziggie, stickies-v, kouloumos! that's why the input can't possibly be less than 165 weight units.
14817:40 <OliverOffing> According to this, Witness data is not
14917:40 <OliverOffing> multiplied by 4 (https://en.bitcoin.it/wiki/Weight_units)
15017:40 <OliverOffing> when calculating the weight… or am I missing something?
15117:41 <glozow> OliverOffing: yes, that's why it's 164 + 1 = 165
15217:41 <glozow> hopefully that makes sense?
15317:41 <bitcoin_pleb_pau> if it were, it would be 164+4=168
15417:41 <glozow> right
15517:41 <stickies-v> the 1 byte empty scriptsig is the only witness data in this basic tx
15617:42 <OliverOffing> got it
15717:42 <glozow> great
15817:42 <OliverOffing> thanks
15917:42 <glozow> In the interface modified by this PR, how would a user call `fundrawtransaction` to specify a maximum input weight?
16017:45 <OliverOffing> With a new argument called `input_weights` which is expected to contain an array of dicts, which should include the fields `txid`, `vout`, and the explicitly defined `weight` of that tx
16117:46 <glozow> OliverOffing: yep! in other words, a mapping from outpoint to max weight
16217:46 <bitcoin_pleb_pau> can we elaborate on the data structure?
16317:46 <bitcoin_pleb_pau> each array entry is a dict?
16417:46 <bitcoin_pleb_pau> and inside each dict includes the fields 'txid' 'vout' and probably others?
16517:47 <tarun> what are the values? the corresponding weight?
16617:47 <OliverOffing> this might make it more clear https://github.com/bitcoin/bitcoin/pull/23201/files#diff-26141d9c7da21eeb4b9e3ffedfaad83212d4710a9e62888f7abea076ca1d0538R676-R686
16717:47 <bitcoin_pleb_pau> thx
16817:48 <glozow> each element of the array corresponds to an input. the element is represented as a dict with keys txid, vout, and weight.
16917:48 <OliverOffing> hm, would it make sense to rename the field to `max_weight` instead then?
17017:49 <glozow> i don't know if that's better. there's not going to be a `min_weight` or anything
17117:50 <OliverOffing> i guess the code base usually calls it just `weight` because it usually knows the exact number -- external inputs is kind of an edge case
17217:52 <glozow> Alright we've got <10 minute so let's try to get through the next few questions
17317:52 <glozow> What is a Compact Size Unsigned Integer? Where is it used in the Bitcoin protocol?
17417:52 <glozow> (Bonus question: is it part of consensus rules?)
17517:53 <Kaizen_Kintsugi_> is it part of serialization?
17617:53 <glozow> Kaizen_Kintsugi_: yep
17717:53 <glozow> elaborate?
17817:53 <OliverOffing> it's a variable-length (unsigned) integer
17917:53 <btckid> it's a varibale size unsigned integer
18017:53 <Kaizen_Kintsugi_> I am unable too
18117:53 <glozow> OliverOffing: btckid: perfect, and where is it used?
18217:54 <Kaizen_Kintsugi_> has something to with endian formatting?
18317:54 <btckid> distinct to varint
18417:54 <svav> https://btcinformation.org/en/developer-reference#compactsize-unsigned-integers
18517:54 <larryruane> definitely part of consensus rules
18617:54 <OliverOffing> well I'm just looking things up now but apparently it's pretty common everywhere in the Bitcoin P2P protocol
18717:55 <OliverOffing> why is it called "Compact" as opposed to "Variable-length Unsigned Integer"? :)
18817:55 <glozow> yep, usually used to encode things like how many stack elements you have, how many bytes something is, etc.
18917:55 <svav> Compact because it saves data probably - a guess
19017:55 <btckid> my confusion is when `Compact Size Unsigned Integer` us used  vs `varint`
19117:55 <glozow> presumably because you save a lot of space when you're encoding small numbers
19217:56 <bitcoin_pleb_pau> so varint would be the one used for smaller fields?
19317:57 <OliverOffing> "If you're reading the Satoshi client code (BitcoinQT) it refers to this encoding as a "CompactSize". Modern Bitcoin Core also has the VARINT macro which implements an even more compact integer for the purpose of local storage (which is incompatible with "CompactSize" described here). VARINT is not a part of the protocol."
19417:57 <glozow> i.e. while you want to be able to encode large numbers, in the vast majority of cases you only need 1 byte to encode size
19517:57 <bitcoin_pleb_pau> sorry i think i lost the plot
19617:57 <sipa> So the Bitcoin P2P protocol only has one variable-length integer serialization format, namely the "compactsize" one.
19717:57 <btckid> thanks
19817:57 <glozow> thanks sipa
19917:57 <sipa> But Bitcoin Core internally has another, more complex but slightly smaller one, which is used in its UTXO set database serialization.
20017:58 <sipa> The latter is called VarInt in the codebase.
20117:58 <sipa> But CompactSize is also "a" varint format, and is often referred to as such.
20217:58 <sipa> (I introduced the second one, so blame me for the confusion)
20317:58 <glozow> understood. so we should try not to use varint when talking about the p2p compact size data type, so we don't get confused
20417:59 <btckid> agree
20517:59 <glozow> let's try to squeeze in one more question: How does the FillInputToWeight() function fill an input to the target weight? Why does it care when the weight to add is between 253 and 263?
20618:00 <sipa> Also, originally CompactSize was only used for the length of vectors or other things, not as a generic number encoding. The BIP152 compact block encoding however started using the compactsize encode for non-length things.
20718:00 <sipa> (don't let me interrupt you)
20818:01 <larryruane> I think that history is extremely useful, thanks sipa
20918:01 <glozow> +1, thanks sipa
21018:01 <glozow> okay we're out of time, but i wanted to throw in that last question because the CompactSize talk was setup for it
21118:01 <glozow> thanks for coming everyone :) happy reviewing
21218:01 <glozow> #endmeeting