--- Day changed Wed Aug 26 2020 00:40 -!- evanlinjin [~evanlinji@2404:4408:43ff:bb00:bba8:a1bb:ac12:ad14] has joined #bitcoin-core-pr-reviews 01:04 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has quit [Quit: jonatack] 01:08 -!- midnight [~midnight@unaffiliated/midnightmagic] has quit [Ping timeout: 272 seconds] 01:18 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews 01:32 -!- digi_james [sid281632@gateway/web/irccloud.com/x-jxgijiagwbfdkdru] has quit [Read error: Connection reset by peer] 01:32 -!- hebasto [sid449604@gateway/web/irccloud.com/x-nekoxatbhfmysbyh] has quit [Read error: Connection reset by peer] 01:33 -!- illlicit_ [uid109953@gateway/web/irccloud.com/x-lqofpaslqmodjrrh] has quit [Read error: Connection reset by peer] 01:34 -!- ethzero [sid396973@gateway/web/irccloud.com/x-dvkypdacpvvxwixn] has quit [Ping timeout: 260 seconds] 01:36 -!- fjahr [sid374480@gateway/web/irccloud.com/x-ljdthgnwgzteqqfh] has quit [Ping timeout: 260 seconds] 01:36 -!- hebasto [sid449604@gateway/web/irccloud.com/x-gbidkjbzcfuvbpsl] has joined #bitcoin-core-pr-reviews 01:39 -!- digi_james [sid281632@gateway/web/irccloud.com/x-zxvanojkpslpezus] has joined #bitcoin-core-pr-reviews 01:39 -!- illlicit_ [uid109953@gateway/web/irccloud.com/x-bdktvipaukiohqws] has joined #bitcoin-core-pr-reviews 01:39 -!- belcher [~belcher@unaffiliated/belcher] has joined #bitcoin-core-pr-reviews 01:39 -!- fjahr [sid374480@gateway/web/irccloud.com/x-fkopmipbioxkvxko] has joined #bitcoin-core-pr-reviews 01:41 -!- ethzero [sid396973@gateway/web/irccloud.com/x-vimfbqgqqhodlduf] has joined #bitcoin-core-pr-reviews 02:03 -!- jonatack [~jon@37.173.221.225] has joined #bitcoin-core-pr-reviews 03:20 -!- Roger69Koelpin [~Roger69Ko@static.57.1.216.95.clients.your-server.de] has joined #bitcoin-core-pr-reviews 03:22 -!- jonatack [~jon@37.173.221.225] has quit [Read error: Connection reset by peer] 03:25 -!- Roger69Koelpin [~Roger69Ko@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 256 seconds] 03:55 -!- vasild_ [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 03:58 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 03:58 -!- vasild_ is now known as vasild 04:32 -!- tryphe_ is now known as tryphe 05:44 -!- troygiorshev [~troygiors@d67-193-140-136.home3.cgocable.net] has joined #bitcoin-core-pr-reviews 05:54 -!- pinheadmz [~pinheadmz@pool-100-33-69-78.nycmny.fios.verizon.net] has joined #bitcoin-core-pr-reviews 05:57 -!- Landryl [~Landryl@ns528256.ip-192-99-10.net] has quit [Quit: Ping timeout (120 seconds)] 05:57 -!- pinheadm_ [~pinheadmz@64.94.215.92] has quit [Ping timeout: 240 seconds] 05:58 -!- Landryl [~Landryl@ns528256.ip-192-99-10.net] has joined #bitcoin-core-pr-reviews 06:05 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection] 06:36 -!- jonatack [~jon@2a01:e0a:53c:a200:bb54:3be5:c3d0:9ce5] has joined #bitcoin-core-pr-reviews 06:38 -!- midnight [~midnight@unaffiliated/midnightmagic] has joined #bitcoin-core-pr-reviews 07:37 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Quit: Konversation terminated!] 07:43 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews 07:56 -!- gzhao408 [uid453516@gateway/web/irccloud.com/x-vpskgznvneyixfrw] has joined #bitcoin-core-pr-reviews 08:02 -!- shesek [~shesek@unaffiliated/shesek] has joined #bitcoin-core-pr-reviews 08:37 -!- thomasb06 [~user@eth-west-pareq2-46-193-0-224.wb.wifirst.net] has joined #bitcoin-core-pr-reviews 09:47 < jnewbery> we'll get started in 15 minutes 09:47 < jnewbery> *13 minutes 09:54 -!- lightlike [~lightlike@88.116.7.34] has joined #bitcoin-core-pr-reviews 09:57 < evanlinjin> Hello 09:58 -!- elliott [~elliott@172.92.4.53] has joined #bitcoin-core-pr-reviews 09:58 -!- elliott is now known as robot-dreams 09:59 -!- shrouded [~shrouded@172.86.186.172.adsl.inet-telecom.org] has joined #bitcoin-core-pr-reviews 09:59 -!- figs [2f293fee@047-041-063-238.res.spectrum.com] has joined #bitcoin-core-pr-reviews 09:59 -!- dhruvmehta [881873f9@249.115.24.136.in-addr.arpa] has joined #bitcoin-core-pr-reviews 10:00 < gzhao408> WOHOOOO LET'S GET THIS PARTY STARTED 10:00 < gzhao408> #startmeeting 10:00 < nehan> hi! 10:00 < pinheadmz> 🍹 10:00 -!- DariusP [49fce2af@c-73-252-226-175.hsd1.ca.comcast.net] has joined #bitcoin-core-pr-reviews 10:00 < amiti> hi ! 10:00 < gzhao408> Welcome to PR Review Club everyone! 10:00 < pinheadmz> hi 10:00 < troygiorshev> hi 10:00 < DariusP> hi! 10:00 < emzy> hi 10:00 < felixweis> hi 10:00 < jnewbery> hi 10:00 < evanlinjin> hi 10:00 < dhruvmehta> hi 10:00 < michaelfolkson> Haha hi! 10:00 < lightlike> hi 10:00 < jb55> hiya 10:00 < ariard> hi 10:00 < robot-dreams> hi 10:00 < shrouded> hi 10:01 < ajonas> hi 10:01 < gzhao408> Today we’re looking at #19339: re-delegate absurd fee checking from mempool to clients. Have y’all had the chance to review the PR? (y/n) 10:01 -!- behradkhodayar [~behrad@static.126.222.46.78.clients.your-server.de] has joined #bitcoin-core-pr-reviews 10:01 < pinheadmz> y 10:01 < amiti> y 10:01 < robot-dreams> y 10:01 < dhruvmehta> y 10:01 < jonatack> hi (half here) 10:01 < evanlinjin> n 10:01 < jnewbery> y 10:01 < willcl_ark> hi 10:01 < nehan> y 10:01 < jonatack> reviewed: a little 10:01 < lightlike> n 10:01 < troygiorshev> n 10:01 < jb55> n 10:01 < DariusP> n 10:02 < emzy> y/n 10:02 < michaelfolkson> y/n too 10:02 < willcl_ark> yes I took a look at it but not done any custom testing of it 10:02 < gzhao408> SO happy! :D Would anyone like to summarize what the PR is doing (and why)? 10:03 < behradkhodayar> hi, looked at it but not tested 10:03 < dhruvmehta> Summary: This PR moves the responsibility to avoid absurdly high fees from ATMP to upstream places like the RPC layer and wallets. It does so by re-using ATMP logic in dry-run mode (while caching expensive results). This reflects existing incentive structures as nodes should protect their owner-users from high fees, but not minimize fees being 10:03 < dhruvmehta> accepted from other owners. 10:04 < gzhao408> dhruvmehta: yep, it re-delegates the responsibility! could anyone tell us why it should be in clients instead of in ATMP? 10:04 < michaelfolkson> And we are worried about us a user creating a transaction with an absurdly high fee rather than accepting a transaction with an absurdly high fee from another peer into our mempool 10:04 < pinheadmz> because its a user-error not a mempool error 10:04 < pinheadmz> form the perspective of our mempool we shouldnt really care if someone wants to throw away money like that 10:05 < pinheadmz> but we should save the user from teh wallet perspecive from shooting themself in the foot 10:05 < evanlinjin> Sorry for the stupid question, but what is ATMP? 10:05 < evanlinjin> Oh memory pool? 10:05 < willcl_ark> (and miners would actively _want_ those transactions anyway) 10:05 < pinheadmz> accept to memory pool! 10:05 < gzhao408> pinheadmz: exactly! and we'll get to the part about "should" later :) 10:05 -!- Guest2020 [d106dbf8@209.6.219.248] has joined #bitcoin-core-pr-reviews 10:05 < gzhao408> AcceptToMemoryPool for reference: https://github.com/bitcoin/bitcoin/blob/c831e105/src/validation.cpp#L1084 10:05 < evanlinjin> ty 10:06 < jnewbery> evanlinjin ATMP = AcceptToMemoryPool() is the function that we call to process a transaction and add it to the mempool if it's acceptable 10:06 < gzhao408> has a really good point as well, we definitely want to protect users from really high fees 10:07 < gzhao408> (just in the clients, not in ATMP 😛) 10:07 -!- vindard [~vindard@190.83.165.233] has joined #bitcoin-core-pr-reviews 10:07 < gzhao408> Alright let's dive in: does this PR change any behavior or is it a pure refactor? 10:08 < robot-dreams> It changes user-facing error messages returned from the `testmempoolaccept` and `sendrawtransaction` RPCs 10:08 < pinheadmz> mainly a refactor (moving functionality around) but in this case it does remove something from the mempool acceptance 10:08 < dhruvmehta> It changes the behavior for ATMP which will no longer reject transactions with too high a fee. ATMP will instead compute the fee after policy checks and has the test_accept option for a dry run. 10:08 < willcl_ark> well it doesn't change behaviour from the user perspective, but your node might respond differently to incoming transactions 10:09 < nehan> i think it's just a refactor (aside from error codes) based on where 0 was passed in to ATMP everywhere 10:09 < robot-dreams> For wallet and RPC transactions, it also moves the absurd fee check later in the validation process, as dhruvmehta mentioned 10:09 < michaelfolkson> How might your node respond differently to incoming transactions willcl_ark? 10:09 < gzhao408> , , correct! 10:10 < nehan> gzhao408: sorry, are you saying it *is* a behavior change? 10:10 < gzhao408> Here's a thought: ATMP is used for validating/mempoolaccepting both transactions received from peers and from clients (RPC/wallet). Before and after this PR, is it possible for ATMP to return a different result for the same transaction received from a peer as opposed to a client? 10:11 < michaelfolkson> I would say no 10:11 < robot-dreams> Interesting question! I think yes, if you and the peer have different settings for maxfee 10:11 < gzhao408> nehan: well I didn't think there was a real "correct" answer to this question to be honest :) you're correct as well (i just typed kind of late) 10:12 < jonatack> For the record, if anyone wants an interesting definition of policy with regards to bitcoin, gzhao408 gave me a way of thinking about it that I like. https://github.com/bitcoin-core-review-club/website/pull/242#discussion_r471059359 10:12 < emzy> I would also say no. Because 0 was passed into ATMP everywhere. 10:12 < gzhao408> You're correct as well that the behavior doesn't change from the perspective of the user, but I'd also like to hear more about what you meant by "your node might respond differently to incoming transactions" 10:12 < gzhao408> emzy: bingo! this leads us to our next question: There was some discussion as to whether AbsurdFee is policy or a client preference. What do you think, and how did you determine this? 10:13 < pinheadmz> absurdity is an opinion :-) 10:13 < pinheadmz> policy / mempool rules are generally DoS mitigations that are added to consensus rules 10:13 < pinheadmz> although luke-jr had a good point about this too 10:13 < gzhao408> (again, no real "correct" answers here - would love to hear a list of arguments for both policy and client preference) 10:14 < pinheadmz> that there are many settings that could be considered opinoins by the node operator 10:14 < emzy> I would say it's only a client preference. 10:15 < amiti> if I were a miner and somebody sent me a txn via p2p that had an insanely high fee, I would definitely want to accept it. as a user I don't want to create those kinds of transactions. that makes it clear to me that its a client preference 10:15 < willcl_ark> gzhao408: as I was reading it, before, we would reject transactions received from peers who were above our _own_ max_fee, whereas this now means this does not happen 10:15 < gzhao408> pinheadmz: is there an easy way for the node operator to set the max fee? :) similar to how they can set the min fee? 10:16 < jnewbery> willcl_ark: no. absurdfee is only used for locally submitted txs 10:16 < pinheadmz> bitcoind command line arg: 10:16 < pinheadmz> -maxtxfee= 10:16 < pinheadmz> Maximum total fees (in BTC) to use in a single wallet transaction; 10:16 < jnewbery> if you look at the calls to ATMP from net_processing, you'll see that they don't specify an absurdfee 10:17 < michaelfolkson> Right I think it has to be client preference. Although you may want to treat your own transactions favorably in your mempool policy over another's peer transaction 10:17 < sipa> gzhao408: philosophical point mostly, but i wouldn't call the absurdfee rule a policy - it's not something bitcoin core enforces on the network 10:17 < willcl_ark> jnewbery: thanks, I tried to trace that backwards... 10:17 < gzhao408> pinheadmz: is that used for wallet transactions or for the node's validation? :) 10:18 < willcl_ark> jnewbery: ah I see, they use the bypass_limits :) thanks 10:18 < jnewbery> (you can also see that -maxtxfee isn't used by the node, since it's defined in wallet/init.cpp) 10:18 < ariard> michaelfolkson: yes but it's quite limited if your transactions don't propagate, unless you have preferential peering with all the same settings 10:19 < sipa> michaelfolkson: you don't want to treat your own transactions differently from other's transactions in the mempool; the mempool is what you use to predict what will be mined 10:19 < pinheadmz> gzhao408 jnewbery ah, my bad i thought that was the value checked in ATMP currently 10:19 < sipa> michaelfolkson: and using a different policy for your own transactions means a possibly observable privacy leak too 10:20 < michaelfolkson> sipa ariard: But you want it to propagate even if it is very low fee. Don't you rebroadcast from your own mempool....? 10:20 < amiti> michaelfolkson: the wallet initiates rebroadcast, not mempool 10:20 < sipa> michaelfolkson: if it's a very low fee, it won't propagate at all; even if you make your own mempool ignore that fact 10:21 < michaelfolkson> Gotcha I thought that might be wrong, thanks amiti 10:21 < gzhao408> Love the discussion! This PR's opinion is that it's client preference and thus belongs in clients :) and as sipa said, ATMP should give consistent results regardless of which who it comes from 10:21 < gzhao408> "which who" haha, which client 10:22 < gzhao408> Ok let's keep on chugging: This PR changes testmempoolaccept and sendrawtransaction - compare the implementations of the two RPCs. What are the differences? (hint: test_accept) 10:22 < jnewbery> michaelfolkson: if the transaction fee is too low to get in your own mempool (eg if prevailing fees are high and your mempool is dropping low fee txs), then you won't rebroadcast it 10:23 < jnewbery> it's impossible to broadcast something that isn't in your mempool 10:23 < robot-dreams> unlike testmempoolaccept, sendrawtransaction calls into `BroadcastTransaction`, which (i) calls ATMP a second time with `test_accept = true` and (ii) does some extra bookkeeping 10:23 < dhruvmehta> test_accept=1 is a dry run. It runs policy checks and signature checks which are cached though so a second "actual" run is significantly cheaper. 10:23 < gzhao408> for reference testmempoolaccept and sendrawtransaction can be found here: https://github.com/bitcoin/bitcoin/blob/master/src/rpc/rawtransaction.cpp#L800 10:24 < robot-dreams> Also, for some reason testmempoolaccept does an extra check `request.params[0].get_array().size() == 1` (maybe in the future, it's intended to test multiple transactions in a single RPC?) 10:25 < sipa> robot-dreams: ah the age old dream of package relay :) 10:25 < gzhao408> 👌 thanks robot-dreams and dhruvmehta for the explanations! 10:25 < jnewbery> robot-dreams: that's exactly the reason. If we implement package relay/package mempool acceptance, then testmempoolaccept can be changed to support that without changing the interface 10:25 < dhruvmehta> gzhao408 I will try and run your branch later and add timing, but do you happen to know how much cheaper a repeat ATMP call is after a dry run? 10:26 < gzhao408> excellent observation robot-dreams, yes - I believe that is the plan, but it gets a little tricky with transactions that depend on each other (would love to talk about this more after pr review club) 10:27 < gzhao408> dhruvmehta: we have a question coming up later about this :) 10:27 < jnewbery> for anyone is wondering what package relay is: https://bitcoinops.org/en/topics/package-relay/ 10:28 < ariard> the golden dream of safe multi-party transactions 10:29 < jonatack> we did a review club about package relay as well: https://bitcoincore.reviews/16401 10:29 < robot-dreams> Thanks for the link jnewbery! I didn't realize the implications of package relay on fee bumping. 10:29 < michaelfolkson> robot-dreams has got us dreaming :) 10:29 < robot-dreams> haha :) 10:29 < pinheadmz> ok i gotta embarass myself for a sec here... before this PR cant tell where nAbsurdFee comes from 10:29 < pinheadmz> im chasing function calls around the codebase :-P 10:29 < ariard> robot-dreams: in theory you can bump feerate of parent transactions whatever network topology 10:30 < ariard> I've a PoC branch here : https://github.com/bitcoin/bitcoin/pull/19621 10:30 < gzhao408> pinheadmz: it only comes from the clients :P 10:30 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #bitcoin-core-pr-reviews 10:30 < ariard> but actually there is different way to implement it and maybe not the best approach 10:30 < jnewbery> pinheadmz: no embarassment necessary. It's a bit convoluted 10:31 < pinheadmz> gzhao408 jnewbery im trying to find a call to ATMP with an actual value there instead of 0 10:31 < gzhao408> I dream about Package Relay too, but let's go onto the next question! What information do you need to check a transaction’s fee? At what point would the node have this information on hand? (there are many ways. hint: look at where in ATMP the fees are calculated) 10:31 < jnewbery> Everything that uses absurdfee goes through BroadcastTransaction() in node/transaction.cpp 10:31 < gzhao408> pinheadmz: also look at calls to BroadcastTransaction maybe 10:31 < pinheadmz> gzhao408 aha ty i see it now 10:31 < robot-dreams> To check a transaction's fee, you'd need to load the actual UTXOs being spent, because the raw transaction only has `txid`, `vout` 10:32 < jnewbery> The argument there is called max_tx_fee 10:32 < jnewbery> https://github.com/bitcoin/bitcoin/blob/93ab136a33e46080c8aa02d59fb7c2a8d03a3387/src/node/transaction.cpp#L16 10:32 < pinheadmz> yep on it now 10:32 < michaelfolkson> Very cool ariard 10:32 < gzhao408> thanks jnewbery for being super helpful 😊 10:32 < pinheadmz> so wait, then i was wrong earlier - a tx relayed by a peer is not subject to absurd fee bc the value there checked against 0 ? 10:32 < sipa> pinheadmz: indeed 10:32 < willcl_ark> pinheadmz: correct 10:32 < gzhao408> pinheadmz: yes 10:32 < pinheadmz> everyone at once! 10:32 < pinheadmz> thanks, i am now on the same page 10:32 < jnewbery> imhelping.gif 10:33 < willcl_ark> john set me straight on that earlier ;) 10:33 < sipa> gzhao408: in a more PSBT-centric world the fee could be calculated from PSBT information, rather than needing the UTXOs being spent... doing it at broadcast time is arguably too late anyway 10:33 < robot-dreams> sipa: do you mean it should arguably be done at signing time instead? 10:34 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 265 seconds] 10:34 < amiti> I was initially confused about that too. I think that's why this PR makes sense because essentially, it was already a client-only check. the code just didn't make that apparent. 10:34 < sipa> robot-dreams: well, it is - that's why decodepsbt/analyzepsbt show fee information, as you wouldn't want to sign something that has a crazy fee 10:34 < jnewbery> sipa: in that case, no harm in *also* doing it at broadcast time as belt-and-suspenders 10:34 < gzhao408> robot-dreams: is off to a great start - fee is inputs minus outputs. are input amounts included in a transaction/ are they guaranteed to exist? what if the transaction is malformed? 10:34 < sipa> jnewbery: of course 10:35 < sipa> but it's only for "bundled" workflows where signing/broadcast happen at the same time that it's sufficient to only do it at ATMP (or around that time) 10:35 < sipa> amiti: indeed 10:35 < michaelfolkson> input amounts aren't included in a transaction 10:36 < michaelfolkson> Are the amounts guaranteed to exist... hmm not sure what that means. Do you mean are the amounts guaranteed to cover the outputs? 10:37 < ariard> michaelfolkson: are you sure they aren't included in segwit transactions ;) ? 10:37 < gzhao408> michaelfolkson yep, and sorry i meant "are the inputs guaranteed to exist" 10:37 < michaelfolkson> ariard: Oof 10:37 < nehan> we have to look the inputs up in our utxo set. hopefully we have them. 10:38 < gzhao408> nehan: yeah! (the point I'm trying to make is, we have to do a tiny bit of sanitization and "validation" in order to check the fee) 10:38 < sipa> nehan: if we don't, they won't be accepted into the mempool anyway :) 10:38 < sipa> gzhao408: right the property you want is that (a) either the fee calculation is correct or (b) the transaction will be invalid anyway 10:39 < nehan> sipa: yes 10:39 < gzhao408> sipa: yaaaas 10:39 < ariard> due to hashPrevouts 10:40 < jonatack> FWIW, in master it is now possible to set an explicit feerate in sentoaddress, sendmany, bumpfee, fundrawtransaction, and walletcreatefundedpsbt 10:40 < dhruvmehta> Even if we have the utxos, merely subtracting sum{output values} from sum{input values} is not enough. We need to check that policy checks and signature checks will pass. That happens in ATMP. If the tx won't be accepted for other reasons, fee is zero :) and testmempoolaccept needs to raise that other rejection reason. 10:40 < sipa> that's the security model underlying tx signing in general... in offline signing workflows there is no way you can know the input amounts/fees/utxos can exist, but you generally have enough information to decide what the fee will be *or* the tx will be invalid (but see the recent input amounts BIP143 issue) 10:40 < jonatack> since https://github.com/bitcoin/bitcoin/pull/11413 was merged last month 10:40 < nehan> dhruvmehta: i would argue to calculate the fee you shouldn't actually *need* to do all those things 10:40 < ariard> sipa: unless merkleproof and headers verification by your signer, but never seen this on any hardware wallet 10:41 < sipa> ariard: yes, but the point is that that isn't (or at least shouldn't be) necessary 10:41 < jonatack> s/last month/June 25/ 10:41 < sipa> if you're given incorrect UTXOs, your signature should be invalid 10:41 < dhruvmehta> nehan yeah i can see that. it depends on whether we think fee = "fee, if the rest of the tx is valid", of fee = "fee, for the tx as-is" 10:41 < nehan> dhruvmehta: the second call to ATMP will check for all that once you've done the fee check. so as sipa says you just need to be sure that either you've calculated the fee correctly or it would be rejected anyway 10:43 < nehan> ariard: i don't understand what you're talking about, could you say more? 10:43 < gzhao408> nehan beat me to it :) dhruvmehta you're right that we need this information for the end result, although it's not strictly necessary to get the fee (which is why it's gathered in PreChecks) 10:43 < ariard> sipa: but how do you that amounts are correct even if you sign valid utxos ? 10:43 < ariard> I mean you need an oracle 10:43 < gzhao408> Alright, next question? :) This PR _doesn’t_ change the wallet. How does the wallet handle the user’s maximum fees? (hint: m_default_max_tx_fee) Do any changes to the wallet need to be made? 10:44 < sipa> ariard: you don't know they're correct, but if they're incorrect, your signature will be invalid 10:44 < dhruvmehta> nehan ah, i think i understand now. thanks. 10:44 < ariard> right, that's my point providing a merkle proof on all spend transaction and doing headers verification would let know the signer about amounts value ? 10:44 < sipa> ariard: or at least should be - again, see the recent segwit amount signing issue 10:44 < ariard> under a PoW security model 10:45 < sipa> ariard: sure, but what's the point? 10:45 < nehan> dhruvmehta: yeah to be clear in the code it does call ATMP twice so it does do all those checks. 10:45 < jnewbery> ariard: getting a little bit in the weeds. perhaps defer this until after the meeting? 10:45 < ariard> sipa: we likely derive from the main thread, but actually hardware wallet flow is to ask user input ? 10:46 < sipa> ariard: let's discuss after meeting 10:46 < robot-dreams> gzhao408: I think the wallet checks max fee in two places: `CreateTransactionInternal` and `SubmitMemoryPoolAndRelay`; the latter eventually calls into ATMP 10:46 < jnewbery> ariard sipa: thanks :) 10:46 < dhruvmehta> nehan yeah the caching makes the overhead small if most transactions are actually valid. but i can see that if we don't want to presume anything about how many transactions are valid, doing light-weight checks separately prior is valuable. 10:46 -!- jeremyrubin [~jr@2601:645:c200:f539:c134:ae7f:2da9:d893] has joined #bitcoin-core-pr-reviews 10:46 < gzhao408> robot-dreams: awesome! yes! and as pinheadmz pointed out earlier, you can set the -maxtxfee in the wallet 10:48 < gzhao408> so `SubmitMemoryPoolAndRelay` - what does it call to submit to the mempool? ATMP directly or...? 10:49 < amiti> goes through broadcast transaction which calls ATMP 10:50 < gzhao408> amiti: we have a winner! so BroadcastTransaction will give it a belt-and-suspenders check :) 10:50 < gzhao408> alright i want to get to the juiciest question now 10:50 < gzhao408> Let’s consider the cost of dry-running ATMP beforehand and then for realsies afterward: what is cached and what do we have to do all over again? Is this a performance concern? Why or why not? 10:51 < dhruvmehta> The git branch with extra logging makes it clear that caching results in fewer calls. I am unsure about the overhead in terms of time though. 10:52 < gzhao408> yes! c a c h i n g! btw, is referring to this branch I made that adds some logging to ATMP so you can see what’s being cached: https://github.com/gzhao408/bitcoin/commit/483cc24a2923514a35f83682822aa29265c61555 10:53 < nehan> it just pains me that ATMP is being used as a fee calculator 10:53 < nehan> (though i still like this PR) 10:53 < jeremyrubin> nehan: I think that's somewhat inavoidable, no? 10:54 < emzy> There are also a limit on how many TX you can broadcast anyway. 10:54 < jnewbery> nehan: why? The logic is already there! 10:54 < gzhao408> indeed :( but we need to run ATMP anyway in all of these cases 10:54 < jeremyrubin> the only way to know if you have enough fee to go in the mempool is to go in the mempool. 10:54 < pinheadmz> yes gzhao408 did a great job of providing proof-of-efficiency 10:54 < nehan> jeremyrubin: this is not checking to see if you have enough fee to go in the mempool. that check has been moved outside. this is literally just calculating the fee to check it outside the call to ATMP. 10:54 < sipa> i'm not sure that's correct; you needed the inputs to be able to sign the transaction anyway 10:54 < pinheadmz> but i sorta feel with nehan i wish there were another way to comput fee that was outside ATMP 10:54 < jnewbery> jeremyrubin: this isn't about enough fee to get in the mempool. It's about not having _too much_ fee 10:55 < jnewbery> nehan: you beat me to it! 10:55 < nehan> jnewbery: no i was slightly confused, your comment was super helpful. jeremyrubin is still wrong though ;) 10:55 < jeremyrubin> i meant more generally because of CPFP rules you need to at least calc ancestors 10:56 < jeremyrubin> I think? 10:56 < jeremyrubin> Could be wrong on how cpfp plays in. Anyways, not on absurd fee stuff, which is the focus here 10:56 < nehan> jeremyrubin: i'm not arguing ATMP shouldn't calculate fees and check them sometimes. I'm arguing you shouldn't *need* to use the full hammer of ATMP if all you want to do is calculate fees. 10:56 < jnewbery> nehan: I tried to do this before without using ATMP to check the fee and it wasn't as good as this PR: https://github.com/bitcoin/bitcoin/pull/15810 10:56 < nehan> jnewbery: i saw it! 10:57 < michaelfolkson> 5 minutes to go so will attempt to start concurrent conversations. Re Luke's comment that all policy should be split from consensus changes not just one policy. Thoughts? 10:57 < nehan> tbf i don't know how to fix this :) 10:57 < gzhao408> nehan: i tried so hard man 10:57 < jeremyrubin> michaelfolkson: cory is working on that a bit 10:57 < nehan> gzhao408: i think what you did makes sense and is an improvement! 10:57 < michaelfolkson> jeremyrubin: As in Cory is working on the rest in a separate PR? 10:57 < gzhao408> to jeremyrubin's point, the failure cases are (1) doesn't even make it into mempool (2) fee too high. so we do kinda need both 10:58 < jnewbery> michaelfolkson: this isn't policy so I think that's a separate conversation 10:58 < jeremyrubin> it's something else. p2p policy, local safety policy, consensus 10:58 < jeremyrubin> I think it's still "policy" 10:58 < michaelfolkson> Oh yes of course. Ok that answers that haha 10:58 < jnewbery> I don't think 'local safety policy' is helpful terminology! 10:59 < jnewbery> policy already means something else 10:59 < jeremyrubin> not sure what else to call it? 10:59 < ariard> it's wallet policy 10:59 < jnewbery> argh! 10:59 < sipa> sanity check 10:59 < gzhao408> yeah, we've only got 1 minute left! 10:59 < jonatack> psa for anyone who hasn't seen it yet, more fun with fees: we have a new fee config option: -maxapsfee 10:59 < shrouded> user preference 10:59 < jonatack> https://github.com/bitcoin/bitcoin/pull/14582 10:59 < gzhao408> any more questions/suggestions about the ATMPx2? I will leave the "should maxrelayfee be policy" as an exercise to the reader 11:00 < gzhao408> wat iz policy?? 11:00 < evanlinjin> I'm getting confused by the definition of policy 11:00 < michaelfolkson> It compiles to Miniscript haha 11:00 < gzhao408> 11AM already! that's a wrap :) Thanks for coming everyone! 11:00 < dhruvmehta> gzhao408 I think that the name for ATMP is probably a loaded thing, but calling it `CheckAndAcceptToMemPool` could perhaps help? 11:00 * michaelfolkson getting my coat 11:00 < nehan> gzhao408: thank you! 11:01 < robot-dreams> gzhao408: thanks for hosting! (and I'm confused about policy too) 11:01 < amiti> thanks gzhao408! that was fun :) 11:01 < willcl_ark> thanks gzhao408! very interesting 11:01 < evanlinjin> thank you! 11:01 < troygiorshev> thanks gzhao408!! 11:01 < jonatack> gzhao408: you know, since policy isn't defined anywhere (that we know of) in the codebase, maybe open a PR to add a definition? 11:01 < shrouded> +1 11:01 < dhruvmehta> gzhao408 thank you for the PR and for walking us through it so well. 11:01 < troygiorshev> +1 11:01 < emzy> Thank you gzhao408 and all! 11:01 < shrouded> thanks gzhao408 11:01 < DariusP> thanks gzhao408 !!! That was helpful and a lot of fun 11:01 < gzhao408> #endmeeting 11:01 < sipa> thanks gzhao408! 11:01 < pinheadmz> thanks gzhao408 ! 11:01 < figs> Thanks 11:01 < jnewbery> thanks gzhao! Great meeting! 11:01 < pinheadmz> you rock, great job amd good hosting 11:02 < ariard> policy/mechanism in the unix terminology I guess 11:02 < jonatack> thanks gzhao408, great job! 11:02 -!- behradkhodayar [~behrad@static.126.222.46.78.clients.your-server.de] has quit [Quit: Leaving] 11:02 < ariard> you have an imperative mechanism (consensus rules) on which application are programming policy (tx-relay policy) 11:02 < gzhao408> Feeling the love! 😊 thanks everyone for participating and being so nice to me 11:02 < felixweis> thanks! 11:03 -!- figs [2f293fee@047-041-063-238.res.spectrum.com] has quit [Remote host closed the connection] 11:03 < wumpus> from what I've noticed "policy" is generally defined as "P2P network transaction policy" 11:03 < ariard> *policies 11:03 < wumpus> that's, at least, always how I interpreted it 11:03 < nehan> ariard: sipa: if you continue your conversation from before, it would be awesome if you could quickly summarize the point you were discussing 11:03 -!- Guest2020 [d106dbf8@209.6.219.248] has quit [Remote host closed the connection] 11:03 < sipa> i don't understand ariard's point :) 11:04 < ariard> wumpus: you have script interpreter flags, is this strictly part of tx-relay policy ? 11:04 < sipa> ariard: yes 11:04 < sipa> (it's even enabled on testnet!) 11:04 < wumpus> ariard: everything that is not consensus: yes 11:04 -!- vindard [~vindard@190.83.165.233] has quit [Remote host closed the connection] 11:04 -!- troygiorshev [~troygiors@d67-193-140-136.home3.cgocable.net] has quit [Quit: leaving] 11:06 < ariard> sipa: I'm not sure anymore that getting a merkle proof would add any security, you still have to get a standard against which to assert spend amount 11:06 < shrouded> maybe: policy = a rule subset of consensus rules that a node will allow to be overridden by blocks/txs in blocks 11:06 < nehan> jeremyrubin: oooh now i see why you brought up CPFP. but yes not relevant for the absurdFee check 11:06 < ariard> ifyou don't allow it you're likely not more in consensus 11:07 -!- vindard [~vindard@190.83.165.233] has joined #bitcoin-core-pr-reviews 11:07 -!- troygiorshev [~troygiors@d67-193-140-136.home3.cgocable.net] has joined #bitcoin-core-pr-reviews 11:07 < sipa> ariard: you just need the tx that created the output you're spending (or if you don't worry about the re-signing issue, just the utxo) 11:07 < sipa> ariard: it doesn't matter if that tx was included in the chain, if it wasn't, the transaction will just be invalid 11:08 < ariard> what about chain of unconfirmed transactions ? 11:08 < sipa> yes, what about it? 11:08 < sipa> if you agree with a transaction, assuming the details provided to you are correct, you sign 11:09 < nehan> ariard: suppose you haven't verified the inputs and they are fake, and you miscalculate the fee. so what? 11:09 < ariard> nehan: I'm supposing the inputs are valid but aren't the ones you want _actually_ to spend because you will burnt irrationally in fee 11:09 < sipa> you can't know if that information you were given is correct, but if it isn't, the tx will be invalid, so no harm done 11:09 < nehan> ^ exactly 11:09 < sipa> ariard: you can compute the fee yourself at signing time 11:10 < sipa> if you don't like it, don't sign it 11:10 < sipa> you don't know if that calculated fee is correct, but if it isn't, the tx will be invalid 11:10 < ariard> how do you evaluate the "like it" you need a standard value fee to evalute against 11:10 < nehan> ariard: are you worried about a case where the fee is miscalculated and is low, but the transaction will eventually get confirmed with an absurd fee? 11:10 < sipa> ariard: you compute it? outputs minus (claimed) inputs 11:11 < sipa> eh, the other way around! 11:11 < ariard> sipa: ah I see , I'm more talking about how do you evaluate fee correctness, like malicious overspending in fees 11:12 < sipa> you can just see it? 11:13 < ariard> your signer challenger is maliciously sending you a utxo you own but with a amount far higher than what you actually want to spend, thus overpaying in fees 11:13 < sipa> then your signature will be invalid 11:13 < ariard> it's not about accessing the data to sign, it's more about sanitizing i 11:13 -!- dhruvmehta [881873f9@249.115.24.136.in-addr.arpa] has quit [Remote host closed the connection] 11:14 < sipa> i think i'm missing what you're trying to do, or you're missing what i'm saying - i'm not sure 11:14 < ariard> yes I know we are talking past each other, trying to formulate it better 11:16 < sipa> if they give you a correct UTXO that you own, you'll be able to assess the fee and decide if it's acceptable to you, and don't sign if it's not 11:16 < ariard> I assume that inputs feeded to signer are valid, just their amounts far higher that what is actually needed to solve the user required spend 11:16 -!- robot-dreams [~elliott@172.92.4.53] has quit [] 11:16 < sipa> if they lie about the UTXO's contents, you may decide incorrectly, but the signature you create will be invalid 11:17 < ariard> voila, how do you determine a "what's it's acceptable to you" for automatic flow, where you might not have user input 11:17 < sipa> oh 11:18 < sipa> if you don't have user input (or no preprogrammed policy that's a proxy for it), having separate signing hardware is pointless 11:18 < sipa> a HSM that signs anything is silly, as an attacker can just steal all your funds (which is almost as bad as stealing the key) 11:19 < ariard> right, unless you re-implement a mempool in your hardware signer and also utxo set, not likely doable 11:19 < sipa> i don't understand what that has to do with anything? 11:19 < sipa> this is about deciding what fee you're willing to pay 11:19 < sipa> which is an economic decision, not something a mempool can do 11:20 < ariard> yes but you might have to decide what fee you're willing to pay in an automatic workflow 11:20 < sipa> yes, again, what does a mempool to do with that? 11:20 < ariard> if you're bounded with a timelock that's a secuirty matter 11:20 < sipa> oh, i see 11:20 < ariard> you want to react to congestion 11:21 < sipa> you need a trusted source of feerate information then 11:21 < sipa> a local mempool won't help with that, unless you have strong non-censorship guarantees between your hardware wallet and the real world... which kind of defeats the point 11:22 < ariard> it's easier to run a pruned full-node on a HSM, right 11:22 < sipa> if you say HSM, you can't realisitically assume censorship resistance 11:23 < sipa> as there is an online component that is your link with the real world, and they can always withold information 11:23 < ariard> right, your networking stack is going to be outside your trusted computing base 11:23 < sipa> if you need to react to market/congestion/... information, you *need* a trusted source for that, no way around it, i think 11:23 < ariard> hmmm you might have wireless links, but I wouldn't trust that much radio stack 11:24 < ariard> yes I agree I don't think there is a better way 11:24 < sipa> then i wouldn't call that an HSM anymore, more "another computer" 11:24 < sipa> (i also think in general that assumptions about your ability to react to such market conditions and guarantee confirms are broken...) 11:25 < sipa> at least in a short term, and without an ability to warn a user 11:25 < ariard> using the headers can still be useful to determine you reach a given block height and authorized a spend 11:25 < ariard> without package relay and redundant tx-relay I agree 11:25 < sipa> perhaps 11:26 < sipa> i'm not sure that's sufficient 11:26 < ariard> rationally you should be able ready to burn as much as there are funds at stake 11:26 < ariard> but you can't be sure that blockspace demand won't override your bid 11:26 < emzy> You can use a fixed fee in the HSM and add a output and use CPFP ouside. 11:27 < ariard> yes but that's still might be a substantial CPFP fee 11:27 < emzy> The outside hot wallet will have limited funds that are at risk. 11:28 < emzy> So there is a maximum that can be stolen. 11:28 < emzy> I think it depends what the actual usecase is. 11:29 < ariard> in the lightning case, you might be ready to burn near as much as you have HTLC in contention 11:30 < emzy> Then you also have to show the HSM that there was a old state on the chain... 11:30 < emzy> I have no idea how to do that without the chain. 11:32 < ariard> you might not an old state on the chain, the broadcast can be initiated on your own 11:32 < ariard> see above, you can use headers as a clock to decide your time-bounded HTLC should be expired onchain 11:33 < ariard> and decide to authorize the signature 11:36 < emzy> Ok at least you need all headers. But new header can be omissioned from the HSM. 11:38 < ariard> right, but in case of site compromise, an attacker won't be able to obtain a commitment transaction signed before expiration time with regards to actual chain tip 11:38 < ariard> which is already an improvement 11:39 < ariard> because you might have multiple chain monitors, and only one of them need to be secure to be safe but if one of them is compromised you want to prevent channel closing abuse 11:40 < emzy> right 12:10 -!- shrouded [~shrouded@172.86.186.172.adsl.inet-telecom.org] has left #bitcoin-core-pr-reviews [] 12:40 -!- DariusP [49fce2af@c-73-252-226-175.hsd1.ca.comcast.net] has quit [Ping timeout: 245 seconds] 13:42 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Quit: Konversation terminated!] 14:35 -!- gzhao408 [uid453516@gateway/web/irccloud.com/x-vpskgznvneyixfrw] has quit [Quit: Connection closed for inactivity] 14:37 -!- thomasb06 [~user@eth-west-pareq2-46-193-0-224.wb.wifirst.net] has quit [Remote host closed the connection] 15:00 -!- seven_ [~seven@2a00:ee2:410c:1300:a897:306b:90dd:f11c] has quit [Read error: Connection reset by peer] 15:01 -!- seven_ [~seven@2a00:ee2:410c:1300:c186:f9ca:bc1a:b369] has joined #bitcoin-core-pr-reviews 15:01 -!- lightlike_ [~lightlike@88.116.7.34] has joined #bitcoin-core-pr-reviews 15:03 -!- ghost43 [~daer@gateway/tor-sasl/daer] has quit [Quit: Leaving] 15:04 -!- ghost43 [~daer@gateway/tor-sasl/daer] has joined #bitcoin-core-pr-reviews 15:04 -!- lightlike [~lightlike@88.116.7.34] has quit [Ping timeout: 258 seconds] 15:09 -!- evanlinjin [~evanlinji@2404:4408:43ff:bb00:bba8:a1bb:ac12:ad14] has quit [Read error: Connection reset by peer] 15:10 -!- evanlinjin [~evanlinji@2404:4408:43ff:bb00:bba8:a1bb:ac12:ad14] has joined #bitcoin-core-pr-reviews 15:12 -!- Brannon56Ankundi [~Brannon56@static.57.1.216.95.clients.your-server.de] has joined #bitcoin-core-pr-reviews 15:20 -!- Brannon56Ankundi [~Brannon56@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 240 seconds] 15:21 -!- lightlike_ [~lightlike@88.116.7.34] has quit [Quit: Leaving] 15:26 < luke-jr> There was some discussion as to whether AbsurdFee is policy or a client preference. What do you think, and how did you determine this? <-- umm, client preference and policy and the same thing? 15:26 < luke-jr> are the same thing* 15:29 < sipa> luke-jr: "client preference" here is the absurdfee rule, which is only applied to locally submitted transactions; it isn't a network policy 15:55 -!- vasild_ [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 15:59 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 15:59 -!- vasild_ is now known as vasild 17:40 -!- dergoegge [uid453889@gateway/web/irccloud.com/x-dxprdbgghyhxzrvj] has quit [Quit: Connection closed for inactivity] 18:46 -!- shesek [~shesek@unaffiliated/shesek] has quit [Remote host closed the connection] 19:25 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Ping timeout: 240 seconds] 19:26 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined #bitcoin-core-pr-reviews 19:56 < luke-jr> sipa: ah, I see 20:16 -!- troygiorshev [~troygiors@d67-193-140-136.home3.cgocable.net] has quit [Ping timeout: 264 seconds] 20:19 -!- shesek [~shesek@unaffiliated/shesek] has joined #bitcoin-core-pr-reviews 20:20 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has quit [Quit: ZNC 1.7.5 - https://znc.in] 20:20 -!- harrigan [~harrigan@ptr-93-89-242-235.ip.airwire.ie] has joined #bitcoin-core-pr-reviews