add coverage for dust mempool policy (-dustrelayfee setting) (tests)

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

Host: larryruane  -  PR author: theStack

Notes

concepts

  • Each bitcoin transaction output carries a specific value. Bitcoin Core defines dust as an output whose value is less than what it would cost to spend this output.

  • An important goal of the Bitcoin network is decentralization, so there are various development efforts to keep the resource costs for running a fully-validating node to a minimum. One way to reduce the storage requirement is to keep the size of the UTXO set small.

  • It would be inexpensive for an attacker, or a careless wallet, to create many tiny-value UTXOs, bloating the UTXO set. Whoever is able to spend these UTXOs (and thus remove them from the UTXO set) would have little to no incentive to do so.

  • For this reason, Bitcoin Core has a policy of not accepting into its mempool or relaying any transaction with a spendable dust output, that is, an output whose value is below a dust limit.

details

  • When validating an incoming transaction, policy code calculates the fee, at a particular feerate, to “pay for” both the output and the (later) spending input. This fee is proportional to the sum of the sizes, measured in virtual bytes, of both the input and output.

  • If the output value is below this (hypothetical) fee, it is considered dust; it would cost more to spend this output than its value.

  • An output’s virtual size is just its physical size, but an input typically also includes witness data which is discounted: 4 bytes of witness data is counted as one byte of virtual data.

  • Rather than the dust feerate being hardcoded, bitcoind includes a configuration option -dustrelayfee=<feerate> to set this value.

  • This feerate is in units of BTC per kvB (1000 virtual bytes). The default is 0.00003 BTC per kvB (3000 sats/kvB or 3 sat/vB).

  • More information can be found on stackexchange.

  • There are several kinds of standard outputs (for example, P2PK, P2PKH). Some have a characteristic size, both of the output itself and for the input that will later spend it. Enforcing the dust limit requires the code to estimate the sizes of the various kinds of inputs and outputs.

  • The concept of dust was first introduced in PR #2577. This commit from PR #9380 introduced the -dustrelayfee option. Previous to that PR, the dust feerate was whatever -minrelaytxfee was set to.

Questions

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

  2. How does the test work? What is its approach?

  3. Why is the concept of dust useful? What problems might occur if it didn’t exist?

  4. A transaction with an output considered “dust” is classified as a valid, but non-standard, transaction. What is the difference between valid transaction and a non-standard transaction? Would it be better if transactions that don’t meet the dust treshhold were considered invalid?

  5. Why does the dust feerate limit apply to each output individually, rather than to all of a transaction’s outputs collectively? Can you think of an anomolous case in which this policy conflicts with being miner-incentive compatatible?

  6. Why is this feerate a configuration option, which makes it fairly static (most node operators probably just accept the default), rather than having it dynamically track the prevailing network feerate?

  7. Why is -dustrelayfee a hidden (or debug) option?

  8. Since -dustrelayfee is a per-node configuration option, what happens if various nodes on the network set different values?

  9. Can you see a future scenario where we’d want to change the default value of -dustrelayfee? Would it more likely be increased or decreased? What does this depend on and which other configuration options would then also very likely be adapted?

  10. What does the largest possible output script that adheres to standardness rules look like? Is it currently implemented in the functional test?

  11. Which of the output scripts need to be inferred from an actual public key (derived from ECKey in the test)? Could some of them also be created with only random data?

  12. The P2TR output script to test is created with pubkey[1:]. What does this expression do and why is this needed? Would that also work with an uncompressed pubkey? (Idea: learn about pubkey encoding and the concept of x-only-pubkeys)

  13. Can you give an example of an output script that is considered standard and is added to the UTXO set (i.e. no null-data), but is still unspendable? Bonus: is there a way to create such an output script where this unspendability can even be mathematically proven?

Meeting Log

  117:00 <d33r_gee> hello
  217:00 <stickies-v> hi
  317:01 <rozehnal_paul> hi
  417:01 <Jmy> Hi everyone
  517:01 <andrewtoth_> hi
  617:01 <guest647> hi
  717:01 <b_101_> hi
  817:01 <ishaanam[m]> hi
  917:02 <LarryRuane> #startmeeting
 1017:02 <LarryRuane> hi!
 1117:02 <emzy> hi
 1217:02 <schmidty_> hi
 1317:02 <theStack> hi
 1417:02 <LarryRuane> welcome everyone! today we'll be discussing https://bitcoincore.reviews/26631 "test: add coverage for dust mempool policy (-dustrelayfee setting)"
 1517:03 <glozow> hi
 1617:03 <LarryRuane> feel free to say hi so we know who's present!
 1717:03 <lightlike> hi
 1817:03 <LarryRuane> By the way, if anyone has a suggestion for a PR to review, or if you'd like to volunteer to host a review club, please leave a comment here or DM me on IRC!
 1917:04 <LarryRuane> So what do people think of this PR, any general thoughts?
 2017:05 <LarryRuane> Any questions on the Notes for today's review? Anything not clear or you'd like to discuss?
 2117:05 <rozehnal_paul> Seems like a non-controversial PR, more testing the better..
 2217:06 <LarryRuane> rozehnal_paul: +1
 2317:06 <LarryRuane> Who had a chance to review the PR?
 2417:07 <d33r_gee> question on running the test... is it done by running test_framework.py?
 2517:07 <b_101_> y/tested ACK
 2617:07 <LarryRuane> We're honored to have the PR author here, @theStack
 2717:08 <theStack> d33r_gee: the simplest is to directly call the python file, i.e. $ ./test/functional/mempool_dust.py
 2817:08 <b_101_> d33r_gee: you can run it directly `test/functional/mempool_dust.py`
 2917:08 <schmidty_> Echoing rozehnal_paul , perhaps question for LarryRuane /group: what would be a common reason to NACK a PR that adds testing coverage?
 3017:08 <LarryRuane> and I want to thank him, and @glozow for their help in preparing for this review club meeting!
 3117:09 <d33r_gee> theStack thanks! will try that
 3217:09 <LarryRuane> I can't think of a reason to NACK, but only to make some suggestions
 3317:09 <glozow> Tests require maintenance too. A test should be clear enough such that, in the future, if it fails, we know what's gone wrong in the code.
 3417:09 <andrewtoth_> yes, but that would be a reason to suggest changes, not NACK right?
 3517:10 <theStack> schmidty_: one reason that i could think of NACKing is if a test is too resource-hungry or takes too long
 3617:10 <theStack> andrewtoth_: +1
 3717:10 <rozehnal_paul> +1 @gloz
 3817:10 <glozow> andrewtoth_: sure
 3917:10 <rozehnal_paul> woops
 4017:10 <LarryRuane> one mistake I used to make in writing tests is to make them too fragile ... if the test makes a very narrow requirement for the result, then it can break in the future when there's not really anything wrong
 4117:11 <rozehnal_paul> +1 glozow : if the approach needs to be rethought, it ought to be NACKd
 4217:11 <LarryRuane> there's quite an art to writing a good test ... you want it to verify correct functionality (not leave something important unverified), but not be overly specific
 4317:12 <ishaanam[m]> andrewtoth_: another reason for NACKing could be if the suggested test could make more sense as a unit test instead of a functional test.
 4417:12 <glozow> ishaanam[m]: +1
 4517:12 <LarryRuane> I think another trend we'd like to promote for testing is move from functional (python) tests to unit tests, when possible
 4617:12 <d33r_gee> tested ACK
 4717:13 <LarryRuane> or even make code changes that make it *possible* to test with unit tests ... when something fails in a unit test, it's often much easier to narrow down where the problem is, because you're not running as much code
 4817:13 <b_101_> LarryRuane: can you elaborate the rationale for that?
 4917:13 <LarryRuane> i think the refactoring effort in the P2P layer has this as one of its goals
 5017:14 <LarryRuane> well, functional tests run one or more full nodes, and there are more chances for false failures due to timing windows or things like, shutting down and restarting nodes being less .... reliable?
 5117:15 <schmidty_> And speed?
 5217:15 <b_101_> LarryRuane: ok, thx
 5317:15 <LarryRuane> also as I said, when something goes wrong, there's so much code that's being run by the test, it may be hard to tell where the problem actually is
 5417:16 <LarryRuane> schmidty_: +1 yes, unit tests can run MUCH faster than functional tests for a given amount of actual testing ... you don't have the delays in starting up the nodes, for example
 5517:17 <LarryRuane> great question! I think we'll always have the functional tests, but the more that can be tested in unit test the better, all else equal
 5617:17 <rozehnal_paul> LarryRuane implying that unit tests do not require full-node usage nor starts&stops?
 5717:17 <rozehnal_paul> just to be clear
 5817:18 <theStack> talking about speed, in an earlier version of the PR the test used multiple nodes, one for each config options (like it's currently done e.g. in mempool_datacarrier.py)... even with a small number of nodes, the test took significantly longer to be setup (non-surprisingly), so i changed to one node that is just restarted
 5917:18 <LarryRuane> rozehnal_paul: yes that is correct
 6017:18 <b_101_> theStack: +1
 6117:19 <LarryRuane> theStack: one thing I wondered, would it be a good idea to have an RPC to change this dustrelayfee, do you think? maybe test-only?
 6217:19 <LarryRuane> then you wouldn't need to restart the node at all ... something i wondered as i reviewed the PR
 6317:19 <LarryRuane> of course that's then changing production code! no longer just a test-only PR! (so harder to get merged)
 6417:20 <LarryRuane> we're kind of into question 2 already, but can anyone tell us, How does the test work? What is its approach?
 6517:21 <b_101_> It creates all posible Script types including a couple future SegWit versions, and try each of this scripts on a list of `-dustrelayfee` arbitrary settings, including the default of `-dustrelayfee` of 3000
 6617:23 <LarryRuane> yes, it tests various combinations along 2 dimensions (that's why you see two nested loops in `run_test`
 6717:24 <rozehnal_paul> sorry if this is off-track, but i couldn't read line 102: [.8f] isn't something i could recognize
 6817:24 <rozehnal_paul> what does .8f mean
 6917:24 <schmidty_> theStack: 1337 haha
 7017:24 <rozehnal_paul> schmidty_ lol
 7117:25 <LarryRuane> rozehnal_paul: no that's not off-track, that's a great question .. are you familiar with the "f" strings that python3 now supports?
 7217:25 <glozow> it means display a float to the 8th decimal point
 7317:26 <LarryRuane> formatted strings ... provides a more convenient way to, well, format strings! I think the .8f means floating point with 8 digits of precision (to the right of the decimal point)
 7417:26 <rozehnal_paul> LarryRuane not really but i just googled it. thanks & thanks glozow
 7517:27 <LarryRuane> good question! it's nice when we cover actual code in these review clubs! :)
 7617:27 <theStack> if someone is wondering why the `.8f` was needed, start up your python interpreter and type in "0.00000001". what you see as a result is a notation that bitcoind can't make sense of
 7717:27 <theStack> schmidty_: xD
 7817:28 <LarryRuane> there's a whole family of script generation functions such as `key_to_p2pk_script()` that are very interesting to examine .. @theStack added those in a previous PR, very helpful to both the tests and for understanding
 7917:28 <rozehnal_paul> theStack python3 changed it to scientific notation [when accessed from a variable name at leat]
 8017:29 <theStack> rozehnal_paul: exactly!
 8117:30 <LarryRuane> feel free to continue previous discussions, but let's get to Q3, Why is the concept of dust useful? What problems might occur if it didn’t exist?
 8217:30 <schmidty_> theStack: thoughts on doing the output_key_to_p2tr_script helper doing the truncation itself?
 8317:31 <michaelfolkson> hi
 8417:31 <LarryRuane> schmidty_: can you explain why the `pubkey[1:]` is there?
 8517:31 <theStack> the review club session LarryRuane is talking about was https://bitcoincore.reviews/22363, hosted by glozow. for anyone learning about scripts and output types, it's a great exercise to fill out this table there
 8617:31 <schmidty_> LarryRuane: I believe x-only-pubkeys, saving a byte: https://bitcoinops.org/en/newsletters/2019/11/13/#x-only-pubkeys
 8717:31 <rozehnal_paul> dust could be used as an attack vector by enlarging the utxo set, as dust has to be accounted for by fullnodes, and if there are 50million dust outpusts to account for, then fullnodes get...tired.
 8817:32 <d33r_gee> Q3:  it helps to keep the size of the UTXO set small, which is important for maintaining the decentralization of the Bitcoin network.
 8917:32 <rozehnal_paul> so we preempt the attack by creating the idea that there are transactions that are too small to be valid\
 9017:32 <LarryRuane> schmidty_: beautiful! Everyone here should be reading Optech each week, by the way!
 9117:32 <b_101_> This expression list the bytes object skipping the first byte, since `pubkey` is a compressed public key, the first byte indicate `x02=even`, `x03=odd`. This piece of data plus the `x` coordinate (bytes 2 to 33) is used to calculate `y` in compressed keys
 9217:33 <LarryRuane> rozehnal_paul: yes, great answer!
 9317:34 <LarryRuane> b_101_: +1
 9417:34 <theStack> schmidty_: theoretically the output_key_to_p2tr_script helper could be adapted to accept both legacy and x-only-pubkeys, by looking at size, yes. not sure if we would really need it that often though, i think for using p2tr we usually create x-only-pubkeys from the start
 9517:36 <ishaanam[m]> rozehnal_paul: these transactions are technically still valid and can be mined into valid blocks. However these transactions are considered non-standard, which means that they are not relayed to other nodes.
 9617:36 <b_101_> My understanding is that uncompressed pubkeys are disallowed for non legacy descriptors/scripts since segwit implementation, is this right?
 9717:36 <LarryRuane> having a dust limit is what i would say an anti-DoS measure ... many things in Bitcoin Core are anti-DoS!
 9817:36 <LarryRuane> (the first part of the Notes covers this)
 9917:36 <theStack> ishaanam[m]: +1
10017:37 <schmidty_> Thoughts on the concept of dust becoming less useful as p2tr gets more widely adopted (more folks using scripts and the cost to spent the output being largely unknown)?
10117:38 <LarryRuane> schmidty_: That's a great point, the dust caclulation has to sort of "guess" at how big the future spending input will be, and with p2tr, that's gets harder
10217:39 <michaelfolkson> No different to P2SH or P2WSH though right? Script is still hashed until you come to spend
10317:40 <michaelfolkson> Maybe encourages use of longer scripts
10417:40 <LarryRuane> michaelfolkson: good point, but is the variation greater with p2tr?
10517:41 <LarryRuane> Q4 has been partially answered by ishaanam[m]: "What is the difference between valid transaction and a non-standard transaction?"
10617:41 <rozehnal_paul> schmidty_ very interesting
10717:41 <rozehnal_paul> ishaanam[m] what stops a malicious miner from accepting dust, and thereby circumventing our Dust-Dos-Defense??
10817:42 <schmidty_> michaelfolkson: exactly. Id anticipate more p2tr since you can bring along complex scripts "for free" (keyspend case)
10917:42 <rozehnal_paul> ishaanam[m] **
11017:42 <rozehnal_paul> could you elaborate or send a link for those (me) who wants to learn about 'keyspend case'
11117:42 <rozehnal_paul> schmidty_ ^
11217:43 <LarryRuane> rozehnal_paul: "what stops a malicious miner from accepting dust" -- nothing! but by not forwarding, it's much less likely that a miner will ever see transactions with dust outputs
11317:43 <ishaanam[m]> LarryRuane: +1
11417:44 <LarryRuane> for that reason (second part of Q4): Would it be better if transactions that don’t meet the dust treshhold were considered invalid?
11517:44 <theStack> rozehnal_paul: this workshop is great for learning taproot https://bitcoinops.org/en/schorr-taproot-workshop/ covers both key-path and script-path spends
11617:45 <rozehnal_paul> LarryRuane A miner creating and trying to mine its own dust transaction would soon go bankrupt, I would imagine.
11717:45 <rozehnal_paul> Q4P2: There is likely a reason I'm missing, but I don't see why dust txs are invalidated outright
11817:45 <rozehnal_paul> thx theStack
11917:47 <ishaanam[m]> For Q4: I don't think that would be better because as mentioned previously, this is more of a "guess" so I don't think that it would make sense to hold all transactions to this partially arbitrary standard for validation.
12017:47 <LarryRuane> I think the answer is (but others chime in), if we make dust part of consensus, then we could never lower it later, because that would be relaxing a rule, which would be a hardfork
12117:48 <LarryRuane> it would make tx that were previously illegal, now legal ... we could *raise* the dust limit, that would be a softfork ... (do i have this right, anyone?)
12217:49 <theStack> LarryRuane: that's also my understanding
12317:49 <LarryRuane> this relates to Q9: "Can you see a future scenario where we’d want to change the default value of -dustrelayfee? Would it more likely be increased or decreased? What does this depend on and which other configuration options would then also very likely be adapted?"
12417:49 <schmidty_> Im not sure how prevalent they are anymore, but protocols built on Bitcoin like counterparty allowed issuance of tokens. Some of those tokens could have a high $ value, but be stored in a low BTC value output.
12517:51 <LarryRuane> schmidty_: interesting! so this might be a reason to lower the dust limit in the future? I was thinking if BTC because much more valuable per unit in the future, what's considered dust today would not be then
12617:51 <LarryRuane> (similar to what you said)
12717:51 <rozehnal_paul> spitballing: if tx fees were somehow lowered in the future, we could lower the dust limit, as it would cost less to spend. not sure how this would affect other config.options
12817:52 <rozehnal_paul> Q9
12917:52 <schmidty_> Not sure either way, just pointing out theres a lot of non BTC value in BTC outputs which complicates the dust idea.
13017:52 <theStack> thought experiment for Q9: let's say for years blocks are more or less constantly full with a minimum fee-rate of hundreds of sats/vbyte. would that be a reason to *increase* the default dust-limit at some point?
13117:54 <LarryRuane> seems like that's true! hard to get my mind around!
13217:55 <schmidty_> theStack: I would think so. Fun fact:There was a period of high fees in 2017 and due to poor UTXO management, Coinbase had a bunch of small value UTXOs that were worth less than the fee to spend them (https://irishtechnews.ie/coinbase-accused-of-incompetence-after-hoarding-millions-of-utxos/).
13317:55 <LarryRuane> i think that's the answer we (or actually @theStack, who wrote this question) was looking for, the `incrementalrelayfee=` option might want to change also
13417:55 <michaelfolkson> A fee market denominated in Bitcoin so shouldn't be impacted by price of Bitcoin in fiat terms but purely the demand for block space
13517:55 <LarryRuane> schmidty_: did they spend them anyway?
13617:56 <michaelfolkson> If demand for block space was permanently much higher then yeah you'd probably want to increase dust feerate as no chance of current dust getting into a block
13717:57 <schmidty_> LarryRuane: consolidated a few months later I believe: https://medium.com/@alcio/when-the-bitcoin-dust-settles-878f3431a71a
13817:57 <rozehnal_paul> schmidty_ that event in 2017 is how i first heard of dust!
13917:58 <LarryRuane> schmidty_: thanks, we're getting close on time, is there any remaining question anyone would like to bring up or answer?
14017:58 <rozehnal_paul> +1 consolidated a few months later
14117:59 <Jmy> Is it possible in theory to merge multiples UTXOs by using cross-input signature aggregation and then spend all these poorly managed UTXOs at once (paying also the tx-fee only once)?
14217:59 <LarryRuane> looks like we've already covered Q12
14318:00 <LarryRuane> cross-input sig aggregation isn't implemented yet, IIUC
14418:00 <b_101_> What did you mean by: Why is -dustrelayfee a hidden (or debug) option?
14518:00 <LarryRuane> ok sorry, we're at time, please feel free to keep discussing!
14618:00 <LarryRuane> #endmeeting
14718:01 <svav> Thanks all
14818:01 <d33r_gee> thanks everyone
14918:01 <emzy> Thanks everyone!
15018:02 <michaelfolkson> Thanks!
15118:02 <theStack> thanks for hosting LarryRuane and to all participants!
15218:02 <LarryRuane> thanks to everyone for contributing! and especially @theStack for the PR!
15318:02 <ishaanam[m]> thanks!
15418:02 <schmidty_> Thanks LarryRuane and theStack
15518:02 <Jmy> Thanks all of you!
15618:02 <theStack> i'm also still around for ~30 minutes if anyone wants to continue discussing
15718:03 <LarryRuane> theStack: what's the answer to Q10 "Q4 has been partially answered by ishaanam[m]: "What is the difference between valid transaction and a non-standard transaction?" ... i'm not sure!
15818:03 <LarryRuane> oh sorry, copy-paste error,
15918:03 <andrewtoth_> thanks LarryRuane!
16018:04 <LarryRuane> What does the largest possible output script that adheres to standardness rules look like? Is it currently implemented in the functional test?
16118:04 <rozehnal_paul> Thanks So much, cant wait till next week
16218:05 <theStack> any participants still around that want to answer Q10?
16318:05 <LarryRuane> my *guess* is that `key_to_p2pk_script(uncompressed_pubkey)` would be the largest
16418:05 <rozehnal_paul> im here
16518:05 <theStack> it's already on the larger side compared to widely used output scripts, but still far from the largest :)
16618:05 <schmidty_> Jmy: Absent cross input aggregation, in theory a miner during low fees could allow a block of dust to be spent with no fees to cleanup UTXO set as a service to node operators. Not sure where the output of all that dust would go though.
16718:06 <b_101_> thank you all
16818:07 <theStack> hint for Q10: think about an output script type that is even less used than P2PK
16918:08 <theStack> (or well, at least less used _nowadays_...)
17018:08 <LarryRuane> schmidty_: this is interesting, so a miner could advertise on twitter or somewhere, "directly send me zero-fee transactions, and if they spend dust and don't create MORE dust, i'll mine them" just to be nice? wouldn't the people sending those transactions decide where the output goes?
17118:08 <LarryRuane> does anyone know how many dust UTXOs exist currently?
17218:09 <michaelfolkson> Less than P2PK? A future witness version :)
17318:09 <schmidty_> LarryRuane: sure they would decide, but it would just be a dust output again unless it was aggregated into larger output amounts.
17418:10 <theStack> michaelfolkson: heh, good one :D those are way too short though for being candidates for the answer
17518:11 <schmidty_> theStack: bare multisig?
17618:11 <theStack> LarryRuane: interesting question about how many dust UTXOs... might be a nice mini-project to find that out
17718:11 <theStack> schmidty_: bingo!
17818:12 <LarryRuane> schmidty_: is that standard tho?
17918:12 <LarryRuane> oh it is! okay cool, TIL!
18018:12 <theStack> what are the specifics of the _largest_ bare multisig considered standard?
18118:12 <LarryRuane> guessing 3/5?
18218:13 <LarryRuane> i see your test does have "bare multisig (3-of-3)"
18318:14 <theStack> yes, but is that specific one really the largest one? (what kind of pubkeys does it use?)
18418:14 <schmidty_> LarryRuane: Unchained did some research on this previously, but its out of date: https://unchained.com/blog/dust-thermodynamics/
18518:14 <LarryRuane> compressed!
18618:15 <LarryRuane> schmidty_: +1 thanks!
18718:15 <theStack> LarryRuane: exactly, i.e. it's not the maximum size
18818:16 <LarryRuane> i changed the test to: `keys_to_multisig_script([uncompressed_pubkey]*3)` and it still passes
18918:16 <schmidty_> "During the high-fees market of late 2017, 15–20% of all UTXOs had value densities below the lowest fee of 50–60 Satoshi/byte, making them almost impossible to spend. 40–50% of all UTXOs had value densities below the average fee of 600–700 Satoshi/byte, making them harder to spend."
19018:16 <LarryRuane> would that be good to add as another test case, do you think?
19118:16 <theStack> LarryRuane: exactly, and that's the maximum AFAICT
19218:16 <theStack> LarryRuane: agree!
19318:17 <Jmy> schmidty_: iicu this means that e.g. once a year people who know that they have some dust around could negotiate where to send all of that, maybe to donate somewhere? And the miner could then creat a new output which then is no dust anymore and could be spend by someone else?
19418:17 <schmidty_> Jmy: They could donate to Brink. Hear those grantees do great work.
19518:17 <LarryRuane> schmidty_: interesting, I heard a rumor that Roger Ver was behind the attack (if you want to think of it that way) to generate very high fees ... to make BCH look better
19618:18 <theStack> in pre nulldata (OP_RETURN) times i think people used primarily bare multisig outputs to store arbitrary data in blocks
19718:18 <LarryRuane> theStack: "that's the maximum AFAICT" -- what about 3 of 5?
19818:18 <theStack> LarryRuane: standardness rules only allow up to n=3
19918:18 <LarryRuane> theStack: i read that too
20018:18 <LarryRuane> theStack: i see, thanks
20118:19 <theStack> the m in m-of-n doesn't matter by the way, as it doesn't change the output script size
20218:19 <LarryRuane> OH, good point
20318:20 <Jmy> schmidty_: cool, thanks for your explanation
20418:20 <rozehnal_paul> neat
20518:21 <LarryRuane> theStack: curious, did you look into doing this test as a unit test, is it possible?
20618:21 <LarryRuane> if not, it would be interesting to see what would be required to make it unit-testable
20718:22 <theStack> LarryRuane: good question, didn't look into it, i'm more familiar with the functional ones
20818:22 <rozehnal_paul> LarryRuane is your preference for unit > functional tests universal in software engineering, or specific to bitcoin? and is unit > functional testing preference controversial or pretty accepted?
20918:22 <LarryRuane> functional tests ARE very cool, i love how easy they are to read!
21018:23 <michaelfolkson> You've got PRs merged with unit tests in the past though right theStack?
21118:23 <michaelfolkson> i swear I've seen some
21218:23 <LarryRuane> rozehnal_paul: i think it's universal, not just bitcoin core
21318:24 <schmidty_> rozehnal_paul: Not Bitcoin specific. Back in my web engineering days the preference was similar. Unit test (run frequently) everything where possible (using mock objects if needed) and "integration" tests to test that everything is working together (run less frequently)
21418:24 <LarryRuane> maybe i overstated it during review club ... it's important to have both, unit tests that zero in on one piece of logic, but also functional tests to make sure everything can work together
21518:24 <theStack> michaelfolkson: yeah there must have been some, a longer time ago
21618:25 <LarryRuane> it's conceivable that unit tests covering two different subsystems both pass, but then when you put them together, one makes an unwarranted assumption about the other, so the functional test fails
21718:25 <michaelfolkson> I didn't realize how many PRs you've got merged. 235! Mostly functional tests I think
21818:26 <rozehnal_paul> thx
21918:26 <LarryRuane> (or integration tests, as @schmidty_ said -- YES, we used that term in earlier projects i've worked on!)
22018:28 <theStack> michaelfolkson: some of them really small refactorings also. i feel like the number of commits/PRs alone is kind of an inadequate measure alone
22118:29 <theStack> anyone wants to give a shot at Q13 "Can you give an example of an output script that is considered standard and is added to the UTXO set (i.e. no null-data), but is still unspendable? Bonus: is there a way to create such an output script where this unspendability can even be mathematically proven?"? i thought this one was fun
22218:30 <LarryRuane> as long as we're on the topic of tests, one thing i learned the hard way over the years is to keep the tests EXTREMELY SIMPLE, they don't have to be efficient! Here's a great example: https://github.com/bitcoin/bitcoin/blob/678889e6c6231cf461de59eefe6fb8eb07468848/src/test/util_tests.cpp#L275
22318:31 <michaelfolkson> theStack: Requiring a signature from a public key that doesn't have an associated private key?
22418:31 <LarryRuane> (this is a positive example, BTW, not a negative example!) this sequence of tests could have been written with some clever loop,
22518:31 <LarryRuane> but then the reader would have to understand how the loop works and confirm that it's correct... the way it's actually written is SO obvious
22618:32 <theStack> LarryRuane: hehe nice one
22718:32 <theStack> michaelfolkson: yes, but can you prove that there is no associated private key?
22818:33 <LarryRuane> michaelfolkson: your answer sounds right ... if the public key is like 0000000000000... then
22918:33 <LarryRuane> we can be PRETTY sure that no one has a private key that goes with that, although theortically possible?
23018:35 <LarryRuane> to go back to what i was saying about testing ... if the test is complex (to save lines of code, cpu time, memory, whatever), then you might feel the need to write a test for the test! we DON'T want to go there!!
23118:36 <theStack> LarryRuane: yeah, for all those "theoretically possible" cases we can't mathematically prove that it's unspendable... but there are possibilities for output scripts that are guaranteed to be unspendable
23218:37 <LarryRuane> OP_FALSE?
23318:37 <theStack> that also, but i mean ones that are considered standard
23418:37 <LarryRuane> oh i see.. hmm no i can't think of it!
23518:38 <theStack> it's scripts that contain a public key that is not on the curve
23618:39 <theStack> i.e. ones that don't fulfill the secp256k1 y^2 = x^3 + 7 equation
23718:39 <michaelfolkson> Does Taproot prevent including public keys not on the curve?
23818:40 <michaelfolkson> Need to check the BIP. It suggests using a random public key *on* the curve when you want to encode an unspendable path
23918:41 <instagibbs> you can't spend them, but you can make them
24018:41 <theStack> michaelfolkson: it doesn't prevent. i naively opened a PR a while ago trying to change those to be considered non-standard, but didn't consider the implications for e.g. exchanges: https://github.com/bitcoin/bitcoin/pull/24106
24118:42 <michaelfolkson> Ha interesting PR, I never saw this one
24218:43 <LarryRuane> really interesting comments on that PR!
24318:44 <michaelfolkson> I think I was preoccupied with something else in January of this year
24418:44 <michaelfolkson> Why I missed it
24518:44 <theStack> here is the wallet-related variant of the same PR btw: https://github.com/bitcoin/bitcoin/pull/24121 it was rejected with the same reasoning
24618:44 <michaelfolkson> But yeah its a fun one
24718:45 <LarryRuane> amazing how much can be learned by closed PRs!
24818:45 <LarryRuane> *by reading closed PRs
24918:47 <michaelfolkson> So should it have been disallowed by consensus in the original soft fork? I'm guessing no
25018:47 <michaelfolkson> Because you might want to use an unspendable path
25118:48 <instagibbs> you'd have to update wallet address decoding software to not send to it
25218:48 <instagibbs> f.e.
25318:49 <michaelfolkson> Oh of course, yeah
25418:49 <instagibbs> I just read that off the closed PR :) good history
25518:50 <schmidty_> Bitcoin NOPtech, for coverage of unmerged PRs
25618:50 <theStack> i wonder if sending to pubkeys that are not on the curve would have been invalid since the beginning, if that would have prevented storing big amounts of data in bare multisig outputs
25718:50 <theStack> on the other hand, one could always use the output scripts with hashes for that...
25818:51 <theStack> schmidty_: haha i'd subscribe immediately
25918:51 <LarryRuane> schmidty_: haha love that!
26018:52 <LarryRuane> theStack: good point, no way to check if a hash has a preimage!
26118:53 <theStack> so maybe it would have been even worse because people stored the same data divided up into a larger number of UTXOs
26218:53 <instagibbs> there's an old (never deployed) gmax idea to gossip partial preimages of p2sh to force people to do lot of hash work to store stuff in utxo set, somewhere on bitcointalk...
26318:56 <theStack> instagibbs: cool, if you happen to find that again, would love to read it
26419:02 <instagibbs> Can't figure out what search terms to use, heh
26519:04 <instagibbs> I suspect the idea was something like: in p2p, p2sh outputs must be accompanied by the ripemd160 preimage(sha2 hash of the redeemscript) in order to be propagated
26619:07 <instagibbs> so to use the utxo set as a storage layer, you'd have to do quite a bit of hashing generating a 32 byte preimage that does that