--- Day changed Mon Nov 30 2015 00:15 < GitHub198> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c28d3937b095...fa93174a7c06 00:15 < GitHub198> bitcoin/master a6cbc02 Luke Dashjr: Bugfix: Default -uiplatform is not actually the platform this build was compiled on 00:15 < GitHub198> bitcoin/master fa93174 Jonas Schnelli: Merge pull request #7127... 00:15 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has joined #bitcoin-core-dev 00:15 < GitHub118> [bitcoin] jonasschnelli closed pull request #7127: Bugfix: Default -uiplatform is not actually the platform this build was compiled on (master...bugfix_uiplatform) https://github.com/bitcoin/bitcoin/pull/7127 00:16 -!- tripleslash [~triplesla@unaffiliated/imsaguy] has joined #bitcoin-core-dev 00:23 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has joined #bitcoin-core-dev 00:26 -!- Apocalyptic [~Apocalypt@62.210.252.63] has joined #bitcoin-core-dev 00:26 -!- Apocalyptic [~Apocalypt@62.210.252.63] has quit [Changing host] 00:26 -!- Apocalyptic [~Apocalypt@unaffiliated/apocalyptic] has joined #bitcoin-core-dev 00:30 -!- Ylbam [uid99779@gateway/web/irccloud.com/session] has quit [Changing host] 00:30 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-mfiqwakmfbtjlyer] has joined #bitcoin-core-dev 00:36 -!- JackH [~Jack@host-80-43-142-236.as13285.net] has quit [Ping timeout: 246 seconds] 00:54 -!- warren [~warren@fedora/wombat/warren] has quit [Ping timeout: 245 seconds] 00:54 -!- Ylbam_ [uid99779@gateway/web/irccloud.com/x-lnpnqxiudktxmyjb] has joined #bitcoin-core-dev 00:55 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-mfiqwakmfbtjlyer] has quit [Ping timeout: 264 seconds] 00:55 -!- Ylbam_ is now known as Ylbam 00:56 -!- warren [~warren@fedora/wombat/warren] has joined #bitcoin-core-dev 01:05 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-lnpnqxiudktxmyjb] has quit [Ping timeout: 264 seconds] 01:10 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-rosposzrvloqcppu] has joined #bitcoin-core-dev 01:10 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…] 01:18 -!- jonasschnelli [~jonasschn@2a01:4f8:200:7025::2] has joined #bitcoin-core-dev 01:18 -!- jonasschnelli [~jonasschn@2a01:4f8:200:7025::2] has quit [Changing host] 01:18 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has joined #bitcoin-core-dev 01:18 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has quit [Excess Flood] 01:18 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has joined #bitcoin-core-dev 01:27 -!- moli [~molly@unaffiliated/molly] has joined #bitcoin-core-dev 01:28 -!- molly [~molly@unaffiliated/molly] has quit [Ping timeout: 245 seconds] 01:29 < jouke> gmaxwell: you are a hero for doing that. 01:30 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has quit [Excess Flood] 01:31 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has joined #bitcoin-core-dev 01:34 -!- Guyver2 [~Guyver2@80.100.156.239] has joined #bitcoin-core-dev 01:34 < wumpus> gmaxwell: yes thanks for doing that and providing some sanity in the always-hysterical reddit 01:42 < gmaxwell> Thank you both for the thanks. 01:42 -!- go1111111 [~go1111111@2604:4080:1128:0:7965:3a52:eb7b:1141] has joined #bitcoin-core-dev 01:43 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has joined #bitcoin-core-dev 01:45 -!- randy-waterhouse [~kiwigb@opentransactions/dev/randy-waterhouse] has joined #bitcoin-core-dev 01:53 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has quit [Ping timeout: 245 seconds] 01:53 < btcdrak> gmaxwell: yeah thanks from me too. I cant stress this enough though, I think you should start a blog or something. You could just use Github pages if you didnt want anything fancy. I fear so much of your writings just get buried. 01:58 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has quit [Excess Flood] 01:58 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has joined #bitcoin-core-dev 01:59 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has joined #bitcoin-core-dev 02:00 < jonasschnelli> sipa: vHashes seems to be and object of "if (!fInitialDownload) {" 02:00 < jonasschnelli> not sure if i can expand the vHashes into the fInitialDownload region. 02:05 < wumpus> gmaxwell: this is another example of how something only gains interest as soon as it is merged. RBF discussions have been going on for as long bitcoin exists, and still they manage to pose it as if this is something new and controversial instead of the compromise of years of argument 02:06 < gmaxwell> If anyone picks up responding at all there, it's best whenever a new question is asked in a comment to break it out and answer it top level, instead of getting in a debate in the comments. 02:07 < gmaxwell> wumpus: I think in this case the PR got a lot of attention first; and most on the noise now was started by an intentional effort to mislead people; there were huge numbers of threads (dozens) created by brand new accounts all repeating the same common misinformation. 02:07 < wumpus> btcdrak: yes, summarizing/copying these things to a blog for future reference would be very useful 02:08 < gmaxwell> That it totally breaks zero conf, that it is something being done by blockstream, etc. 02:08 < gmaxwell> So I think poor Opt-in RBF is just being made a proxy in someone's battle for something else. :( 02:08 < wumpus> btcdrak: the reddit format is good for interaction, but not for e.g. linking or referencing 02:08 < gmaxwell> petertodd: sorry. :( 02:09 < wumpus> gmaxwell: but I mean everyone has those worries and those have been considered from the very start 02:09 < gmaxwell> with the _complete_ lack of complaint (even from the expected parties, hell dgenr8 ACKed) on the PR, I was not anticipating this. :( 02:09 < wumpus> gmaxwell: some people suddenly awake 'oh no! zero-conf'.. if zero conf was not a concern the first iteration would have been merged 02:09 < btcdrak> wumpus: Let's be honest here. Trouble makers are just waiting for topics that are easy to misunderstand and potentially emotive for general public, so they can jump on it and willfully misrepresent it. 02:09 < wumpus> btcdrak: sure, that's true, it's kind of an attack I suppose 02:10 < btcdrak> wumpus: also think about the timing. they want to drum up as much negative attention going into the conference as possible because this is literally their last stand. 02:10 < gmaxwell> I think we haven't been really loud enough about zero conf not being a supported feature, its sort of shocking to hear people's expectations. 02:10 < wumpus> people feeling like they need to raise a rabble about everything because they're suuch an anarchist 02:11 < tulip> zero confirmation transactions are sort of unfortunate because there's no clean line of breakage, no matter how broken anybody can describe it, there's always a secret sauce solution which will "fix" it. 02:11 < wumpus> gmaxwell: well WE have been loud enough about that 02:12 < wumpus> it's just that no one really listens, unless there is a controversy 02:12 < wumpus> (or perceived controversy) 02:12 < btcdrak> gmaxwell: I thoroughly agree with you. In fact, we sort of need a PR side to act as a communication bridge with the general public. The weekly IRC meetings have been really well received, but because they are posted on social media they get buried. This is why I discussed with harding to make a new section on bitcoin.org so we can collate these kind of 02:12 < btcdrak> things. 02:12 < wumpus> tulip: yep 02:12 < gmaxwell> tulip: "Homeopathic confirmation." 02:12 < btcdrak> gmaxwell: LOL 02:13 < wumpus> gmaxwell: that's a good comparison 02:14 < btcdrak> gmaxwell: if only we could get homeopathic theory to work for us, the more the message gets diluted and agitated the more powerful it becomes. 02:14 < gmaxwell> The problem is that the reasons that it doesn't provide the security people think it does are really unintutive to people; and so they stick to easy and wrong naratives instead. E.g. thinking that we're overobessing about perfect security, or saying that they can't use it in places where there is external trust or recourse, where they obviously have; or the latest because blockstream is behind i 02:14 < gmaxwell> t (muhaha). 02:15 < gmaxwell> And so you get this duality where people who grock distributed systems and security reasoning "Get It"; and keep posting "don't do it!" -- while everyone else tells themselves but it's fine and pats each other on the back about how smart they are. :( 02:16 < gmaxwell> we can't save everyone; but its irritating if it gets in the way of progress. They're also attacking my mempool p2p message limiting PR, :( 02:17 < randy-waterhouse> more and more mainstream, layman derpiness, afraid it might only keep tending that way 02:17 < btcdrak> but like seriously gmaxwell please please collate your writings somewhere static. Better to cover a topic in your blog then paste it into reddit as replies. general consensus is your posts are extremely informative. 02:18 < jgarzik> gmaxwell, Trying to help a bit, https://twitter.com/jgarzik/status/671271910634889216 02:18 < jgarzik> gmaxwell, +1 to btcdrak's comment 02:19 -!- MarcoFalke [8af60236@gateway/web/cgi-irc/kiwiirc.com/ip.138.246.2.54] has joined #bitcoin-core-dev 02:21 < wumpus> gmaxwell: jonasschnelli: #7112 #7037 both move the uiInterface.NotifyBlockTip 02:21 < wumpus> who should win? :p 02:22 < gmaxwell> maybe blocknotify should be made seperate? :) 02:23 < gmaxwell> 7112 looks like it would make it get processed even later. :) 02:23 < gmaxwell> (I say with a 10 second glance) 02:23 < wumpus> gmaxwell: and should include the notification for zmq 02:23 < wumpus> gmaxwell: no, I'd say otherwise 02:23 < wumpus> gmaxwell: the WALLET notification needs to be separte 02:24 < wumpus> gmaxwell: all the others, the GUI, ZMQ, blocknotify all take very short 02:24 < gmaxwell> Makes sense. 02:24 < wumpus> it's the wallet that is the problem and should probably get its own signal 02:24 < wumpus> 'delayedBlockTipNotify' :p 02:24 < sipa> jonasschnelli: let me look 02:25 < wumpus> ideally the wallet would also only notify a processing thread instead of doing heavy work in the notification, but that's for later 02:26 < jonasschnelli> gmaxwell: wumpus: i think #7037 is included in #7112 02:26 < wumpus> but at least all the well-behaved clients should get a first stab at the signal 02:26 < wumpus> jonasschnelli: no; #7037 moves the GUI notification earlier, you move it later 02:27 < jonasschnelli> Argh.. yes. Right. 02:27 < jgarzik> separate signal was always the plan... 02:27 < wumpus> jonasschnelli: could you live with moving it to the beginning? 02:27 < jgarzik> for wallet 02:28 < wumpus> jonasschnelli: (move the UI notification *before* notifying the wallet etc) 02:28 < jonasschnelli> wumpus: just checking... yes. I think this would be okay. 02:28 < sipa> jonasschnelli: you can use if (pindexFork != pindexNewTip) { ...} 02:28 < wumpus> jonasschnelli: ok, if you include that in your pull, we hit two flies with one stone 02:29 < jonasschnelli> sipa: okay. Will add the if 02:33 < sipa> jonasschnelli: in fact that whole notifications section can be skipped in that case 02:34 < sipa> gmaxwell, jgarzik, wumpus: IMHO we should only have one signal, and different handlers that each run in their own thread can register 02:34 < wumpus> sipa: well all the handlers are well-behaved in that regard. except the wallet 02:34 < jgarzik> sipa, For this specific situation you need cascading notifications and some additional parallelization in the wallet 02:34 < wumpus> UI handles things in their own thread, ZMQ is low-latency, blocknotify forks a thread, 02:35 < wumpus> just the wallet does expensive work in the signal handler 02:35 < jonasschnelli> sipa: the change results in a ugly diff. But i think it makes sense: https://github.com/jonasschnelli/bitcoin/commit/9af5f9cb8773da2904aa3819234aaebd2efb5d15 02:35 < wumpus> so I agree we should have only one signal, but that means the wallet wil have to handle the notification differently first 02:36 < jonasschnelli> the GetMainSignals().UpdatedBlockTip is unused by the wallet 02:36 < jonasschnelli> it was added only for ZMQ IIRC 02:36 < gmaxwell> sipa: if there is only one signal, then we must make sure all things that handle take no time. 02:36 < sipa> jonasschnelli: pro tip: add ?w=1 after the url 02:36 < sipa> gmaxwell: yes, that's the only sane design :) 02:36 < jonasschnelli> sipa: Ah. Right. Much better... 02:37 < sipa> gmaxwell: not saying that's viable now 02:37 < wumpus> right, we can't block progress on the wallet getting fixed 02:37 < sipa> but yes, signal handlers should never do work and never block 02:37 < jgarzik> That's the standard signal processing ideal - do very little - just enough to trigger further processing - not blocking 02:37 < wumpus> yes 02:37 < sipa> we all agree :) 02:39 < CodeShark> just have the signal handler all tasks into a queue and use separate threads for those :) 02:39 < sipa> CodeShark: some things do have time comstraints 02:40 < jonasschnelli> But could we not eliminate uiInterface.NotifyBlockTip and use GetMainSignals().UpdatedBlockTip instead? 02:40 < sipa> but indeed, that's perfectly viable for a lot of thingfs 02:40 < jonasschnelli> Or would this be hard for the UI? 02:40 < wumpus> CodeShark: all but one of the handlers already take (almost) no time, and do their own "second half" handling, it's not necessary to use a solution like that globally 02:40 < sipa> jonasschnelli: imho yes 02:41 < jonasschnelli> But for the current PRs,... let's keep the signal. Combine and remove can be done later. 02:41 < sipa> especially when it includes the initialsync bool 02:41 < wumpus> CodeShark: in the case of e.g. the GUI a message is already sent in a queue, so adding another queue to insert into the second queue would just add latency 02:42 < CodeShark> yes, of course - Qt already has its own messaging system for that 02:42 < wumpus> also there is the problem of things blocking the dispatch thread then 02:42 < sipa> i wish we could set w=1 on github by default for c++ files 02:42 < wumpus> CodeShark: so does zmq, and so does blocknotify (which old-fashioned forks) :) 02:44 < wumpus> it's just the wallet code that needs to do something smarter. Right now it is made sure that the wallet is always in sync with the chain, but this isn't necessary, it could catch up in its own thread 02:45 < CodeShark> it's necessary for coin selection...but that's about it 02:45 < sipa> ... coin selection? 02:46 < CodeShark> yes - selecting outputs to spend when creating a new transaction 02:46 < sipa> there is no coin selection when updating the tip i hope! 02:46 < CodeShark> well, it's not necessary if you only use coins that are sufficiently buried 02:46 < sipa> we're talking about milliseconds difference 02:46 < sipa> blocks already propagate in the order of seconds 02:47 < sipa> you really don't need to inform the wallet within miliseconds 02:47 < wumpus> well obviously you don't want the wallet to be many blocks behind, that'd also mess with fee computation 02:47 < sipa> obviously 02:47 < wumpus> but that's not what we're considering here 02:47 < sipa> but async does not mean it has to lag behind 02:48 < wumpus> sure, I agree 02:48 < sipa> just that main can continue before the wallet is done handling things 02:48 < CodeShark> yes, for sure 02:49 < wumpus> could add a 'catch up fence' before doing coin selection / sending a transaction 02:49 < sipa> sounds reasonable 02:49 < wumpus> but that's mostly important if the wallet runs separately, e.g. if it can be stopped/started separately like monero's 02:49 < wumpus> if it still runs in the same process it'll never get out of sync much 02:51 < gmaxwell> in wizkids case, I don't think the node with the low notify had a wallet (other than maybe 100 addresses that had never been paid) 02:52 < sipa> gmaxwell: i think i'm not fully understanding the problem there 02:53 < gmaxwell> right now we trigger blocknotify last, after sending to peers. Wizkid noticed that rpc calls were often returning the new block before blocknotify fired, I suggested he move the notify up, he did. It was first every time. 02:53 < sipa> oh, right 02:54 < sipa> i'd say the right order is zmq,blocknotify,peers,wallet 02:54 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has joined #bitcoin-core-dev 02:54 < sipa> hmm, but your notify may trigger wallet rpcs 02:54 < CodeShark> wumpus: my stuff lets you do coin selection and create a transaction even if you're not connected to a peer...and peer sync is entirely asynchronous and can be started and stopped independently...but it's not very recommended that you try creating new transactions if you're not fully synched 02:55 < CodeShark> :) 02:56 < CodeShark> I guess I could make it smarter with conflict resolution as it syncs 02:56 < CodeShark> i.e. replace inputs with new ones as they are seen spent during sync 02:57 < CodeShark> and require resigning 02:58 < CodeShark> but meh...probably not worth the effort :p 02:58 < wumpus> ... 02:59 < wumpus> bitcoin core's wallet makes the explicit assumption that we're the only one with the keys in this wallet.dat, they cannot be shared, so that should never happen 02:59 < CodeShark> right...that too 02:59 < sipa> so if blocknotify runs before the wallet update, then the notified script can issue a wallet rpc and miss the uodate 02:59 < CodeShark> my stuff does not make that assumption at all 03:00 < CodeShark> and in fact, for many typical use cases it would be very bad to make that assumption 03:00 < sipa> CodeShark: this is bitcoin core dev 03:01 < CodeShark> yes, just acknowledging the design feature 03:01 < sipa> i think it is completely reasonable that if you get informed about a new block, and as a result go query the wallet, you actually also see that update 03:02 -!- guest234234 [~guest2342@223.207.206.176] has joined #bitcoin-core-dev 03:03 < CodeShark> so block all wallet calls during a tip update? 03:03 < sipa> yuck 03:04 < wumpus> maybe the wallet should have its own notification for new blocks 03:04 < sipa> wumpus: i was just thinking that... 03:04 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has quit [] 03:04 < wumpus> 'walletblockprocessed' or something like that 03:05 < wumpus> there's a conceptual difference between 'new block came in' and 'new block processed by wallet' 03:05 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has joined #bitcoin-core-dev 03:05 < wumpus> or to remain backward compatible: add a -blockearlynotify 03:06 < sipa> ... and the same in zmq? 03:06 < sipa> or is zmq blocks only anyway already 03:06 < tulip> zmq does transactions too 03:06 < wumpus> zmq should be completely decoupled from the wallet already 03:06 < wumpus> it's meant as a low-latency notification interface from the core, there is no wallet anything zmq 03:07 < sipa> oh, it just notifies for all txn 03:07 < sipa> if you subscribe? 03:07 < wumpus> yes 03:07 < sipa> so: zmq,blockearlynotify,peers,wallet,blocknotify 03:08 < wumpus> ui should be notified as early as possible too 03:08 < tulip> either transaction hashes or transaction binaries depending what you subscribe to. 03:09 < wumpus> ui's notify just sends an async message to the UI thread to update the block counts etc, it does not make any assumptions about the wallet being updated or not 03:09 < sipa> ok 03:10 < wumpus> (there are CWallet signals that the UI subscribes to for wallet information, this is already handled properly) 03:15 < GitHub144> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/9ebedc1756e3...6fc287f2df54 03:15 < GitHub144> bitcoin/master c6973ca MarcoFalke: [qa] keypool: Fix white space to prepare transition to test framework 03:15 < GitHub144> bitcoin/master 4ea1790 MarcoFalke: [qa] keypool: DRY: Use test framework 03:15 < GitHub144> bitcoin/master 6fc287f Wladimir J. van der Laan: Merge pull request #7027... 03:15 < GitHub101> [bitcoin] laanwj closed pull request #7027: [qa] rpc-tests: Properly use test framework (master...MarcoFalke-2015-rpcTestCleanup) https://github.com/bitcoin/bitcoin/pull/7027 03:18 < GitHub24> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/6fc287f2df54...a7751824ce8a 03:18 < GitHub24> bitcoin/master 4b89f01 Ryan Havar: Default fPayAtLeastCustomFee to false... 03:18 < GitHub24> bitcoin/master fa506c0 MarcoFalke: [wallet] Add rpc tests to verify fee calculations 03:18 < GitHub24> bitcoin/master a775182 Wladimir J. van der Laan: Merge pull request #7103... 03:18 < GitHub124> [bitcoin] laanwj closed pull request #7103: [wallet, rpc tests] Fix settxfee, paytxfee (master...FixSettxfee) https://github.com/bitcoin/bitcoin/pull/7103 03:31 < wumpus> imo needs more code review: Add pre-allocated vector type and use it for CScript #6914 03:32 < wumpus> (lots of concept ACK, but as this involves new data structures and pretty advanced c++, and affects consensus code, I think it needs more thorough review and testing) 03:36 < GitHub37> [bitcoin] laanwj pushed 2 new commits to 0.11: https://github.com/bitcoin/bitcoin/compare/595c8d6301cf...5f09cda0bf4c 03:36 < GitHub37> bitcoin/0.11 7d0a05f Ryan Havar: Default fPayAtLeastCustomFee to false... 03:36 < GitHub37> bitcoin/0.11 5f09cda MarcoFalke: [wallet] Add rpc tests to verify fee calculations... 03:37 < wumpus> more generally: please help moving anything with 0.12 milestone along https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+milestone%3A0.12.0 deadline is tomorrow 03:38 -!- jgarzik [~jgarzik@unaffiliated/jgarzik] has quit [Ping timeout: 246 seconds] 03:39 -!- instagibbs [~greg@pool-108-31-210-40.washdc.fios.verizon.net] has quit [Ping timeout: 276 seconds] 03:41 -!- fkhan [weechat@gateway/vpn/mullvad/x-ifcceekdywreglre] has quit [Ping timeout: 276 seconds] 03:42 < sipa> wumpus: seems reasonable 03:42 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has quit [Quit: My MacBook has gone to sleep. ZZZzzz…] 03:44 < sipa> wumpus: i think something like 7105 is needed now with mempool limiting, as we'll otherwise automatically respend evicted inputs 03:45 < sipa> i'll help rebasing pr's tagged for 0.12 if that helps 03:47 -!- instagibbs [~greg@pool-108-31-210-40.washdc.fios.verizon.net] has joined #bitcoin-core-dev 03:52 -!- randy-waterhouse [~kiwigb@opentransactions/dev/randy-waterhouse] has quit [Quit: Leaving.] 03:54 -!- fkhan [~weechat@unaffiliated/loteriety] has joined #bitcoin-core-dev 03:55 < wumpus> sipa: agree 04:01 < wumpus> I'm not sure about #6898 (Rewrite CreateNewBlock), if it has not been tested for mainnet mining yet it may be too risky to include in a release 04:04 < sipa> i understand the concern, but that'll likely be the case for any such change; people won't start using it before it's considered stable... 04:04 < GitHub77> [bitcoin] laanwj pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/a7751824ce8a...96b802510da0 04:04 < GitHub77> bitcoin/master 012fc91 Jonas Schnelli: NotifyBlockTip signal: switch from hash (uint256) to CBlockIndex*... 04:04 < GitHub77> bitcoin/master e6d50fc Jonas Schnelli: [Qt] update block tip (height and date) without locking cs_main, update always (each block) 04:04 < GitHub77> bitcoin/master 947d20b Jonas Schnelli: [Qt] reduce cs_main in getVerificationProgress() 04:04 < GitHub88> [bitcoin] laanwj closed pull request #7112: [Qt] reduce cs_main locks during tip update, more fluently update UI (master...2015/11/qt_tipupdate) https://github.com/bitcoin/bitcoin/pull/7112 04:05 < wumpus> sipa: but some people are mining with master, if it has been in master for a while before the release the concern is mostly gone 04:05 < wumpus> sipa: my problem is not with merging it, but with merging it one day before a release branch-off 04:08 < sipa> understood; i think the benefits are worth it though. the code produces the same blocks as the old code modulo a few rounding error (says morcos), so i'm not too worried about it accidentally producing blocks that are totally off in terms of what we expect to be in there 04:08 < sipa> i will review the code again 04:08 < wumpus> what I want to avoid is a 0.12 release where we have to spend a lot of time mopping up issues introduced last-minute before the release, whereas master as it is now is good as a release already. The goal shouldnt be to stash in everything last minute, at this point it's important to consider the risk not just the benefits 04:24 -!- guest234234 [~guest2342@223.207.206.176] has quit [Ping timeout: 244 seconds] 04:30 -!- BashCo [~BashCo@unaffiliated/bashco] has joined #bitcoin-core-dev 04:31 < GitHub20> [bitcoin] sipa opened pull request #7133: Replace setInventoryKnown with a rolling bloom filter (rebase of #7100) (master...known_bloom) https://github.com/bitcoin/bitcoin/pull/7133 04:44 < morcos> sipa: wumpus: i'm back now, so i can rebase 6898 (i'll do that right now) and starting this afternoon help with review 04:45 < morcos> personally i think the most risky part about 6898 is not the code in the pull itself but the fact that it now relies on the mempool to correctly maintain some state such as number of sig ops, no orphans, etc ... 04:46 < sipa> but we still run TBV afterwards, right? 04:46 < morcos> correct 04:46 < morcos> so then my question is , is that the best way to deal with a situation where that is broken at some point in the future 04:47 < morcos> i'm relatively confident that its not broken for normal situations now 04:47 < sipa> well that's what master and release candidates are for 04:47 < sipa> though as wumpus notes, it is already quite late, and we don't have knowledge of people using this code in production 04:48 < sipa> what about turning it into a separate createnewblockbeta RPC for now? 04:48 < morcos> i generated a new block template and tested it on 1 out of every 128 transactions for 6 weeks in historical simulation, and of course they all passed 04:48 < sipa> oh wow 04:49 < morcos> i don't really have any objection to that if its preferred.. i think my concern was something about maintaining two sets of code last time it was mentioned 04:50 < morcos> to be honest, i'm not sure too many people look at this stuff. that pull has been open for a full month and received almost no conversation on the PR 04:50 < sipa> which is strange, because evidence shows that there are miners running master 04:50 < morcos> i'm not really in a position to complain since i didn't review anyone elses PR's recently though... 04:50 < morcos> Dec 1st just came too fast! 04:50 < sipa> you'd think they'd also test PRs... 04:50 < sipa> or show interest in things like this 04:51 < sipa> so i'm afraid that the only way to actually test it is getting it in master 04:51 < sipa> s/test/get production feedback/ 04:51 < morcos> well the nice thing is that its relatively easy to sub in and out 04:52 < morcos> doesn't affect much more than 1 function 04:52 < sipa> indeed, just looked 04:52 < sipa> it doesn't seem hard 04:52 < morcos> i think it would be slightly cleaner to just replace than maintain both, but either option is ok with me 04:53 < morcos> let me rebase it really quick.. 04:53 < sipa> the question is mostly whether adding it as a separate RPC would actually make more people use it 04:53 < sipa> wumpus: opinion? 04:55 < morcos> actually wait 04:55 < morcos> i know why i didn't like that idea 04:55 < morcos> it basically makes it impossible to make forward progress for another release 04:55 < morcos> well, yeah, depending on the answer to your question.. it would have to get a lot of uses as an alternate RPC 04:55 < morcos> of if you have original and beta for 0.12 04:56 < morcos> then what do we have for 0.13 ? 04:58 -!- ParadoxSpiral [~ParadoxSp@p508B94C6.dip0.t-ipconnect.de] has joined #bitcoin-core-dev 04:59 < sipa> i'd say after branching off 0.12, beta goes away and becomes the only one 05:00 < sipa> making this beta RPC a preview for an upcoming version, if no problems are found 05:00 < wumpus> Dec 1st just came too fast! <- I'm sure May 1st will come soon enough too, then :) 05:01 < morcos> sipa: yes but my point is that it won't be the upcoming version 05:01 < wumpus> morcos: I agree it's cleaner to just replace it, I'm just very afraid of breaking mining in some subtle way just before a release 05:02 < morcos> i'd like to make more changes, to actually improve the algorithm, not just speed it up 05:02 < sipa> right, that's where the real benefit lies, the fact that this new code can incorporate efficient CPFP etc 05:03 < sipa> and that won't be in such a beta 05:03 < wumpus> if you're fully confident that that cannot happen, then there's no reason to not just replace the old code, but the part about it not being tested on the main network at all scares me 05:03 < wumpus> getting some ACKs from actual miners would be great 05:04 < morcos> wumpus: i think sipa is right, lets put it in master. and announce we need actual miners to test it 05:04 < morcos> its easy to sub out if we lose confidence before release, but i don't think we will 05:04 < wumpus> then revert if there are none? 05:04 < wumpus> ok 05:04 < morcos> let me finish rebasing 05:10 < btcdrak> we should ping wangchun_, his pool is pretty proactive in testing stuff out. also Luke-Jr of course. 05:18 < wumpus> sipa: re: #7105, does listunspent need any change? 05:18 < wumpus> (the default is already to only show outputs with at least one confirmation, but asking just in case) 05:20 < GitHub142> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/96b802510da0...9b8fc6c89a08 05:20 < GitHub142> bitcoin/master 4531fc4 Daniel Cousens: torcontrol: only output disconnect if -debug=tor 05:20 < GitHub142> bitcoin/master 9b8fc6c Wladimir J. van der Laan: Merge pull request #7035... 05:21 < sipa> wumpus: i have not checked, but i don't expect it does 05:21 < morcos> wumpus: 7008 needs to be merged first. i just rebased that. or i need to rewrite 6898 without it 05:22 < wumpus> 7008 seems ready for merging 05:22 < sipa> wumpus: at worst, it will show non-conflicted non-mempool things now 05:23 < sipa> if you ask for unconfirmed results 05:23 < wumpus> sipa: ok - maybe report 'trusted' in that case? 05:23 < wumpus> not sure it's really relevant 05:31 < morcos> wumpus: where are we on the deprecating priority question. i haven't looked at the changes needed yet, but i'm assuming my blockprioritysize=0 default PR breaks all the RPC tests. 05:32 < morcos> it seems to me that we really should have that in for 0.12 if we're hoping to rip out priority for 0.13, and if so makes sense to put it in before the split 05:32 < morcos> i'm happy to make it work right today if we agree, but otherwise i'll spend my time on something else 05:32 < sipa> morcos: it's easy enough to keep the test and give them an explicit -blockprioritysize 05:33 < sipa> those should keep working anyway 05:33 < wumpus> yes, I'd say just make the tests pass in the right parameter, and add one or so test that tests the new state 05:33 < morcos> sipa: yep, sure the fix isn't hard, but it might be a lot of RPC tests that need it, so we should fix make the changes pre split 05:33 < sipa> agree 05:34 < morcos> although i was thinking it might make sense (other than a test for that parameter) to make the tests just work right without needing free space, but maybe thats too tedious for now.. ok, so i'll do something anyway. 05:34 < morcos> 6898 is rebased as well now 05:35 < morcos> i'll be back online in a couple hours 05:36 < sipa> cool 05:46 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has quit [Remote host closed the connection] 05:52 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-rosposzrvloqcppu] has quit [Ping timeout: 264 seconds] 05:58 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-lplypgjvvyilzlfc] has joined #bitcoin-core-dev 06:01 -!- ParadoxSpiral [~ParadoxSp@p508B94C6.dip0.t-ipconnect.de] has quit [Quit: cya] 06:01 -!- instagibbs [~greg@pool-108-31-210-40.washdc.fios.verizon.net] has quit [Ping timeout: 260 seconds] 06:16 -!- MarcoFalke [8af60236@gateway/web/cgi-irc/kiwiirc.com/ip.138.246.2.54] has quit [Ping timeout: 257 seconds] 06:28 -!- CodeShark_ [CodeShark@cpe-76-167-237-202.san.res.rr.com] has quit [Ping timeout: 245 seconds] 06:30 -!- tulip [~tulip@unaffiliated/tulip] has quit [] 06:36 -!- BashCo [~BashCo@unaffiliated/bashco] has quit [] 06:39 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has quit [Ping timeout: 250 seconds] 07:03 < jonasschnelli> sipa: any recommendation for decorating a "limbo"-transaction in the UI? 07:03 < jonasschnelli> The "?" (questionmark) is already in use of unconfirmed transaction... 07:03 < jonasschnelli> maybe a "?!" 07:03 < sipa> jonasschnelli: ha! 07:04 < wumpus> ah yes the ‽ 07:04 < jonasschnelli> haha 07:05 < wumpus> I suppose using the same decoration as unconfirmed transaction makes some sense, it's a relatively rare occurenc 07:06 < jonasschnelli> Okay. Then i'll add the "Trusted" (=in mempool) info to the transaction detail window only. 07:06 < wumpus> don't think it warrants a new icon, isn't there some other way to distinguish it? 07:07 < wumpus> hmm only in transaction detail window is the other extreme 07:08 < wumpus> I'd like to avoid a christmas tree effect with too many implementation details in the UI - what kind of dysfunctional transaction of the day is this? - but it makes sense to be able to recognize them in some way I guess... 07:09 < jonasschnelli> yes. Makes sense. 07:10 < wumpus> hmm it determines whether we regard it as spendable or not, does it? 07:10 < sipa> wumpus: indeed 07:11 < wumpus> that's what the brackets around the balance are used for [...] 07:11 < sipa> 0 confirmations (only for transactions from youself) are spendable when in the mempool, unspendable otherwise 07:11 < sipa> but the GUI afaik has no way of exposing the distinction between the two types of unconfirmed 07:11 < wumpus> e.g. if it shows [123] it doesn't count to the balance, if it's 123 it does 07:12 < wumpus> status.countsForBalance = wtx.IsTrusted() && !(wtx.GetBlocksToMaturity() > 0); 07:12 < sipa> ah, good! 07:12 < wumpus> so, seems we already check isTrusted() 07:12 < sipa> ok, so no real change needed 07:13 < sipa> that can be simplified to just wtx.IsTrusted btw 07:13 < sipa> ah, no 07:13 < sipa> sorry; it's a maturity check afterwards, not a confirmation check 07:16 < jonasschnelli> sipa: wumpus: is this bad? https://github.com/jonasschnelli/bitcoin/commit/f5957b88ca4d6205824bcf348feb8f865913b99c 07:16 < jonasschnelli> sipa: I think you could pull https://github.com/jonasschnelli/bitcoin/commit/965a1e6d9c5c1af5555665c0a94cde85dfe19a7b into your #7105 07:17 < sipa> i think "trusted 07:17 < sipa> i think "trusted" will confuse people 07:17 < sipa> if they receive an unconfirmed transaction from someone else it will always say untrusted 07:18 < sipa> trusted is stronger than being in the mempool; it's also only true for your own transactions 07:18 < wumpus> yes, if it's in transaction details you can give a longer description 07:19 < wumpus> but trusted is very overused and ambigious 07:19 < wumpus> let's not use that 07:19 < jonasschnelli> "In mempool" | "Not in mempool"? 07:19 < sipa> maybe use "0/unknown" if it's not in the mempool? 07:19 < wumpus> nooo not unknown 07:20 < sipa> that's probably even scarier :) 07:20 < wumpus> yeah unknown is very, very scary :) 07:20 < wumpus> 'your wallet is glitching' 07:20 < sipa> 0/pray 07:20 < wumpus> jonasschnelli: yes, let's just call it what it is, 'in memory pool', 'not in memory pool' 07:21 < jonasschnelli> ok. 07:21 < wumpus> heh 07:21 < sipa> while we're at it: maybe also actually show the depth of the conflict for conflicted? 07:21 < jonasschnelli> right... will do 07:21 < jonasschnelli> sipa: "conflicted (-X)"? 07:22 < sipa> sgtm 07:22 < wumpus> or conflicted (at depth X) ? 07:22 < sipa> sgtm 07:25 < sipa> or "Conflicts with a transaction with X confirmations" 07:26 < wumpus> that does get very long 07:26 < wumpus> that's fine for the expanded transaction details, but probably too long for the onmouseover 07:27 < sipa> ok; we have a GUI maintainer to decide such things :) 07:27 < wumpus> yes 07:27 < wumpus> :D 07:27 < jonasschnelli> hah. 07:27 < jonasschnelli> i'll check how it looks... 07:30 < sipa> jonasschnelli: if you want to show "In mempool", you can't just use IsTrusted() 07:32 < jonasschnelli> sipa: right.. need to refactor the mempool.exists check. 07:32 < jonasschnelli> public expose "bool CWalletTx::InMempool()"? 07:32 < sipa> sgtm 07:36 -!- jgarzik [~jgarzik@unaffiliated/jgarzik] has joined #bitcoin-core-dev 07:39 -!- rubensayshi [~ruben@91.206.81.13] has joined #bitcoin-core-dev 07:39 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has joined #bitcoin-core-dev 07:41 -!- MarcoFalke [8af60236@gateway/web/cgi-irc/kiwiirc.com/ip.138.246.2.54] has joined #bitcoin-core-dev 07:50 -!- instagibbs [~instagibb@pool-108-31-210-40.washdc.fios.verizon.net] has joined #bitcoin-core-dev 07:50 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has joined #bitcoin-core-dev 08:00 -!- dermoth [~thomas@dsl-66-36-138-7.mtl.aei.ca] has quit [Remote host closed the connection] 08:01 -!- Taek42 is now known as Taek 08:01 < wumpus> if you do want to use IsTrusted, use "counts towards balance" instead of "in mempool"? 08:02 < wumpus> otherwise it needs yet another flag on the ui's transaction structure, for kind of little gain 08:08 -!- gavinand1esen [~gavin@66.172.11.36] has quit [Changing host] 08:08 -!- gavinand1esen [~gavin@unaffiliated/gavinandresen] has joined #bitcoin-core-dev 08:13 -!- ParadoxSpiral [~ParadoxSp@p508B94C6.dip0.t-ipconnect.de] has joined #bitcoin-core-dev 08:16 < wumpus> also we don't expose "transaction in mempool" on RPC, should definitely do that if it's shown in the GUI... 08:17 < sipa> agree 08:23 -!- BashCo [~BashCo@unaffiliated/bashco] has joined #bitcoin-core-dev 08:30 -!- gavinand1esen is now known as gavinandresen 08:38 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has quit [Ping timeout: 260 seconds] 08:41 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has joined #bitcoin-core-dev 08:51 -!- gavinandresen [~gavin@unaffiliated/gavinandresen] has quit [Ping timeout: 260 seconds] 08:53 -!- gavinandresen [~gavin@unaffiliated/gavinandresen] has joined #bitcoin-core-dev 09:00 -!- challisto [~challisto@unaffiliated/challisto] has quit [Quit: Leaving] 09:24 -!- Apocalyptic [~Apocalypt@unaffiliated/apocalyptic] has quit [Ping timeout: 260 seconds] 09:27 -!- Apocalyptic [~Apocalypt@unaffiliated/apocalyptic] has joined #bitcoin-core-dev 09:31 < jgarzik> Double-check: is it correct that MTP is available via RPC, via getinfo.timeoffset? (curtime + timeoffset) 09:32 < sipa> i would expect that to be about GetAdjustedTime 09:33 < gmaxwell> it's in getblockchaininfo 09:33 < gmaxwell> "mediantime" 09:33 < gmaxwell> In master. 09:34 < jgarzik> Yeah, the definition of GetAdjustedTime() is curtime + GetTimeOffset 09:35 < jgarzik> but mediantime is better 09:36 < sipa> jgarzik: do you mean getmediantimepast of the last block? 09:36 < sipa> or the median time offset reported by peers? 09:36 < instagibbs> wumpus, I think #7044 is merge-ready. Trying to slip it in for 0.12 09:38 < jgarzik> sipa, A fair question - I'm trying to pick which is the best number to poll, for an external agent to trigger a "if $network_time > x, do y" action. MTP with a reorg safety margin seems most applicable. 09:39 < sipa> jgarzik: agree 09:41 -!- rubensayshi [~ruben@91.206.81.13] has quit [Remote host closed the connection] 09:42 -!- zookolaptop [~user@c-73-217-96-13.hsd1.co.comcast.net] has joined #bitcoin-core-dev 09:44 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has joined #bitcoin-core-dev 09:46 < GitHub22> [bitcoin] sdaftuar opened pull request #7137: Tests: Explicitly set chain limits in replace-by-fee test (master...fix-rbf-test) https://github.com/bitcoin/bitcoin/pull/7137 09:46 -!- zookolap` [~user@c-73-217-16-2.hsd1.co.comcast.net] has joined #bitcoin-core-dev 09:48 -!- zookolaptop [~user@c-73-217-96-13.hsd1.co.comcast.net] has quit [Ping timeout: 245 seconds] 09:49 -!- ParadoxSpiral [~ParadoxSp@p508B94C6.dip0.t-ipconnect.de] has quit [Ping timeout: 260 seconds] 09:52 -!- cocoBTC [~cocoBTC__@c-233a71d5.136-1-64736c10.cust.bredbandsbolaget.se] has joined #bitcoin-core-dev 09:52 -!- JackH [~Jack@host-80-43-143-143.as13285.net] has joined #bitcoin-core-dev 09:55 < gmaxwell> bluematt: apparently the 0.11.2 ppa is 'build failed' and not up yet? 10:01 -!- zookolap` [~user@c-73-217-16-2.hsd1.co.comcast.net] has quit [Ping timeout: 260 seconds] 10:02 -!- ParadoxSpiral [~ParadoxSp@p508B80ED.dip0.t-ipconnect.de] has joined #bitcoin-core-dev 10:05 -!- MarcoFalke [8af60236@gateway/web/cgi-irc/kiwiirc.com/ip.138.246.2.54] has quit [Quit: http://www.kiwiirc.com/ - A hand crafted IRC client] 10:17 -!- lightningbot` [supybot@2400:8900::f03c:91ff:fedf:3a06] has joined #bitcoin-core-dev 10:17 -!- lightningbot [supybot@2400:8900::f03c:91ff:fedf:3a06] has quit [Ping timeout: 240 seconds] 10:17 -!- phantomcircuit [phantomcir@2600:3c01::f03c:91ff:fe73:6892] has quit [Ping timeout: 264 seconds] 10:17 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has quit [Ping timeout: 250 seconds] 10:19 -!- cfields_ [~quassel@unaffiliated/cfields] has quit [Quit: No Ping reply in 180 seconds.] 10:21 -!- cfields [~quassel@unaffiliated/cfields] has joined #bitcoin-core-dev 10:22 -!- asoltys [~adam@li92-10.members.linode.com] has quit [Ping timeout: 260 seconds] 10:24 -!- phantomcircuit [phantomcir@2600:3c01::f03c:91ff:fe73:6892] has joined #bitcoin-core-dev 10:30 -!- arowser [~quassel@106.120.101.38] has joined #bitcoin-core-dev 10:30 -!- arowser_ [~quassel@106.120.101.38] has quit [Ping timeout: 246 seconds] 10:45 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has joined #bitcoin-core-dev 10:48 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has quit [Excess Flood] 10:48 -!- asoltys [~adam@li92-10.members.linode.com] has joined #bitcoin-core-dev 10:50 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has joined #bitcoin-core-dev 10:55 < BlueMatt> gmaxwell: yea, for 12.04 the build hasnt worked in like...6 months? I think I've received a total of about 10 complaints 10:55 < BlueMatt> gmaxwell: it seems launchpad cant install a package which is in the repos according to ubuntu's package listing site 10:56 < BlueMatt> gmaxwell: mostly, internet folks need to go complain to launchpad 10:58 < gmaxwell> bleh. 10:59 < gmaxwell> BlueMatt: Have you complained to ubuntu? 10:59 < BlueMatt> next step for me would be to reproduce locally...its been really low on my priority list considering I've hard complaints about this only 5-10 times 10:59 < BlueMatt> but I can go do that 11:01 -!- paveljanik [~paveljani@79-98-72-216.sys-data.com] has joined #bitcoin-core-dev 11:01 -!- paveljanik [~paveljani@79-98-72-216.sys-data.com] has quit [Changing host] 11:01 -!- paveljanik [~paveljani@unaffiliated/paveljanik] has joined #bitcoin-core-dev 11:07 < gmaxwell> BlueMatt: I think its mildly important. My expirence is that you can break a piece of software in a way that impacts _millions_ of people, but if its broken in a really obvious way where they assume you know, no one will report it. 11:08 < gmaxwell> I think on wikipedia when I introduced bugs that would throw consistent errors to readers I'd get about one bug report per million people who hit the error or something at that level. 11:12 -!- warren [~warren@fedora/wombat/warren] has quit [Ping timeout: 260 seconds] 11:13 -!- warren [~warren@fedora/wombat/warren] has joined #bitcoin-core-dev 11:15 < jgarzik> Vaguely related - Fedora keeps inching along towards shipping bitcoin - I got a patch from Harald Hoyer to picocoin, adding bitcoin's curve to an otherwise functional openssl lib. Once libsecp256k1 is packaged into Fedora, that makes a nice end run around the patent/copyright annoyances blocking Fedora deployment. 11:16 < jgarzik> (picocoin thing just an anecdote noting Fedora people working on the overall ecosystem; Harald also works on bitcoin) 11:18 < BlueMatt> jgarzik: yea, warren has been pushing that one forward as well, iirc 11:20 < Luke-Jr> sipa: what benefits? the only benefit is performance AFAIK, which isn't even a real concern for this; and CPFP went unmerged since 2011 despite lots of real-world testing.. (playing devil's advocate here - I'm fine with merging it if there are really no changes, including priority working correctly) 11:29 -!- zookolaptop [~user@c-73-217-16-2.hsd1.co.comcast.net] has joined #bitcoin-core-dev 11:29 < sipa> Luke-Jr: yes, it produces the same blocks as the old code, ignoring some rounding errors, afaik 11:29 < morcos> Luke-Jr: in addition to the latency improvement, it also has the benefit of not blowing away your cache by loading up the inputs for every tx in your mempool 11:31 < morcos> sipa: requires 6357 for the blocks to be the same, but they are quite close without that 11:33 < sipa> right 11:41 -!- zookolaptop [~user@c-73-217-16-2.hsd1.co.comcast.net] has quit [Ping timeout: 250 seconds] 11:43 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has quit [Ping timeout: 245 seconds] 11:47 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has joined #bitcoin-core-dev 11:52 < sdaftuar> BlueMatt: I updated 6915, hopefully this version looks good to everyone... Can you take a look? 12:02 -!- zookolaptop [~user@2601:281:8001:26aa:7db3:9077:b884:b5f4] has joined #bitcoin-core-dev 12:10 < sipa> jonasschnelli: opinion about asking on bitcoin-qt first startup how much RAM the user wants to make available for chainstate? 12:11 < sipa> the default (100 MiB) makes sense for smallish devices and VPS'es to not OOM all the time, but on modern desktop systems it can certainly go higher, and will significantly improve sync speed 12:12 < gmaxwell> it would be useful to figure out where the point of diminishing returns is. E.g. perhaps it's 200MB. in which case it might makes sense to actually check how much the system has. 12:12 < gmaxwell> and switch defaults based on the result. 12:14 < sipa> now that the limit is actually honored accurately, maybe it's more useful to do that 12:15 < gmaxwell> it's probably more useful to measure how much physical ram the system has, rather than free.. changing behavior randomly at startup would not be good. 12:16 < gmaxwell> mempool limit could also change on that basis. 12:17 < morcos> sipa: now that its honored? 12:19 < sipa> morcos: -dbcache value is pretty accurately followed now 12:19 < morcos> i think that the default of 100 MB is too small given the default mempool is 300MB and the rest of the random memory usage 12:19 < sipa> agree 12:19 < morcos> i thought that without 6872 you'd still blow through the limit? 12:20 < sipa> not considering miners :) 12:20 < sipa> but yes 12:20 -!- BashCo [~BashCo@unaffiliated/bashco] has quit [] 12:20 < sipa> and we should probably try to make checkmempool also not pull in the entire mempool into the cache 12:20 < morcos> i thought there was still a problem of all the txs that coudl come in between blocks 12:21 < morcos> especially rejected ones 12:22 < sipa> hmm, didn't we fix that? 12:22 < sipa> i can't remember how; only thinking about it 12:23 < sdaftuar> that's what #6872 is (tagged for 0.12 but still not yet merged) 12:23 < sdaftuar> looks like it needs to be rebased 12:24 < sipa> oh 12:26 -!- JackH [~Jack@host-80-43-143-143.as13285.net] has quit [Ping timeout: 250 seconds] 12:26 < GitHub26> [bitcoin] gmaxwell pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/34e02e014718...438ee59839ad 12:26 < GitHub26> bitcoin/master d52fbf0 Gregory Sanders: Added additional config option for multiple RPC users. 12:26 < GitHub26> bitcoin/master 438ee59 Gregory Maxwell: Merge pull request #7044... 12:26 < sipa> sdaftuar: sorry, i didn't look at the number :) 12:26 < GitHub15> [bitcoin] gmaxwell closed pull request #7044: RPC: Added additional config option for multiple RPC users. (master...multrpc) https://github.com/bitcoin/bitcoin/pull/7044 12:26 < sipa> BlueMatt: feel like rebasing 6872? 12:27 -!- JackH [~Jack@host-80-43-143-143.as13285.net] has joined #bitcoin-core-dev 12:28 -!- jtimon [~quassel@74.29.134.37.dynamic.jazztel.es] has joined #bitcoin-core-dev 12:31 < morcos> there is a check in init.cpp which prevents minrelaytxfee=0 12:32 < morcos> it seems to me we should consider removing that check? 12:32 < morcos> in a world without priority, it'll be impossible to mine 0 fee txs unless we do 12:39 < jtimon> morcos: sounds reasonable to me 12:42 < gmaxwell> I think we key othre things off the assumption that this can't be zero. 12:42 < morcos> gmaxwell: first thing i found is that the absurdFee test would be a problem 12:42 < gmaxwell> And it's a misconfiguration hazard, e.g. 0.0000001 is a lot more reasonable a setting than 0. 12:42 < gmaxwell> Also perhaps dust limits? 12:43 < morcos> well having the dust limit be 0 makes sense right 12:43 < morcos> you might want to have that 12:43 < sipa> and the minimum increase for mempool eviction 12:43 < morcos> again 12:43 < morcos> you might want that 12:43 < sipa> indeed 12:43 < sipa> not sure it'd be a good thing for the network, though 12:43 < morcos> this came about for fixing rpc tests 12:44 < morcos> i was going to make blockprioritysize=non-zero be a default for RPC tests 12:44 < gmaxwell> "I support low fees!" which would drive the setting doesn't necessarily mean "I want my node to be trivialy vulnerable to DOS attack and a DOS amplifier towards other nodes" though. 12:44 < morcos> but i realized that would break as soon as we remove priority for 0.13 12:44 < morcos> gmaxwell: sure, but do you want to make it impossible to accept free txs 12:44 < gmaxwell> morcos: then we must support priority. 12:45 < morcos> there is not approriate emoji 12:45 < gmaxwell> My understanding and my sadness at dropping priority is that it went hand in hand with not supporting free transactions. 12:45 < gmaxwell> haha 12:45 < morcos> yeah so i guess i'm distinguishing between supporting it as a normal mode of operation vs not allowing it period 12:46 < morcos> for instance rpc tests, or mining some old tx authored a long time ago? 12:46 < morcos> perhaps fee deltas is the solution to the second (and perhaps teh first) 12:47 < morcos> but these issues are why we should rip priority out as soon as 0.12 is released, so we have time to get everythign working right 12:48 < sipa> going to run master + memory-usage-impacting-PRs-i-expect-to-be-merged on bitcoin.sipa.be, with empty bitcoin.conf 12:49 -!- d_t [~textual@c-50-136-139-144.hsd1.ca.comcast.net] has quit [Quit: Textual IRC Client: www.textualapp.com] 12:53 < sipa> and see what memory usage is 12:58 -!- CodeShark_ [~CodeShark@cpe-76-167-237-202.san.res.rr.com] has joined #bitcoin-core-dev 13:05 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has quit [] 13:05 -!- rook520 [~rook520@c-71-194-206-169.hsd1.in.comcast.net] has joined #bitcoin-core-dev 13:16 -!- tulip [~tulip@unaffiliated/tulip] has joined #bitcoin-core-dev 13:16 -!- JackH [~Jack@host-80-43-143-143.as13285.net] has quit [Ping timeout: 250 seconds] 13:16 < jtimon> rewarding the absurd fee check, see #7070 13:20 < morcos> jtimon: ah ok nice... but i'll save trying to let minrelaytxfee=0 for 0.13 13:22 < jtimon> morcos: ack, priority won't be removed for 0.12 anyway, no? 13:23 < morcos> jtimon: correct, just updating the pull to set the default blockprioritysize=0 13:27 -!- CodeShark_ [~CodeShark@cpe-76-167-237-202.san.res.rr.com] has quit [Ping timeout: 260 seconds] 13:37 -!- paveljanik [~paveljani@unaffiliated/paveljanik] has quit [Quit: Leaving] 14:15 < BlueMatt> sipa: I was waiting on 6936 14:15 < BlueMatt> it looks like that replaces 6872? (morcos?) 14:17 -!- warren [~warren@fedora/wombat/warren] has quit [Ping timeout: 245 seconds] 14:17 < morcos> BlueMatt: heh.. my ideal combination is some version of 6936 (warm cache) with a subset of 6872 (granular removals from the cache) 14:18 < morcos> but i hadn't really put the effort into that, and was expecting 6936 to wait for 0.13 14:18 < BlueMatt> oh? if you're gonna have a scoring metric for what to remove from mempool/cache, then we should only use that 14:19 < BlueMatt> no need to also remove things manually 14:19 < sipa> from morcos' benchmarks it seems that 6936 has a much less proven effect? 14:20 < morcos> BlueMatt: well the problem was there was some unexplainable slowdown with frequent cache flushes which I believe was due to deallocation time. 14:20 -!- warren [~warren@fedora/wombat/warren] has joined #bitcoin-core-dev 14:20 < BlueMatt> lol, fucking C++ 14:20 < morcos> so yeah i think 6936 is a net positive, but i don't think its ideally tune and i was kind of waiting for the prevector stuff 14:20 < morcos> since that ought to change that whole equation 14:20 < BlueMatt> hmm 14:20 < BlueMatt> fucking dep tree 14:21 < sipa> i think that we can do much better than prevector even for the coinscache if we write a custom memory-packed CCoins 14:21 < sipa> so maybe we can try that for 0.13 14:21 < sipa> but i think 6872 is necessary just for memory conservation 14:21 < BlueMatt> at what point do we decide we've gone too far down the rabbit hole? 14:22 < morcos> when we started working on bitcoin 14:22 < sipa> when people stop being willing to review code :) 14:23 < morcos> to be honest, i don't love 6872, but i reviewed it and test its performance, so i think if we want to address the potential memory growth problem between blocks, lets do 6872 now and we can always take some of it back out later 14:23 < BlueMatt> morcos: yea, it was minimum-to-fix-the-bug 14:23 < sipa> btw, 6872 rebase is trivial; already running it 14:23 < BlueMatt> morcos: hence i stopped paying attention to it when you did 6936 14:24 < BlueMatt> i figured you did that as a "oh, your hack? yea, i can do that better...." 14:24 -!- Guyver2 [~Guyver2@80.100.156.239] has quit [Quit: :)] 14:24 < morcos> ha, that was the original plan. :) 14:25 < morcos> the other thing that makes 6936 not as good as it should be is stupid priority 14:25 < BlueMatt> yup 14:25 < BlueMatt> long overdue to be killed 14:26 < morcos> have you guys thought about the performance hit to certain rejected txs from 6872 (see my comment) 14:29 < morcos> i guess my vote is to go ahead and merge 6872, but be open to a better fix for 0.13? 14:34 < BlueMatt> so libsecp256k1 has java bindings in-tree...what are people's opinions of doing something like that for libbitcoinconsensus? 14:34 < BlueMatt> otherwise you end up with a thin wrapper library doing the jni stuff which then links libbitcoinconsensus 14:34 < BlueMatt> very nice to have them in the same .so 14:35 -!- jgarzik_ [~jgarzik@104-178-201-106.lightspeed.tukrga.sbcglobal.net] has joined #bitcoin-core-dev 14:37 -!- jgarzik [~jgarzik@unaffiliated/jgarzik] has quit [Ping timeout: 260 seconds] 14:37 < BlueMatt> sipa: ? 14:38 -!- BashCo [~BashCo@unaffiliated/bashco] has joined #bitcoin-core-dev 14:39 < BlueMatt> jtimon: ? 14:41 < jtimon> BlueMatt: ? 14:41 < BlueMatt> jtimon: java wrapper in bitcoin core for libbitcoinconsensus? 14:41 < BlueMatt> eg https://github.com/bitcoin/secp256k1/tree/master/src/java 14:41 < btcdrak> o.O 14:41 < btcdrak> that's pretty cool. 14:42 < jtimon> BlueMatt: don't see why not 14:43 < sipa> BlueMatt: do those bindings even work? 14:43 < sipa> someone started updating them, but never got past the concept of read write locks, i think 14:44 < BlueMatt> sipa: now? i have no idea, they used to 14:44 < sipa> morcos: i think it's acceptable 14:44 < jtimon> it should be included with tests that start failing if it becomes unmaintained 14:44 < BlueMatt> i highly doubt anyone uses them, but I'm gonna try to push bitcoinj to remove its script engine entirely so I need some kind of replacement 14:45 < BlueMatt> so people would actually use that one 14:45 < sipa> jtimon: yup, that was my requirement; travis testable 14:45 < jtimon> but more things that can potentially break if you somehow break the consensus encapsulation helps protect what's already encapsulated 14:46 < jtimon> I mean, I don't plan to do it myself, but concept ack 14:46 < sipa> jtimon: the wrapper should only use the already exposed C api 14:47 < sipa> but yes, i'm certainly fine with that 14:47 < sipa> not just for java 14:48 -!- ParadoxSpiral [~ParadoxSp@p508B80ED.dip0.t-ipconnect.de] has quit [Remote host closed the connection] 14:48 < jtimon> sipa: yeah it's only going to be broken when the C api is changed, which shouldn't happen much once it's complete (and in the meantime) 14:50 < jtimon> and of course not just java, although I think I would put these in the consensus dir directly in preparation for "subtreeing" rather than in src 14:51 < jtimon> BlueMatt: ping #7091 15:00 < rook520> Why is it wrong to get involved in the bitcoin trade?? Wouldnt it be smart to buy some shares and just let it sit and not touch it...and just let it evolve with bitcoin itself?? 15:01 < rook520> Andreas Antonopoulos....are there people in here smarter then him?? is he a joke?? Or does he know hes shit?? 15:02 < jcorgan> rook520: take it to #bitcoin 15:02 < rook520> nvm i rather sit here quiet and watch you guys...Im learning in the process 15:03 -!- go1111111 [~go1111111@2604:4080:1128:0:7965:3a52:eb7b:1141] has quit [Ping timeout: 260 seconds] 15:08 < jtimon> morcos: sipa: BlueMatt: can we decide on one of the two options in #7115 ? 15:09 < jtimon> or another options, there's infinite ways to remove the new circular dependency, let's just chose one 15:09 < BlueMatt> jtimon: yea, give me a day and I'll go review that one 15:09 < jtimon> BlueMatt: awesome 15:12 < morcos> jtimon: i'm confused. i thought you only presented 1 way of removing the circular dependency 15:12 < jtimon> BlueMatt: there's ways inbetween (ie hide the global usage in main [only use it in GetMinFee] but still do it through a global instead of an attribute) 15:12 < morcos> calling pool->getMinFee() from inside CTxMemPool::estimateSmartFee 15:12 < jtimon> morcos: here's the other one: https://github.com/bitcoin/bitcoin/compare/master...jtimon:mempool-circular-dependency 15:13 < jtimon> and as said I can produce an infinite amount of them with enough time 15:13 < jtimon> but you guys are doing some incompatible requests 15:14 < sipa> i'm staying out of this now, but morcos's suggestion seems reasonable 15:15 < jtimon> for example, sipa says policy code shouldn't be created in the mempool, but then GetMinFee should be in the estimator instead of the mempool 15:15 < morcos> sipa: but you still want to remove the circular dependency now? 15:15 -!- go1111111 [~go1111111@104.232.116.217] has joined #bitcoin-core-dev 15:15 < morcos> jtimon: as for which method of removing circular dependency of the two you present, i like the one in the PR better 15:15 < morcos> what i don't like in the PR is where you are setting that attribute 15:16 < morcos> can you change it to be lastTrimmedSize and set it in TrimToSize? 15:16 < jtimon> yeah in init is better like in the other option 15:16 < morcos> its not necessarily a fixed value, its the value that was last trimmed to, it should be set from TrimToSize 15:16 < morcos> if you want TrimToSize to call some global variable set in init, thats ok with me 15:17 < morcos> but the mempool attribute should be set by TrimToSize 15:17 < jtimon> but then I would prefer to use a global instead of an attribute (the global can be wherever we want, for example, policy/fee) 15:17 < morcos> sigh.. why? 15:17 < morcos> its really only localized to GetMinFee 15:17 < morcos> why make it a globabl 15:17 < morcos> you're talking about two separate things 15:18 < morcos> the command line argument for maxmempool (fine make that a global) 15:18 < morcos> and the high water mark for the decay in GetMinFee (which should not be a global or a fixed constant, it should be whatever number TrimToSize was called with) 15:19 < jtimon> I haven't looked much yet, but how disruptive it seems to move GetMinFee to the estimator? 15:19 < jtimon> then we can make it an attribute there 15:19 < morcos> i haven't looked either but i also thought of that 15:20 < jtimon> and the mempool can take the estimator as a parameter for its constructor instead of the minRelayfee 15:20 < morcos> lets just concentrate on what MUST be done for 0.12. 15:20 < morcos> having the mempool take the estimator as a parameter seems very weird to me 15:21 < morcos> why does the circular dependency have to go right now? 15:21 < jtimon> then we can call the estimator without necessarily going through the mempool (there's many facade methods in mempool) and start decoupling the mempool from the estimator 15:21 < jtimon> morcos: again, because you introduced it right now 15:21 < morcos> jtimon: agree with that last statement, but too much for 0.12 15:21 < morcos> jtimon: but why is it a problem? 15:21 < sipa> jtimon: it seems very natural for the estimator to deoend on the mempool; why does that have to go? 15:22 < jtimon> if you hadn't introduced it unnecessarily there would be no hurry to remove it 15:22 < sipa> we can of course disconnect everything, and have a ton of glue code to connect things together 15:22 < jtimon> please let's discuss that properly when it's "needed" then, this is clearly not the case 15:23 < jtimon> my plan, as said multiple times, is to make them both completely independent from each other (but acceptToMempool depends on both) 15:23 < sipa> i dislike circular dependencies; they're a sign of badly separated code 15:24 < sipa> what must be done: i don't think it's worth so much discussion 15:24 < jtimon> if we introduce this unnecesssary dependency it will be harder to present my case later 15:24 < sipa> maybe i'm fine with just having a plan to fix it 15:24 < morcos> sipa: sure but if it makes sense for estimator to depend on mempool. then introducing that dependency isn't a regression even if mempool already depends on estimator, it just points out that at some point we should clean up the mempool depending on the estimator 15:24 < jtimon> morcos: why make it dependent right now when it has been shown that is not necessary? 15:25 < sipa> jtimon: no dependency is ever "necessary" 15:25 < sipa> ever seen juice code? 15:25 < sipa> all dependencies resolved at runtime 15:25 < morcos> sipa: ok, i'm happy to make sure the dependency is at most 1 way, but i'd rather not worry about it before 0.12 15:25 < sipa> that doesn't mean it's natural or readable 15:25 < jtimon> sipa: exactly 15:25 < jtimon> not seen juicy code but ack on no dependencies ever being necessary 15:26 < sipa> jtimon: but it sometimes comes at great cost in needing glue to bind things together 15:26 < sipa> the fact that a dependency is not necessary does not make it the best design 15:27 < jtimon> so morcos and I agree that the mempool should not depend on the estimator but we disagree on whether or not the estimator should depend on the mempool or not, step one, whatever is more "natural" both are equally possible 15:27 < jtimon> sipa: agreed 15:27 < jtimon> so we disagree on what is the best design 15:27 < sipa> it seems to me (but i'm not very familiar with the details) that having a mempool estimator depend on the mempool is perfectly natural... if the mempool changes, the estimator (or the glue code) will need to change too 15:28 < sipa> anyway 15:28 < jtimon> leaving the circular dependency there would be already deciding in his favor, since more couplings will rapidly follow 15:28 < morcos> jtimon: but half of what we're arguing about is the stupid GetArg calls and has nothing to do with the circular dependency! 15:28 < sipa> jtimon: if the coupling is natural, of course it will follow! 15:28 -!- zookolaptop [~user@2601:281:8001:26aa:7db3:9077:b884:b5f4] has quit [Remote host closed the connection] 15:29 < jtimon> morcos: no, from the PR: "But if people disagree, I can easily write a third patch that removes the circular dependency while maintaining all the GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000 verbosity." 15:29 < sipa> i've said enough about this 15:29 < jtimon> that's what sipa and matt are arguing about, it's the less prioritary thing for me 15:30 < morcos> jtimon: ok well do you want to try that, remove the dependency by calling GetMinFee inside of the CTxMemPool function with the GetArg and then I'll stop objecting. If I need to introduce the dependency for something else we can argue then 15:30 < morcos> leave all the GetArg's for now and we can celan those up separately 15:31 < jtimon> morcos: exactly we can argue later about the dependency (while, I hope, collaborating n removing the inverse dependency we both dislike) 15:31 < morcos> sounds like a plan 15:32 < morcos> i have to run now 15:33 < jtimon> ok, removing all the GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000 verbosity seemed like an obvious thing to do because I thought that was the general trend, I think we started arguing on where to put the global/attribute, but IMO the global is still better than argsMap (another global) 15:37 < sipa> best is a global in init or mail, whose value is passed to the trim calls in ATMP :) 15:38 < sipa> main 15:39 < jtimon> yep main and initialization in init is what the great majority of globals currently do, I don't see why we should be inconsistent in this case 15:40 < jtimon> sipa: thus https://github.com/bitcoin/bitcoin/compare/master...jtimon:mempool-circular-dependency I thought you would like it more than the initial one 15:40 < sipa> yup 15:42 < sipa> superficial ack 15:42 < jtimon> I specially would like to remove that nMempoolSizeMax local variable that's only used for a check but doesn't actually gurantee anything about the future contents of "GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000" line https://github.com/bitcoin/bitcoin/compare/master...jtimon:mempool-circular-dependency#diff-c865a8939105e6350a50af02766291b7L902 15:45 < jtimon> anyway, I'll wait for BlueMatt to have an opinion, but the main thing is that morcos and I have agreed to disagree later 15:47 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has quit [Ping timeout: 260 seconds] 15:47 < jtimon> well, I will prepare a 3-step thing for people to ack or nack the last 2 commits, the first one will be the minimal way of removing the circular dependency 15:48 < jtimon> so, 1 minimal commit plus 2 squash-or-nack commits 15:48 -!- da2ce7 [~da2ce7@opentransactions/dev/da2ce7] has joined #bitcoin-core-dev 15:53 < Luke-Jr> phantomcircuit: Author: Patick Strateman 15:53 < Luke-Jr> typo? 15:57 < sipa> Luke-Jr: no, twin brother 15:57 -!- randy-waterhouse [~kiwigb@opentransactions/dev/randy-waterhouse] has joined #bitcoin-core-dev 15:58 < Luke-Jr> O.o 16:03 < phantomcircuit> Luke-Jr, ha 16:17 -!- zookolaptop [~user@2601:281:8001:26aa:94ab:eb5d:4d3f:abe8] has joined #bitcoin-core-dev 16:27 -!- cocoBTC [~cocoBTC__@c-233a71d5.136-1-64736c10.cust.bredbandsbolaget.se] has quit [Quit: Leaving] 16:49 -!- dcousens [~anon@c110-22-219-15.sunsh4.vic.optusnet.com.au] has joined #bitcoin-core-dev 17:24 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-lplypgjvvyilzlfc] has quit [Quit: Connection closed for inactivity] 17:26 < morcos> jtimon: surely you don't need to make estimateSmartFee take a mempoolsize argument? as in that branch 17:52 < jtimon> morcos: getMinFee doesn't need to be called inside policy/fee yet, but nGlobalMempoolSizeLimit can be already in policy/fee instead of main if you want (I mean, no complain on my part) 17:52 < morcos> one sec and i'll show you what i had in mind 17:53 < jtimon> yep, I will update the PR with the minimal thing and a bunch of stuff for you to discard soon as well 17:53 < jtimon> just got distracted with something else 17:55 < jtimon> morcos: updated #7115 focus on the first commit and then nack the other two if you want 17:55 < morcos> https://github.com/bitcoin/bitcoin/compare/master...morcos:trackTrimSize 17:56 < morcos> and then if you want i'd be happy for you to add a commit that has a global mempool limit variable which TrimToSize calls 17:56 < morcos> and per our recent agreement i'll stop objecting if you move GetMinFee call to inside CTxMemPool, although I will silently still disagree 17:57 < jtimon> ok, so you agree with hiding the param in getminfee, I thought that was what you disliked more for some reason and removed it 17:58 < jtimon> morcos: do you have plans for dynamic trimming beyond testing? 17:58 < jtimon> I really don't understand that or what it even has to do with our discussion 17:58 < morcos> jtimon: no i don't, we talked about using a different trim limit during IBD, but to me its really about encapsulating properly what GetMinFee cares about 17:59 < jtimon> IBD? 17:59 < morcos> i looked at your pull, i don't like adding a global argument to estimateSmartFee 17:59 < morcos> yes, during IBD you could allocate less size to the mempool and more to your cache 17:59 < morcos> or during initial sync if you're not caught up or whatever 18:00 < morcos> doesn't have to technically be IBD 18:00 < jtimon> in the future that can be fixed if getminfee moves to the estimator 18:00 < jtimon> it could be an attribute in the estimator 18:00 < morcos> what is it you don't like about the commit i just showed you. i realize it still doesn't solve your problem, but i'm ok with you solving your problem on top of that. 18:01 < jtimon> what is IDB? 18:01 < morcos> initial block download 18:01 < jtimon> thanks 18:01 < jtimon> so, wait 18:02 < jtimon> you want to have a different limit for IBD but a constant one for the rest right? 18:02 < morcos> i don't want to do any of that right now 18:02 < morcos> i would like to fix it so GetMinFee doesn' tneed an argument 18:02 < morcos> but i don't think the proper value for it to use is your command line argument 18:02 < morcos> i think the proper value for it to use is the last size the mempool was trimmed to 18:02 < jtimon> I want to understand your long term vision, and I would like you to understand mine 18:04 < morcos> so yes, if GetMinFee moves to the estimator, then that attribute could also move to the estimator (or i mean policy code), perhaps a bit tricky since trim to size needs to set it, but perhaps trim to size should be a policy function anyway 18:05 < jtimon> morcos: ok GetMinFee doesn't need an argument, a global (a different one from argsMap I mean) can fix that problem, where do you want the global? 18:05 < morcos> jtimon: a global what? 18:05 < morcos> this is where we are miscommunicating 18:05 < jtimon> main, mempool or policy/fees? 18:05 < morcos> the value GetMinFee needs to know about isn't the command line argument, it is literally the last agrument that was given to TrimToSize 18:06 < jtimon> argsMap is the global we're currently using for this command line param 18:06 < jtimon> we should use an extern global initialized in init.cpp like everybody else 18:06 < morcos> but i think you're missing my point 18:07 < morcos> if TrimToSize still takes an argument, which I think it should 18:07 < morcos> then GetMinFee has to be aware of that argument, not the global just because we happen to call TrimToSize with the global 18:07 < jtimon> should TrimToSize use a different value 2 different times for a given comman-line call? 18:09 < morcos> lets step back for a second, what is your objection to doing it the way i'm proposing? 18:09 < jtimon> why does TrimToSize needs to be "reset" every time, why everyone else must depend on "whatever TrimToSize was called with last time" instead of just -maxmempool? 18:10 < morcos> because thats literally what the logic is, what was the mempool trimmed to 18:10 < morcos> and now how far below that number are we 18:10 < jtimon> morcos: it doesn't make any sense to me, I think you should start with why is this command line option different from all the rest 18:11 < morcos> i don't really think i have anything else to say thats not repeating myself 18:12 < jtimon> morcos: I just made the logic to be "never change once the user sets it, TrimToSize will eat whatever was set in init.cpp like anybody else" in 3 different ways 18:13 < jtimon> that can be changed as shown, why do you want ot maintain it as a parameter in TrimToSize but not int getminfee? what's the fundamental difference you see but I don't? 18:13 < morcos> well i think one evidence of what i'm saying is the fact that you had to create two TrimToSize functions 18:13 < jtimon> I could make them one if you prefer that 18:14 < jtimon> trvially I must add 18:14 < morcos> i think it would be hard to get the unit tests to work with no argument 18:14 < jtimon> the same goes for getminfee, again, what's the difference? 18:14 < morcos> I think TrimToSize should take an argument because its logic that should exist outside the mempool as to what size it should be trimmed to 18:15 < jtimon> morcos we're using other globals in the tests 18:15 < jtimon> for example argsMap 18:15 < morcos> I think GetMinFee should not take an argument, because it is constrained to have to use the value that TrimToSize was called with 18:15 < jtimon> like we're currently using 18:15 < morcos> the unit tests call TrimToSize with different arguments within the same tests or GetMinFee or whatever 18:15 < morcos> both probably 18:16 < jtimon> calling GetArg in 11 places instead of one is just a style choice 18:16 < jtimon> would you even oppose to a macro? 18:16 < morcos> ok, well at this point we're cluttering up IRC with a ridiculous conversation. you are the one who is trying to get something merged. 18:16 < morcos> i've tried to give you some reasonable compromises that i would support 18:17 < jtimon> "I think GetMinFee should not take an argument, because it is constrained to have to use the value that TrimToSize was called with" this doesn't make any sense, if you call TrimToSize with a value, you want GetMinFee to use that value 18:17 < morcos> if it gets merged without my support, that's also fine... but its just a bit annoying since i'm the one spending the most time coding in this area 18:17 < phantomcircuit> jtimon, theoretically we could fix GetArg but we cant if noboey uses it 18:18 < morcos> jtimon: huh? yes, this is correct: "if you call TrimToSize with a value, you want GetMinFee to use that value" 18:18 < jtimon> morcos: whatever, I can leave it at the minimal fix-circular-dependency commit and focus on what we agree instead of what we disagree 18:18 < morcos> i have not seen any commits that i can agree with 18:19 < jtimon> phantomcircuit: ?? init.cpp uses it plenty 18:19 < phantomcircuit> jtimon, GetArg isn't thread safe, so we cant change settings outside of init.cpp 18:20 < phantomcircuit> if we aren't using GetArg it becomes a hastle to find and change settings all over the place 18:20 < jtimon> morcos: if you can't agree with https://github.com/jtimon/bitcoin/commit/3c30cea7245a2fd44bbd9cf8844c6730855f63e4 you will have to explain why we need to introduce an unecessary dependency "right now" 18:21 < jtimon> sorry https://github.com/jtimon/bitcoin/commit/3c30cea7245a2fd44bbd9cf8844c6730855f63e4 18:22 < morcos> jtimon: the part i don't like about that is passing through nMempoolSizeLimit through estimateSmartFee. that just doesnt make sense to me. callers want to just say estimateSmartFee, why do they need to be aware of certain global variables. Furthermore it limits the ability to call TrimToSize with different values for no good reason. but talk about circular dependencies, you and i have a good one going now... so let 18:22 -!- calibre720 [~calibre72@triband-mum-120.62.204.215.mtnl.net.in] has joined #bitcoin-core-dev 18:22 < morcos> so lets just let it go. 18:23 < jtimon> "the part i don't like about that is passing through nMempoolSizeLimit through estimateSmartFee. that just doesnt make sense to me" do you have another simple alternative beyond creating an unnecessary circular dependency? 18:23 < morcos> i pasted it to you above!!! 18:24 < jtimon> callers should call the estimator directly without calling the mempool as a facade, but that's another topic entirily 18:24 < morcos> https://github.com/bitcoin/bitcoin/compare/master...morcos:trackTrimSize 18:24 < morcos> then you can fix the circular dependency on top of that. 18:25 < jtimon> "Furthermore it limits the ability to call TrimToSize with different values for no good reason" do you expect to need to call TrimToSize with values different from -maxmempool beyond testing? 18:27 < morcos> jtimon: sorry, i have to stop discussing this. no i don't expect to right now except for the edge case of doing so in IBD as mentioned above. 18:28 < jtimon> btw, I asked many times without anwer, do you agree that getminfee belongs in policy/fees ? 18:28 < jtimon> morcos: ok, whenever you have time to answer questions come back to me 18:29 -!- zookolaptop [~user@2601:281:8001:26aa:94ab:eb5d:4d3f:abe8] has quit [Ping timeout: 250 seconds] 18:35 < GitHub150> [bitcoin] jtimon closed pull request #7115: Mempool: Decouple CBlockPolicyEstimator from CTxMemPool (fix #6134) (master...6134-nits) https://github.com/bitcoin/bitcoin/pull/7115 18:35 -!- calibre720 [~calibre72@triband-mum-120.62.204.215.mtnl.net.in] has quit [Ping timeout: 245 seconds] 19:00 -!- pmienk_ [~pmienk@c-71-227-177-179.hsd1.wa.comcast.net] has quit [Ping timeout: 276 seconds] 19:23 -!- afk11 [~afk11@unaffiliated/afk11] has joined #bitcoin-core-dev 19:50 -!- jgarzik_ [~jgarzik@104-178-201-106.lightspeed.tukrga.sbcglobal.net] has quit [Quit: This computer has gone to sleep] 19:57 -!- guest234234 [~guest2342@223.207.206.176] has joined #bitcoin-core-dev 20:12 -!- afk11 [~afk11@unaffiliated/afk11] has quit [Ping timeout: 250 seconds] 20:16 -!- dcousens [~anon@c110-22-219-15.sunsh4.vic.optusnet.com.au] has quit [Ping timeout: 250 seconds] 20:29 -!- Dyanisus [~Dyanisus@unaffiliated/dyanisus] has quit [Ping timeout: 244 seconds] 20:34 -!- mempool [~mempool@unaffiliated/tulip/bot/mempool] has joined #bitcoin-core-dev 20:42 -!- jgarzik [~jgarzik@104-178-201-106.lightspeed.tukrga.sbcglobal.net] has joined #bitcoin-core-dev 20:42 -!- jgarzik [~jgarzik@104-178-201-106.lightspeed.tukrga.sbcglobal.net] has quit [Changing host] 20:42 -!- jgarzik [~jgarzik@unaffiliated/jgarzik] has joined #bitcoin-core-dev 21:25 -!- mempool [~mempool@unaffiliated/tulip/bot/mempool] has quit [Quit: Textual IRC Client: www.textualapp.com] 22:04 -!- Madars [~null@unaffiliated/madars] has quit [Ping timeout: 276 seconds] 22:05 -!- Madars [~null@unaffiliated/madars] has joined #bitcoin-core-dev 22:30 -!- CodeShark_ [~CodeShark@cpe-76-167-237-202.san.res.rr.com] has joined #bitcoin-core-dev 22:36 -!- randy-waterhouse [~kiwigb@opentransactions/dev/randy-waterhouse] has quit [Remote host closed the connection] 22:42 -!- tulip [~tulip@unaffiliated/tulip] has quit [Ping timeout: 260 seconds] 22:59 < GitHub18> [bitcoin] gmaxwell pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/438ee59839ad...c143c499c85b 22:59 < GitHub18> bitcoin/master 996d311 Nick: [RPC] Add transaction size to JSON output... 22:59 < GitHub18> bitcoin/master c143c49 Gregory Maxwell: Merge pull request #7072... 22:59 < GitHub134> [bitcoin] gmaxwell closed pull request #7072: [RPC] Add transaction size to JSON output (master...master) https://github.com/bitcoin/bitcoin/pull/7072 23:04 -!- randy-waterhouse [~kiwigb@opentransactions/dev/randy-waterhouse] has joined #bitcoin-core-dev 23:34 -!- paveljanik [~paveljani@79-98-72-216.sys-data.com] has joined #bitcoin-core-dev 23:34 -!- paveljanik [~paveljani@79-98-72-216.sys-data.com] has quit [Changing host] 23:34 -!- paveljanik [~paveljani@unaffiliated/paveljanik] has joined #bitcoin-core-dev 23:53 -!- PaulCapestany [~PaulCapes@204.28.124.82] has quit [Quit: .] 23:54 -!- PaulCapestany [~PaulCapes@204.28.124.82] has joined #bitcoin-core-dev 23:58 -!- Ylbam [uid99779@gateway/web/irccloud.com/x-lmpkubwdmeoftnbu] has joined #bitcoin-core-dev