Check for non-representable CFeeRates (tx fees and policy, wallet, refactoring)

https://github.com/bitcoin/bitcoin/pulls/20546

Host: jonatack  -  PR author: jonatack

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

Notes

  • Today’s review club is A Tale of Two Fee Rate Units, and the ability starting from Bitcoin Core 0.21 to set one of them explicitly in Bitcoin Core’s various send RPCs.

  • Bitcoin Core has historically used fee rates denominated in bitcoins per 1000 bytes (BTC/kB); post-segwit, the unit is 1000 virtual bytes (BTC/kvB).

  • You can see this by running ./src/bitcoin-cli help settxfee and ./src/bitcoind -help-debug | grep fee

  • Nevertheless, users have expressed what appears to be a long-standing preference for fee rates to be expressed in satoshis per vbyte (sat/vB) instead, which is a commonly used unit in other wallets. This demand can be seen, for instance, in Bitcoin Core issues and in twitter polls.

Background

  • PR 11413, “Add explicit feerate option,” was merged in June 2020. That pull enabled users to specify for the first time a specific fee rate in some send RPCs. However, it did so by overloading the integer conf_target param/option to also accept floats or strings. Then, if a float or string was passed to represent a fee rate instead, the unit was taken from the estimate_mode param/option that the PR also overloaded. See this comment for more context.

  • Issue 19543, “Normalize fee units for RPC (BTC/kB and sat/B),” was opened in July 2020 to address the problem.

  • PR 20220, “Explicit fee rate follow-ups/fixes for 0.21,” was merged in November 2020 to provide a spec and regression coverage for the potential next step of moving to a universal fee rate argument in satoshis per vbyte (sat/vB) as well as immediate coverage and fixes for 0.21.

  • PR 20305, “Introduce fee_rate sat/vB param/option,” was merged in November 2020 to address issue 19543 and is part of the 0.21 release. It replaces overloading the conf_target and estimate_mode params/options with a new fee_rate param/option denominated in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs, as well as other fixes. It was followed by PR 20246, “Allow zero-fee fundrawtransaction/walletcreatefundedpsbt and other fixes,” that was backported to 0.21.

Current situation

  • RPCs sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee all allow specifying an explicit fee rate in sat/vB starting from Bitcoin Core v0.21 released in January 2021.

  • It would be ideal for all fee rates to be in the same units. However, if you run git grep feeRate, you will see that we still have a similarly spelled feeRate (BTC/kvB) option in RPCs fundrawtransaction and walletcreatefundedpsbt that could confuse users, as fee_rate and feeRate have different units and similar spellings.

  • PR 20483, “deprecate feeRate in fundrawtransaction/walletcreatefundedpsbt,” was opened to deprecate the feeRate option but cannot yet be considered. (Why?)

  • All of the fee rate configuration options are currently still denominated in BTC/kvB units and need to be updated.

  • Issue 20534, “sat/b values aren’t validated to be in range,” was opened in November 2020 and remains unresolved.

Code

  • Today’s PR is an accessible change to resolve issue 20534 while doing some CFeeRate refactoring (see the pull request description for details).

  • The CFeeRate class in src/policy/feerate.{h,cpp} is responsible for constructing fee rates of different units. It also contains comparison operators and a ToString() helper function.

  • SetFeeEstimateMode() in src/wallet/rpcwallet.cpp is the central function for setting fee estimation or the fee rate that is used by the wallet transaction RPCs and by FundTransaction() that some of the RPCs call through. These are all located in the same file, rpcwallet.cpp.

  • AmountFromValue() in src/rpc/util.cpp is a frequently used utility that converts UniValue amounts from user input to CAmount values suitable for constructing CFeeRate objects.

Questions

  1. Explain the difference between BTC/kvB and sat/vB units. A fee rate of 1 BTC/kvB is equivalent to what fee rate in sat/vB?

  2. What is the difference between an RPC parameter and an RPC option?

  3. When making transactions, what fee practice does the ability to specify an explicit fee rate replace?

  4. At the time of this writing, why is it too early to deprecate the feeRate (BTC/kvB) option in fundrawtransaction and walletcreatefundedpsbt to avoid confusion with fee_rate (sat/vB)?

  5. After some discussion, a plan was made to complete the migration from BTC/kvB to sat/vB units. What is it? Does it make sense to you?

  6. In the meantime, if users confuse fee_rate with feeRate when making a transaction or fee bumping, why was the risk of losing funds considered low enough to be temporarily acceptable?

  7. What range of values is non-representable by CFeeRate? How does this PR resolve that? Is there a better approach?

  8. What is the “Named Constructor Idiom”? Why does this PR use it?

  9. The new utility function, FeeRateFromValueInSatB(), calls another utility function named AmountFromValue(). Why are there two of them in the codebase (one in src/rpc/util.cpp and one in src/bitcoin-tx.cpp)?

  10. Running ./src/bitcoind -help-debug | grep fee returns the fee rate configuration options. After setfeerate and estimatefeerate in sat/vB are added and settxfee and estimatesmartfee in BTC/kvB become hidden RPCs, what would be the best way to migrate the fee rate units in these configuration options from BTC/kvB to sat/vB?

Meeting Log

  118:00 <jonatack> #startmeeting
  218:00 <kanzure> hi
  318:00 <jnewbery> hi
  418:00 <murtyjones> hi
  518:00 <svav> hi
  618:00 <jonatack> Hi all! Welcome to this week's episode of the Bitcoin Core PR Review club. 🎉
  718:00 <AnthonyRonning> hi
  818:00 <pinheadmz> hi
  918:00 <michaelfolkson> hi
 1018:00 <jonatack> We usually start Bitcoin Core IRC meetings with a 'hi' to know who is here. Feel free to say hi! If you have a signet address, append it your "hi" to receive some sBTC (signet coins) for good answers. 🍰
 1118:00 <norisgOG> Hi
 1218:00 <larryruane_> hi!
 1318:00 <jonatack> This week, we're talking about PR #20564 "Check for non-representable CFeeRates (tx fees and policy, wallet, refactoring)" which is a bugfix and part of our ongoing migration of fee rates units from BTC/kvB to sat/vB units (by popular demand). 🚀
 1418:00 <glozow> hiya
 1518:00 <jonatack> Notes and questions are at https://bitcoincore.reviews/20546
 1618:00 <n3wBEEEE> hi
 1718:01 <schmidty> howdy
 1818:01 <jonatack> pinheadz or I will send signet coins to people with good answers using the explicit fee rate feature. 🍰
 1918:01 <andozw> Hiiiii
 2018:01 <comfy> hi
 2118:01 <thomasb06> hi
 2218:01 <jonatack> Here is an RPC command template you can use to try sending coins to people (or to yourself) from your signet node using the new fee_rate param (just add their address):
 2318:01 <jonatack> ./src/bitcoin-cli -signet -named sendtoaddress address="" amount=0.01 fee_rate=1
 2418:01 <ccdle12> hi
 2518:01 <jonatack> or to yourself if no one posts a signet address :)))
 2618:01 <michaelfolkson> Don't know where the private key is but here's mine: tb1qmmn6u4y9wv3jvwgktyw42nc5ql7t6shs0f2229
 2718:02 <michaelfolkson> Lesson: don't lose your Signet private key
 2818:02 <sunon> Hi!
 2918:02 <AnthonyRonning> still need to set up a signet node
 3018:02 <jonatack> Anyone get a chance to read the notes and review the PR? y/n y/n
 3118:02 <AnthonyRonning> y
 3218:02 <norisgOG> y
 3318:02 <thomasb06> n
 3418:02 <murtyjones> yes to notes, no to PR
 3518:02 <sishir> ^^
 3618:03 <svav> y
 3718:03 <n3wBEEEE> nn
 3818:03 <sunon> y n
 3918:03 <michaelfolkson> y / 0.3y
 4018:03 <jonatack> Great, let's get started!
 4118:03 <jonatack> 1. Explain the difference between BTC/kvB and sat/vB units. A fee rate of 1 BTC/kvB is equivalent to what fee rate in sat/vB?
 4218:03 <sishir> “difference between BTC/kvB and sat/vB units means that a transaction with a fee rate mistakenly calculated in BTC/kvB rather than sat/vB should raise an error due to the fee rate being set too low.”
 4318:03 <norisgOG> 100000 sats per byte
 4418:03 <larryruane_> 100,000?
 4518:03 <emzy> hi
 4618:03 <goums> hi
 4718:03 <AnthonyRonning> 1 BTC/kvB = 100000 sat/vB
 4818:03 <emzy> n
 4918:04 <sunon> 100k
 5018:04 <thomasb06> same
 5118:04 <jonatack> nice! sBTC to sishir norisgOG larryruane_ and AnthonyRonning (if you post an address)
 5218:04 <larryruane_> is one advantage avoiding messy floating point?
 5318:04 <jonatack> sunon: yes!
 5418:04 <jonatack> A sat/vB is 1e5 times larger than a BTC/kvB, so 1 BTC/kvB is equal to 100000 sat/vB.
 5518:04 <jnewbery> 1e5
 5618:04 <larryruane_> tb1qc32gukwejqkyh7z3cyfafsg5spj0cd4hfml4rm
 5718:05 <michaelfolkson> 100 million sats in a Bitcoin. 1000 vB in a kvB
 5818:05 <jonatack> sBTC to jnewbery and sunon too
 5918:05 <jonatack> 2. What is the difference between an RPC *parameter* and an RPC *option*?
 6018:05 <thomasb06> a parameter is mandatory
 6118:05 <pinheadmz> 1 sBTC to larryruane_ !
 6218:05 <larryruane_> option is a boolean param?
 6318:05 <AnthonyRonning> parameters are required and options are optional
 6418:05 <murtyjones> required vs optional
 6518:06 <sishir> one is mandatory, the other is optional
 6618:06 <jonatack> interesting
 6718:06 <michaelfolkson> option has a dash before it, parameter doesn't
 6818:06 <jonatack> My answer was: An RPC parameter is a standalone input argument; an option is an argument inside the options JSON hash.
 6918:06 <jnewbery> I think the distinction is pretty pointless since we got named arguments a few releases ago
 7018:07 <michaelfolkson> Does this match standard Unix definitions? https://stackoverflow.com/questions/36495669/difference-between-terms-option-argument-and-parameter
 7118:07 <jnewbery> the options object doesn't benefit from the built-in type checking / help formatting that RPC parameters do
 7218:07 <jnewbery> and just make the call signature unnecessarily complex
 7318:08 <jonatack> i wonder if it was to allow avoiding positional param issues before we had named params
 7418:08 <jonatack> let's move on
 7518:08 <jonatack> (see bitcoin-cli help send for an example of both.)
 7618:08 <jonatack> 3. When making transactions, what fee practice does the ability to specify an explicit fee rate replace?
 7718:09 <AnthonyRonning> It replaces estimating fee based on desired confirmation target
 7818:09 <ccdle12> replaces the user of the fee estimator
 7918:09 <jonatack> Yes! Explicit fee rate is an alternative to using fee estimation with the conf_target and estimate_mode params/options, to allow Bitcoin Core to set the fee rate for you.
 8018:10 <jonatack> 4. Why is it too early to deprecate the `feeRate` (BTC/kvB) option in `fundrawtransaction` and `walletcreatefundedpsbt` to avoid confusion with `fee_rate` (sat/vB)?
 8118:10 <sishir> https://github.com/bitcoin/bitcoin/pull/20483
 8218:10 <sishir> The estimatesmartfee is still in BTC/kB, so merging this would force all users to do the conversion themselves (or force to pass -deprecatedrpc).
 8318:10 <AnthonyRonning> There was already a feature freeze for 0.21 but it should now be considered okay for 22.0, right?
 8418:10 <michaelfolkson> What's the history on the fee estimator? I'm presuming that this is pretty similar to the fee estimators on other non-Core wallets?
 8518:10 <jonatack> sishir: correct!
 8618:11 <jonatack> AnthonyRonning: all of the feerate changes were merged after the 0.21 FF given they were considered fixes to an open issue
 8718:12 <AnthonyRonning> ahh, gotcha. wasn't sure if some where merged in before.
 8818:13 <jonatack> I proposed to deprecate feeRate but because people may use RPC estimatesmartfee to estimate the fee needed, but that RPC returns the fee rate in BTC/kvB and not in sat/vB, it was considered to early.
 8918:13 <svav> What is the difference between a virtual byte and a byte?
 9018:13 <jonatack> michaelfolkson: I don't know. I do like to be able to set the fee rate explicitly and use this feature most of the time.
 9118:14 <jonatack> svav: good question. anyone want to reply?
 9218:14 <michaelfolkson> svav: https://bitcoin.stackexchange.com/questions/89385/is-there-a-difference-between-bytes-and-virtual-bytes-vbytes
 9318:14 <ccdle12> svav: vbytes referes to the txs size in weight (segwit), bytes is simply the raw byte lenght of the tx
 9418:14 <thomasb06> vB is 1000 instead of 1024
 9518:15 <thomasb06> arg..
 9618:15 <michaelfolkson> Bytes are bytes. Virtual bytes are adjusted bytes based on SegWit adjustment
 9718:15 <sishir> svav vsize aka vytes is tx using weighted size under segwit rules
 9818:15 <sishir> * vbytes
 9918:15 <larryruane_> so a tx's size in vbytes is always greater-or-equal to its size in bytes?
10018:15 <comfy> vB is a synthetic unit to estimate the byte load for transactions on the chain, post-segwit
10118:16 <michaelfolkson> Lesser or equal larryruane_
10218:16 <jonatack> have a look at https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki
10318:16 <sishir> I think vbytes is lesser
10418:16 <norisgOG> thomasb06 I thought vbytes of a presegwit tx are equal to psot segwit therefore I think still both 1024 bytes
10518:17 <norisgOG> but the meaning is diffrent
10618:17 <thomasb06> norisgOG: definitely
10718:17 <sunon> Bit late but tb1qg0x546029q0ee3cuuvlx46anxrlsfkk5qy27eg
10818:18 <jonatack> feel free to continue discussing while we keep moving
10918:18 <jonatack> sunon: yay!
11018:18 <jonatack> 5. A plan was made to complete the migration from BTC/kvB to sat/vB units. What is it? Does it make sense to you?
11118:18 <jonatack> hint https://github.com/bitcoin/bitcoin/pull/20484#issuecomment-734786305
11218:19 <AnthonyRonning> ah cool, i couldn't for the life of me figure out where the plan was
11318:19 <michaelfolkson> I think so. Don't understand exactly why that ordering made the most sense but seems reasonable
11418:19 <emzy> I think mostly user demand.
11518:19 <sishir> Wallet demands as well
11618:20 <jonatack> emzy: yes, dev pushback on not breaking user space
11718:20 <pinheadmz> sunon 1 sBTC for u !
11818:20 <jonatack> It seemed the best way to normalize and transition to sat/vB per user demand, protect users by not having feeRate/feerate in BTC/kvB and fee_rate in sat/vB in the same RPCs, and minimize the amount of breaking API changes.
11918:20 <jonatack> 6. In the meantime, if users confuse fee_rate with feeRate when making a transaction or fee bumping, why is the risk of losing funds considered low enough to be temporarily acceptable?
12018:21 <AnthonyRonning> because they get a nice fast confirmation :)
12118:22 <michaelfolkson> This was the rationale that the user would underpay on fee when overpaying on the fee would be more of a concern. Right?
12218:22 <murtyjones> i presume the wallet software would reject fees that are wildly high/low?
12318:22 <svav> When instroducing new feerate variable, why not name it more differently from existing fee rate to avoid confusion?
12418:22 <jonatack> note that we deliberately broke backward compatibility wrt fee_rate in RPC bumpfee
12518:22 <larryruane_> because specifying the old units (BTC/kvB) results in a too-low fee when the actual units are sat/vB?
12618:22 <emzy> I think it will be so high that there sould be a warning.
12718:22 <norisgOG> users fee estimate is 1/100000 lower if he confuses both
12818:23 <jonatack> svav: initially i named it fee_rate_sat_vb, but reviewers considered it best to use fee_rate and i agree
12918:23 <larryruane_> there's been a worldwide shortage of underscores, we must conserve them!
13018:23 <jonatack> Pretty good replies. Luckily, the 1e5 difference is large enough that any user confusion should be benign.
13118:24 <jonatack> Specifying BTC/vkB (current value) in place of the new sat/vB would always be a much lower feerate. So it'd most likely be too low and error, or worst case lower than you intended and you can just bump it again to fix.
13218:24 <AnthonyRonning> ahh i had that backwards
13318:24 <jonatack> Or as Murch wrote, "In the worst case, someone is going to get upped to the minRelayTxFee silently and send at 1 sat/vB. Since RBF is on by default, they should be able to bump when they notice."
13418:25 <svav> In your new code, could you code it so that the units are specified in a separate variable, in case you ever want to change units again?
13518:25 <jonatack> Ok, let's move into the code
13618:25 <jonatack> 7. What range of values is non-representable by CFeeRate? How does this PR resolve that? Is there a better approach?
13718:25 <AnthonyRonning> 0 to 0.001, non inclusive. It checks if, after conversion, if the value is zero but the initial value was not zero. Checking for `> 0 && < 0.001` instead of allocating a `CFeeRate` could be another approach.
13818:26 <jonatack> AnthonyRonning: correct!
13918:26 <Murch> Hey, sorry, I had a conflicting meeting, but this one is totally down my alley ^^
14018:27 <Murch> sishir: Yes, a transactions virtualsize is strictly smaller or equal to its raw byte size.
14118:27 <jonatack> Murch: hi! yeah Murch was co-author and a major reviewer behind these changes
14218:27 <Murch> norisgOG: there are 1000 bytes in a kB in Bitcoin :-/
14318:28 <norisgOG> Murch thx
14418:28 <jonatack> AnthonyRonning: which approach of the two would you use?
14518:29 <sishir> Murch Thankyou
14618:29 <AnthonyRonning> i typically would use the range check. Is there a reason why allocating `CFeeRate` is done here? Is there a hidden benefit i'm missing?
14718:29 <AnthonyRonning> performance, or otherwise
14818:30 <jonatack> AnthonyRonning: no, strangely enough I only thought of the first way
14918:31 <AnthonyRonning> less use of magic numbers though, so there's a benefit there
15018:31 <jonatack> AnthonyRonning: thanks for the ideas.
15118:31 <jonatack> 8. What is the Named Constructor Idiom? Why does this PR use it?
15218:32 <AnthonyRonning> A way to be more explicit about the object you are creating. This PR uses it by `CFeeRate::FromSatB` and `CFeeRate::FromBtcKb` .
15318:32 <svav> To distinguish between the old charge units and the new ones.
15418:32 <AnthonyRonning> never heard of that term before but i think that's the idea
15518:33 <jonatack> Yes, several contributors found the current constructor confusing to use
15618:33 <michaelfolkson> https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Named_Constructor
15718:33 <AnthonyRonning> I still see some places in code that use `CFeeRate(X, 1000)`, is there further refactoring that could be implemented to be consistent?
15818:33 <AnthonyRonning> for instance, here https://github.com/jonatack/bitcoin/blob/ed414f6dd72c103b5ba9e17c6b6bd2bcc8548b5b/src/wallet/wallet.cpp#L3962
15918:33 <jonatack> AnthonyRonning: yes
16018:34 <jonatack> I found it in isocpp.org/wiki/faq/ctors#named-ctor-idiom by Stroustrup, Cline, Sutter and Alexandrescu. Its goal is to provide more intuitive and/or safer construction operations for users of your class.
16118:35 <jonatack> AnthonyRonning: yes, good eye. I didn't change them all, maybe I should.
16218:36 <jonatack> There are a few that cannot be changed, however, but most cases are covered by the named ctors
16318:36 <jonatack> michaelfolkson: nice link, thanks
16418:37 <jonatack> This PR uses the Named Constructor Idiom to encapsulate the confusing "pass COIN for sat/vB, 1000 for
16518:37 <jonatack> BTC/kvB to the ctor" into CFeeRate itself, by popular demand as requested by several contributors. One example is at https://github.com/bitcoin/bitcoin/pull/20305#discussion_r519986635
16618:37 <larryruane_> Seems like named constructors are nice when the argument list can't be used to distinguish between different forms of initializers (e.g. both take uint64_t but the units are different, as is the case here)... nice! I wasn't aware of this!
16718:38 <jonatack> larryruane_: right! i like it too
16818:39 <jonatack> Question: are you people concept ACK on using the IsZero() member function to replace CFeeRate() == 0 conditionals?
16918:40 <jonatack> I found it curious to construct just to check for a zero CFeeRate
17018:40 <AnthonyRonning> yeah i think it makes sense to me to be more explicit about it
17118:40 <murtyjones> encapsulating code like that seems useful to me, for auditability if nothing else
17218:41 <jonatack> and there a quite a few of those in the codebase
17318:41 <larryruane_> i like it too, it's like isNull
17418:42 <jonatack> 9. FeeRateFromValueInSatB calls another utility function named AmountFromValue. Why are there two AmountFromValue functions in the codebase (one in src/rpc/util.cpp and one in src/bitcoin-tx.cpp)?
17518:42 <jonatack> (thanks for the feedback!)
17618:42 <AnthonyRonning> Because you can work with transactions through the RPC and the command line (from what i can tell), and `AmountFromValue` should always be used when inputing numerical values from the user.
17718:43 <SurajUpadhyay> Is the meeting for today over ??
17818:43 <jonatack> about question 9: when i needed to call AmountFromValue from src/bitcoin-cli.cpp, i found i needed to make a *third* copy of this function
17918:44 <michaelfolkson> SurajUpadhyay: No, in progress
18018:44 <jonatack> AnthonyRonning: right! but why are there two of them?
18118:45 <AnthonyRonning> ah yeah, better question. I'm not sure, is there not a better place for utility functions to exist? Or are they slightly different, need to check again.
18218:45 <jonatack> hint, look in test/lint/lint-circular-dependencies.sh
18318:45 <larryruane_> i notice the exceptions they throw is different
18418:47 <jonatack> I couldn't pull it into bitcoin-cli.cpp without adding a circular dependency
18518:47 <jonatack> "policy/fees -> txmempool -> policy/fees"
18618:47 <jonatack> at least, that is my recollection, i could be wrong!
18718:48 <Murch> Given that there are quite a few types of amounts in the codebase that have distinct ranges and shouldn't be easily added together, have there been more attempts to make classes for specific types of amounts?
18818:48 <larryruane_> if this doesn't sidetrack the discussion too much (feel free to ignore), what's the reason we even check for circular dependencies? what goes wrong if we don't? there are exceptions anyway....
18918:48 <murtyjones> i see. given that there's a manual linter, does C++ not do any kind of circularity checking when it compiles?
19018:48 <michaelfolkson> I have same question larryruane_
19118:48 <jonatack> larryruane_: i think we really really don't want more circular dependencies, but it requires some refactoring to remove them
19218:49 <svav> How is a circular dependency introduced?
19318:50 <jonatack> the current assumeUXTO PR adds one, for instance, but pulling things out of validation code isn't easy
19418:51 <jonatack> svav: by code pulling in headers that link to each other, though that's not an official definition
19518:52 <michaelfolkson> And this bash script checks no new ones are introduced?
19618:52 <jonatack> murtyjones: i'd have to look
19718:52 <murtyjones> no biggie just curious.
19818:52 <jonatack> michaelfolkson: yes, other than the permitted exceptions
19918:53 <jonatack> in EXPECTED_CIRCULAR_DEPENDENCIES
20018:53 <jonatack> if you add one and don't add it to that list, the CI fails
20118:54 <michaelfolkson> I wouldn't describe that as linter/linting but heh
20218:54 <jonatack> 10. What would be the best way to migrate the config option fee rate units in the from BTC/kvB to sat/vB?
20318:54 <jonatack> ./src/bitcoind -help-debug | grep fee
20418:55 <AnthonyRonning> i've seen other projects create a new variable. So an example could be `maxtxfeesat` instead of `maxtxfee`
20518:55 <jonatack> will show you the config options
20618:55 <AnthonyRonning> then deprecate the old one eventually
20718:56 <jonatack> AnthonyRonning: one thing i've noticed is the older options often use "fee" both for an absolute fee and a fee rate
20818:56 <jonatack> which helps for naming, e.g. RPC settxfee to setfeerate for the sat/vB version
20918:56 <jonatack> or estimatesmartfee -> estimatefeerate
21018:56 <michaelfolkson> Do you need to do the normal two version deprecating or can you just supply error messages and do it straight away?
21118:57 <michaelfolkson> I guess you don't know
21218:57 <michaelfolkson> So like a warning rather than an error message
21318:57 <Murch> michaelfolkson: It's preferable not to break userspace
21418:57 <jonatack> michaelfolkson: idk, we did simply remove a banscore config option in 0.21 without any deprecation period
21518:57 <Murch> there are a lot of people running automated scripts against full nodes
21618:57 <jonatack> we wouldn't have done that with the RPC API
21718:58 <michaelfolkson> Right, good point Murch
21818:58 <AnthonyRonning> oh didn't notice that, yeah i see that now. It could have been better to be more explicit between the two in the first place, in retrospect
21918:58 <Murch> I mean, good practice is to test new versions first, but I know a lot of businesses just yolo upgrades
22018:58 <jonatack> I would imagine we'd have to add as AnthonyRonning suggests
22118:58 <Murch> It would be fair to say that it's their fault, but still
22218:59 <TechMiX> AnthonyRonning: sounds like a good idea to me
22318:59 <AnthonyRonning> i think c-lightning and/or lnd had to introduce a new one for `msat`, and eventually deprecating `sat`
22418:59 <jonatack> that makes sense to me
22518:59 <Murch> AnthonyRonning: The downside with making parameter names even more explicit is that you're then stuck with that name even when the old one is long forgotten
22619:00 <jonatack> That's time! Thanks everyone!
22719:00 <jonatack> #endmeeting
22819:00 <Murch> so it comes down to a trade-off between a shortterm confusion and a long term clunky name
22919:00 <comfy> ty jonatack et al
23019:00 <AnthonyRonning> Murch: yeah good point
23119:00 <AnthonyRonning> thank you jonatack! And everyone else. Very informative.
23219:00 <jnewbery> thanks jonatack!
23319:00 <michaelfolkson> Great notes and sess jonatack. Thanks!
23419:00 <thomasb06> thanks jonatack
23519:00 <schmidty> thanks jonatack !
23619:00 <emzy> Thank you jonatack
23719:00 <ecola> thanks jonatack
23819:00 <jnewbery> next week, glozow hosts. Notes and questions will be up this Friday
23919:01 <jonatack> I learned some things from you all and see things to think more about, thanks!
24019:01 <glozow> great notes as always jonatack, hard to follow u
24119:01 <svav> Thanks jonatack
24219:01 <norisgOG> thanks jonatack
24319:02 <n3wBEEEE> thank you all - this was very i9nformative for me