12015-11-30T00:04:58 *** Ylbam has joined #bitcoin-core-dev
22015-11-30T00:18:48 *** jonasschnelli has quit IRC
32015-11-30T00:22:43 <andytoshi> does fundmany support a way to restrict the input set to only sufficiently confirmed outputs?
42015-11-30T00:22:58 <andytoshi> if i submit a PR to add this functionality (haven't scoped this out, idk if it's easy) would it be supported?
52015-11-30T00:24:10 <andytoshi> err, fundrawtransaction
62015-11-30T00:26:26 *** jonasschnelli has joined #bitcoin-core-dev
72015-11-30T00:35:47 <gmaxwell> andytoshi: it uses the normal selection logic: only mature inputs unless they're from yourself.
82015-11-30T00:36:25 <sipa> you can globally disable -spendzeroconfchange=0 or so
92015-11-30T00:36:37 <gmaxwell> then only >=1 conf, unless it still can't be successful, then unconfirmed, unless -spendzeroconfchange=0
102015-11-30T00:40:18 <sipa> it first tries only 6 confirms; if that doesn't work it tries 1 confirm; if that does work and spednzeroconfchange is set it tries 0 confirmed change and 1 confirms for all the rest
112015-11-30T00:43:29 *** Apocalyptic has quit IRC
122015-11-30T00:43:50 *** Dyanisus has quit IRC
132015-11-30T00:46:24 *** da2ce7 has quit IRC
142015-11-30T00:46:50 <andytoshi> sipa: gmaxwell: elements alpha uses this RPC to fund withdrawwatch transactions, which basically means the functionary policy is set by CAre
152015-11-30T00:46:52 <andytoshi> Core
162015-11-30T00:47:00 *** jonasschnelli has quit IRC
172015-11-30T00:47:12 <andytoshi> well, in this specific are
182015-11-30T00:47:18 <andytoshi> area
192015-11-30T00:47:44 <sipa> andytoshi: good thing bitcoin core uses a reasonable policy :)
202015-11-30T00:47:50 <andytoshi> hehe : )
212015-11-30T00:48:49 *** Apocalyptic has joined #bitcoin-core-dev
222015-11-30T01:09:00 *** harding_ is now known as harding
232015-11-30T01:09:31 *** jonasschnelli has joined #bitcoin-core-dev
242015-11-30T01:09:32 *** tripleslash_u is now known as tripleslash
252015-11-30T01:16:09 *** Dyanisus has joined #bitcoin-core-dev
262015-11-30T01:21:12 *** da2ce7 has joined #bitcoin-core-dev
272015-11-30T01:35:16 *** CodeShark_ has joined #bitcoin-core-dev
282015-11-30T01:42:47 *** guest234234 has quit IRC
292015-11-30T01:43:24 *** jonasschnelli has quit IRC
302015-11-30T01:49:26 *** jonasschnelli has joined #bitcoin-core-dev
312015-11-30T01:52:55 *** guest234234 has joined #bitcoin-core-dev
322015-11-30T01:54:32 *** Ylbam has quit IRC
332015-11-30T01:55:24 *** jonasschnelli has quit IRC
342015-11-30T02:05:26 *** jonasschnelli has joined #bitcoin-core-dev
352015-11-30T02:15:12 *** jonasschnelli has quit IRC
362015-11-30T02:23:04 *** jonasschnelli has joined #bitcoin-core-dev
372015-11-30T02:33:54 *** CodeShark_ has quit IRC
382015-11-30T02:37:29 *** guest234234 has quit IRC
392015-11-30T03:19:24 *** jonasschnelli has quit IRC
402015-11-30T03:20:28 *** jonasschnelli has joined #bitcoin-core-dev
412015-11-30T03:25:23 *** jonasschnelli has quit IRC
422015-11-30T03:35:45 *** guest234234 has joined #bitcoin-core-dev
432015-11-30T03:40:31 *** proem has joined #bitcoin-core-dev
442015-11-30T03:45:58 *** jonasschnelli has joined #bitcoin-core-dev
452015-11-30T03:51:12 *** jonasschnelli has quit IRC
462015-11-30T03:51:24 *** da2ce7 has quit IRC
472015-11-30T03:57:28 *** jonasschnelli has joined #bitcoin-core-dev
482015-11-30T04:16:30 *** CodeShark_ has joined #bitcoin-core-dev
492015-11-30T04:22:26 *** jtimon has quit IRC
502015-11-30T04:39:12 *** jonasschnelli has quit IRC
512015-11-30T04:48:59 *** jonasschnelli has joined #bitcoin-core-dev
522015-11-30T05:10:21 *** proem has quit IRC
532015-11-30T05:13:24 *** jonasschnelli has quit IRC
542015-11-30T05:16:32 *** d_t has quit IRC
552015-11-30T05:17:39 *** Dyanisus has quit IRC
562015-11-30T05:21:02 *** jonasschnelli has joined #bitcoin-core-dev
572015-11-30T05:28:23 *** jonasschnelli has quit IRC
582015-11-30T05:30:31 *** jonasschnelli has joined #bitcoin-core-dev
592015-11-30T05:40:26 <dgenr8> s/optimal/incentive-aligned/... relay policy can't be thought of as an optimization problem, too many independent actors involved.
602015-11-30T05:40:27 <dgenr8> Child-alone-pays-for-all-ancestors is simple and incentive-aligned. It should work for CNB too, though it may miss edge case opportunities.
612015-11-30T05:43:26 *** tripleslash has quit IRC
622015-11-30T05:45:09 *** Apocalyptic has quit IRC
632015-11-30T05:51:24 *** decalobate has joined #bitcoin-core-dev
642015-11-30T05:54:44 *** decalobate has quit IRC
652015-11-30T06:13:24 *** jonasschnelli has quit IRC
662015-11-30T06:18:02 *** jonasschnelli has joined #bitcoin-core-dev
672015-11-30T06:23:16 *** randy-waterhouse has quit IRC
682015-11-30T06:37:23 *** jonasschnelli has quit IRC
692015-11-30T06:47:02 *** jonasschnelli has joined #bitcoin-core-dev
702015-11-30T06:55:23 *** jonasschnelli has quit IRC
712015-11-30T06:55:33 *** jonasschnelli has joined #bitcoin-core-dev
722015-11-30T06:56:16 *** Ylbam has joined #bitcoin-core-dev
732015-11-30T07:08:49 *** Ylbam has quit IRC
742015-11-30T07:09:08 *** Ylbam has joined #bitcoin-core-dev
752015-11-30T07:09:46 *** guest234234 has quit IRC
762015-11-30T07:14:35 *** jonasschnelli has quit IRC
772015-11-30T07:18:13 *** guest234234 has joined #bitcoin-core-dev
782015-11-30T07:23:43 *** guest234234 has quit IRC
792015-11-30T07:29:53 *** guest234234 has joined #bitcoin-core-dev
802015-11-30T07:34:34 *** guest234234 has quit IRC
812015-11-30T07:40:20 *** Dyanisus has joined #bitcoin-core-dev
822015-11-30T07:55:42 <gmaxwell> Just so people aren't caught surprised, the addtion of Opt-in RBF to Bitcoin Core has inspired a seemingly organized misinformation campaign that has been spiraling all over the place. I've been maintaining a FAQ on reddit: https://www.reddit.com/r/Bitcoin/comments/3urm8o/optin_rbf_is_misunderstood_ask_questions_about_it/
832015-11-30T08:15:04 <GitHub198> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c28d3937b095...fa93174a7c06
842015-11-30T08:15:04 <GitHub198> bitcoin/master a6cbc02 Luke Dashjr: Bugfix: Default -uiplatform is not actually the platform this build was compiled on
852015-11-30T08:15:04 <GitHub198> bitcoin/master fa93174 Jonas Schnelli: Merge pull request #7127...
862015-11-30T08:15:10 *** d_t has joined #bitcoin-core-dev
872015-11-30T08:15:14 <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
882015-11-30T08:16:59 *** tripleslash has joined #bitcoin-core-dev
892015-11-30T08:23:53 *** da2ce7 has joined #bitcoin-core-dev
902015-11-30T08:26:04 *** Apocalyptic has joined #bitcoin-core-dev
912015-11-30T08:26:04 *** Apocalyptic has joined #bitcoin-core-dev
922015-11-30T08:30:43 *** Ylbam has quit IRC
932015-11-30T08:30:43 *** Ylbam has joined #bitcoin-core-dev
942015-11-30T08:36:11 *** JackH has quit IRC
952015-11-30T08:54:44 *** warren has quit IRC
962015-11-30T08:54:53 *** Ylbam_ has joined #bitcoin-core-dev
972015-11-30T08:55:24 *** Ylbam has quit IRC
982015-11-30T08:55:24 *** Ylbam_ is now known as Ylbam
992015-11-30T08:56:53 *** warren has joined #bitcoin-core-dev
1002015-11-30T09:05:36 *** Ylbam has quit IRC
1012015-11-30T09:10:39 *** Ylbam has joined #bitcoin-core-dev
1022015-11-30T09:10:51 *** d_t has quit IRC
1032015-11-30T09:18:01 *** jonasschnelli has joined #bitcoin-core-dev
1042015-11-30T09:18:02 *** jonasschnelli has quit IRC
1052015-11-30T09:18:02 *** jonasschnelli has joined #bitcoin-core-dev
1062015-11-30T09:18:49 *** jonasschnelli has joined #bitcoin-core-dev
1072015-11-30T09:27:22 *** moli has joined #bitcoin-core-dev
1082015-11-30T09:28:29 *** molly has quit IRC
1092015-11-30T09:29:50 <jouke> gmaxwell: you are a hero for doing that.
1102015-11-30T09:30:43 *** jonasschnelli has quit IRC
1112015-11-30T09:31:35 *** jonasschnelli has joined #bitcoin-core-dev
1122015-11-30T09:34:23 *** Guyver2 has joined #bitcoin-core-dev
1132015-11-30T09:34:57 <wumpus> gmaxwell: yes thanks for doing that and providing some sanity in the always-hysterical reddit
1142015-11-30T09:42:12 <gmaxwell> Thank you both for the thanks.
1152015-11-30T09:42:28 *** go1111111 has joined #bitcoin-core-dev
1162015-11-30T09:43:36 *** d_t has joined #bitcoin-core-dev
1172015-11-30T09:45:11 *** randy-waterhouse has joined #bitcoin-core-dev
1182015-11-30T09:53:29 *** BlueMatt has quit IRC
1192015-11-30T09:53:40 <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.
1202015-11-30T09:58:23 *** jonasschnelli has quit IRC
1212015-11-30T09:58:35 *** jonasschnelli has joined #bitcoin-core-dev
1222015-11-30T09:59:18 *** BlueMatt has joined #bitcoin-core-dev
1232015-11-30T10:00:11 <jonasschnelli> sipa: vHashes seems to be and object of "if (!fInitialDownload) {"
1242015-11-30T10:00:45 <jonasschnelli> not sure if i can expand the vHashes into the fInitialDownload region.
1252015-11-30T10:05:36 <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
1262015-11-30T10:06:36 <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.
1272015-11-30T10:07:50 <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.
1282015-11-30T10:07:53 <wumpus> btcdrak: yes, summarizing/copying these things to a blog for future reference would be very useful
1292015-11-30T10:08:14 <gmaxwell> That it totally breaks zero conf, that it is something being done by blockstream, etc.
1302015-11-30T10:08:35 <gmaxwell> So I think poor Opt-in RBF is just being made a proxy in someone's battle for something else. :(
1312015-11-30T10:08:36 <wumpus> btcdrak: the reddit format is good for interaction, but not for e.g. linking or referencing
1322015-11-30T10:08:43 <gmaxwell> petertodd: sorry. :(
1332015-11-30T10:09:03 <wumpus> gmaxwell: but I mean everyone has those worries and those have been considered from the very start
1342015-11-30T10:09:31 <gmaxwell> with the _complete_ lack of complaint (even from the expected parties, hell dgenr8 ACKed) on the PR, I was not anticipating this. :(
1352015-11-30T10:09:32 <wumpus> gmaxwell: some people suddenly awake 'oh no! zero-conf'.. if zero conf was not a concern the first iteration would have been merged
1362015-11-30T10:09:35 <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.
1372015-11-30T10:09:55 <wumpus> btcdrak: sure, that's true, it's kind of an attack I suppose
1382015-11-30T10:10:48 <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.
1392015-11-30T10:10:52 <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.
1402015-11-30T10:10:55 <wumpus> people feeling like they need to raise a rabble about everything because they're suuch an anarchist
1412015-11-30T10:11:42 <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.
1422015-11-30T10:11:50 <wumpus> gmaxwell: well WE have been loud enough about that
1432015-11-30T10:12:04 <wumpus> it's just that no one really listens, unless there is a controversy
1442015-11-30T10:12:11 <wumpus> (or perceived controversy)
1452015-11-30T10:12:22 <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
1462015-11-30T10:12:22 <btcdrak> things.
1472015-11-30T10:12:29 <wumpus> tulip: yep
1482015-11-30T10:12:44 <gmaxwell> tulip: "Homeopathic confirmation."
1492015-11-30T10:12:53 <btcdrak> gmaxwell: LOL
1502015-11-30T10:13:14 <wumpus> gmaxwell: that's a good comparison
1512015-11-30T10:14:08 <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.
1522015-11-30T10:14:44 <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
1532015-11-30T10:14:50 <gmaxwell> t (muhaha).
1542015-11-30T10:15:36 <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. :(
1552015-11-30T10:16: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, :(
1562015-11-30T10:17:01 <randy-waterhouse> more and more mainstream, layman derpiness, afraid it might only keep tending that way
1572015-11-30T10:17:39 <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.
1582015-11-30T10:18:11 <jgarzik> gmaxwell, Trying to help a bit, https://twitter.com/jgarzik/status/671271910634889216
1592015-11-30T10:18:32 <jgarzik> gmaxwell, +1 to btcdrak's comment
1602015-11-30T10:19:10 *** MarcoFalke has joined #bitcoin-core-dev
1612015-11-30T10:21:10 <wumpus> gmaxwell: jonasschnelli: #7112 #7037 both move the uiInterface.NotifyBlockTip
1622015-11-30T10:21:18 <wumpus> who should win? :p
1632015-11-30T10:22:52 <gmaxwell> maybe blocknotify should be made seperate? :)
1642015-11-30T10:23:03 <gmaxwell> 7112 looks like it would make it get processed even later. :)
1652015-11-30T10:23:17 <gmaxwell> (I say with a 10 second glance)
1662015-11-30T10:23:18 <wumpus> gmaxwell: and should include the notification for zmq
1672015-11-30T10:23:42 <wumpus> gmaxwell: no, I'd say otherwise
1682015-11-30T10:23:49 <wumpus> gmaxwell: the WALLET notification needs to be separte
1692015-11-30T10:24:09 <wumpus> gmaxwell: all the others, the GUI, ZMQ, blocknotify all take very short
1702015-11-30T10:24:28 <gmaxwell> Makes sense.
1712015-11-30T10:24:31 <wumpus> it's the wallet that is the problem and should probably get its own signal
1722015-11-30T10:24:44 <wumpus> 'delayedBlockTipNotify' :p
1732015-11-30T10:24:50 <sipa> jonasschnelli: let me look
1742015-11-30T10:25:27 <wumpus> ideally the wallet would also only notify a processing thread instead of doing heavy work in the notification, but that's for later
1752015-11-30T10:26:08 <jonasschnelli> gmaxwell: wumpus: i think #7037 is included in #7112
1762015-11-30T10:26:18 <wumpus> but at least all the well-behaved clients should get a first stab at the signal
1772015-11-30T10:26:50 <wumpus> jonasschnelli: no; #7037 moves the GUI notification earlier, you move it later
1782015-11-30T10:27:04 <jonasschnelli> Argh.. yes. Right.
1792015-11-30T10:27:11 <jgarzik> separate signal was always the plan...
1802015-11-30T10:27:13 <wumpus> jonasschnelli: could you live with moving it to the beginning?
1812015-11-30T10:27:20 <jgarzik> for wallet
1822015-11-30T10:28:13 <wumpus> jonasschnelli: (move the UI notification *before* notifying the wallet etc)
1832015-11-30T10:28:36 <jonasschnelli> wumpus: just checking... yes. I think this would be okay.
1842015-11-30T10:28:51 <sipa> jonasschnelli: you can use if (pindexFork != pindexNewTip) { ...}
1852015-11-30T10:28:55 <wumpus> jonasschnelli: ok, if you include that in your pull, we hit two flies with one stone
1862015-11-30T10:29:31 <jonasschnelli> sipa: okay. Will add the if
1872015-11-30T10:33:15 <sipa> jonasschnelli: in fact that whole notifications section can be skipped in that case
1882015-11-30T10:34:12 <sipa> gmaxwell, jgarzik, wumpus: IMHO we should only have one signal, and different handlers that each run in their own thread can register
1892015-11-30T10:34:33 <wumpus> sipa: well all the handlers are well-behaved in that regard. except the wallet
1902015-11-30T10:34:55 <jgarzik> sipa, For this specific situation you need cascading notifications and some additional parallelization in the wallet
1912015-11-30T10:34:57 <wumpus> UI handles things in their own thread, ZMQ is low-latency, blocknotify forks a thread,
1922015-11-30T10:35:08 <wumpus> just the wallet does expensive work in the signal handler
1932015-11-30T10:35:11 <jonasschnelli> sipa: the change results in a ugly diff. But i think it makes sense: https://github.com/jonasschnelli/bitcoin/commit/9af5f9cb8773da2904aa3819234aaebd2efb5d15
1942015-11-30T10:35:53 <wumpus> so I agree we should have only one signal, but that means the wallet wil have to handle the notification differently first
1952015-11-30T10:36:00 <jonasschnelli> the GetMainSignals().UpdatedBlockTip is unused by the wallet
1962015-11-30T10:36:09 <jonasschnelli> it was added only for ZMQ IIRC
1972015-11-30T10:36:09 <gmaxwell> sipa: if there is only one signal, then we must make sure all things that handle take no time.
1982015-11-30T10:36:22 <sipa> jonasschnelli: pro tip: add ?w=1 after the url
1992015-11-30T10:36:40 <sipa> gmaxwell: yes, that's the only sane design :)
2002015-11-30T10:36:57 <jonasschnelli> sipa: Ah. Right. Much better...
2012015-11-30T10:37:00 <sipa> gmaxwell: not saying that's viable now
2022015-11-30T10:37:19 <wumpus> right, we can't block progress on the wallet getting fixed
2032015-11-30T10:37:31 <sipa> but yes, signal handlers should never do work and never block
2042015-11-30T10:37:40 <jgarzik> That's the standard signal processing ideal - do very little - just enough to trigger further processing - not blocking
2052015-11-30T10:37:51 <wumpus> yes
2062015-11-30T10:38:00 <sipa> we all agree :)
2072015-11-30T10:39:31 <CodeShark> just have the signal handler all tasks into a queue and use separate threads for those :)
2082015-11-30T10:39:54 <sipa> CodeShark: some things do have time comstraints
2092015-11-30T10:40:32 <jonasschnelli> But could we not eliminate uiInterface.NotifyBlockTip and use GetMainSignals().UpdatedBlockTip instead?
2102015-11-30T10:40:34 <sipa> but indeed, that's perfectly viable for a lot of thingfs
2112015-11-30T10:40:43 <jonasschnelli> Or would this be hard for the UI?
2122015-11-30T10:40:45 <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
2132015-11-30T10:40:48 <sipa> jonasschnelli: imho yes
2142015-11-30T10:41:22 <jonasschnelli> But for the current PRs,... let's keep the signal. Combine and remove can be done later.
2152015-11-30T10:41:27 <sipa> especially when it includes the initialsync bool
2162015-11-30T10:41:39 <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
2172015-11-30T10:42:11 <CodeShark> yes, of course - Qt already has its own messaging system for that
2182015-11-30T10:42:14 <wumpus> also there is the problem of things blocking the dispatch thread then
2192015-11-30T10:42:26 <sipa> i wish we could set w=1 on github by default for c++ files
2202015-11-30T10:42:38 <wumpus> CodeShark: so does zmq, and so does blocknotify (which old-fashioned forks) :)
2212015-11-30T10:44:05 <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
2222015-11-30T10:45:40 <CodeShark> it's necessary for coin selection...but that's about it
2232015-11-30T10:45:59 <sipa> ... coin selection?
2242015-11-30T10:46:15 <CodeShark> yes - selecting outputs to spend when creating a new transaction
2252015-11-30T10:46:25 <sipa> there is no coin selection when updating the tip i hope!
2262015-11-30T10:46:30 <CodeShark> well, it's not necessary if you only use coins that are sufficiently buried
2272015-11-30T10:46:46 <sipa> we're talking about milliseconds difference
2282015-11-30T10:46:58 <sipa> blocks already propagate in the order of seconds
2292015-11-30T10:47:16 <sipa> you really don't need to inform the wallet within miliseconds
2302015-11-30T10:47:25 <wumpus> well obviously you don't want the wallet to be many blocks behind, that'd also mess with fee computation
2312015-11-30T10:47:33 <sipa> obviously
2322015-11-30T10:47:36 <wumpus> but that's not what we're considering here
2332015-11-30T10:47:47 <sipa> but async does not mean it has to lag behind
2342015-11-30T10:48:08 <wumpus> sure, I agree
2352015-11-30T10:48:11 <sipa> just that main can continue before the wallet is done handling things
2362015-11-30T10:48:21 <CodeShark> yes, for sure
2372015-11-30T10:49:09 <wumpus> could add a 'catch up fence' before doing coin selection / sending a transaction
2382015-11-30T10:49:24 <sipa> sounds reasonable
2392015-11-30T10:49:40 <wumpus> but that's mostly important if the wallet runs separately, e.g. if it can be stopped/started separately like monero's
2402015-11-30T10:49:50 <wumpus> if it still runs in the same process it'll never get out of sync much
2412015-11-30T10:51:41 <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)
2422015-11-30T10:52:28 <sipa> gmaxwell: i think i'm not fully understanding the problem there
2432015-11-30T10:53:33 <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.
2442015-11-30T10:53:48 <sipa> oh, right
2452015-11-30T10:54:31 <sipa> i'd say the right order is zmq,blocknotify,peers,wallet
2462015-11-30T10:54:37 *** rook520 has joined #bitcoin-core-dev
2472015-11-30T10:54:52 <sipa> hmm, but your notify may trigger wallet rpcs
2482015-11-30T10:54:56 <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
2492015-11-30T10:55:04 <CodeShark> :)
2502015-11-30T10:56:13 <CodeShark> I guess I could make it smarter with conflict resolution as it syncs
2512015-11-30T10:56:45 <CodeShark> i.e. replace inputs with new ones as they are seen spent during sync
2522015-11-30T10:57:33 <CodeShark> and require resigning
2532015-11-30T10:58:07 <CodeShark> but meh...probably not worth the effort :p
2542015-11-30T10:58:23 <wumpus> ...
2552015-11-30T10:59:34 <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
2562015-11-30T10:59:41 <CodeShark> right...that too
2572015-11-30T10:59:43 <sipa> so if blocknotify runs before the wallet update, then the notified script can issue a wallet rpc and miss the uodate
2582015-11-30T10:59:55 <CodeShark> my stuff does not make that assumption at all
2592015-11-30T11:00:22 <CodeShark> and in fact, for many typical use cases it would be very bad to make that assumption
2602015-11-30T11:00:33 <sipa> CodeShark: this is bitcoin core dev
2612015-11-30T11:01:00 <CodeShark> yes, just acknowledging the design feature
2622015-11-30T11:01:30 <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
2632015-11-30T11:02:37 *** guest234234 has joined #bitcoin-core-dev
2642015-11-30T11:03:29 <CodeShark> so block all wallet calls during a tip update?
2652015-11-30T11:03:48 <sipa> yuck
2662015-11-30T11:04:06 <wumpus> maybe the wallet should have its own notification for new blocks
2672015-11-30T11:04:18 <sipa> wumpus: i was just thinking that...
2682015-11-30T11:04:26 *** rook520 has quit IRC
2692015-11-30T11:04:44 <wumpus> 'walletblockprocessed' or something like that
2702015-11-30T11:05:13 <wumpus> there's a conceptual difference between 'new block came in' and 'new block processed by wallet'
2712015-11-30T11:05:17 *** rook520 has joined #bitcoin-core-dev
2722015-11-30T11:05:45 <wumpus> or to remain backward compatible: add a -blockearlynotify
2732015-11-30T11:06:04 <sipa> ... and the same in zmq?
2742015-11-30T11:06:12 <sipa> or is zmq blocks only anyway already
2752015-11-30T11:06:26 <tulip> zmq does transactions too
2762015-11-30T11:06:26 <wumpus> zmq should be completely decoupled from the wallet already
2772015-11-30T11:06:56 <wumpus> it's meant as a low-latency notification interface from the core, there is no wallet anything zmq
2782015-11-30T11:07:11 <sipa> oh, it just notifies for all txn
2792015-11-30T11:07:18 <sipa> if you subscribe?
2802015-11-30T11:07:21 <wumpus> yes
2812015-11-30T11:07:49 <sipa> so: zmq,blockearlynotify,peers,wallet,blocknotify
2822015-11-30T11:08:07 <wumpus> ui should be notified as early as possible too
2832015-11-30T11:08:08 <tulip> either transaction hashes or transaction binaries depending what you subscribe to.
2842015-11-30T11:09:28 <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
2852015-11-30T11:09:37 <sipa> ok
2862015-11-30T11:10:19 <wumpus> (there are CWallet signals that the UI subscribes to for wallet information, this is already handled properly)
2872015-11-30T11:15:40 <GitHub144> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/9ebedc1756e3...6fc287f2df54
2882015-11-30T11:15:40 <GitHub144> bitcoin/master c6973ca MarcoFalke: [qa] keypool: Fix white space to prepare transition to test framework
2892015-11-30T11:15:41 <GitHub144> bitcoin/master 4ea1790 MarcoFalke: [qa] keypool: DRY: Use test framework
2902015-11-30T11:15:41 <GitHub144> bitcoin/master 6fc287f Wladimir J. van der Laan: Merge pull request #7027...
2912015-11-30T11:15:44 <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
2922015-11-30T11:18:39 <GitHub24> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/6fc287f2df54...a7751824ce8a
2932015-11-30T11:18:40 <GitHub24> bitcoin/master 4b89f01 Ryan Havar: Default fPayAtLeastCustomFee to false...
2942015-11-30T11:18:41 <GitHub24> bitcoin/master fa506c0 MarcoFalke: [wallet] Add rpc tests to verify fee calculations
2952015-11-30T11:18:41 <GitHub24> bitcoin/master a775182 Wladimir J. van der Laan: Merge pull request #7103...
2962015-11-30T11:18:49 <GitHub124> [bitcoin] laanwj closed pull request #7103: [wallet, rpc tests] Fix settxfee, paytxfee (master...FixSettxfee) https://github.com/bitcoin/bitcoin/pull/7103
2972015-11-30T11:31:04 <wumpus> imo needs more code review: Add pre-allocated vector type and use it for CScript #6914
2982015-11-30T11:32:11 <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)
2992015-11-30T11:35:59 <GitHub37> [bitcoin] laanwj pushed 2 new commits to 0.11: https://github.com/bitcoin/bitcoin/compare/595c8d6301cf...5f09cda0bf4c
3002015-11-30T11:36:00 <GitHub37> bitcoin/0.11 7d0a05f Ryan Havar: Default fPayAtLeastCustomFee to false...
3012015-11-30T11:36:00 <GitHub37> bitcoin/0.11 5f09cda MarcoFalke: [wallet] Add rpc tests to verify fee calculations...
3022015-11-30T11:37:27 <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
3032015-11-30T11:38:32 *** jgarzik has quit IRC
3042015-11-30T11:39:11 *** instagibbs has quit IRC
3052015-11-30T11:41:08 *** fkhan has quit IRC
3062015-11-30T11:42:12 <sipa> wumpus: seems reasonable
3072015-11-30T11:42:29 *** d_t has quit IRC
3082015-11-30T11:44:24 <sipa> wumpus: i think something like 7105 is needed now with mempool limiting, as we'll otherwise automatically respend evicted inputs
3092015-11-30T11:45:07 <sipa> i'll help rebasing pr's tagged for 0.12 if that helps
3102015-11-30T11:47:35 *** instagibbs has joined #bitcoin-core-dev
3112015-11-30T11:52:24 *** randy-waterhouse has quit IRC
3122015-11-30T11:54:02 *** fkhan has joined #bitcoin-core-dev
3132015-11-30T11:55:15 <wumpus> sipa: agree
3142015-11-30T12:01:51 <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
3152015-11-30T12:04:17 <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...
3162015-11-30T12:04:49 <GitHub77> [bitcoin] laanwj pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/a7751824ce8a...96b802510da0
3172015-11-30T12:04:50 <GitHub77> bitcoin/master 012fc91 Jonas Schnelli: NotifyBlockTip signal: switch from hash (uint256) to CBlockIndex*...
3182015-11-30T12:04:51 <GitHub77> bitcoin/master e6d50fc Jonas Schnelli: [Qt] update block tip (height and date) without locking cs_main, update always (each block)
3192015-11-30T12:04:51 <GitHub77> bitcoin/master 947d20b Jonas Schnelli: [Qt] reduce cs_main in getVerificationProgress()
3202015-11-30T12:05:00 <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
3212015-11-30T12:05:11 <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
3222015-11-30T12:05:25 <wumpus> sipa: my problem is not with merging it, but with merging it one day before a release branch-off
3232015-11-30T12:08:19 <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
3242015-11-30T12:08:32 <sipa> i will review the code again
3252015-11-30T12:08:52 <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
3262015-11-30T12:24:25 *** guest234234 has quit IRC
3272015-11-30T12:30:37 *** BashCo has joined #bitcoin-core-dev
3282015-11-30T12:31:44 <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
3292015-11-30T12:44:57 <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
3302015-11-30T12:45:50 <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 ...
3312015-11-30T12:46:16 <sipa> but we still run TBV afterwards, right?
3322015-11-30T12:46:22 <morcos> correct
3332015-11-30T12:46: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
3342015-11-30T12:47:10 <morcos> i'm relatively confident that its not broken for normal situations now
3352015-11-30T12:47:14 <sipa> well that's what master and release candidates are for
3362015-11-30T12:47:45 <sipa> though as wumpus notes, it is already quite late, and we don't have knowledge of people using this code in production
3372015-11-30T12:48:01 <sipa> what about turning it into a separate createnewblockbeta RPC for now?
3382015-11-30T12:48:04 <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
3392015-11-30T12:48:49 <sipa> oh wow
3402015-11-30T12:49:38 <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
3412015-11-30T12:50:06 <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
3422015-11-30T12:50:27 <sipa> which is strange, because evidence shows that there are miners running master
3432015-11-30T12:50:31 <morcos> i'm not really in a position to complain since i didn't review anyone elses PR's recently though...
3442015-11-30T12:50:41 <morcos> Dec 1st just came too fast!
3452015-11-30T12:50:47 <sipa> you'd think they'd also test PRs...
3462015-11-30T12:50:59 <sipa> or show interest in things like this
3472015-11-30T12:51:20 <sipa> so i'm afraid that the only way to actually test it is getting it in master
3482015-11-30T12:51:30 <sipa> s/test/get production feedback/
3492015-11-30T12:51:55 <morcos> well the nice thing is that its relatively easy to sub in and out
3502015-11-30T12:52:03 <morcos> doesn't affect much more than 1 function
3512015-11-30T12:52:48 <sipa> indeed, just looked
3522015-11-30T12:52:53 <sipa> it doesn't seem hard
3532015-11-30T12:52:55 <morcos> i think it would be slightly cleaner to just replace than maintain both, but either option is ok with me
3542015-11-30T12:53:06 <morcos> let me rebase it really quick..
3552015-11-30T12:53:23 <sipa> the question is mostly whether adding it as a separate RPC would actually make more people use it
3562015-11-30T12:53:38 <sipa> wumpus: opinion?
3572015-11-30T12:55:03 <morcos> actually wait
3582015-11-30T12:55:03 <morcos> i know why i didn't like that idea
3592015-11-30T12:55:03 <morcos> it basically makes it impossible to make forward progress for another release
3602015-11-30T12:55:22 <morcos> well, yeah, depending on the answer to your question.. it would have to get a lot of uses as an alternate RPC
3612015-11-30T12:55:56 <morcos> of if you have original and beta for 0.12
3622015-11-30T12:56:04 <morcos> then what do we have for 0.13 ?
3632015-11-30T12:58:19 *** ParadoxSpiral has joined #bitcoin-core-dev
3642015-11-30T12:59:27 <sipa> i'd say after branching off 0.12, beta goes away and becomes the only one
3652015-11-30T13:00:08 <sipa> making this beta RPC a preview for an upcoming version, if no problems are found
3662015-11-30T13:00:57 <wumpus> <morcos> Dec 1st just came too fast! <- I'm sure May 1st will come soon enough too, then :)
3672015-11-30T13:01:56 <morcos> sipa: yes but my point is that it won't be the upcoming version
3682015-11-30T13:01:58 <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
3692015-11-30T13:02:21 <morcos> i'd like to make more changes, to actually improve the algorithm, not just speed it up
3702015-11-30T13:02:57 <sipa> right, that's where the real benefit lies, the fact that this new code can incorporate efficient CPFP etc
3712015-11-30T13:03:27 <sipa> and that won't be in such a beta
3722015-11-30T13:03:44 <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
3732015-11-30T13:03:52 <wumpus> getting some ACKs from actual miners would be great
3742015-11-30T13:04:19 <morcos> wumpus: i think sipa is right, lets put it in master. and announce we need actual miners to test it
3752015-11-30T13:04:36 <morcos> its easy to sub out if we lose confidence before release, but i don't think we will
3762015-11-30T13:04:44 <wumpus> then revert if there are none?
3772015-11-30T13:04:45 <wumpus> ok
3782015-11-30T13:04:51 <morcos> let me finish rebasing
3792015-11-30T13:10:31 <btcdrak> we should ping wangchun_, his pool is pretty proactive in testing stuff out. also Luke-Jr of course.
3802015-11-30T13:18:00 <wumpus> sipa: re: #7105, does listunspent need any change?
3812015-11-30T13:18:27 <wumpus> (the default is already to only show outputs with at least one confirmation, but asking just in case)
3822015-11-30T13:20:39 <GitHub142> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/96b802510da0...9b8fc6c89a08
3832015-11-30T13:20:40 <GitHub142> bitcoin/master 4531fc4 Daniel Cousens: torcontrol: only output disconnect if -debug=tor
3842015-11-30T13:20:40 <GitHub142> bitcoin/master 9b8fc6c Wladimir J. van der Laan: Merge pull request #7035...
3852015-11-30T13:21:51 <sipa> wumpus: i have not checked, but i don't expect it does
3862015-11-30T13:21:58 <morcos> wumpus: 7008 needs to be merged first. i just rebased that. or i need to rewrite 6898 without it
3872015-11-30T13:22:51 <wumpus> 7008 seems ready for merging
3882015-11-30T13:22:57 <sipa> wumpus: at worst, it will show non-conflicted non-mempool things now
3892015-11-30T13:23:07 <sipa> if you ask for unconfirmed results
3902015-11-30T13:23:32 <wumpus> sipa: ok - maybe report 'trusted' in that case?
3912015-11-30T13:23:46 <wumpus> not sure it's really relevant
3922015-11-30T13:31:20 <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.
3932015-11-30T13:32:19 <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
3942015-11-30T13:32:46 <morcos> i'm happy to make it work right today if we agree, but otherwise i'll spend my time on something else
3952015-11-30T13:32:48 <sipa> morcos: it's easy enough to keep the test and give them an explicit -blockprioritysize
3962015-11-30T13:33:07 <sipa> those should keep working anyway
3972015-11-30T13:33:16 <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
3982015-11-30T13:33:21 <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
3992015-11-30T13:33:48 <sipa> agree
4002015-11-30T13:34:21 <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.
4012015-11-30T13:34:48 <morcos> 6898 is rebased as well now
4022015-11-30T13:35:16 <morcos> i'll be back online in a couple hours
4032015-11-30T13:36:31 <sipa> cool
4042015-11-30T13:46:49 *** rook520 has quit IRC
4052015-11-30T13:52:23 *** Ylbam has quit IRC
4062015-11-30T13:58:28 *** Ylbam has joined #bitcoin-core-dev
4072015-11-30T14:01:53 *** ParadoxSpiral has quit IRC
4082015-11-30T14:01:59 *** instagibbs has quit IRC
4092015-11-30T14:16:54 *** MarcoFalke has quit IRC
4102015-11-30T14:28:54 *** CodeShark_ has quit IRC
4112015-11-30T14:30:31 *** tulip has quit IRC
4122015-11-30T14:36:00 *** BashCo has quit IRC
4132015-11-30T14:39:56 *** BlueMatt has quit IRC
4142015-11-30T15:03:13 <jonasschnelli> sipa: any recommendation for decorating a "limbo"-transaction in the UI?
4152015-11-30T15:03:30 <jonasschnelli> The "?" (questionmark) is already in use of unconfirmed transaction...
4162015-11-30T15:03:48 <jonasschnelli> maybe a "?!"
4172015-11-30T15:03:59 <sipa> jonasschnelli: ha!
4182015-11-30T15:04:35 <wumpus> ah yes the ‽
4192015-11-30T15:04:44 <jonasschnelli> haha
4202015-11-30T15:05:33 <wumpus> I suppose using the same decoration as unconfirmed transaction makes some sense, it's a relatively rare occurenc
4212015-11-30T15:06:30 <jonasschnelli> Okay. Then i'll add the "Trusted" (=in mempool) info to the transaction detail window only.
4222015-11-30T15:06:52 <wumpus> don't think it warrants a new icon, isn't there some other way to distinguish it?
4232015-11-30T15:07:02 <wumpus> hmm only in transaction detail window is the other extreme
4242015-11-30T15:08:32 <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...
4252015-11-30T15:09:30 <jonasschnelli> yes. Makes sense.
4262015-11-30T15:10:42 <wumpus> hmm it determines whether we regard it as spendable or not, does it?
4272015-11-30T15:10:50 <sipa> wumpus: indeed
4282015-11-30T15:11:05 <wumpus> that's what the brackets around the balance are used for [...]
4292015-11-30T15:11:16 <sipa> 0 confirmations (only for transactions from youself) are spendable when in the mempool, unspendable otherwise
4302015-11-30T15:11:50 <sipa> but the GUI afaik has no way of exposing the distinction between the two types of unconfirmed
4312015-11-30T15:11:53 <wumpus> e.g. if it shows [123] it doesn't count to the balance, if it's 123 it does
4322015-11-30T15:12:28 <wumpus> status.countsForBalance = wtx.IsTrusted() && !(wtx.GetBlocksToMaturity() > 0);
4332015-11-30T15:12:39 <sipa> ah, good!
4342015-11-30T15:12:41 <wumpus> so, seems we already check isTrusted()
4352015-11-30T15:12:45 <sipa> ok, so no real change needed
4362015-11-30T15:13:17 <sipa> that can be simplified to just wtx.IsTrusted btw
4372015-11-30T15:13:28 <sipa> ah, no
4382015-11-30T15:13:40 <sipa> sorry; it's a maturity check afterwards, not a confirmation check
4392015-11-30T15:16:20 <jonasschnelli> sipa: wumpus: is this bad? https://github.com/jonasschnelli/bitcoin/commit/f5957b88ca4d6205824bcf348feb8f865913b99c
4402015-11-30T15:16:45 <jonasschnelli> sipa: I think you could pull https://github.com/jonasschnelli/bitcoin/commit/965a1e6d9c5c1af5555665c0a94cde85dfe19a7b into your #7105
4412015-11-30T15:17:36 <sipa> i think "trusted
4422015-11-30T15:17:41 <sipa> i think "trusted" will confuse people
4432015-11-30T15:17:56 <sipa> if they receive an unconfirmed transaction from someone else it will always say untrusted
4442015-11-30T15:18:37 <sipa> trusted is stronger than being in the mempool; it's also only true for your own transactions
4452015-11-30T15:18:48 <wumpus> yes, if it's in transaction details you can give a longer description
4462015-11-30T15:19:06 <wumpus> but trusted is very overused and ambigious
4472015-11-30T15:19:09 <wumpus> let's not use that
4482015-11-30T15:19:23 <jonasschnelli> "In mempool" | "Not in mempool"?
4492015-11-30T15:19:40 <sipa> maybe use "0/unknown" if it's not in the mempool?
4502015-11-30T15:19:52 <wumpus> nooo not unknown
4512015-11-30T15:20:00 <sipa> that's probably even scarier :)
4522015-11-30T15:20:07 <wumpus> yeah unknown is very, very scary :)
4532015-11-30T15:20:20 <wumpus> 'your wallet is glitching'
4542015-11-30T15:20:42 <sipa> 0/pray
4552015-11-30T15:20:51 <wumpus> jonasschnelli: yes, let's just call it what it is, 'in memory pool', 'not in memory pool'
4562015-11-30T15:21:00 <jonasschnelli> ok.
4572015-11-30T15:21:06 <wumpus> heh
4582015-11-30T15:21:21 <sipa> while we're at it: maybe also actually show the depth of the conflict for conflicted?
4592015-11-30T15:21:43 <jonasschnelli> right... will do
4602015-11-30T15:21:57 <jonasschnelli> sipa: "conflicted (-X)"?
4612015-11-30T15:22:05 <sipa> sgtm
4622015-11-30T15:22:32 <wumpus> or conflicted (at depth X) ?
4632015-11-30T15:22:42 <sipa> sgtm
4642015-11-30T15:25:24 <sipa> or "Conflicts with a transaction with X confirmations"
4652015-11-30T15:26:28 <wumpus> that does get very long
4662015-11-30T15:26:48 <wumpus> that's fine for the expanded transaction details, but probably too long for the onmouseover
4672015-11-30T15:27:19 <sipa> ok; we have a GUI maintainer to decide such things :)
4682015-11-30T15:27:26 <wumpus> yes
4692015-11-30T15:27:27 <wumpus> :D
4702015-11-30T15:27:34 <jonasschnelli> hah.
4712015-11-30T15:27:48 <jonasschnelli> i'll check how it looks...
4722015-11-30T15:30:50 <sipa> jonasschnelli: if you want to show "In mempool", you can't just use IsTrusted()
4732015-11-30T15:32:01 <jonasschnelli> sipa: right.. need to refactor the mempool.exists check.
4742015-11-30T15:32:23 <jonasschnelli> public expose "bool CWalletTx::InMempool()"?
4752015-11-30T15:32:30 <sipa> sgtm
4762015-11-30T15:36:59 *** jgarzik has joined #bitcoin-core-dev
4772015-11-30T15:39:41 *** rubensayshi has joined #bitcoin-core-dev
4782015-11-30T15:39:54 *** BlueMatt has joined #bitcoin-core-dev
4792015-11-30T15:41:43 *** MarcoFalke has joined #bitcoin-core-dev
4802015-11-30T15:50:12 *** instagibbs has joined #bitcoin-core-dev
4812015-11-30T15:50:59 *** rook520 has joined #bitcoin-core-dev
4822015-11-30T16:00:52 *** dermoth has quit IRC
4832015-11-30T16:01:24 *** Taek42 is now known as Taek
4842015-11-30T16:01:41 <wumpus> if you do want to use IsTrusted, use "counts towards balance" instead of "in mempool"?
4852015-11-30T16:02:16 <wumpus> otherwise it needs yet another flag on the ui's transaction structure, for kind of little gain
4862015-11-30T16:08:58 *** gavinand1esen has quit IRC
4872015-11-30T16:08:58 *** gavinand1esen has joined #bitcoin-core-dev
4882015-11-30T16:13:48 *** ParadoxSpiral has joined #bitcoin-core-dev
4892015-11-30T16:16:51 <wumpus> also we don't expose "transaction in mempool" on RPC, should definitely do that if it's shown in the GUI...
4902015-11-30T16:17:59 <sipa> agree
4912015-11-30T16:23:39 *** BashCo has joined #bitcoin-core-dev
4922015-11-30T16:30:53 *** gavinand1esen is now known as gavinandresen
4932015-11-30T16:38:39 *** da2ce7 has quit IRC
4942015-11-30T16:41:58 *** da2ce7 has joined #bitcoin-core-dev
4952015-11-30T16:51:15 *** gavinandresen has quit IRC
4962015-11-30T16:53:03 *** gavinandresen has joined #bitcoin-core-dev
4972015-11-30T17:00:00 *** challisto has quit IRC
4982015-11-30T17:24:23 *** Apocalyptic has quit IRC
4992015-11-30T17:27:54 *** Apocalyptic has joined #bitcoin-core-dev
5002015-11-30T17:31:00 <jgarzik> Double-check: is it correct that MTP is available via RPC, via getinfo.timeoffset? (curtime + timeoffset)
5012015-11-30T17:32:22 <sipa> i would expect that to be about GetAdjustedTime
5022015-11-30T17:33:34 <gmaxwell> it's in getblockchaininfo
5032015-11-30T17:33:47 <gmaxwell> "mediantime"
5042015-11-30T17:33:56 <gmaxwell> In master.
5052015-11-30T17:34:59 <jgarzik> Yeah, the definition of GetAdjustedTime() is curtime + GetTimeOffset
5062015-11-30T17:35:03 <jgarzik> but mediantime is better
5072015-11-30T17:36:11 <sipa> jgarzik: do you mean getmediantimepast of the last block?
5082015-11-30T17:36:18 <sipa> or the median time offset reported by peers?
5092015-11-30T17:36:55 <instagibbs> wumpus, I think #7044 is merge-ready. Trying to slip it in for 0.12
5102015-11-30T17:38:28 <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.
5112015-11-30T17:39:46 <sipa> jgarzik: agree
5122015-11-30T17:41:35 *** rubensayshi has quit IRC
5132015-11-30T17:42:33 *** zookolaptop has joined #bitcoin-core-dev
5142015-11-30T17:44:21 *** d_t has joined #bitcoin-core-dev
5152015-11-30T17:46:05 <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
5162015-11-30T17:46:48 *** zookolap` has joined #bitcoin-core-dev
5172015-11-30T17:48:04 *** zookolaptop has quit IRC
5182015-11-30T17:49:35 *** ParadoxSpiral has quit IRC
5192015-11-30T17:52:36 *** cocoBTC has joined #bitcoin-core-dev
5202015-11-30T17:52:44 *** JackH has joined #bitcoin-core-dev
5212015-11-30T17:55:20 <gmaxwell> bluematt: apparently the 0.11.2 ppa is 'build failed' and not up yet?
5222015-11-30T18:01:14 *** zookolap` has quit IRC
5232015-11-30T18:02:02 *** ParadoxSpiral has joined #bitcoin-core-dev
5242015-11-30T18:05:13 *** MarcoFalke has quit IRC
5252015-11-30T18:17:19 *** lightningbot` has joined #bitcoin-core-dev
5262015-11-30T18:17:26 *** lightningbot has quit IRC
5272015-11-30T18:17:36 *** phantomcircuit has quit IRC
5282015-11-30T18:17:59 *** midnightmagic has quit IRC
5292015-11-30T18:19:43 *** cfields_ has quit IRC
5302015-11-30T18:21:50 *** cfields has joined #bitcoin-core-dev
5312015-11-30T18:22:43 *** asoltys has quit IRC
5322015-11-30T18:24:00 *** phantomcircuit has joined #bitcoin-core-dev
5332015-11-30T18:30:29 *** arowser has joined #bitcoin-core-dev
5342015-11-30T18:30:50 *** arowser_ has quit IRC
5352015-11-30T18:45:44 *** midnightmagic has joined #bitcoin-core-dev
5362015-11-30T18:48:41 *** midnightmagic has quit IRC
5372015-11-30T18:48:55 *** asoltys has joined #bitcoin-core-dev
5382015-11-30T18:50:28 *** midnightmagic has joined #bitcoin-core-dev
5392015-11-30T18:55:11 <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
5402015-11-30T18:55:32 <BlueMatt> gmaxwell: it seems launchpad cant install a package which is in the repos according to ubuntu's package listing site
5412015-11-30T18:56:51 <BlueMatt> gmaxwell: mostly, internet folks need to go complain to launchpad
5422015-11-30T18:58:34 <gmaxwell> bleh.
5432015-11-30T18:59:00 <gmaxwell> BlueMatt: Have you complained to ubuntu?
5442015-11-30T18:59:47 <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
5452015-11-30T18:59:50 <BlueMatt> but I can go do that
5462015-11-30T19:01:55 *** paveljanik has joined #bitcoin-core-dev
5472015-11-30T19:01:55 *** paveljanik has joined #bitcoin-core-dev
5482015-11-30T19:07:39 <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.
5492015-11-30T19:08:28 <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.
5502015-11-30T19:12:11 *** warren has quit IRC
5512015-11-30T19:13:28 *** warren has joined #bitcoin-core-dev
5522015-11-30T19:15:43 <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.
5532015-11-30T19:16:12 <jgarzik> (picocoin thing just an anecdote noting Fedora people working on the overall ecosystem; Harald also works on bitcoin)
5542015-11-30T19:18:39 <BlueMatt> jgarzik: yea, warren has been pushing that one forward as well, iirc
5552015-11-30T19:20:52 <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)
5562015-11-30T19:29:18 *** zookolaptop has joined #bitcoin-core-dev
5572015-11-30T19:29:36 <sipa> Luke-Jr: yes, it produces the same blocks as the old code, ignoring some rounding errors, afaik
5582015-11-30T19:29:59 <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
5592015-11-30T19:31:57 <morcos> sipa: requires 6357 for the blocks to be the same, but they are quite close without that
5602015-11-30T19:33:30 <sipa> right
5612015-11-30T19:41:58 *** zookolaptop has quit IRC
5622015-11-30T19:43:29 *** da2ce7 has quit IRC
5632015-11-30T19:47:34 *** da2ce7 has joined #bitcoin-core-dev
5642015-11-30T19:52:35 <sdaftuar> BlueMatt: I updated 6915, hopefully this version looks good to everyone... Can you take a look?
5652015-11-30T20:02:38 *** zookolaptop has joined #bitcoin-core-dev
5662015-11-30T20:10:31 <sipa> jonasschnelli: opinion about asking on bitcoin-qt first startup how much RAM the user wants to make available for chainstate?
5672015-11-30T20:11:20 <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
5682015-11-30T20:12:05 <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.
5692015-11-30T20:12:21 <gmaxwell> and switch defaults based on the result.
5702015-11-30T20:14:14 <sipa> now that the limit is actually honored accurately, maybe it's more useful to do that
5712015-11-30T20:15:29 <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.
5722015-11-30T20:16:10 <gmaxwell> mempool limit could also change on that basis.
5732015-11-30T20:17:48 <morcos> sipa: now that its honored?
5742015-11-30T20:19:09 <sipa> morcos: -dbcache value is pretty accurately followed now
5752015-11-30T20:19:25 <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
5762015-11-30T20:19:36 <sipa> agree
5772015-11-30T20:19:50 <morcos> i thought that without 6872 you'd still blow through the limit?
5782015-11-30T20:20:09 <sipa> not considering miners :)
5792015-11-30T20:20:14 <sipa> but yes
5802015-11-30T20:20:29 *** BashCo has quit IRC
5812015-11-30T20:20:30 <sipa> and we should probably try to make checkmempool also not pull in the entire mempool into the cache
5822015-11-30T20:20:55 <morcos> i thought there was still a problem of all the txs that coudl come in between blocks
5832015-11-30T20:21:00 <morcos> especially rejected ones
5842015-11-30T20:22:33 <sipa> hmm, didn't we fix that?
5852015-11-30T20:22:47 <sipa> i can't remember how; only thinking about it
5862015-11-30T20:23:08 <sdaftuar> that's what #6872 is (tagged for 0.12 but still not yet merged)
5872015-11-30T20:23:31 <sdaftuar> looks like it needs to be rebased
5882015-11-30T20:24:00 <sipa> oh
5892015-11-30T20:26:10 *** JackH has quit IRC
5902015-11-30T20:26:27 <GitHub26> [bitcoin] gmaxwell pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/34e02e014718...438ee59839ad
5912015-11-30T20:26:27 <GitHub26> bitcoin/master d52fbf0 Gregory Sanders: Added additional config option for multiple RPC users.
5922015-11-30T20:26:28 <GitHub26> bitcoin/master 438ee59 Gregory Maxwell: Merge pull request #7044...
5932015-11-30T20:26:29 <sipa> sdaftuar: sorry, i didn't look at the number :)
5942015-11-30T20:26:32 <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
5952015-11-30T20:26:55 <sipa> BlueMatt: feel like rebasing 6872?
5962015-11-30T20:27:40 *** JackH has joined #bitcoin-core-dev
5972015-11-30T20:28:24 *** jtimon has joined #bitcoin-core-dev
5982015-11-30T20:31:57 <morcos> there is a check in init.cpp which prevents minrelaytxfee=0
5992015-11-30T20:32:10 <morcos> it seems to me we should consider removing that check?
6002015-11-30T20:32:36 <morcos> in a world without priority, it'll be impossible to mine 0 fee txs unless we do
6012015-11-30T20:39:14 <jtimon> morcos: sounds reasonable to me
6022015-11-30T20:42:17 <gmaxwell> I think we key othre things off the assumption that this can't be zero.
6032015-11-30T20:42:41 <morcos> gmaxwell: first thing i found is that the absurdFee test would be a problem
6042015-11-30T20:42:48 <gmaxwell> And it's a misconfiguration hazard, e.g. 0.0000001 is a lot more reasonable a setting than 0.
6052015-11-30T20:42:53 <gmaxwell> Also perhaps dust limits?
6062015-11-30T20:43:02 <morcos> well having the dust limit be 0 makes sense right
6072015-11-30T20:43:07 <morcos> you might want to have that
6082015-11-30T20:43:09 <sipa> and the minimum increase for mempool eviction
6092015-11-30T20:43:12 <morcos> again
6102015-11-30T20:43:15 <morcos> you might want that
6112015-11-30T20:43:18 <sipa> indeed
6122015-11-30T20:43:41 <sipa> not sure it'd be a good thing for the network, though
6132015-11-30T20:43:51 <morcos> this came about for fixing rpc tests
6142015-11-30T20:44:04 <morcos> i was going to make blockprioritysize=non-zero be a default for RPC tests
6152015-11-30T20:44:15 <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.
6162015-11-30T20:44:16 <morcos> but i realized that would break as soon as we remove priority for 0.13
6172015-11-30T20:44:45 <morcos> gmaxwell: sure, but do you want to make it impossible to accept free txs
6182015-11-30T20:44:54 <gmaxwell> morcos: then we must support priority.
6192015-11-30T20:45:10 <morcos> there is not approriate emoji
6202015-11-30T20:45:17 <gmaxwell> My understanding and my sadness at dropping priority is that it went hand in hand with not supporting free transactions.
6212015-11-30T20:45:20 <gmaxwell> haha
6222015-11-30T20:45:51 <morcos> yeah so i guess i'm distinguishing between supporting it as a normal mode of operation vs not allowing it period
6232015-11-30T20:46:29 <morcos> for instance rpc tests, or mining some old tx authored a long time ago?
6242015-11-30T20:46:54 <morcos> perhaps fee deltas is the solution to the second (and perhaps teh first)
6252015-11-30T20:47:25 <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
6262015-11-30T20:48:34 <sipa> going to run master + memory-usage-impacting-PRs-i-expect-to-be-merged on bitcoin.sipa.be, with empty bitcoin.conf
6272015-11-30T20:49:13 *** d_t has quit IRC
6282015-11-30T20:53:32 <sipa> and see what memory usage is
6292015-11-30T20:58:01 *** CodeShark_ has joined #bitcoin-core-dev
6302015-11-30T21:05:23 *** rook520 has quit IRC
6312015-11-30T21:05:56 *** rook520 has joined #bitcoin-core-dev
6322015-11-30T21:16:24 *** tulip has joined #bitcoin-core-dev
6332015-11-30T21:16:26 *** JackH has quit IRC
6342015-11-30T21:16:42 <jtimon> rewarding the absurd fee check, see #7070
6352015-11-30T21:20:27 <morcos> jtimon: ah ok nice... but i'll save trying to let minrelaytxfee=0 for 0.13
6362015-11-30T21:22:41 <jtimon> morcos: ack, priority won't be removed for 0.12 anyway, no?
6372015-11-30T21:23:06 <morcos> jtimon: correct, just updating the pull to set the default blockprioritysize=0
6382015-11-30T21:27:58 *** CodeShark_ has quit IRC
6392015-11-30T21:37:53 *** paveljanik has quit IRC
6402015-11-30T22:15:38 <BlueMatt> sipa: I was waiting on 6936
6412015-11-30T22:15:48 <BlueMatt> it looks like that replaces 6872? (morcos?)
6422015-11-30T22:17:14 *** warren has quit IRC
6432015-11-30T22:17:58 <morcos> BlueMatt: heh.. my ideal combination is some version of 6936 (warm cache) with a subset of 6872 (granular removals from the cache)
6442015-11-30T22:18:43 <morcos> but i hadn't really put the effort into that, and was expecting 6936 to wait for 0.13
6452015-11-30T22:18:50 <BlueMatt> oh? if you're gonna have a scoring metric for what to remove from mempool/cache, then we should only use that
6462015-11-30T22:19:01 <BlueMatt> no need to also remove things manually
6472015-11-30T22:19:58 <sipa> from morcos' benchmarks it seems that 6936 has a much less proven effect?
6482015-11-30T22:20:02 <morcos> BlueMatt: well the problem was there was some unexplainable slowdown with frequent cache flushes which I believe was due to deallocation time.
6492015-11-30T22:20:15 *** warren has joined #bitcoin-core-dev
6502015-11-30T22:20:23 <BlueMatt> lol, fucking C++
6512015-11-30T22:20:32 <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
6522015-11-30T22:20:41 <morcos> since that ought to change that whole equation
6532015-11-30T22:20:42 <BlueMatt> hmm
6542015-11-30T22:20:59 <BlueMatt> fucking dep tree
6552015-11-30T22:21:18 <sipa> i think that we can do much better than prevector even for the coinscache if we write a custom memory-packed CCoins
6562015-11-30T22:21:31 <sipa> so maybe we can try that for 0.13
6572015-11-30T22:21:48 <sipa> but i think 6872 is necessary just for memory conservation
6582015-11-30T22:21:55 <BlueMatt> at what point do we decide we've gone too far down the rabbit hole?
6592015-11-30T22:22:06 <morcos> when we started working on bitcoin
6602015-11-30T22:22:13 <sipa> when people stop being willing to review code :)
6612015-11-30T22:23:15 <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
6622015-11-30T22:23:42 <BlueMatt> morcos: yea, it was minimum-to-fix-the-bug
6632015-11-30T22:23:48 <sipa> btw, 6872 rebase is trivial; already running it
6642015-11-30T22:23:56 <BlueMatt> morcos: hence i stopped paying attention to it when you did 6936
6652015-11-30T22:24:11 <BlueMatt> i figured you did that as a "oh, your hack? yea, i can do that better...."
6662015-11-30T22:24:15 *** Guyver2 has quit IRC
6672015-11-30T22:24:23 <morcos> ha, that was the original plan. :)
6682015-11-30T22:25:02 <morcos> the other thing that makes 6936 not as good as it should be is stupid priority
6692015-11-30T22:25:16 <BlueMatt> yup
6702015-11-30T22:25:25 <BlueMatt> long overdue to be killed
6712015-11-30T22:26:10 <morcos> have you guys thought about the performance hit to certain rejected txs from 6872 (see my comment)
6722015-11-30T22:29:40 <morcos> i guess my vote is to go ahead and merge 6872, but be open to a better fix for 0.13?
6732015-11-30T22:34:09 <BlueMatt> so libsecp256k1 has java bindings in-tree...what are people's opinions of doing something like that for libbitcoinconsensus?
6742015-11-30T22:34:24 <BlueMatt> otherwise you end up with a thin wrapper library doing the jni stuff which then links libbitcoinconsensus
6752015-11-30T22:34:36 <BlueMatt> very nice to have them in the same .so
6762015-11-30T22:35:05 *** jgarzik_ has joined #bitcoin-core-dev
6772015-11-30T22:37:31 *** jgarzik has quit IRC
6782015-11-30T22:37:52 <BlueMatt> sipa: ?
6792015-11-30T22:38:48 *** BashCo has joined #bitcoin-core-dev
6802015-11-30T22:39:29 <BlueMatt> jtimon: ?
6812015-11-30T22:41:10 <jtimon> BlueMatt: ?
6822015-11-30T22:41:23 <BlueMatt> jtimon: java wrapper in bitcoin core for libbitcoinconsensus?
6832015-11-30T22:41:32 <BlueMatt> eg https://github.com/bitcoin/secp256k1/tree/master/src/java
6842015-11-30T22:41:37 <btcdrak> o.O
6852015-11-30T22:41:52 <btcdrak> that's pretty cool.
6862015-11-30T22:42:15 <jtimon> BlueMatt: don't see why not
6872015-11-30T22:43:27 <sipa> BlueMatt: do those bindings even work?
6882015-11-30T22:43:54 <sipa> someone started updating them, but never got past the concept of read write locks, i think
6892015-11-30T22:44:21 <BlueMatt> sipa: now? i have no idea, they used to
6902015-11-30T22:44:21 <sipa> morcos: i think it's acceptable
6912015-11-30T22:44:42 <jtimon> it should be included with tests that start failing if it becomes unmaintained
6922015-11-30T22:44:52 <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
6932015-11-30T22:45:01 <BlueMatt> so people would actually use that one
6942015-11-30T22:45:43 <sipa> jtimon: yup, that was my requirement; travis testable
6952015-11-30T22:45:59 <jtimon> but more things that can potentially break if you somehow break the consensus encapsulation helps protect what's already encapsulated
6962015-11-30T22:46:39 <jtimon> I mean, I don't plan to do it myself, but concept ack
6972015-11-30T22:46:48 <sipa> jtimon: the wrapper should only use the already exposed C api
6982015-11-30T22:47:00 <sipa> but yes, i'm certainly fine with that
6992015-11-30T22:47:09 <sipa> not just for java
7002015-11-30T22:48:07 *** ParadoxSpiral has quit IRC
7012015-11-30T22:48:32 <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)
7022015-11-30T22:50:07 <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
7032015-11-30T22:51:23 <jtimon> BlueMatt: ping #7091
7042015-11-30T23:00:33 <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??
7052015-11-30T23:01:41 <rook520> Andreas Antonopoulos....are there people in here smarter then him?? is he a joke?? Or does he know hes shit??
7062015-11-30T23:02:04 <jcorgan> rook520: take it to #bitcoin
7072015-11-30T23:02:30 <rook520> nvm i rather sit here quiet and watch you guys...Im learning in the process
7082015-11-30T23:03:38 *** go1111111 has quit IRC
7092015-11-30T23:08:55 <jtimon> morcos: sipa: BlueMatt: can we decide on one of the two options in #7115 ?
7102015-11-30T23:09:37 <jtimon> or another options, there's infinite ways to remove the new circular dependency, let's just chose one
7112015-11-30T23:09:38 <BlueMatt> jtimon: yea, give me a day and I'll go review that one
7122015-11-30T23:09:51 <jtimon> BlueMatt: awesome
7132015-11-30T23:12:16 <morcos> jtimon: i'm confused. i thought you only presented 1 way of removing the circular dependency
7142015-11-30T23:12:24 <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)
7152015-11-30T23:12:42 <morcos> calling pool->getMinFee() from inside CTxMemPool::estimateSmartFee
7162015-11-30T23:12:58 <jtimon> morcos: here's the other one: https://github.com/bitcoin/bitcoin/compare/master...jtimon:mempool-circular-dependency
7172015-11-30T23:13:21 <jtimon> and as said I can produce an infinite amount of them with enough time
7182015-11-30T23:13:59 <jtimon> but you guys are doing some incompatible requests
7192015-11-30T23:14:53 <sipa> i'm staying out of this now, but morcos's suggestion seems reasonable
7202015-11-30T23:15:09 <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
7212015-11-30T23:15:17 <morcos> sipa: but you still want to remove the circular dependency now?
7222015-11-30T23:15:33 *** go1111111 has joined #bitcoin-core-dev
7232015-11-30T23:15:39 <morcos> jtimon: as for which method of removing circular dependency of the two you present, i like the one in the PR better
7242015-11-30T23:15:49 <morcos> what i don't like in the PR is where you are setting that attribute
7252015-11-30T23:16:00 <morcos> can you change it to be lastTrimmedSize and set it in TrimToSize?
7262015-11-30T23:16:08 <jtimon> yeah in init is better like in the other option
7272015-11-30T23:16:36 <morcos> its not necessarily a fixed value, its the value that was last trimmed to, it should be set from TrimToSize
7282015-11-30T23:16:55 <morcos> if you want TrimToSize to call some global variable set in init, thats ok with me
7292015-11-30T23:17:10 <morcos> but the mempool attribute should be set by TrimToSize
7302015-11-30T23:17:13 <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)
7312015-11-30T23:17:24 <morcos> sigh.. why?
7322015-11-30T23:17:38 <morcos> its really only localized to GetMinFee
7332015-11-30T23:17:41 <morcos> why make it a globabl
7342015-11-30T23:17:47 <morcos> you're talking about two separate things
7352015-11-30T23:18:02 <morcos> the command line argument for maxmempool (fine make that a global)
7362015-11-30T23:18:36 <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)
7372015-11-30T23:19:04 <jtimon> I haven't looked much yet, but how disruptive it seems to move GetMinFee to the estimator?
7382015-11-30T23:19:13 <jtimon> then we can make it an attribute there
7392015-11-30T23:19:46 <morcos> i haven't looked either but i also thought of that
7402015-11-30T23:20:06 <jtimon> and the mempool can take the estimator as a parameter for its constructor instead of the minRelayfee
7412015-11-30T23:20:42 <morcos> lets just concentrate on what MUST be done for 0.12.
7422015-11-30T23:20:54 <morcos> having the mempool take the estimator as a parameter seems very weird to me
7432015-11-30T23:21:08 <morcos> why does the circular dependency have to go right now?
7442015-11-30T23:21:14 <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
7452015-11-30T23:21:30 <jtimon> morcos: again, because you introduced it right now
7462015-11-30T23:21:31 <morcos> jtimon: agree with that last statement, but too much for 0.12
7472015-11-30T23:21:41 <morcos> jtimon: but why is it a problem?
7482015-11-30T23:21:46 <sipa> jtimon: it seems very natural for the estimator to deoend on the mempool; why does that have to go?
7492015-11-30T23:22:08 <jtimon> if you hadn't introduced it unnecessarily there would be no hurry to remove it
7502015-11-30T23:22:22 <sipa> we can of course disconnect everything, and have a ton of glue code to connect things together
7512015-11-30T23:22:39 <jtimon> please let's discuss that properly when it's "needed" then, this is clearly not the case
7522015-11-30T23:23:23 <jtimon> my plan, as said multiple times, is to make them both completely independent from each other (but acceptToMempool depends on both)
7532015-11-30T23:23:30 <sipa> i dislike circular dependencies; they're a sign of badly separated code
7542015-11-30T23:24:20 <sipa> what must be done: i don't think it's worth so much discussion
7552015-11-30T23:24:22 <jtimon> if we introduce this unnecesssary dependency it will be harder to present my case later
7562015-11-30T23:24:35 <sipa> maybe i'm fine with just having a plan to fix it
7572015-11-30T23:24:39 <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
7582015-11-30T23:24:49 <jtimon> morcos: why make it dependent right now when it has been shown that is not necessary?
7592015-11-30T23:25:01 <sipa> jtimon: no dependency is ever "necessary"
7602015-11-30T23:25:08 <sipa> ever seen juice code?
7612015-11-30T23:25:17 <sipa> all dependencies resolved at runtime
7622015-11-30T23:25:22 <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
7632015-11-30T23:25:26 <sipa> that doesn't mean it's natural or readable
7642015-11-30T23:25:29 <jtimon> sipa: exactly
7652015-11-30T23:25:57 <jtimon> not seen juicy code but ack on no dependencies ever being necessary
7662015-11-30T23:26:12 <sipa> jtimon: but it sometimes comes at great cost in needing glue to bind things together
7672015-11-30T23:26:34 <sipa> the fact that a dependency is not necessary does not make it the best design
7682015-11-30T23:27:13 <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
7692015-11-30T23:27:27 <jtimon> sipa: agreed
7702015-11-30T23:27:37 <jtimon> so we disagree on what is the best design
7712015-11-30T23:27:55 <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
7722015-11-30T23:28:06 <sipa> anyway
7732015-11-30T23:28:18 <jtimon> leaving the circular dependency there would be already deciding in his favor, since more couplings will rapidly follow
7742015-11-30T23:28:25 <morcos> jtimon: but half of what we're arguing about is the stupid GetArg calls and has nothing to do with the circular dependency!
7752015-11-30T23:28:39 <sipa> jtimon: if the coupling is natural, of course it will follow!
7762015-11-30T23:28:52 *** zookolaptop has quit IRC
7772015-11-30T23:29:11 <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."
7782015-11-30T23:29:32 <sipa> i've said enough about this
7792015-11-30T23:29:56 <jtimon> that's what sipa and matt are arguing about, it's the less prioritary thing for me
7802015-11-30T23:30:11 <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
7812015-11-30T23:30:32 <morcos> leave all the GetArg's for now and we can celan those up separately
7822015-11-30T23:31:12 <jtimon> morcos: exactly we can argue later about the dependency (while, I hope, collaborating n removing the inverse dependency we both dislike)
7832015-11-30T23:31:52 <morcos> sounds like a plan
7842015-11-30T23:32:44 <morcos> i have to run now
7852015-11-30T23:33:52 <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)
7862015-11-30T23:37:54 <sipa> best is a global in init or mail, whose value is passed to the trim calls in ATMP :)
7872015-11-30T23:38:00 <sipa> main
7882015-11-30T23:39:41 <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
7892015-11-30T23:40:15 <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
7902015-11-30T23:40:16 <sipa> yup
7912015-11-30T23:42:14 <sipa> superficial ack
7922015-11-30T23:42:59 <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
7932015-11-30T23:45:43 <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
7942015-11-30T23:47:31 *** da2ce7 has quit IRC
7952015-11-30T23:47:40 <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
7962015-11-30T23:48:26 <jtimon> so, 1 minimal commit plus 2 squash-or-nack commits
7972015-11-30T23:48:46 *** da2ce7 has joined #bitcoin-core-dev
7982015-11-30T23:53:14 <Luke-Jr> phantomcircuit: Author: Patick Strateman <patrick.strateman@gmail.com>
7992015-11-30T23:53:15 <Luke-Jr> typo?
8002015-11-30T23:57:26 <sipa> Luke-Jr: no, twin brother
8012015-11-30T23:57:33 *** randy-waterhouse has joined #bitcoin-core-dev
8022015-11-30T23:58:27 <Luke-Jr> O.o