12017-07-17T00:10:41 *** fanquake has joined #bitcoin-core-dev
22017-07-17T00:10:44 *** rhavar has joined #bitcoin-core-dev
32017-07-17T00:10:54 *** fanquake has quit IRC
42017-07-17T00:12:56 *** jamesob has joined #bitcoin-core-dev
52017-07-17T00:13:32 *** arowser has joined #bitcoin-core-dev
62017-07-17T00:15:49 <sipa> gmaxwell: we likely want that to be dynamic, based on the chosen implementation
72017-07-17T00:16:47 <gmaxwell> I don't think always buffering two would hurt, other than a slight latency hit.
82017-07-17T00:17:00 <gmaxwell> 8 would be kinda nuts
92017-07-17T00:17:05 *** jamesob has quit IRC
102017-07-17T00:17:08 <sipa> well, we can benchmark
112017-07-17T00:19:44 <gmaxwell> I could be wrong about it mattering at all too. Though I don't think so.
122017-07-17T00:23:52 *** belcher has quit IRC
132017-07-17T00:25:28 *** belcher has joined #bitcoin-core-dev
142017-07-17T00:32:43 *** Chris_Stewart_5 has quit IRC
152017-07-17T00:35:38 *** arowser has quit IRC
162017-07-17T00:38:45 *** Murch has joined #bitcoin-core-dev
172017-07-17T00:40:26 *** Murch has quit IRC
182017-07-17T00:42:17 *** arowser has joined #bitcoin-core-dev
192017-07-17T00:56:09 *** arowser has quit IRC
202017-07-17T00:58:41 *** goatpig has quit IRC
212017-07-17T01:00:13 <sipa> hmm, i wonder why i think that sse 4.2 is needed for that asm code
222017-07-17T01:00:20 <sipa> it seems to only use sse 4.1 instructions
232017-07-17T01:00:28 <sipa> crc32 needs sse 4.2
242017-07-17T01:07:00 *** promag has quit IRC
252017-07-17T01:18:47 *** arowser has joined #bitcoin-core-dev
262017-07-17T01:21:51 *** Murch has joined #bitcoin-core-dev
272017-07-17T01:25:09 *** discreteunit has joined #bitcoin-core-dev
282017-07-17T01:28:01 *** chjj has quit IRC
292017-07-17T01:31:54 *** chjj has joined #bitcoin-core-dev
302017-07-17T01:34:45 *** discreteunit has quit IRC
312017-07-17T01:42:06 *** arowser has quit IRC
322017-07-17T01:49:48 *** Ylbam has quit IRC
332017-07-17T01:51:28 *** arowser has joined #bitcoin-core-dev
342017-07-17T02:05:25 *** arowser has quit IRC
352017-07-17T02:11:50 *** arowser has joined #bitcoin-core-dev
362017-07-17T02:14:11 *** jamesob has joined #bitcoin-core-dev
372017-07-17T02:18:43 *** jamesob has quit IRC
382017-07-17T02:26:39 <bitcoin-git> [bitcoin] MarcoFalke pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/3895e25a7736...bf0a08be281d
392017-07-17T02:26:40 <bitcoin-git> bitcoin/master ff7365e John Newbery: [tests] fix flake8 warnings in zapwallettxes.py
402017-07-17T02:26:40 <bitcoin-git> bitcoin/master e7a2181 John Newbery: [wallet] fix zapwallettxes interaction with persistent mempool...
412017-07-17T02:26:41 <bitcoin-git> bitcoin/master 4c3b538 John Newbery: [logs] fix zapwallettxes startup logs
422017-07-17T02:26:55 *** Chris_Stewart_5 has joined #bitcoin-core-dev
432017-07-17T02:26:56 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #10330: [wallet] fix zapwallettxes interaction with persistent mempool (master...zapwallettxes) https://github.com/bitcoin/bitcoin/pull/10330
442017-07-17T02:27:50 *** rhavar has quit IRC
452017-07-17T02:34:38 *** Chris_Stewart_5 has quit IRC
462017-07-17T02:40:02 *** handlex has joined #bitcoin-core-dev
472017-07-17T02:40:05 *** twistedline has quit IRC
482017-07-17T02:40:06 *** twistedline_ has joined #bitcoin-core-dev
492017-07-17T02:44:05 *** twistedline_ has quit IRC
502017-07-17T02:44:12 *** twistedline has joined #bitcoin-core-dev
512017-07-17T02:48:01 *** Chris_Stewart_5 has joined #bitcoin-core-dev
522017-07-17T02:48:22 *** twistedline has quit IRC
532017-07-17T02:52:34 *** twistedline has joined #bitcoin-core-dev
542017-07-17T03:08:23 *** jamesob has joined #bitcoin-core-dev
552017-07-17T03:08:39 *** handlex has quit IRC
562017-07-17T03:11:21 *** handlex has joined #bitcoin-core-dev
572017-07-17T03:17:48 *** Giszmo has quit IRC
582017-07-17T03:32:48 *** Giszmo has joined #bitcoin-core-dev
592017-07-17T03:40:32 *** Chris_Stewart_5 has quit IRC
602017-07-17T03:55:21 *** handlex has quit IRC
612017-07-17T04:18:07 <bitcoin-git> [bitcoin] jnewbery closed pull request #10802: [tests] skip zapwallettxes.py (master...skip_zapwallettxes) https://github.com/bitcoin/bitcoin/pull/10802
622017-07-17T04:25:39 <gmaxwell> sipa: you broke the sse4 thing by renaming incompletely. fixy fixy.
632017-07-17T04:37:14 <sipa> gmaxwell: fixed
642017-07-17T04:42:28 <cfields> jeez, qt 5.9 made a mess of _everything_
652017-07-17T04:44:05 <gmaxwell> What did they do
662017-07-17T04:46:54 <cfields> they're working on modularizing their source/build. but the result (so far) is just a new tangled mess of dependencies. And no working configs without patches
672017-07-17T04:46:57 *** JackH has quit IRC
682017-07-17T04:47:38 <cfields> want cocoa for osx? you'll need a printer of course. And obviously printers require opengl...
692017-07-17T04:47:49 <luke-jr> O.o
702017-07-17T04:47:50 <sipa> any reason why we'd really want 5.9?
712017-07-17T04:48:02 <sipa> cfields: they're 3D printers perhaps? :D
722017-07-17T04:48:44 <cfields> sipa: lol
732017-07-17T04:49:13 <cfields> sipa: ironically i was looking forward to 5.9 and its slimmer build
742017-07-17T04:49:15 <gmaxwell> they probably need opengl for 'lpad()'...
752017-07-17T04:50:00 <sipa> or for is_integer
762017-07-17T04:50:38 <cfields> but after messing with it all day, it's not offering many advantages so far
772017-07-17T04:51:00 *** Giszmo has quit IRC
782017-07-17T04:51:00 <cfields> (other than providing me a stack of patches to upstream)
792017-07-17T04:52:20 <cfields> so no, i don't think it makes sense to bump
802017-07-17T04:52:40 <cfields> as-is, we don't build with 5.9. But I haven't seen a bug report yet, so I'm assuming distros aren't shipping it
812017-07-17T04:52:57 <cfields> (and it'll be a pain to support once they do :( )
822017-07-17T04:53:44 <luke-jr> Qt used to care about backward compat :/
832017-07-17T04:54:02 <luke-jr> I noticed some stuff I have won't build with 5.7 because it requires C++11 >_<
842017-07-17T04:54:02 <cfields> luke-jr: i suspect all is fine if you use qmake
852017-07-17T04:54:12 <luke-jr> ah
862017-07-17T04:54:22 <cfields> yea, c++11 became a hard dep for 5.7 i believe
872017-07-17T04:54:25 <luke-jr> well, FWIW, Gentoo doesn't have 5.8 as even an option yet
882017-07-17T04:54:46 <cfields> but stl is still optional i think :p
892017-07-17T04:54:50 <luke-jr> hard dep makes sense; but 5.7 won't allow Qt programs to use non-C++11 :p
902017-07-17T04:55:26 <sipa> gcc 6 by default already compiles with c++14
912017-07-17T04:55:26 <luke-jr> anyhow, so I changed my USE flags to build that app for Qt4 <.<
922017-07-17T04:55:36 <cfields> mm, that's annoying. but kinda makes sense from a support standpoint
932017-07-17T05:05:39 *** Giszmo has joined #bitcoin-core-dev
942017-07-17T05:05:43 *** QBcrusher_ has joined #bitcoin-core-dev
952017-07-17T05:07:09 *** Murch has quit IRC
962017-07-17T05:08:17 *** QBcrusher has quit IRC
972017-07-17T05:23:58 *** CubicEarth has joined #bitcoin-core-dev
982017-07-17T05:28:19 *** paveljanik has quit IRC
992017-07-17T05:58:45 <wumpus> any reason to not just stick with our current qt for 0.15? that sounds like a nightmare, better let them sort out their problems upstream and not be a guinea pig
1002017-07-17T05:59:46 <wumpus> from a user PoV there seems to be no difference between different qt version for a long time now, as we basically only use the 4.x API
1012017-07-17T06:06:17 <luke-jr> they don't even have a Qt5.9 dance, so..
1022017-07-17T06:06:46 <wumpus> hehe
1032017-07-17T06:12:23 <wumpus> disabling -getinfo on the day of the feature freeze, really?
1042017-07-17T06:13:27 <luke-jr> meh, why not. there's an option to get it back
1052017-07-17T06:13:33 <wumpus> I don't get it, what is the sudden rush?
1062017-07-17T06:14:00 <wumpus> sure, but we could have done that 100 times during 0.15's release cycle
1072017-07-17T06:14:07 <wumpus> but somehow it has to be added in the last day
1082017-07-17T06:14:11 <gmaxwell> leeroy jenkins!
1092017-07-17T06:14:18 <luke-jr> I suppose it adds a new string
1102017-07-17T06:14:27 <luke-jr> (for the -help)
1112017-07-17T06:15:19 <gmaxwell> I think it should be done at the beginning of a cycle. It's bound to cause severe irritation for people who use it at the CLI and we will predictably get PRs to improve other things to compensate.
1122017-07-17T06:15:42 <wumpus> gmaxwell: heh indeed, in this way we don't even get used to it ourself
1132017-07-17T06:15:51 <wumpus> before exposing it to end users
1142017-07-17T06:15:53 *** CubicEarth has quit IRC
1152017-07-17T06:16:14 <wumpus> can't we just add a comment to the getinfo output instead?
1162017-07-17T06:16:22 <wumpus> Warning: this will be deprecated!
1172017-07-17T06:16:26 <gmaxwell> (or perhaps I'm alone in continuing to use getinfo myself? :) )
1182017-07-17T06:16:42 <wumpus> instead of the add a command line argument dance
1192017-07-17T06:16:54 <gmaxwell> "nag": "This RPC will be deprecated."
1202017-07-17T06:17:21 <wumpus> gmaxwell: I officially stopped using it at least, replaced it with a client-side script that shows more useful information
1212017-07-17T06:17:51 <luke-jr> wumpus: +1, maybe even put it in the warnings software already should look at?
1222017-07-17T06:18:06 <wumpus> getinfo's output is pretty meh, do you ever lok at anything besides the number of connections or the wallet balance?
1232017-07-17T06:18:34 <wumpus> luke-jr: not sure that's a good idea
1242017-07-17T06:18:44 <wumpus> luke-jr: it's not part of the node-wide warnings
1252017-07-17T06:19:29 <luke-jr> not sure why that'd be a reason not to do it
1262017-07-17T06:19:44 <wumpus> because it's a false alarm
1272017-07-17T06:19:51 <wumpus> it's not actually an alarm
1282017-07-17T06:20:07 <luke-jr> "your software is about to break" isn't? ;)
1292017-07-17T06:20:23 <gmaxwell> Connections, block count, balance, warings. And unfortunately, to use seperate rpcs to get this info is three rpcs (four?) and most of them each give a full screen full of information each. :)
1302017-07-17T06:20:55 <luke-jr> getblockcount and getbalance are scalars
1312017-07-17T06:21:27 <sipa> is there any rpc besides getinfo that actually lists the warning?
1322017-07-17T06:21:29 <wumpus> what the hell
1332017-07-17T06:21:40 <wumpus> BlueMatt concerned about atomicity in #8843?!
1342017-07-17T06:21:41 <gmaxwell> true, though I was using getblockchaininfo and getwalletinfo as replacements.
1352017-07-17T06:21:41 <gribble> https://github.com/bitcoin/bitcoin/issues/8843 | rpc: Handle `getinfo` client-side in bitcoin-cli w/ `-getinfo` by laanwj · Pull Request #8843 · bitcoin/bitcoin · GitHub
1362017-07-17T06:22:07 <wumpus> I don't...
1372017-07-17T06:22:18 <luke-jr> gmaxwell: "don't do that"? :p
1382017-07-17T06:22:23 <bitcoin-git> [bitcoin] laanwj closed pull request #8843: rpc: Handle `getinfo` client-side in bitcoin-cli w/ `-getinfo` (master...2016_09_getinfo_clientside) https://github.com/bitcoin/bitcoin/pull/8843
1392017-07-17T06:22:35 <wumpus> never mind, going to put this discussion on the backburner, there's more important things to worry about
1402017-07-17T06:22:40 <wumpus> what needs to get in today?
1412017-07-17T06:23:47 <luke-jr> BIP148, but I assume if the hold-outs on that had changed their mind, they'd say so :/ so probably nothing to do / discuss there
1422017-07-17T06:24:13 <wumpus> no no no no no
1432017-07-17T06:24:22 <sipa> #10831 would be nice, together with #10830 (which i hope can go in s a bugfix later)
1442017-07-17T06:24:23 <gribble> https://github.com/bitcoin/bitcoin/issues/10831 | Batch flushing operations to the walletdb during top up and increase keypool size. by gmaxwell · Pull Request #10831 · bitcoin/bitcoin · GitHub
1452017-07-17T06:24:24 <gribble> https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
1462017-07-17T06:26:30 <wumpus> I'll just maintain my own client-side info scripts, I'm not going to try to introduce convenient info functionality in bitcoin-cli anymore
1472017-07-17T06:26:38 <wumpus> this is just too bizarry
1482017-07-17T06:26:40 <achow101> re getinfo: can we add something to the errors field warning about it being removed soon
1492017-07-17T06:26:49 <luke-jr> wumpus: put it in contrib/?
1502017-07-17T06:26:57 <wumpus> luke-jr: no, people can write their own
1512017-07-17T06:27:21 <wumpus> luke-jr: would probably get 100 comments again from people that want something slightly differetn
1522017-07-17T06:27:30 <wumpus> luke-jr: just tired of it
1532017-07-17T06:27:31 <luke-jr> sigh
1542017-07-17T06:27:54 <gmaxwell> please don't add things to the errors field.
1552017-07-17T06:28:00 <wumpus> that's what I said
1562017-07-17T06:28:05 <wumpus> just add an extra 'nag' field or so
1572017-07-17T06:28:11 <gmaxwell> yes, that would be okay.
1582017-07-17T06:28:37 <wumpus> errors would be a horrible place to add it, because it's not a block chain error
1592017-07-17T06:28:43 <sipa> agree
1602017-07-17T06:29:08 <sipa> there are more urgent things
1612017-07-17T06:29:24 <sipa> #10706 should get review
1622017-07-17T06:29:25 <gribble> https://github.com/bitcoin/bitcoin/issues/10706 | Improve wallet fee logic and fix GUI bugs by morcos · Pull Request #10706 · bitcoin/bitcoin · GitHub
1632017-07-17T06:30:00 <sipa> and #10829 or #10650
1642017-07-17T06:30:02 <gribble> https://github.com/bitcoin/bitcoin/issues/10829 | Simple, backwards compatible RPC multiwallet support. by ryanofsky · Pull Request #10829 · bitcoin/bitcoin · GitHub
1652017-07-17T06:30:05 <gribble> https://github.com/bitcoin/bitcoin/issues/10650 | Multiwallet: add RPC endpoint support by jonasschnelli · Pull Request #10650 · bitcoin/bitcoin · GitHub
1662017-07-17T06:31:22 <gmaxwell> sipa: I was just finishing testing/reviewing 10706.
1672017-07-17T06:31:56 <bitcoin-git> [bitcoin] luke-jr closed pull request #10074: Block size/weight fraud proofs (master...sizefp) https://github.com/bitcoin/bitcoin/pull/10074
1682017-07-17T06:47:27 *** Ylbam has joined #bitcoin-core-dev
1692017-07-17T06:58:03 *** mmgen has joined #bitcoin-core-dev
1702017-07-17T07:10:24 <luke-jr> anyone have anything urgent / must be done before August I can help with for the next hour?
1712017-07-17T07:11:44 *** paveljanik has joined #bitcoin-core-dev
1722017-07-17T07:26:05 <bitcoin-git> [bitcoin] laanwj pushed 7 new commits to master: https://github.com/bitcoin/bitcoin/compare/bf0a08be281d...6859ad2936bf
1732017-07-17T07:26:06 <bitcoin-git> bitcoin/master ecd81df Alex Morcos: Make CoinControl a required argument to CreateTransaction
1742017-07-17T07:26:06 <bitcoin-git> bitcoin/master 03ee701 Alex Morcos: Refactor to use CoinControl in GetMinimumFee and FeeBumper...
1752017-07-17T07:26:07 <bitcoin-git> bitcoin/master 1983ca6 Alex Morcos: Use CoinControl to pass custom fee setting from QT....
1762017-07-17T07:26:28 <bitcoin-git> [bitcoin] laanwj closed pull request #10706: Improve wallet fee logic and fix GUI bugs (master...improveWalletFeeLogic) https://github.com/bitcoin/bitcoin/pull/10706
1772017-07-17T07:26:53 *** coredump_ has quit IRC
1782017-07-17T07:30:08 <wumpus> anyone opposed to adding #10829 as a last-ditch try to still have RPC multiwallet support in 0.15?
1792017-07-17T07:30:10 <gribble> https://github.com/bitcoin/bitcoin/issues/10829 | Simple, backwards compatible RPC multiwallet support. by ryanofsky · Pull Request #10829 · bitcoin/bitcoin · GitHub
1802017-07-17T07:30:46 <wumpus> it is simple to review at least
1812017-07-17T07:34:12 <gmaxwell> I support it.
1822017-07-17T07:39:16 *** Evel-Knievel has quit IRC
1832017-07-17T07:41:22 *** promag has joined #bitcoin-core-dev
1842017-07-17T07:41:52 *** promag1 has joined #bitcoin-core-dev
1852017-07-17T07:43:08 *** Walter2 has joined #bitcoin-core-dev
1862017-07-17T07:49:58 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6859ad2936bf...91edda8f3c81
1872017-07-17T07:49:58 <bitcoin-git> bitcoin/master 1cc251f Patrick Strateman: Explicitly search for bdb5.3....
1882017-07-17T07:49:59 <bitcoin-git> bitcoin/master 91edda8 Wladimir J. van der Laan: Merge #10803: Explicitly search for bdb5.3....
1892017-07-17T07:50:20 *** Evel-Knievel has joined #bitcoin-core-dev
1902017-07-17T07:50:25 <bitcoin-git> [bitcoin] laanwj closed pull request #10803: Explicitly search for bdb5.3. (master...2017-07-02-bdb5.3) https://github.com/bitcoin/bitcoin/pull/10803
1912017-07-17T07:52:10 *** BashCo has quit IRC
1922017-07-17T07:54:20 <gmaxwell> so on #10831 I'm ambivlent to the performance improvement; I created the PR because I understood that performance issue to be a blocker for increasing the default keypool size, which I think is really important to get out due to bad interactions with hd wallet and rescan.
1932017-07-17T07:54:21 <gribble> https://github.com/bitcoin/bitcoin/issues/10831 | Batch flushing operations to the walletdb during top up and increase keypool size. by gmaxwell · Pull Request #10831 · bitcoin/bitcoin · GitHub
1942017-07-17T07:55:21 <luke-jr> we should probably just do keypool in the background..?
1952017-07-17T07:55:49 <bitcoin-git> [bitcoin] practicalswift closed pull request #10847: Enable devirtualization opportunities by using the final specifier (C++11) (master...devirtualization) https://github.com/bitcoin/bitcoin/pull/10847
1962017-07-17T07:56:12 <wumpus> well, performance improvement or not, doing the keypool top-up in a single db transaction makes sense
1972017-07-17T07:56:54 <luke-jr> doing it in a single dbtx would necessitate blocking on it, though, no?
1982017-07-17T07:57:07 <sipa> ?
1992017-07-17T07:57:08 <luke-jr> at least before any of the keys can be used
2002017-07-17T07:57:08 <wumpus> especially on VMs with slow sync (most of them), it's super slow as it is right now
2012017-07-17T07:57:19 <wumpus> blocking on *what*? key generation is so fast
2022017-07-17T07:57:57 <sipa> if it weren't for db syncing, we can generate 10000 per second or so
2032017-07-17T07:58:04 <wumpus> the only reason it takes noticable time right now is because of the sync per key
2042017-07-17T07:58:08 <gmaxwell> luke-jr: the topup is already a single operation that holds the relevant locks the whole time.
2052017-07-17T07:58:09 <luke-jr> i c
2062017-07-17T07:58:31 <luke-jr> gmaxwell: yes, I was thinking we shouldn't do that, but otoh, if this is so much of a gain, maybe it doesn't matter
2072017-07-17T07:58:48 <wumpus> it's only done once anyhow!
2082017-07-17T07:58:49 <gmaxwell> sipa: well not quite that fast, the code paths it goes through per key are very twisty and inefficient, and we aren't multithreaded for it, and we do validate after create. :)
2092017-07-17T07:59:01 <wumpus> (at least, so much at once)
2102017-07-17T07:59:03 <gmaxwell> but thousands per second sure.
2112017-07-17T08:00:00 <wumpus> keypool generation happens a lot during testing though, so #10831 is going to speed up testing
2122017-07-17T08:00:01 <gribble> https://github.com/bitcoin/bitcoin/issues/10831 | Batch flushing operations to the walletdb during top up and increase keypool size. by gmaxwell · Pull Request #10831 · bitcoin/bitcoin · GitHub
2132017-07-17T08:00:44 <wumpus> (euh.. it would at least if if the number of keys generated stayed the same)
2142017-07-17T08:00:48 <gmaxwell> luke-jr: if someone really cared, they could make topup run in batches of no more than X to reduce worst case blocking, it would be a couple lines improvement probably.
2152017-07-17T08:01:02 <gmaxwell> wumpus: it'll be faster even with the 1000 size increase, esp on things with slow IO.
2162017-07-17T08:01:13 <wumpus> ah yes it's 1000, yes srue
2172017-07-17T08:01:27 <wumpus> does it still log a line for every key?
2182017-07-17T08:01:32 <wumpus> probably want to get rid of that too
2192017-07-17T08:01:33 <luke-jr> well, if it's so fast, it doesn't matter
2202017-07-17T08:01:39 <gmaxwell> yes.
2212017-07-17T08:01:46 <luke-jr> "added N keys to keypool" âº
2222017-07-17T08:02:05 *** promag1 has quit IRC
2232017-07-17T08:03:55 <gmaxwell> instagibbs reported for 1000 keys (new default) on his presumably SSD equipmt system it went from 20 second to 1.6. so I expect it's still a 20% speedup. We could in the future make many of the tests turn their keypool sizes down to speed them up further.
2242017-07-17T08:05:17 <gmaxwell> it would be a trivial (2 loc-ish) change to make it log only one line in the topup.
2252017-07-17T08:06:08 <wumpus> should certainly do that, I've always found the per-key message annoying during first run, and it's 10 times as annoying now :)
2262017-07-17T08:06:13 <gmaxwell> as in, remove the current line, re-add it two below using the missingInternal missingExternal variables as the count.
2272017-07-17T08:06:24 <gmaxwell> okay, should I just add another commit that does that to my PR?
2282017-07-17T08:06:29 <sipa> ack
2292017-07-17T08:06:56 <wumpus> remove the current line, or move it to debug category if someone theoretically could be interested while troubleshooting
2302017-07-17T08:07:25 <gmaxwell> I can't see the reason, it conveys no useful information that the summary wouldn't have.
2312017-07-17T08:07:40 <wumpus> agree
2322017-07-17T08:08:43 <gmaxwell> and yes, the old one annoyed me too. though leveldb logging with debug on has made me blind to log annoyances... (funny, I did that nice bitfield thing and I don't use it to deactivate leveldb)
2332017-07-17T08:09:35 <wumpus> heh yes that's kind of what it was meant for
2342017-07-17T08:10:30 <wumpus> though now that it's possible to enable/disable debug bits at runtime, I don't enable any by default anymore
2352017-07-17T08:11:01 <wumpus> well, bench. That one is nice.
2362017-07-17T08:11:02 *** Walter2 has quit IRC
2372017-07-17T08:17:29 <gmaxwell> pushed, not yet tested because it'll take me a half hour to compile it.
2382017-07-17T08:19:41 *** BashCo has joined #bitcoin-core-dev
2392017-07-17T08:23:48 <wumpus> I can test np
2402017-07-17T08:24:56 <gmaxwell> I will test, just... when the build finishes.
2412017-07-17T08:25:29 <wumpus> half an hour for a single-line change, did you change to rpi for builds? :-)
2422017-07-17T08:26:47 <gmaxwell> wumpus: my laptop is slow, and I've switched trees, so its recompiling all the things.
2432017-07-17T08:34:47 *** timothy has joined #bitcoin-core-dev
2442017-07-17T08:41:36 <wumpus> ccache can help with that a lot
2452017-07-17T08:43:22 <gmaxwell> 2017-07-17 08:42:25 keypool added 2000 keys (1000 internal), size=2000 (1000 internal)
2462017-07-17T08:43:25 <wumpus> though, you need to set the cache size high to survive tree switches
2472017-07-17T08:43:41 <gmaxwell> yes, and with a full blockchain on a 256GB disk, I can't do that. :(
2482017-07-17T08:44:13 <gmaxwell> okay, only took me 25 minutes.
2492017-07-17T08:44:22 * wumpus wonders how well lzo compression fs will work for ccache objects
2502017-07-17T08:44:47 *** harrymm has quit IRC
2512017-07-17T08:45:18 <wumpus> 2017-07-17 08:45:03 Performing wallet upgrade to 60000
2522017-07-17T08:45:18 <wumpus> 2017-07-17 08:45:05 keypool added 2000 keys (1000 internal), size=2000 (1000 internal)
2532017-07-17T08:45:21 <wumpus> two seconds woohoo
2542017-07-17T08:45:56 <gmaxwell> yea, thats still pretty slow considering how fast the underlying crypto is... but fast enough.
2552017-07-17T08:52:14 <wumpus> well it's short enough not to be noticable in the overall process, could always be optimzied further if anyone cares
2562017-07-17T08:52:46 <gmaxwell> if we later change it to 10k it might be worth it.
2572017-07-17T08:53:20 <wumpus> 184s for 10k keys here, yea that's definitely too slow
2582017-07-17T08:53:33 <wumpus> sorry - 100k keys
2592017-07-17T08:54:06 <wumpus> but 18.4 seconds would also be too slow
2602017-07-17T08:54:46 <gmaxwell> it could also be broken up and run in the background, I didn't push for 10k for a different reason: bloats up the wallet currently.
2612017-07-17T08:55:01 <gmaxwell> esp if you use encryption and immediately mark all the keys you just generated as used.
2622017-07-17T08:55:01 <wumpus> then again you had a good point about the encryption
2632017-07-17T08:55:04 <wumpus> yes
2642017-07-17T08:55:42 <gmaxwell> but we can fix this later by not even creating the wallet until you do something like hit getnewaddress or do encryption.
2652017-07-17T08:55:51 <wumpus> there could be some kind of shortcut - if you *never* dealt out a key before encrypting
2662017-07-17T08:56:05 <wumpus> then it could just start over with an encrypted wallet
2672017-07-17T08:56:24 <gmaxwell> or that.. and also, by using smaller records for keypool entries.
2682017-07-17T08:56:38 <wumpus> for multiwallet it'd also be nice to be able to create wallets in encrypted mode
2692017-07-17T08:57:26 <wumpus> yes
2702017-07-17T08:57:58 *** promag has joined #bitcoin-core-dev
2712017-07-17T08:59:56 *** harrymm has joined #bitcoin-core-dev
2722017-07-17T09:05:55 <gmaxwell> I think we should generally consider defered init of the wallet because it would allow us to better integrate mandatory backup/recovery.
2732017-07-17T09:07:07 <gmaxwell> e.g. in the GUI at least you don't do a wallet unless you complete a series of slightly paternalistic steps that make it likely you actually got some kind of backup.
2742017-07-17T09:07:53 <gmaxwell> so: starting with no wallet, and any effort to get an address prompts you to make one, along with a backup of some form.
2752017-07-17T09:28:58 *** ivan has quit IRC
2762017-07-17T09:53:02 *** Dyaheon has quit IRC
2772017-07-17T09:54:15 *** Dyaheon has joined #bitcoin-core-dev
2782017-07-17T10:08:17 <morcos> wumpus thanks for merging 10706. The last thing I want to nag about is #10707. It hasn't gotten much review b/c it was dependent on 10706, but its not big. I think its important in terms of finalizing estimatesmartfee API now that it is no longer marked unstable. Not sure if you feel the cutoff for it is today.
2792017-07-17T10:08:19 <gribble> https://github.com/bitcoin/bitcoin/issues/10707 | Better API for estimatesmartfee RPC by morcos · Pull Request #10707 · bitcoin/bitcoin · GitHub
2802017-07-17T10:09:17 <morcos> #10817 would be nice (and isn't yet tagged 0.15) but isn't critical, and so far I haven't heard anyone other than gmaxwell and I's opinion on it.
2812017-07-17T10:09:18 <gribble> https://github.com/bitcoin/bitcoin/issues/10817 | Add a discard_rate to avoid small change by morcos · Pull Request #10817 · bitcoin/bitcoin · GitHub
2822017-07-17T10:14:48 <bitcoin-git> [bitcoin] jonasschnelli opened pull request #10849: Multiwallet: simplest endpoint support (master...2017/07/mw_endpoint_simple) https://github.com/bitcoin/bitcoin/pull/10849
2832017-07-17T10:18:14 <gmaxwell> morcos: I would not for 10817 that the definition used there is a bit different then the one were thinking of using for the exact matching hurestic, because the dust threshold has a *3 in it.
2842017-07-17T10:19:12 <gmaxwell> I think thats okay, but its slightly less of a no-brainer than the factor of one hurestic (where the output is worthless at the long term feerate).
2852017-07-17T10:24:36 <morcos> gmaxwell: Yeah I argued a release ago we should remove the *3 from the definition of dust and correspondingly change the dust rate to 3 sat/B . It would make more sense to talk about everything like that.
2862017-07-17T10:25:33 <gmaxwell> morcos: I would agree with that change, makes a lot of sense.
2872017-07-17T10:25:59 <gmaxwell> if you make a commit that does that, fix the comment in IsDust which just seems to be inexplicable.
2882017-07-17T10:26:29 <gmaxwell> (there is no integer you can multiply 148 by to get 546)
2892017-07-17T10:26:34 <morcos> still want to try to do that for 0.15? or forget it for now
2902017-07-17T10:26:58 <morcos> heh!
2912017-07-17T10:27:12 <gmaxwell> Well I think you should put it in 10817, other than the 10817 behavior it's a no-op
2922017-07-17T10:27:13 <morcos> not forget, but postpone
2932017-07-17T10:27:25 <morcos> ok
2942017-07-17T10:27:52 <gmaxwell> I'd like to get it in 0.15, but wladimir's call. I think the PR would be better with it, esp as it makes it even a more benign change.
2952017-07-17T10:28:34 <gmaxwell> (also, I don't really think we have any other utxo bloat reducers in this release; gotta meet quota)
2962017-07-17T10:28:48 <morcos> But then you would make the discard rate still 5 sat/B or you'd make it higher
2972017-07-17T10:28:57 <gmaxwell> morcos: the 3x in there was due to the original somewhat stupid formulation that tied it to minimum relay fee.
2982017-07-17T10:29:05 <morcos> in the PR I gave the long term fee estimation rate can only serve to LOWER the discard rate
2992017-07-17T10:29:08 <morcos> it is not a max
3002017-07-17T10:29:34 <morcos> I think it is dangerous to use the existing long term fee estimation rate as a discard rate
3012017-07-17T10:29:41 <morcos> it got as high as 100 sat/B a couple months ago
3022017-07-17T10:30:16 <gmaxwell> Could be argued that leaving it at 5 is just more conservative and better than not having the feature.
3032017-07-17T10:31:13 <gmaxwell> 5 means the threshold is about at 1.5 cents at the current prices.
3042017-07-17T10:31:44 <morcos> it seems not worth the complication to increase the discard rate from the preexisting 3 sat/B b/c of dust to 5
3052017-07-17T10:31:49 <morcos> from 3 to 15 then yes
3062017-07-17T10:32:37 <morcos> and i think the fact that it gets MIN'ed with long term fee estimation makes me not worry too much that the 15 will end up biting people in the ass later. (also it's configurable)
3072017-07-17T10:33:05 <gmaxwell> Yes, that also sounds fine to me.
3082017-07-17T10:33:38 <morcos> only other question is it's a bit dicey to change the definition of a command line argument like dustrelayfee, but in this case it's a hidden argument that i doubt anyone specified, so its probably ok.
3092017-07-17T10:33:51 <morcos> but thats more reason to do it now while it's only been out for one release
3102017-07-17T10:34:48 <gmaxwell> it's also good to get this in because the exact matcher code should signficantly increase the number of cases where this happens, so if people are gonna squak about the wallet throwing away a few more cents to avoid change it'll be good to hear about it.
3112017-07-17T10:35:18 <morcos> gmaxwell: 546 = 3 * (148 + 34)
3122017-07-17T10:35:22 *** Guyver2 has joined #bitcoin-core-dev
3132017-07-17T10:36:17 <gmaxwell> ah thats where it comes from; it's including the txout itself.
3142017-07-17T10:40:26 *** AaronvanW has joined #bitcoin-core-dev
3152017-07-17T10:43:27 *** promag has quit IRC
3162017-07-17T10:43:39 *** goatpig has joined #bitcoin-core-dev
3172017-07-17T10:47:05 *** rockhouse has quit IRC
3182017-07-17T10:52:48 *** promag has joined #bitcoin-core-dev
3192017-07-17T11:06:29 *** Lorena has joined #bitcoin-core-dev
3202017-07-17T11:12:13 *** BashCo has quit IRC
3212017-07-17T11:12:49 *** BashCo has joined #bitcoin-core-dev
3222017-07-17T11:15:53 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/91edda8f3c81...8bc6d1f179a0
3232017-07-17T11:15:53 <bitcoin-git> bitcoin/master a8ae0b2 Dag Robole: Fix resource leak
3242017-07-17T11:15:54 <bitcoin-git> bitcoin/master 8bc6d1f Wladimir J. van der Laan: Merge #10837: Fix resource leak on error in GetDevURandom...
3252017-07-17T11:16:33 <bitcoin-git> [bitcoin] laanwj closed pull request #10837: Fix resource leak on error in GetDevURandom (master...20170715-fix-leak-1) https://github.com/bitcoin/bitcoin/pull/10837
3262017-07-17T11:20:39 <morcos> gmaxwell: ok updated 10817 and reduced the discard rate from 15 to 10
3272017-07-17T11:39:05 *** harrymm has quit IRC
3282017-07-17T11:49:51 <luke-jr> wumpus: hmm, what about ;wallet=abc (path segment parameter)?
3292017-07-17T11:49:58 <wumpus> :-(
3302017-07-17T11:50:22 * luke-jr wonders if anything actually uses that part of the spec
3312017-07-17T11:50:22 <wumpus> let's pile up even more alternatives
3322017-07-17T11:50:33 <wumpus> heh I wonder too
3332017-07-17T11:50:38 <luke-jr> wumpus: well, it doesn't have the issue you were concerned with
3342017-07-17T11:50:58 <wumpus> maybe we should do something really obscure so that at least no one is happy with it either :p
3352017-07-17T11:51:23 <luke-jr> custom HTTP header?
3362017-07-17T11:51:34 <wumpus> :-)
3372017-07-17T11:51:49 <luke-jr> although that's not so obscure :/
3382017-07-17T11:51:56 <wumpus> or a custom field in the JSON-RPC outer structure
3392017-07-17T11:52:03 <luke-jr> let's encode it in the Accept header!
3402017-07-17T11:52:17 <wumpus> or in the user agent
3412017-07-17T11:52:24 <luke-jr> lol
3422017-07-17T11:52:25 <jonasschnelli> :/
3432017-07-17T11:52:42 <jonasschnelli> lets use sessions and states ... *duck*
3442017-07-17T11:52:52 <luke-jr> oooh perfect
3452017-07-17T11:53:08 <luke-jr> that would actually be useful for other stuff too
3462017-07-17T11:53:36 <luke-jr> historically anyway, maybe not post-named params
3472017-07-17T11:53:41 <jonasschnelli> Yes. Would be useful to lose all your coins.. :)
3482017-07-17T11:53:42 <jonasschnelli> *loose
3492017-07-17T11:53:47 <luke-jr> (I'm thinking settxfee)
3502017-07-17T11:53:59 <wumpus> how would that lose coins - store private keys in a session identifier?
3512017-07-17T11:54:19 <luke-jr> sounds like bc.i
3522017-07-17T11:54:36 <wumpus> "sorry, your session expired" but isntead of losing that long mail, you lose all your coins
3532017-07-17T11:54:38 <jonasschnelli> You may loose them because your session with "the other" wallet is still active...
3542017-07-17T11:54:48 <jonasschnelli> Though loosing may not directly be possible..
3552017-07-17T11:54:52 <jonasschnelli> But sessions is the worst for APIS
3562017-07-17T11:55:30 <wumpus> I'm sure someone somewhere thinks it's a good idea
3572017-07-17T11:55:55 <jonasschnelli> hehe
3582017-07-17T11:55:57 <jonasschnelli> Me too
3592017-07-17T11:56:06 <luke-jr> Coinbase
3602017-07-17T11:56:23 <jonasschnelli> Does their API work keep state in sessions?
3612017-07-17T11:56:25 <luke-jr> OAuth2 is kinda inherently session-based tho.
3622017-07-17T11:56:39 <luke-jr> not state, I think
3632017-07-17T11:56:43 <jonasschnelli> For Auth, header fields are better IMO.
3642017-07-17T11:56:58 *** harrymm has joined #bitcoin-core-dev
3652017-07-17T11:57:02 <jonasschnelli> Bitpays stuff is pretty nice (ECDSA auth via signature of post in header)
3662017-07-17T11:57:06 <luke-jr> couldn't do session-less for OAuth2's use case
3672017-07-17T11:57:22 <jonasschnelli> Just sign each request
3682017-07-17T11:57:28 <luke-jr> because the user is authenticating, and the app is just accessing that session
3692017-07-17T11:57:41 <jonasschnelli> And add an upcounting nonce
3702017-07-17T11:57:59 <luke-jr> jonasschnelli: the authenticator is not the API-accessor
3712017-07-17T11:58:13 <jonasschnelli> OAuth, yeah
3722017-07-17T12:01:12 <gmaxwell> the bitpay auth scheme is insecure. IIRC, has not anti-replay nonce
3732017-07-17T12:02:08 <gmaxwell> s/not/no/
3742017-07-17T12:03:58 <morcos> settxfee needs to go away
3752017-07-17T12:04:17 <morcos> we're getting close to being able to just do it on a per rpc call
3762017-07-17T12:04:25 <jonasschnelli> gmaxwell: Indeed
3772017-07-17T12:04:34 <luke-jr> morcos: probably replacing it with sessions isn't the way though :p
3782017-07-17T12:06:34 *** Ch4rlesM4ck4y has joined #bitcoin-core-dev
3792017-07-17T12:08:06 *** Ch4rlesM4ck4y has left #bitcoin-core-dev
3802017-07-17T12:34:28 *** blznblzn2 has joined #bitcoin-core-dev
3812017-07-17T12:46:33 *** RoyceX has joined #bitcoin-core-dev
3822017-07-17T12:50:01 *** cheese_ has quit IRC
3832017-07-17T12:51:54 *** blznblzn2 has quit IRC
3842017-07-17T13:06:06 <wumpus> <morcos> settxfee needs to go away <- yes please
3852017-07-17T13:06:28 <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/8bc6d1f179a0...2b0179d8a9b7
3862017-07-17T13:06:29 <bitcoin-git> bitcoin/master e061d8d practicalswift: Remove declaration of unused function: void UpdatedTransaction(const uint256 &)
3872017-07-17T13:06:29 <bitcoin-git> bitcoin/master 2b0179d MarcoFalke: Merge #10834: Remove declaration of unused method: void UpdatedTransaction(const uint256 &)...
3882017-07-17T13:06:58 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #10834: Remove declaration of unused method: void UpdatedTransaction(const uint256 &) (master...remove-UpdatedTransaction) https://github.com/bitcoin/bitcoin/pull/10834
3892017-07-17T13:07:35 <wumpus> happy that we're moving to an API where all the important things can be set per transaction, so that different callers don't have to worry about messing up each other's global state
3902017-07-17T13:11:21 <promag> wumpus: remove https://github.com/bitcoin/bitcoin/pull/10650 from project 8?
3912017-07-17T13:12:31 <wumpus> only 3 things in project 8 anyhow - maybe we should clean it out for now, and use the 0.15 milestone list instead
3922017-07-17T13:12:42 <wumpus> (it's what I'm doing at least)
3932017-07-17T13:13:02 <wumpus> (one thing now)
3942017-07-17T13:13:39 *** CubicEarth has joined #bitcoin-core-dev
3952017-07-17T13:28:31 *** Dyaheon has quit IRC
3962017-07-17T13:31:30 <jonasschnelli> CodeShark: have you started to work on client side filtering after roasbeef's specs?
3972017-07-17T13:31:33 *** Dyaheon has joined #bitcoin-core-dev
3982017-07-17T13:39:19 *** CubicEarth has quit IRC
3992017-07-17T13:42:12 <jnewbery> Chris_Stewart_5: to get all blocks and transactions to propagate, try BitcoinTestFramework.sync_all()
4002017-07-17T13:42:30 <jnewbery> wumpus gmaxwell: for getinfo, "nag": "This RPC will be deprecated." will be silently dropped by any clients that are picking individual fields from getinfo.
4012017-07-17T13:42:33 <jnewbery> Hiding getinfo behind a flag for one version *forces* users to acknowledge that the RPC is being deprecated (and gives them one release grace period to fix their scripts/whatever that is using getinfo)
4022017-07-17T13:42:56 <jnewbery> wumpus: > keypool generation happens a lot during testing though
4032017-07-17T13:42:57 <jnewbery> unit testing or functional testing? Default keypool size in the functional tests is 1 (https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/test_framework.py#L212)
4042017-07-17T13:44:42 <wumpus> jnewbery: didn't know that the keypool size was overridden for testing, so in that case this doesn't help testing become faster, well too bad :)
4052017-07-17T13:45:02 <wumpus> jnewbery: yes, we can do the other thing for 0.16, just too late to make such a change for 0.15
4062017-07-17T13:45:14 <wumpus> jnewbery: I honestly don't understand why this comes up on the last day before feature freeze
4072017-07-17T13:50:35 <jnewbery> I agree, it's late. But hiding getinfo behind a flag in v0.16 => removing in 0.17. getinfo is one of the bitcoin_server -> bitcoin_wallet dependencies. I was hoping we'd be able to remove all of those for 0.16
4082017-07-17T13:53:10 <wumpus> sure, but couldn't you have brought it up a few weeks earlier?
4092017-07-17T13:53:58 <gmaxwell> I'd still complain that we haven't dogfooded it enough.
4102017-07-17T13:55:16 <wumpus> we could just remove the wallet balance from there
4112017-07-17T13:55:29 <gmaxwell> I wouldn't complain about that.
4122017-07-17T13:55:33 <wumpus> it makes no sense with multiwallet anyway
4132017-07-17T13:56:19 <wumpus> then again, that doesn't need to be done last-minute for 0.15, the circular dependency won't be solved before then anyhow
4142017-07-17T13:56:42 <wumpus> but let's say it's the last thing holding back solving the build dependency, we could do that
4152017-07-17T13:57:19 <wumpus> I still think adding the nag: makes sense, most people using getinfo will likely use it from the command line
4162017-07-17T13:57:38 <gmaxwell> even if they don't use it always that way, it's more visible than even release notes.
4172017-07-17T13:57:49 <wumpus> yes it would be in addition to release notes, not instead
4182017-07-17T13:57:50 <gmaxwell> and we've mentioned it in release notes before (I think, I hope)
4192017-07-17T13:57:55 <wumpus> yes, we did
4202017-07-17T13:58:12 <wumpus> even with the new places where everything could be found
4212017-07-17T13:58:36 <wumpus> https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.14.0.md#getinfo-deprecated
4222017-07-17T13:59:55 <wumpus> I don't know why we waited for 0.14 before doing that
4232017-07-17T14:04:28 <BlueMatt> yeesh y'all been busy this morning
4242017-07-17T14:04:37 <wumpus> anyhow as it was deprecated in 0.14 (the deprecation message in the help was even backported to 0.13.1), removing it completely in 0.16 should be fine
4252017-07-17T14:04:55 <BlueMatt> wumpus: I mean its not a strong concern, really, I pointed it out cause it would be a problem for automated users (which I hope are mostly not calling -cli)
4262017-07-17T14:05:51 <wumpus> BlueMatt: yes, the -cli change would mostly be for command line users, but now agree we shouldn't do that, external stats/info scripts should be external
4272017-07-17T14:06:27 <BlueMatt> well i was more curious to find out what the demand for it was (re: the "IRC discussion" referenced in that pr)
4282017-07-17T14:06:29 <wumpus> and -cli should be a minimal, dumb client utiltiy
4292017-07-17T14:06:37 <gmaxwell> I think we'll have to do ~something~ but the best way to find that and get it done is just to live with it for a while.
4302017-07-17T14:08:25 <wumpus> maybe some example of a stats script? I don't know - generally, those things have gone unmaintained fast, as they're personal, everyone is interested in different info
4312017-07-17T14:08:48 *** CubicEarth has joined #bitcoin-core-dev
4322017-07-17T14:09:32 <wumpus> there is also such a low threshold to writing them
4332017-07-17T14:10:13 <gmaxwell> connection count + height + errors is pretty universal; and is a bit obnoxious from the cli in our post getinfo world.
4342017-07-17T14:11:08 <jnewbery> > couldn't you have brought it up a few weeks earlier?
4352017-07-17T14:11:22 <wumpus> well for example personally I'm not interested in totai connection count - my script shows in/out versus ipv4/ipv6/tor
4362017-07-17T14:11:48 <jnewbery> I brought it up in response to #10841. I thought it would be friendlier to hide the RPC behind a flag before removing it one release later
4372017-07-17T14:11:50 <gribble> https://github.com/bitcoin/bitcoin/issues/10841 | [rpc] Give users one final warning before removing getinfo by jnewbery · Pull Request #10841 · bitcoin/bitcoin · GitHub
4382017-07-17T14:12:07 <wumpus> this is much more useful to troubleshoot any connectivity issues than the total number
4392017-07-17T14:12:18 <jnewbery> sorry, in response to #10838
4402017-07-17T14:12:19 <gribble> https://github.com/bitcoin/bitcoin/issues/10838 | (finally) remove the longest-ever-deprecated RPC call getinfo by TheBlueMatt · Pull Request #10838 · bitcoin/bitcoin · GitHub
4412017-07-17T14:12:36 <gmaxwell> wumpus: the normal thing I'm interested in is 0/8/many. seperate in/out counts would be interesting. From a cli perspective there are a number of other interesting things to show; e.g. best block hash, without getting into those huge info dumps like getblockchaininfo.
4422017-07-17T14:12:37 * BlueMatt has 7 4k monitors full of stats monitoring scripts...anyone need some?
4432017-07-17T14:12:55 <wumpus> jnewbery: sure! BlueMatt was the original wtf 'why do you bring this up now', you tried to do a friendlier version, which is why I tagged that one
4442017-07-17T14:12:57 <jnewbery> If we're going to remove getinfo in v0.16, I suggest I close #10841
4452017-07-17T14:12:58 <gribble> https://github.com/bitcoin/bitcoin/issues/10841 | [rpc] Give users one final warning before removing getinfo by jnewbery · Pull Request #10841 · bitcoin/bitcoin · GitHub
4462017-07-17T14:13:18 <wumpus> jnewbery: but I stil think it is too much for such a late change
4472017-07-17T14:13:48 <wumpus> gmaxwell: exactly, none of which getinfo currently does, it's been frozen for years (on purpose)
4482017-07-17T14:14:00 * BlueMatt had assumed it had been longer-deprecated, so figured it could just be removed, when it was pointed out that it was only deprecated in 14 (damn it) i withdrew, but making the deprecation more explicit is nice
4492017-07-17T14:14:10 <BlueMatt> i guess it doesnt matter much, though
4502017-07-17T14:14:25 <jnewbery> Yeah, doesn't matter too much. I'll close 10841.
4512017-07-17T14:14:30 <wumpus> we should have mentioned it in the release notes much earlier
4522017-07-17T14:14:48 <wumpus> jnewbery: I still think it makes sense to add a 'nag': to the result with your message, but anyhow
4532017-07-17T14:14:48 <gmaxwell> wumpus: I would have log ago proposed a 'status' rpc, but feared that the response would be "omg not another getinfo!"
4542017-07-17T14:15:10 <wumpus> gmaxwell: yes we're definitely not going to have any new server side call like that, that spans subsystems
4552017-07-17T14:15:22 <jnewbery> yeah, adding a "nag" field does no harm
4562017-07-17T14:15:37 <wumpus> client-side would have been ok with me, but runs into too much opposition, so we';re not going to do that either
4572017-07-17T14:15:45 <bitcoin-git> [bitcoin] jnewbery closed pull request #10841: [rpc] Give users one final warning before removing getinfo (master...deprecate_getinfo) https://github.com/bitcoin/bitcoin/pull/10841
4582017-07-17T14:16:47 <gmaxwell> wumpus: if it didn't span into the wallet, but was just blockchain stats and p2p stats?
4592017-07-17T14:17:33 <wumpus> gmaxwell: I don't think that's a good idea, no
4602017-07-17T14:17:41 <gmaxwell> I figured.
4612017-07-17T14:17:56 <wumpus> could potentially create another dependency in the future that will be so hard to get rid of
4622017-07-17T14:18:15 <wumpus> if getting rid of getinfo would have been easier, I would have thought differently about it, but this keeps haunting us forever
4632017-07-17T14:18:34 <gmaxwell> well, we have to do something. I don't pretend to know what. But having to run multiple commands just to get the most basic of health information, is a real burden for support.
4642017-07-17T14:18:58 <BlueMatt> can we call #10526 a fix for a performance regression (really, disk space usage regression) and tag it 15 and merge it later this week?
4652017-07-17T14:18:59 <gribble> https://github.com/bitcoin/bitcoin/issues/10526 | Force on-the-fly compaction during pertxout upgrade by sipa · Pull Request #10526 · bitcoin/bitcoin · GitHub
4662017-07-17T14:19:04 <wumpus> then do something: write a useful script taht collects that information, submit it as PR
4672017-07-17T14:19:06 <wumpus> I TRIED
4682017-07-17T14:19:19 <gmaxwell> I think we haven't gotten rid of it because we haven't replaced it for anything but programmatic users who care little about how many calls they make or how many pages of uninteresting data they toss.
4692017-07-17T14:19:31 <wumpus> I just mistakingly made it part of -cli
4702017-07-17T14:19:49 <gmaxwell> wumpus: I know. Though I admit I have mixed feelings about cli being anything but a dumb client too. I am not yelling at you.
4712017-07-17T14:19:57 <gmaxwell> you did great.
4722017-07-17T14:20:22 <gmaxwell> BlueMatt: I believe we were waiting on feedback from wumpus to determine if there actually was a need there.
4732017-07-17T14:20:38 <wumpus> things that don't have to be done server side don't need to be done server side, this is the same reason I argue against #10804
4742017-07-17T14:20:40 <gribble> https://github.com/bitcoin/bitcoin/issues/10804 | Add histunspent RPC by promag · Pull Request #10804 · bitcoin/bitcoin · GitHub
4752017-07-17T14:20:53 <wumpus> client side has the information, so can roll the statistics and display them in any way
4762017-07-17T14:21:01 <gmaxwell> yes, I also saw 10804 and thought that.
4772017-07-17T14:21:02 <wumpus> the RPC interface doesn't have to acoomodate end users specifics
4782017-07-17T14:21:27 <gmaxwell> then our issue is that we just don't have a cli interface, except we have a highly used cli interface.
4792017-07-17T14:21:30 <gmaxwell> :)
4802017-07-17T14:21:56 <gmaxwell> BlueMatt: there was some question about the state of the node in question where he saw the not good results.
4812017-07-17T14:21:59 <wumpus> the same problem we had in 2012, and have had extensive discussions about since then, and no one is making that tool it seems
4822017-07-17T14:22:14 <wumpus> which I understand, I'm certainly not going to do it, too many opinions about it, I only write simple utility scripts for myself :)
4832017-07-17T14:22:18 <BlueMatt> wumpus: hmm, ok, now I understand the desire, thanks, yea thanks for making that pr. would be nice to just make it a bash script, but sadly fucking json :(
4842017-07-17T14:23:10 <wumpus> BlueMatt: it's trivial in python though
4852017-07-17T14:23:20 <gmaxwell> rename bitcoin-cli bitcoin-rpc and we create a real bitcoin-cli ? :P
4862017-07-17T14:23:21 <BlueMatt> true
4872017-07-17T14:23:24 <BlueMatt> lol
4882017-07-17T14:23:39 <mmgen> Just adding my two bits here: how about creating an addition cli command that aggregates info from various RPC calls and presents it in a readable way?
4892017-07-17T14:23:41 <wumpus> I have that script, could post it, but don't really feel like submitting such trivial things to the repo
4902017-07-17T14:23:51 <wumpus> mmgen: I DID THAT
4912017-07-17T14:24:02 <mmgen> wumpus: what's it called?
4922017-07-17T14:24:02 <wumpus> mmgen: no one liked it!
4932017-07-17T14:24:07 <BlueMatt> lol
4942017-07-17T14:24:14 <mmgen> I think that's the best solution
4952017-07-17T14:24:21 <mmgen> It could even be coded in Python
4962017-07-17T14:24:34 <wumpus> mmgen: I have a python and c++ implementation!
4972017-07-17T14:25:17 <gmaxwell> wumpus: I liked it except for one concern, the parity between the CLI and RPC is a major learing curve improvement. If we went down the path of a highly cooked bitcoin-cli then knoweldge wouldn't automatically transfer between them, improvements wouldn't automatically transfer, etc.
4982017-07-17T14:25:20 <wumpus> #8843 was the c++ one
4992017-07-17T14:25:21 <gribble> https://github.com/bitcoin/bitcoin/issues/8843 | rpc: Handle `getinfo` client-side in bitcoin-cli w/ `-getinfo` by laanwj · Pull Request #8843 · bitcoin/bitcoin · GitHub
5002017-07-17T14:25:32 <mmgen> wumpus: need to convince the others I think
5012017-07-17T14:25:41 <wumpus> I don't have the python one online, but could post it if anyone cares
5022017-07-17T14:25:41 * BlueMatt was not aware of the python version
5032017-07-17T14:25:51 <BlueMatt> yea, I'd be interested in putting the python one in contrib
5042017-07-17T14:26:13 *** CubicEarth has quit IRC
5052017-07-17T14:26:17 <gmaxwell> for a getinfo replacement that cooked vs not cooked argument doesn't really hold, since it would just be one command.
5062017-07-17T14:26:24 <wumpus> gmaxwell: sure, that's just a matter of documentation though
5072017-07-17T14:26:43 <gmaxwell> which could be documented ('here are the underlying rpcs')
5082017-07-17T14:26:44 <wumpus> gmaxwell: it could show where it collects the various fields from for getinfo
5092017-07-17T14:26:47 <wumpus> yeah...
5102017-07-17T14:26:53 <mmgen> My wallet basically does just that, so I have a bit of experience in this area
5112017-07-17T14:27:00 <wumpus> a python script is self documentingi nthat regard
5122017-07-17T14:27:09 <wumpus> and easier to edit if you want something else
5132017-07-17T14:27:22 <wumpus> so ok, I'll add that to contrib, not in 0.15 though
5142017-07-17T14:27:26 <gmaxwell> wumpus: if it went as far as .e.g the ncurses interface, then that isn't the case. :)
5152017-07-17T14:27:38 <gmaxwell> wumpus: we could use more python rpc examples, IMO, in any case.
5162017-07-17T14:28:05 <wumpus> gmaxwell: yes I mean a simple getinfo replacement
5172017-07-17T14:28:37 <wumpus> ncurses isn't trivial (though also not rocket science, I've gotten people to start linux coding using it)
5182017-07-17T14:29:17 <promag> I'm happy to close #10804.. the problem is that to compute the histogram it takes really long to get and dump the json to the client with large utxo set
5192017-07-17T14:29:22 <gribble> https://github.com/bitcoin/bitcoin/issues/10804 | Add histunspent RPC by promag · Pull Request #10804 · bitcoin/bitcoin · GitHub
5202017-07-17T14:29:36 <mmgen> wumpus: ncurses is for wimps. real programmers use ANSI escape codes
5212017-07-17T14:29:54 <wumpus> mmgen: HAHA ansi escape codes are easier imo
5222017-07-17T14:30:04 <mmgen> wumpus: the fact is they are
5232017-07-17T14:30:27 <BlueMatt> morcos: wait, i thought min conftarget was 2?
5242017-07-17T14:30:31 <wumpus> (depending on what you want to do, if you need windowing and partial refresh etc, and menus, ncurses is definitely the way to go)
5252017-07-17T14:31:03 <mmgen> wumpus: true
5262017-07-17T14:31:04 <gmaxwell> promag: how long is really long? for how large? With thousands of txouts it still takes a fraction of a second to listunspent last I checked.
5272017-07-17T14:31:18 <wumpus> if you want to go wild with displaying 24-bit color unicode graphs, ansi escape colors are definitely easier
5282017-07-17T14:33:23 <wumpus> promag: sure, computing the statistics server-side will be faster, but it's all a compromise...
5292017-07-17T14:34:02 <wumpus> promag: in that case maintaining a private patch set might make sense - the use for those statistics is probably specific to you
5302017-07-17T14:34:26 <gmaxwell> there are certantly things where I've written a python script to collect some data.. started it.... got tired of waiting, wrote it into the daemon on another host, recompiled, ran it, got the result...
5312017-07-17T14:34:36 <promag> gmaxwell: let me get fresh numbers for you
5322017-07-17T14:34:58 <gmaxwell> its unfortunate that rpc from python (and perhaps rpc in general) is so slow, though it is much faster than it used to be.
5332017-07-17T14:35:26 <BlueMatt> yea, Ive run python rpc clients that literally look days to complete
5342017-07-17T14:35:28 <mmgen> gmaxwell: I use it all the time. Haven't noticed that it's slow
5352017-07-17T14:36:23 <mmgen> but this is with my own RPC library
5362017-07-17T14:36:30 <wumpus> in that case it makes sense to implement it server-side in c++, but less sense for it to be merged upstream
5372017-07-17T14:36:41 <gmaxwell> mmgen: try doing something simple like graphing the size of all blocks from it... takes rather long time. (or better, fee income from each block) at least with the normal python bitcoinrpc stuff
5382017-07-17T14:37:23 <wumpus> there's not really any API that can help you in that case, I guess there could be a dynlib plugin interface to load things into bitcoind, but does that ever make things easier than just recompiling?
5392017-07-17T14:37:27 <mmgen> gmaxwell: I don't use python-bitcoinrpc. I just connect to the daemon directly
5402017-07-17T14:38:00 <gmaxwell> I believe we've factored up the relevant interfaces now that it shouldn't be hard to maintain external patches for things like this. Presumably we could take other changes that improved it further.
5412017-07-17T14:38:49 <gmaxwell> mmgen: may be that it's mostly the python side handling making it slow now. (didn't used to be the case, but the rpc has become a lot faster)
5422017-07-17T14:39:12 <mmgen> gmaxwell: Probably is. Python is slow
5432017-07-17T14:39:17 <mmgen> in general
5442017-07-17T14:39:37 *** whphhg has left #bitcoin-core-dev
5452017-07-17T14:39:44 <morcos> BlueMatt: huh?
5462017-07-17T14:40:00 <BlueMatt> morcos: new docs for estimatesmartfee says nblocks minimum is 1?
5472017-07-17T14:40:10 <wumpus> sometimes speeding up python is easy, a lot of code works as-is with pypy
5482017-07-17T14:40:10 <BlueMatt> in #10707
5492017-07-17T14:40:12 <gribble> https://github.com/bitcoin/bitcoin/issues/10707 | Better API for estimatesmartfee RPC by morcos · Pull Request #10707 · bitcoin/bitcoin · GitHub
5502017-07-17T14:41:19 <mmgen> wumpus: never tried pypy
5512017-07-17T14:41:21 <morcos> BlueMatt: you mean conf_target :)
5522017-07-17T14:41:38 <morcos> it is the minimum you can request, however asking for 1 will give you an answer for 2
5532017-07-17T14:41:41 <mmgen> wumpus: but everything remains single-threaded anyway
5542017-07-17T14:41:43 <BlueMatt> morcos: yes, sorry, was reviewing the first commit first :p
5552017-07-17T14:41:51 <BlueMatt> morcos: so lets just make the minimum 1?
5562017-07-17T14:41:56 <BlueMatt> (or at least mention that in the docs)
5572017-07-17T14:42:04 <morcos> it always worked to ask for 1, i didn't want to change that
5582017-07-17T14:42:15 <morcos> it seems like a semi-common thing a user might ask for
5592017-07-17T14:42:17 <BlueMatt> ok, so mention it in the docs, i guess
5602017-07-17T14:42:22 <wumpus> mmgen: it's still quite a big linear speedup for tight loops, better than you'd get with multithreading in most cases
5612017-07-17T14:43:01 <wumpus> (and it's no extra work, unlike multithreading)
5622017-07-17T14:43:32 <morcos> Also I think it preserves the ability to in the future return something different/smarter for a as fast as possible conf_target of 1
5632017-07-17T14:45:00 *** Chris_Stewart_5 has joined #bitcoin-core-dev
5642017-07-17T14:45:26 *** Murch has joined #bitcoin-core-dev
5652017-07-17T14:47:58 <morcos> BlueMatt: ok, while I'm at it, should I change the return parameter name from "blocks" now that we changed the argument to "conf_target" or leave it?
5662017-07-17T14:48:17 <morcos> I guess I vote leave it
5672017-07-17T14:49:34 <bitcoin-git> [bitcoin] promag closed pull request #10804: Add histunspent RPC (master...2017-07-rpc-add-histunspent) https://github.com/bitcoin/bitcoin/pull/10804
5682017-07-17T14:49:48 <BlueMatt> morcos: leave it, id say
5692017-07-17T14:51:35 *** owowo has quit IRC
5702017-07-17T15:00:28 <BlueMatt> morcos: "fee estimation is *only* able to return"....
5712017-07-17T15:02:19 <morcos> BlueMatt: did you read the whole sentence?
5722017-07-17T15:02:38 <morcos> that would make no sense
5732017-07-17T15:03:22 <BlueMatt> morcos: oh, its one sentence
5742017-07-17T15:03:24 <BlueMatt> ok, whatever
5752017-07-17T15:04:36 <morcos> just ACK it already
5762017-07-17T15:05:01 <BlueMatt> i did
5772017-07-17T15:07:44 *** BashCo has quit IRC
5782017-07-17T15:08:18 *** BashCo has joined #bitcoin-core-dev
5792017-07-17T15:13:46 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/2b0179d8a9b7...89bb0365b97a
5802017-07-17T15:13:46 <bitcoin-git> bitcoin/master dba485d Wladimir J. van der Laan: init: Factor out AppInitLockDataDirectory...
5812017-07-17T15:13:47 <bitcoin-git> bitcoin/master 89bb036 Wladimir J. van der Laan: Merge #10832: init: Factor out AppInitLockDataDirectory and fix startup core dump issue...
5822017-07-17T15:14:16 <bitcoin-git> [bitcoin] laanwj closed pull request #10832: init: Factor out AppInitLockDataDirectory and fix startup core dump issue (master...2017_07_appinitlockdatadirectory) https://github.com/bitcoin/bitcoin/pull/10832
5832017-07-17T15:16:39 <bitcoin-git> [bitcoin] laanwj pushed 5 new commits to master: https://github.com/bitcoin/bitcoin/compare/89bb0365b97a...0b019357ff09
5842017-07-17T15:16:40 <bitcoin-git> bitcoin/master 3a53f19 Gregory Maxwell: Pushdown walletdb object through GenerateNewKey/DeriveNewChildKey....
5852017-07-17T15:16:40 <bitcoin-git> bitcoin/master 30d8f3a Gregory Maxwell: Pushdown walletdb though CWallet::AddKeyPubKey to avoid flushes....
5862017-07-17T15:16:41 <bitcoin-git> bitcoin/master 41dc163 Gregory Maxwell: Increase wallet default keypool size to 1000.
5872017-07-17T15:17:12 <bitcoin-git> [bitcoin] laanwj closed pull request #10831: Batch flushing operations to the walletdb during top up and increase keypool size. (master...topup_batch_flush) https://github.com/bitcoin/bitcoin/pull/10831
5882017-07-17T15:19:48 <cfields> wumpus: re qt, no, i don't think it's worth messing with for now
5892017-07-17T15:22:07 <wumpus> ok
5902017-07-17T15:23:51 <BlueMatt> gah, github is missing notification emails :(
5912017-07-17T15:26:46 <BlueMatt> wumpus: hmmm....I think (in addition to github actually missing notification emails) github wont send an email for a merge unless you comment merged.
5922017-07-17T15:26:48 <BlueMatt> can you do that?
5932017-07-17T15:27:06 <wumpus> hm?
5942017-07-17T15:27:12 <morcos> BlueMatt: I've never gotten more than about 1 in 3 of my expected github notification emails
5952017-07-17T15:27:14 <BlueMatt> eg #10831
5962017-07-17T15:27:16 <gribble> https://github.com/bitcoin/bitcoin/issues/10831 | Batch flushing operations to the walletdb during top up and increase keypool size. by gmaxwell · Pull Request #10831 · bitcoin/bitcoin · GitHub
5972017-07-17T15:27:20 <BlueMatt> morcos: did you mail support@?
5982017-07-17T15:27:23 <wumpus> github should send an automated mail when something gets merged
5992017-07-17T15:27:29 <BlueMatt> morcos: i just for the first time missed some emails
6002017-07-17T15:27:31 <morcos> i complained to sdaftuar, does that count
6012017-07-17T15:27:35 <BlueMatt> ........
6022017-07-17T15:27:44 <morcos> it didn't used to happen to him, but now it does too
6032017-07-17T15:27:51 <BlueMatt> well whatever, I have all mailserver logs and didnt get any, so I'm gonna bitch to support@
6042017-07-17T15:27:51 <wumpus> but I don't know what you mean with 'comment merged'
6052017-07-17T15:28:05 <BlueMatt> wumpus: as in usually when sipa merges he comments on the pr merged when he closes it
6062017-07-17T15:28:06 <BlueMatt> iirc
6072017-07-17T15:28:12 <BlueMatt> saying "Merged"
6082017-07-17T15:28:20 <wumpus> huh? no, he doesn't
6092017-07-17T15:29:17 <BlueMatt> oh?
6102017-07-17T15:29:23 <wumpus> see https://github.com/bitcoin/bitcoin/pull/10735 https://github.com/bitcoin/bitcoin/pull/10844
6112017-07-17T15:29:35 <wumpus> https://github.com/bitcoin/bitcoin/pull/10840
6122017-07-17T15:30:00 <BlueMatt> hmm, oh, you're right!
6132017-07-17T15:30:04 <wumpus> being the last things he merged, he commented on none of them, besides an utACK in some cases, but no "Merged". Github does that.
6142017-07-17T15:30:07 <BlueMatt> ugh, yea, so github notifications ar ejust fucked
6152017-07-17T15:30:14 <BlueMatt> yea, ok, ill go bitch at them
6162017-07-17T15:31:31 <BlueMatt> clearly we should be doing dev on our own hosted infrastructure so that we dont miss notification emails :p
6172017-07-17T15:32:00 <wumpus> as if sending mails from your own infrastructure (and having them actually arrive) is a sure thing, or was that the joke? :)
6182017-07-17T15:32:08 <BlueMatt> it was a joke
6192017-07-17T15:32:09 <gmaxwell> "now you have two problems"
6202017-07-17T15:32:09 <BlueMatt> yea
6212017-07-17T15:32:19 <BlueMatt> i mean they'd arrive for me, cause i run the receiving mailserver too :p
6222017-07-17T15:33:03 <gmaxwell> BlueMatt: but feel free to setup mirrored infra... :)
6232017-07-17T15:33:12 <BlueMatt> lol, I'll pass
6242017-07-17T15:33:15 <BlueMatt> i already manage too much shit
6252017-07-17T15:37:37 *** Murch has quit IRC
6262017-07-17T15:38:10 <gmaxwell> What do we think of #10817 ? I didn't notice until a day ago that it wasn't 15 flagged, I think it's ready to go. And its a likely useful lead up to work in 0.16-- in that it will increase change discarding somewhat, while planned work for 0.16 (the exact match coin selecter) will increase it a lot more... might be good to have in field feedback (are people going to be irate about it throwing
6272017-07-17T15:38:12 <gribble> https://github.com/bitcoin/bitcoin/issues/10817 | Redefine Dust and add a discard_rate by morcos · Pull Request #10817 · bitcoin/bitcoin · GitHub
6282017-07-17T15:38:17 <gmaxwell> away an extra 5 cents in bitcoin to avoid change outputs)
6292017-07-17T15:39:09 <wumpus> tagged
6302017-07-17T15:39:20 <gmaxwell> danke.
6312017-07-17T15:39:42 <bitcoin-git> [bitcoin] theuni opened pull request #10851: depends: bump fontconfig to 2.12.4 (master...fontconfig-bump) https://github.com/bitcoin/bitcoin/pull/10851
6322017-07-17T15:43:00 *** Murch has joined #bitcoin-core-dev
6332017-07-17T15:44:09 *** SopaXorzTaker has joined #bitcoin-core-dev
6342017-07-17T15:48:45 <gmaxwell> I think 9728 can be untagged for 15, retagged for 16.
6352017-07-17T15:49:36 <instagibbs> freeze is after today yes?
6362017-07-17T15:49:49 <wumpus> gmaxwell: ok, agree
6372017-07-17T15:49:52 <instagibbs> ack re 9728, we still have work on it
6382017-07-17T15:50:02 <instagibbs> but should def be ready for 16
6392017-07-17T15:50:03 <wumpus> instagibbs: seems we're slipping with regard to multiwallet
6402017-07-17T15:50:24 <instagibbs> yeah I noticed, but I don't have the strongest opinion so kinda ducked out of that
6412017-07-17T15:50:28 <wumpus> instagibbs: but yeah, please no new stuff now
6422017-07-17T15:51:01 <wumpus> (unless they're bugfixes, ofcourse)
6432017-07-17T15:51:16 <gmaxwell> that was the only thing in the 15 tags that seemed to me that obviously wouldn't make it. (I'm sure some other things won't too)
6442017-07-17T15:53:09 <morcos> #10830 is missing the 0.15 tag and probably still needs the most work
6452017-07-17T15:53:11 <gribble> https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
6462017-07-17T15:53:14 <BlueMatt> ok, what are we doing about multiwallet for 15?
6472017-07-17T15:53:33 * BlueMatt ducks before the impending swats
6482017-07-17T15:53:35 <gmaxwell> morcos: Agreed, though I consider 10830 a bugfix. It should still be tagged. (the other PR was tagged)
6492017-07-17T15:54:10 <gmaxwell> sipa: Have you looked at the redo of the endpoint change... it's much simpler and drop-in usable than the earlier one.
6502017-07-17T15:54:52 *** pandabull has joined #bitcoin-core-dev
6512017-07-17T15:55:07 *** pandabull has quit IRC
6522017-07-17T15:55:17 <bitcoin-git> [bitcoin] jnewbery opened pull request #10853: [tests] Fix RPC failure testing (again) (master...cleanup_jsonrpc_asserts) https://github.com/bitcoin/bitcoin/pull/10853
6532017-07-17T15:55:28 <morcos> As for multiwallet, everyone just vote 0, 1, or 2 votes for either #10829 or #10849 based on how stronly you feel, and lets just pick one. honestly, we'll be fine with either choice, but lets concentrate our effort on one of them
6542017-07-17T15:55:29 <gribble> https://github.com/bitcoin/bitcoin/issues/10829 | Simple, backwards compatible RPC multiwallet support. by ryanofsky · Pull Request #10829 · bitcoin/bitcoin · GitHub
6552017-07-17T15:55:30 <gribble> https://github.com/bitcoin/bitcoin/issues/10849 | Multiwallet: simplest endpoint support by jonasschnelli · Pull Request #10849 · bitcoin/bitcoin · GitHub
6562017-07-17T15:55:50 <gmaxwell> #10821 tag for 0.16
6572017-07-17T15:55:52 <gribble> https://github.com/bitcoin/bitcoin/issues/10821 | Add SSE4 optimized SHA256 by sipa · Pull Request #10821 · bitcoin/bitcoin · GitHub
6582017-07-17T15:56:06 <morcos> I vote 0 votes
6592017-07-17T15:56:14 *** Murch has quit IRC
6602017-07-17T15:56:24 <gmaxwell> do not vote for SSE4 optimized multiwallet.
6612017-07-17T15:56:37 <BlueMatt> oooo, 2 votes for that (whatever the hell it is)
6622017-07-17T15:56:56 <morcos> as my 2 year old would say whenever we tell him no.. "not yet?"
6632017-07-17T15:56:58 <instagibbs> 1.6x as secure as regular multiwallet
6642017-07-17T15:57:02 *** Murch has joined #bitcoin-core-dev
6652017-07-17T15:57:13 *** owowo has joined #bitcoin-core-dev
6662017-07-17T16:02:37 <jnewbery> Is #10650 definitely dead? It had 4 ACKs and instagibbs and I had both tested it.
6672017-07-17T16:02:40 <gribble> https://github.com/bitcoin/bitcoin/issues/10650 | Multiwallet: add RPC endpoint support by jonasschnelli · Pull Request #10650 · bitcoin/bitcoin · GitHub
6682017-07-17T16:03:11 <jnewbery> If it is dead, then my 1 vote is for #10849, since I've reviewed and tested that manually, and I think endpoints is the correct way to go
6692017-07-17T16:03:13 <gribble> https://github.com/bitcoin/bitcoin/issues/10849 | Multiwallet: simplest endpoint support by jonasschnelli · Pull Request #10849 · bitcoin/bitcoin · GitHub
6702017-07-17T16:03:20 <instagibbs> 1 very vehement NACK, basically
6712017-07-17T16:03:26 <instagibbs> (not saying right or wrong)
6722017-07-17T16:04:15 <jnewbery> but if other people have reviewed and tested #10829 then I can also live with that. I see the multiwallet interface in v0.15 as unstable/experimental, so as long as something gets in, I'm happy
6732017-07-17T16:04:17 <gribble> https://github.com/bitcoin/bitcoin/issues/10829 | Simple, backwards compatible RPC multiwallet support. by ryanofsky · Pull Request #10829 · bitcoin/bitcoin · GitHub
6742017-07-17T16:05:07 <instagibbs> I mostly reviewed that things behaved like they should, I can do the same today for either of the other two. I'm happy with whatever works.
6752017-07-17T16:06:59 <gmaxwell> -1 on 10650, I think like 10849 better than 10829, as I think it will be more usable in practice but I will be fully willing to help throw out either interface later, so I don't care overly much. 10849 is a newer pr... both presumably still need a lot of review love.
6762017-07-17T16:07:10 *** mmgen has quit IRC
6772017-07-17T16:12:18 <gmaxwell> morcos: care to review #10672
6782017-07-17T16:12:20 <gribble> https://github.com/bitcoin/bitcoin/issues/10672 | Avoid division by zero in the case of a corrupt estimates file by practicalswift · Pull Request #10672 · bitcoin/bitcoin · GitHub
6792017-07-17T16:17:01 <cfields> BlueMatt: are you still wanting #10652 for 0.15 ?
6802017-07-17T16:17:03 <gribble> https://github.com/bitcoin/bitcoin/issues/10652 | Small step towards demangling cs_main from CNodeState by TheBlueMatt · Pull Request #10652 · bitcoin/bitcoin · GitHub
6812017-07-17T16:17:20 *** Guyver2 has quit IRC
6822017-07-17T16:20:06 <gmaxwell> #10335 sounds like something should be done.
6832017-07-17T16:20:07 <gribble> https://github.com/bitcoin/bitcoin/issues/10335 | back-compat: add fallback getentropy implementation by theuni · Pull Request #10335 · bitcoin/bitcoin · GitHub
6842017-07-17T16:35:05 <cfields> gmaxwell: thanks for the reminder. fixing that up now.
6852017-07-17T16:35:06 *** mmgen has joined #bitcoin-core-dev
6862017-07-17T16:35:13 *** Dyaheon has quit IRC
6872017-07-17T16:36:50 *** Dyaheon has joined #bitcoin-core-dev
6882017-07-17T16:40:16 *** fengling has quit IRC
6892017-07-17T16:40:17 *** [Author] has quit IRC
6902017-07-17T16:40:17 *** Magma has quit IRC
6912017-07-17T16:41:29 <BlueMatt> fizzwont: nah, think its gonna slip now
6922017-07-17T16:41:33 <BlueMatt> cfields: ...
6932017-07-17T16:41:44 <instagibbs> little worried about 10829 since named support is still a bit flakey. I think merging that would mean fixing up named arg support becomes priority.
6942017-07-17T16:41:49 <BlueMatt> it would've been nice, but i dont think its critical enough to care, just untag 10672
6952017-07-17T16:42:20 <gmaxwell> BlueMatt: whats the 'fizzwont' context for your message?
6962017-07-17T16:42:36 <BlueMatt> gmaxwell: bad auto-tab, its a user on here
6972017-07-17T16:42:39 <cfields> :(
6982017-07-17T16:43:11 <BlueMatt> cfields: meh, its mostly a bugfix for not-really-issues bugs
6992017-07-17T16:43:20 <BlueMatt> the cleanup to do the demangle is gonna be a through-16 process
7002017-07-17T16:44:17 <fizzwont> i'm honored...but just observing
7012017-07-17T16:59:19 *** Murch has quit IRC
7022017-07-17T17:01:45 *** Murch has joined #bitcoin-core-dev
7032017-07-17T17:03:25 <bitcoin-git> [bitcoin] gmaxwell opened pull request #10854: Avoid using sizes on non-fixed-width types to derive protocol constants. (master...rbf-numlimit-fix) https://github.com/bitcoin/bitcoin/pull/10854
7042017-07-17T17:06:59 *** arowser has quit IRC
7052017-07-17T17:10:34 *** herzmeister[m] has quit IRC
7062017-07-17T17:11:07 *** draadpiraat[m] has quit IRC
7072017-07-17T17:12:01 *** kewde[m] has quit IRC
7082017-07-17T17:15:57 <bitcoin-git> [bitcoin] theuni closed pull request #10335: back-compat: add fallback getentropy implementation (master...getentropy-back-compat) https://github.com/bitcoin/bitcoin/pull/10335
7092017-07-17T17:16:10 *** arowser has joined #bitcoin-core-dev
7102017-07-17T17:18:07 <bitcoin-git> [bitcoin] theuni opened pull request #10855: random: only use getentropy on openbsd (master...getentropy-openbsd) https://github.com/bitcoin/bitcoin/pull/10855
7112017-07-17T17:19:39 *** herzmeister[m] has joined #bitcoin-core-dev
7122017-07-17T17:23:06 *** Murch has quit IRC
7132017-07-17T17:24:53 *** Murch has joined #bitcoin-core-dev
7142017-07-17T17:26:42 *** kexkey has joined #bitcoin-core-dev
7152017-07-17T17:27:06 *** promag has quit IRC
7162017-07-17T17:30:52 *** jamesob_ has joined #bitcoin-core-dev
7172017-07-17T17:37:29 *** CubicEarth has joined #bitcoin-core-dev
7182017-07-17T17:42:28 *** arubi has quit IRC
7192017-07-17T17:42:28 *** dermoth has quit IRC
7202017-07-17T17:43:24 *** dermoth has joined #bitcoin-core-dev
7212017-07-17T17:44:52 *** arubi has joined #bitcoin-core-dev
7222017-07-17T17:47:29 *** jtimon has joined #bitcoin-core-dev
7232017-07-17T17:50:11 *** nakaluna has joined #bitcoin-core-dev
7242017-07-17T17:52:10 <morcos> yeah someone please tag #10758 for 0.15
7252017-07-17T17:53:28 <sipa> done
7262017-07-17T17:54:21 <morcos> instagibbs: to be clear, there are no rpc calls where you can't use named arguments right? there just might be some where you need to specifically specify the default value if you want to name a later argument (but this is the same thing you have to do with positional)
7272017-07-17T17:54:55 <sipa> wasnt't there a PR to fix that?
7282017-07-17T17:55:05 *** kewde[m] has joined #bitcoin-core-dev
7292017-07-17T17:55:05 *** draadpiraat[m] has joined #bitcoin-core-dev
7302017-07-17T17:55:18 <instagibbs> morcos, oh, true
7312017-07-17T17:55:28 <instagibbs> yes you'll have to enter in all values for some
7322017-07-17T17:55:37 <gribble> https://github.com/bitcoin/bitcoin/issues/10758 | Fix some chainstate-init-order bugs. by TheBlueMatt · Pull Request #10758 · bitcoin/bitcoin · GitHub
7332017-07-17T17:55:44 <instagibbs> so, less urgent :)
7342017-07-17T17:55:47 <morcos> sipa: yeah, i was just trying to understand the urgency
7352017-07-17T17:56:02 <instagibbs> merely annoying
7362017-07-17T17:56:55 *** timothy has quit IRC
7372017-07-17T17:57:25 <BlueMatt> lol, somehow i thought 10758 was already tagged 15
7382017-07-17T17:57:27 *** CubicEarth has quit IRC
7392017-07-17T17:57:28 <morcos> i guess people didn't like my voting idea or don't have an opinon on which multiwallet PR to merge. can we decide so we can make it happen? i'm happy to review 10849 if we prefer that one
7402017-07-17T17:57:47 <sipa> i'll review the last PR before giving an opinion
7412017-07-17T17:57:51 *** PaulCape_ has quit IRC
7422017-07-17T18:00:28 *** PaulCapestany has joined #bitcoin-core-dev
7432017-07-17T18:03:28 <ryanofsky> i'd give one vote to 10829 (obvs), and one conditional vote to 10849 if it uses a plain query parameter, or has some documentation explaining the path schema
7442017-07-17T18:05:41 <sipa> i think a query parameter is inferior to just a named parameter, and doesn't really simplify later moving to a new process
7452017-07-17T18:06:09 <sipa> i'm fine with named parameters for now, as a stopgap measure because a full new API isn't complete
7462017-07-17T18:06:16 <sipa> documentation can come later
7472017-07-17T18:06:26 <sipa> that doesn't need to be ready by the feature freeze
7482017-07-17T18:08:03 <sipa> but i do think a new endpoint is eventually the way to go - whether that happens in 0.15 or not
7492017-07-17T18:08:05 <ryanofsky> sipa i think moving to new process is basically orthogonal. reason why i think query parameter is better is it leave uri-path space open for future uses and an actual thoughtful design. current design is changing multiple times a day
7502017-07-17T18:08:47 <ryanofsky> add v1, subtract v1, forward node calls through wallet, then stop, then start again
7512017-07-17T18:09:16 <sipa> ryanofsky: i really disagree with that... the point of an endpoint is that clients can be configured to point to a new process
7522017-07-17T18:09:36 <ryanofsky> i'm trying to understand...
7532017-07-17T18:09:41 <sipa> you can't do that with query parameters
7542017-07-17T18:09:55 <ryanofsky> ? why not?
7552017-07-17T18:10:20 <sipa> what if some of them are process-specific, and some or not?
7562017-07-17T18:10:31 <sipa> the client software would need to go modify the url to put in extra things
7572017-07-17T18:10:59 <sipa> i guess it isn't worse - as long as everything you'd put in the path becomes a query parameter and nothing else
7582017-07-17T18:11:05 <sipa> but it also doesn't gain you anything
7592017-07-17T18:11:05 <jtimon> ryanofsky: it's about simplyfing concurrency
7602017-07-17T18:11:11 <sipa> jtimon: wut?
7612017-07-17T18:11:17 <jtimon> is it not?
7622017-07-17T18:11:23 <sipa> what does that have to do with anything
7632017-07-17T18:11:40 *** AaronvanW has quit IRC
7642017-07-17T18:11:58 <ryanofsky> i'm not understanding what you're saying sipa... did you change your mind midway?
7652017-07-17T18:12:28 <sipa> ryanofsky: yes, i did; i agree that query parameters can do anything that a URI can... but it also doesn't gain you anything
7662017-07-17T18:12:32 <ryanofsky> advantage of query parameter is it leaves more options for the future that don't require us to break compatibility
7672017-07-17T18:12:39 <ryanofsky> nothing to do with multiprocess
7682017-07-17T18:13:53 <ryanofsky> specific example of what i'm talking about: maybe we want to have web interface listening on /wallet. maybe we want to add versioning in the future
7692017-07-17T18:13:59 <sipa> ryanofsky: but the point is creating a namespace
7702017-07-17T18:14:12 <jtimon> right, each wallet can get its own lock even with query paramters, then I don't undesrtand what's the point either...can't you serve the rpc more concurrently with different http addresses more concurrently either (ie maybe a server for each or something)?
7712017-07-17T18:14:15 <sipa> separating things off, which can later move elsewhere
7722017-07-17T18:14:24 <ryanofsky> doing these things cleanly means planning out url design somewhat, which hasn't been done. in these prs url design is constantly in flux
7732017-07-17T18:14:33 <sipa> jtimon: that's completely orthogonal; we're talking about interface here
7742017-07-17T18:14:59 <jtimon> well, I remembered that about the most convincing argument in favor of this interface
7752017-07-17T18:15:13 <jtimon> s/about/as/
7762017-07-17T18:15:22 <sipa> jtimon: we already have multithreaded RPC...
7772017-07-17T18:15:37 <jtimon> sipa: oh, nice
7782017-07-17T18:15:40 <sipa> jtimon: ...
7792017-07-17T18:16:25 <ryanofsky> jtimon, fwiw, i don't see connection there either
7802017-07-17T18:16:51 <sipa> jtimon: the RPC implementation isn't very concurrent, but that's an implementation detail that can be improved independently
7812017-07-17T18:17:00 <jtimon> I didn't know, sorry, then I don't understand why this wallet/<my wallet> is better than wallet=<mywallet> in the rpc either. but I'll read your conversation and see if I get it
7822017-07-17T18:17:00 <sipa> (overeager locking in many places)
7832017-07-17T18:17:19 <sipa> jtimon: it's so that things can move to a different process
7842017-07-17T18:17:41 <sipa> if the wallet weren't handled by bitcoind anymore, it would have its own RPC server, which runs on a different port or something
7852017-07-17T18:17:53 <jtimon> sipa: how is moving things to a different process unrelated to concurrency?
7862017-07-17T18:18:03 <sipa> jtimon: because it's already multithreaded!
7872017-07-17T18:18:27 <jtimon> then what's the point of moving it to a different process?
7882017-07-17T18:18:37 <sipa> the reason for moving things to a different process is for security (don't have your private keys connected to a network interface) and modularity (run it on a different machine)
7892017-07-17T18:19:04 <jtimon> ok, thanks, I wrongly assumed the reason was concurrency
7902017-07-17T18:19:06 <ryanofsky> my the way my conditional vote for 10849 had an OR in it. if query parameter is worse for some reason (aesthetics?) and path interpretation is way to go, please just document/respond what actual plans are for path interpretation. what compatibility is being promised
7912017-07-17T18:19:24 <sipa> ryanofsky: of course it needs documenting
7922017-07-17T18:20:15 <sipa> ryanofsky: if we aren't clear on how the namespace separation should happen, i agree that just an RPC named parameter for selecting a wallet is better for now
7932017-07-17T18:20:38 <sipa> ryanofsky: but if we are, we should do it... and url parameters seem like a very odd thing for a namespace
7942017-07-17T18:21:38 <ryanofsky> i don't understand the point about a namespace. is an aesthetic thing, or a practical difference?
7952017-07-17T18:22:05 *** DaggerHashimoto has joined #bitcoin-core-dev
7962017-07-17T18:22:29 <ryanofsky> i'm asking for documentation not just because documentation is needed, but also because i really think i am missing something about rationales (or that rationales haven't actually been thought through)
7972017-07-17T18:22:46 <sipa> ryanofsky: my concern is that uri parameters don't force us to think about separation
7982017-07-17T18:22:57 <jtimon> but the parameters don't need to go in the url, do they? they can go with the json data (ie with method, params, jsonrpc and id)
7992017-07-17T18:23:18 <sipa> jtimon: yes, that's the discussion... there are 4 approaches suggested
8002017-07-17T18:24:11 <jtimon> I think I'm missing one from the last meeting...
8012017-07-17T18:24:54 <sipa> jtimon: in the URL (http://localhost:8333/v1/wallet/[walletname]), in a URL parameter (http://localhost:8333/v1?wallet=[walletname]), in a JSON RPC parameter (pass wallet: "[walletname]" as named argument), through RPC auth (every rpc user has his own wallet)
8022017-07-17T18:25:24 <jtimon> I see, rpc auth was the one I was mising
8032017-07-17T18:25:44 <sipa> ryanofsky: people may be inclined to add things to URI parameters that don't naturally correspond to separation (say, an option for changing the default currency unit)
8042017-07-17T18:25:51 <sipa> ryanofsky: or expect those to go there
8052017-07-17T18:25:55 <jtimon> no, I thought we agreed some users will have more than one, and I think some users may not want one too
8062017-07-17T18:26:31 <sipa> ryanofsky: sure you can say, "well don't do that"
8072017-07-17T18:26:45 <ryanofsky> not sure why a query option like that would be bad at all. do you also think a timeout query option would be bad?
8082017-07-17T18:26:46 <BlueMatt> it may be unfair, but ryanofsky succcessfully convinced me over lunch....the user experience of upgrading is gonna be the same no matter how we do it, and the args approach is much simpler for users to add
8092017-07-17T18:26:53 <BlueMatt> rather than the endpoints approach
8102017-07-17T18:27:22 <sipa> ryanofsky: ok, say we have ?wallet=[walletname]&timeout=30
8112017-07-17T18:27:31 <sipa> all in one process
8122017-07-17T18:27:41 <BlueMatt> the endpoints stuff is kinda nice in that it makes users thing, but realistically its just another hop to upgrade, cause if we move to process isolation now its all a different ip/port connection anyway
8132017-07-17T18:27:46 <BlueMatt> so its just a useless upgrade hop
8142017-07-17T18:28:13 <sipa> now the wallets move to different processes
8152017-07-17T18:28:30 <BlueMatt> and now the split is by connection port, and not endpoint anyway :p
8162017-07-17T18:28:40 <sipa> you can't just go reconfigure the client software
8172017-07-17T18:29:08 <sipa> it needs to mix something from the configured URI (which likely has the ?wallet= part) and from the application (which likely has the ?timeout= part)
8182017-07-17T18:29:09 <BlueMatt> actually args makes it easier at that point than endpoints?
8192017-07-17T18:29:16 <BlueMatt> precisely cause it doesnt require client reconfiguration
8202017-07-17T18:29:29 <BlueMatt> wait, now maybe I'm confused, why are we re-adding uri parameters now
8212017-07-17T18:29:34 <sipa> but client reconfiguration is easy
8222017-07-17T18:29:36 <ryanofsky> sipa are you talking about a node process that forwards requests to various wallets?
8232017-07-17T18:29:40 <sipa> ryanofsky: no
8242017-07-17T18:29:41 <ryanofsky> BlueMatt, i have same confusion
8252017-07-17T18:30:02 <ryanofsky> so you are talking about what matt is talking about. wallet process listening on own port
8262017-07-17T18:30:08 <BlueMatt> doing wallet split today is very doable - have multiple rpc clients pointed at different listening ports
8272017-07-17T18:30:10 <sipa> i want to just tell my client app (instead of http://localhost:8333, you can use http://localhost:8333/v1/wallet/bla)
8282017-07-17T18:30:21 <BlueMatt> but you cant with most clients/
8292017-07-17T18:30:22 <BlueMatt> ?
8302017-07-17T18:30:31 <ryanofsky> wait sipa, that sounds like you are saying one port for node and wallet?
8312017-07-17T18:30:38 <sipa> ryanofsky: ???
8322017-07-17T18:30:55 <sipa> as long as they're in the same process
8332017-07-17T18:31:00 <ryanofsky> port 8333 is wallet process or node process?
8342017-07-17T18:31:07 <sipa> it's the process process
8352017-07-17T18:31:09 <sipa> there is just one
8362017-07-17T18:31:10 <BlueMatt> instead of btc = AuthServiceProxy(); btc.getwalletinfo(); btc.getmininginfo(); its now wallet_btc = AuthServiceProxy(...:8332); node_btc = AuthServiceProxy(...:8331)......
8372017-07-17T18:31:22 <sipa> ryanofsky: i'm talking about the case before things move to a idfferent process
8382017-07-17T18:31:25 <BlueMatt> sipa: I'm horribly confused
8392017-07-17T18:31:28 <ryanofsky> AH ok
8402017-07-17T18:31:33 <ryanofsky> so single process just like today
8412017-07-17T18:32:13 <BlueMatt> sipa: I think ryanofsky's point (and my point) is that we should focus on making it easier for clients to do the upgrade (which wallet arg is, imo) cause the isolation is gonna come from different listening ports in the future anyway
8422017-07-17T18:32:15 <sipa> however, afterwards, when things move to a new process, you don't need to change the application software, just reconfigure it to use a different URI (which is now maybe running on a different port, or a different host)
8432017-07-17T18:32:24 <BlueMatt> so trying to force endpoints is just an extra step for users for no reason
8442017-07-17T18:32:41 <BlueMatt> sipa: you can already do that?
8452017-07-17T18:32:47 <ryanofsky> sipa, how is wallet named arg a problem in that scenario?
8462017-07-17T18:32:53 <BlueMatt> just tell your app software that you have two different rpc hosts
8472017-07-17T18:32:59 <ryanofsky> i think wallet named arg is a feature even in that scenario
8482017-07-17T18:33:15 <sipa> ryanofsky: because wallet named arg requires at the very least the client application to know which wallet it is talking to
8492017-07-17T18:33:23 <ryanofsky> because it's easy to imagine starting wallet processes and confusing yourself about port numbers
8502017-07-17T18:33:29 <sipa> while it can be encapsulated in the uri
8512017-07-17T18:34:51 <BlueMatt> sipa: if you're gonna change the rpc library to support endpoints, you can also change it to support a global wallet argument
8522017-07-17T18:35:00 <ryanofsky> i'm confused again. now we are talking about separate wallet process listening it's separate wallet port. what is the harm of wallet param there (used or unused)?
8532017-07-17T18:35:12 <BlueMatt> whereas arguments are nice cause you dont have to change the library, if it already supports named args
8542017-07-17T18:35:13 <sipa> BlueMatt: but after process separation there may not even be a need for a global wallet argument
8552017-07-17T18:35:22 <jtimon> for what is worth, I like v1 in the uri more than "jsonrpc": "1.0" in the json data, but do we need 2 of them?
8562017-07-17T18:35:23 <sipa> BlueMatt: endpoint support means there is just one change
8572017-07-17T18:35:32 <BlueMatt> sipa: sure, and its, in fact, easier to remove the wallet arg at that point than the endpoints
8582017-07-17T18:35:33 <sipa> to the application
8592017-07-17T18:35:39 <BlueMatt> cause we can even start ignoring the arg
8602017-07-17T18:36:10 <sipa> you really think so? that first requiring everything to add a wallet configuration option, and then later change it again because now it's done in a different URI
8612017-07-17T18:36:17 <sipa> is easier than just allowing to configure a URI?
8622017-07-17T18:36:43 <ryanofsky> oh sipa, now i finally get it, yes i agree
8632017-07-17T18:36:50 <BlueMatt> sipa: think about eg the python library which is never updated
8642017-07-17T18:36:58 <BlueMatt> you could use multiwallet even before the library is updated
8652017-07-17T18:37:08 <BlueMatt> and if you update the library then its the same
8662017-07-17T18:37:18 <ryanofsky> yes that is an argument against named json-rpc params in favor of urls
8672017-07-17T18:37:20 <BlueMatt> (either the library silently adds the wallet arg to all calls, or it doesnt)
8682017-07-17T18:37:21 <sipa> i'm confused
8692017-07-17T18:37:28 <BlueMatt> instead of silently adding the wallet endpoint to all calls, or not
8702017-07-17T18:37:40 <ryanofsky> it's not an argument for wallet in uri-path instead of wallet in url query-string
8712017-07-17T18:37:43 <sipa> BlueMatt: it shouldn't silently add wallet endpoints!
8722017-07-17T18:37:53 <sipa> BlueMatt: you should tell it "MY URL IS ...../v1/wallet"
8732017-07-17T18:38:08 <sipa> it shouldn't even know there is such a thing as wallet names
8742017-07-17T18:38:08 <BlueMatt> sipa: does the current python client support that?
8752017-07-17T18:38:18 <BlueMatt> (I dont know, I'm asking)
8762017-07-17T18:38:20 <sipa> BlueMatt: i don't know, but it'd be trivial to add
8772017-07-17T18:38:38 <sipa> and certainly easier than updating it to selectively go add wallet parameters everywhere
8782017-07-17T18:38:41 <sipa> and then later making that optional
8792017-07-17T18:39:28 <jtimon> sipa: how is "encapsulated in the uri" different from "encapsulated on the json data like 'jsonrpc' and 'method'"? if that's better, why not move the method to the uri too?
8802017-07-17T18:39:50 <sipa> jtimon: because thr rpc method and parameter are decided by the application
8812017-07-17T18:39:55 *** SopaXorzTaker has quit IRC
8822017-07-17T18:40:00 <sipa> jtimon: and the URI is decided by the person configuring the software
8832017-07-17T18:40:02 <jtimon> sipa: and so it's the uri?
8842017-07-17T18:40:08 <sipa> no
8852017-07-17T18:40:20 <sipa> you tell your application which host and port the RPC server runs on,
8862017-07-17T18:40:26 <sipa> you can also tell it what URI to use
8872017-07-17T18:40:45 <sipa> ryanofsky: i agree that technically url query-string can do as much as uri-path... but i do believe that this approach can only work if we clearly think about what can be separated
8882017-07-17T18:40:46 *** Dyaheon has quit IRC
8892017-07-17T18:40:46 <BlueMatt> sipa: issue is I do not believe most client libraries support that today
8902017-07-17T18:40:48 <jtimon> mhmm, well, that's were I'm confused, I don't see how you could have control of the json data bur not of the uri
8912017-07-17T18:40:51 <sipa> BlueMatt: sure, so?
8922017-07-17T18:40:53 <BlueMatt> (I was informed that the python-bitcoinrpc library does not)
8932017-07-17T18:40:54 <sipa> BlueMatt: they can be updated
8942017-07-17T18:40:55 <jtimon> s/were/where
8952017-07-17T18:41:15 <BlueMatt> sipa: whereas client libraies may already support a wallet argument
8962017-07-17T18:41:17 <BlueMatt> silently, easily
8972017-07-17T18:41:21 <ryanofsky> sipa, maybe an example of where query-string would not work & uri-path would work?
8982017-07-17T18:41:55 <sipa> 18:10:59 < sipa> i guess it isn't worse - as long as everything you'd put in the path becomes a query parameter and nothing else
8992017-07-17T18:41:59 <ryanofsky> by the way, one reason i am more inclined to named wallet param is that i think it is actually better for safety
9002017-07-17T18:41:59 <sipa> ryanofsky: there isn't
9012017-07-17T18:42:24 *** Dyaheon has joined #bitcoin-core-dev
9022017-07-17T18:42:30 <jtimon> sipa: oh, ok, then that's where I was confused! what are we discussing then?
9032017-07-17T18:42:39 <sipa> jtimon: JSON RPC argument vs URI
9042017-07-17T18:42:53 <sipa> jtimon: i'm not talking about URI path vs URI query string
9052017-07-17T18:43:08 <ryanofsky> named arguments are safer than positional arguments, because harder to screw up ordering, explicitly specifying wallet is better than not because easy to mess up port numbers
9062017-07-17T18:43:42 <ryanofsky> so i tend to think even with multiprocess wallet, param has utility
9072017-07-17T18:44:36 <jtimon> sipa: right, so there's no difference between JSON RPC argument vs URI, right?
9082017-07-17T18:44:47 <sipa> jtimon: yes there is
9092017-07-17T18:45:04 <sipa> jtimon: with URI based approaches, we can avoid the need for applications to know what wallet they're talking to
9102017-07-17T18:45:13 <sipa> or even know that there is such a thing as multiwallet
9112017-07-17T18:45:23 <jtimon> I thought you just said there isn't an example of what ryanofsky was asking for?
9122017-07-17T18:45:30 <jtimon> I'm getting more confused, sorry
9132017-07-17T18:45:37 <sipa> jtimon: sigh... that was about URI path vs URI query
9142017-07-17T18:45:42 <jtimon> I'll read the full conversation
9152017-07-17T18:46:09 <ryanofsky> jtimon, talking about 3 different things. (1) wallet in jsonrpc param (2) wallet in uri query param (3) wallet in uri-path
9162017-07-17T18:46:13 <jtimon> I don't think ryanofsky was asking anything about URI query, I know I wasn't
9172017-07-17T18:46:18 <BlueMatt> sipa: but thats my point...we cant cause none of the client libraries support it, and they're all like barely maintained last I heard
9182017-07-17T18:46:36 <sipa> ryanofsky: every JSON-RPC library supports passing in a URI
9192017-07-17T18:46:39 <sipa> eh, BlueMatt ^
9202017-07-17T18:46:51 <sipa> BlueMatt: the bitcoin specific shims can be trivially patched to pass that through
9212017-07-17T18:47:28 <sipa> adding a named wallet arg everywhere is far harder, and requires logic that imho is totally unneeded at that level
9222017-07-17T18:47:28 <jtimon> ryanofsky: and who is defending 2 ?
9232017-07-17T18:48:00 <jtimon> ryanofsky: I really thought we were only duscussing 1 vs 3 already, sorry
9242017-07-17T18:48:25 *** THoVer has joined #bitcoin-core-dev
9252017-07-17T18:48:38 <sipa> jtimon: the main discussion is (1) vs (3)... though ryanofsky has suggested that he'd prefer (2) over (3)
9262017-07-17T18:48:40 <ryanofsky> i'm defending 2. i think there are practical tradeoffs between 1 & 2, but that 3 has no practical advantages over either and has disadvange of introducing undocumented half-baked url scheme
9272017-07-17T18:48:43 <BlueMatt> sipa: you could add a shim that passes the wallet arg in everywhere, too, which is already supported by some rpc client libraries, and all of this debate is useless if we have a process split anyway, cause then its about port numbers anyway
9282017-07-17T18:49:01 <BlueMatt> at least then we wont have new endpoints to maintain, just an extra arg that we can ignore
9292017-07-17T18:49:06 <sipa> BlueMatt: port numbers are also part of the URI
9302017-07-17T18:49:11 <jtimon> ok, I missed that, as said I will read the beginning of the discussion...
9312017-07-17T18:49:21 <BlueMatt> yes, but are treated differently by some rpc client software :p
9322017-07-17T18:49:27 <sipa> ?
9332017-07-17T18:49:48 <BlueMatt> well apparently at least python-bitcoinrpc does not support endpoints, but does, obviously, already support a different port to connect to
9342017-07-17T18:49:53 <BlueMatt> or so I'm told
9352017-07-17T18:49:58 <sipa> yes, but that's easy to fix
9362017-07-17T18:50:14 <sipa> and doesn't change that's better if client libraries don't need to know what wallet they're connected to
9372017-07-17T18:50:35 <jtimon> yeah, don't know python-bitcoinrpc but I can't imagine how it wouldn't be trivial to adapt either way
9382017-07-17T18:50:47 <ryanofsky> BlueMatt, python-bitcoinrpc doesn't support multiple endpoints currently, you have to choose one in advance if you want to write a test that uses multiple wallets for example. but jonas pr changes that
9392017-07-17T18:51:08 <BlueMatt> ohoh, wait, so it does support endpoints but you just need multiple AuthServiceProxy's for it?
9402017-07-17T18:51:10 <jtimon> afk
9412017-07-17T18:51:12 <BlueMatt> wait, that might change my view
9422017-07-17T18:51:12 <BlueMatt> ugh
9432017-07-17T18:51:14 <BlueMatt> i give up
9442017-07-17T18:51:18 <ryanofsky> BlueMatt, exactly
9452017-07-17T18:51:30 <BlueMatt> oh, well i mean thats kinda more analygous to the port number changes :(
9462017-07-17T18:51:33 <BlueMatt> lol, sorry sipa
9472017-07-17T18:51:59 <sipa> ryanofsky: i think that you do bring up a good point that there is a risk in creation a half-baked url scheme
9482017-07-17T18:52:11 <ryanofsky> sipa, that is why i am fine with 2
9492017-07-17T18:52:34 <sipa> ryanofsky: i believe (2) has more risk in going the wrong way than (3)
9502017-07-17T18:53:02 <sipa> and i guess part of that argument is aesthetics
9512017-07-17T18:53:05 <ryanofsky> i also want to point out that there we are talking about 1 practical tradeoff between 1 & 2/3, there are other practical advantages to 1 like not having to modify bitcoin-cli
9522017-07-17T18:53:15 <BlueMatt> can we use the lsb of the next blockhash?
9532017-07-17T18:54:03 <ryanofsky> sipa, ok that is what i think i'm not understanding. is there more to the argument than aesthetics...
9542017-07-17T18:54:05 <sipa> ryanofsky: i am fine with (1) over (2)/(3) if we believe the separation isn't thought out enough
9552017-07-17T18:54:40 <sipa> ryanofsky: it's a slippery slope argument, so you can counter anything i say with "well, we can just decide not to do that"
9562017-07-17T18:55:06 <sipa> ryanofsky: but i believe there is a risk that (2) will make it too easy for us to say "oh, let's put the currency unit in the URI"
9572017-07-17T18:55:09 <ryanofsky> sipa, not sure i understand an example of something bad....
9582017-07-17T18:55:30 <ryanofsky> you were saying some other parameters are bad to put in query strings?
9592017-07-17T18:56:53 <sipa> actually, i change my mind - that wouldn't be bad
9602017-07-17T18:57:36 <ryanofsky> also just to list other practical advantages i see in (1) over (2/3). Requires no changes to bitcoin-cli. Is properly documented, easy to understand, just a param. Encourages named arguments. Allows checking wallet name for safety even with multiprocess.
9612017-07-17T18:58:08 <sipa> so yes, my preference for (3) over (2) is aesthetics (and, _if_ we have a well thought-out separation, i also think (2) does not really have advantages over (3))
9622017-07-17T18:58:50 *** promag has joined #bitcoin-core-dev
9632017-07-17T18:59:21 <ryanofsky> agree maybe (2) may not have advantages over (3) if you have a well-thought uri-path schema
9642017-07-17T18:59:52 <sipa> and things like a currency type or timeout can still be query string options rather than paths
9652017-07-17T19:00:21 <ryanofsky> i definitely have opposite aesthetics though, straight key=value named argument vastly preferable to me to inflexible /positional/path/stuff
9662017-07-17T19:01:01 *** d_t_ has joined #bitcoin-core-dev
9672017-07-17T19:01:28 <sipa> well, (purely aesthetics argument), paths feel like "i am now talking to a different subsystem!"; query options feel like "here is some extra stuff that may or may not affect how you're interpreting my request"
9682017-07-17T19:01:59 *** AaronvanW has joined #bitcoin-core-dev
9692017-07-17T19:02:55 <ryanofsky> sure, i can see that
9702017-07-17T19:03:29 <sipa> and given an intention (if we agree to that) to move the wallet out, i think it makes sense to treat it as a different subsystem
9712017-07-17T19:03:41 <morcos> at the risk of derailing this convo on the brink of agreement, sipa: achow: do you still want #10579 for 0.15, it doesn't look as far along as 10571
9722017-07-17T19:03:41 <ryanofsky> maybe i don't understand what you see in multiprocess world. i see wallet=filename still being something you can specify for safety
9732017-07-17T19:03:42 <gribble> https://github.com/bitcoin/bitcoin/issues/10579 | [RPC] Split signrawtransaction into wallet and non-wallet RPC command by achow101 · Pull Request #10579 · bitcoin/bitcoin · GitHub
9742017-07-17T19:04:16 <ryanofsky> but you are seeing us do this elaborate /v1/wallet/ /v1/node thing and then the /v1/wallet stuff goes in the trash because it is no longer a "separate subsystem"?
9752017-07-17T19:04:24 <morcos> oops achow101 ^
9762017-07-17T19:04:55 <sipa> ryanofsky: no, /v1/wallet would remain - the wallet process just would not expose /v1/node anymore
9772017-07-17T19:05:15 <ryanofsky> sipa, ok. that is aesthetically ugly to me :)
9782017-07-17T19:05:24 <sipa> ryanofsky: please clarify
9792017-07-17T19:05:54 *** paveljanik has joined #bitcoin-core-dev
9802017-07-17T19:05:54 *** paveljanik has joined #bitcoin-core-dev
9812017-07-17T19:06:38 <sipa> (as in, i'd genuinely interested in hearing why)
9822017-07-17T19:06:41 <sipa> *i'm
9832017-07-17T19:06:53 <ryanofsky> wallet filename at that point is no longer "specifying a subsystem". it is just redundant at that point. we have to go on treating it as this magical thing different from other parameters forever
9842017-07-17T19:07:12 <sipa> ryanofsky: well i expect one process to remain capable of handling multiple wallets
9852017-07-17T19:07:49 <sipa> a lightweight node is far cheaper than a full node, but handling an individual wallet compared to that is still orders of magnitude less
9862017-07-17T19:08:21 <achow101> morcos: I would like to have it in 0.15 but it hasn't been getting any review
9872017-07-17T19:09:19 <sipa> ryanofsky: actually, no
9882017-07-17T19:09:42 <sipa> ryanofsky: the whole point of having it in a URI is that it shouldn't be treated as part of the interface
9892017-07-17T19:09:54 *** BashCo has quit IRC
9902017-07-17T19:10:19 *** helo has quit IRC
9912017-07-17T19:10:31 <sipa> someone could create a new lightweight implementation that has the same API (unlikely, i know), which only exposes what we have now under /v1/wallet/[walletname], but just exposes it as '/blah'
9922017-07-17T19:10:33 *** BashCo has joined #bitcoin-core-dev
9932017-07-17T19:10:34 *** helo has joined #bitcoin-core-dev
9942017-07-17T19:12:40 <jnewbery> sorry, I was away from my desk. Seems like I missed out on all the fun.
9952017-07-17T19:12:42 <sipa> however, if people feel that we haven't thought through the implication of that, and whether we can do that separation cleanly... we should just do (1)
9962017-07-17T19:12:54 <jnewbery> At the risk of going over old ground, I prefer (3) for a couple of reasons:
9972017-07-17T19:13:20 <jnewbery> 1. each wallet is conceptually a separate resource, so it makes sense to me to have different URIs
9982017-07-17T19:13:39 <jnewbery> That's true whether or not we go for wallet separation in future
9992017-07-17T19:14:20 <jnewbery> *wallet process separation
10002017-07-17T19:14:22 <ryanofsky> thanks jnewbery, you are finally writing the documentatino i was asking for :)
10012017-07-17T19:14:58 <jnewbery> 2. It offers a smoother upgrade path for clients if we do go for wallet process separation and each wallet binds to its own port
10022017-07-17T19:15:27 <jnewbery> namely: change the endpoint now to /wallet/<blah>, and then change the endpoint later to <wallet port>
10032017-07-17T19:15:53 *** THoVer has quit IRC
10042017-07-17T19:16:13 <jnewbery> I'm *much* less worried than ryanofsky about implementing a uri scheme now that we want to get rid of later. This should all be considered unstable/experimental anyway
10052017-07-17T19:16:26 *** promag has quit IRC
10062017-07-17T19:16:35 <jnewbery> yes, I'll happily write documentation
10072017-07-17T19:17:20 <sipa> ryanofsky: if anything - thanks for discussing this; it's made your point clearer to me, and helped me understand my own preferences better
10082017-07-17T19:17:49 <ryanofsky> your second reason is reason sipa & gmaxwell like approaches (2)&(3) over approach (1), in that respect there is no distiniction between (2)/(3), and countervailing tradeoffs like having to modify bitcoin-cli and other things i mentioned above
10092017-07-17T19:18:38 <ryanofsky> and yes, it is clear that i am wayyy more concerned with backwards compatibility than other people
10102017-07-17T19:19:15 <jnewbery> In general, I'm also concerned about backwards compatibility. But in this specific case, I'm not
10112017-07-17T19:19:20 <ryanofsky> i'm probably just an outlier in that respect
10122017-07-17T19:20:08 <instagibbs> jnewbery, a brief(!) recap in the PR itself on the design choice being made would be nice for review/historicity sake
10132017-07-17T19:21:07 <jnewbery> > countervailing tradeoffs like having to modify bitcoin-cli
10142017-07-17T19:21:25 <ryanofsky> yes named arguments require no changes to bitcoin-cli
10152017-07-17T19:21:38 <sipa> ok, i just looked over #10849, and it is suffuciently simple that i don't think i care about implementation complexity compared to 10829
10162017-07-17T19:21:39 <gribble> https://github.com/bitcoin/bitcoin/issues/10849 | Multiwallet: simplest endpoint support by jonasschnelli · Pull Request #10849 · bitcoin/bitcoin · GitHub
10172017-07-17T19:22:15 <jnewbery> I think (2) is better in that respect. We need to modify bitcoin-cli, but we have those changes already coded. With (1), every script that calls wallet methods using bitcoin-cli needs to be changed because positional arguments are no longer supported
10182017-07-17T19:22:24 <jnewbery> or have I misunderstood?
10192017-07-17T19:22:30 *** d_t has joined #bitcoin-core-dev
10202017-07-17T19:22:45 *** tiagotrs has joined #bitcoin-core-dev
10212017-07-17T19:22:52 <sipa> jnewbery: that is also a good argument
10222017-07-17T19:23:21 <ryanofsky> jnewbery, you understood. i see that is an argument the other way because named arguments are safer
10232017-07-17T19:24:22 <jnewbery> with (2), the bitcoin-cli caller needs to change to include a `-usewallet` argument. With (1) the entire invocation needs to change
10242017-07-17T19:24:37 <ryanofsky> jnewbery, correct
10252017-07-17T19:24:59 <jnewbery> so with (2), it's just as safe. There's no default wallet - it has to be explicitly specified
10262017-07-17T19:25:18 <ryanofsky> i'm talking about general safety of named parameters vs positional arguments
10272017-07-17T19:25:51 <ryanofsky> i don't think discouraging positional arguments is bad, i actually think it is good
10282017-07-17T19:25:52 *** d_t_ has quit IRC
10292017-07-17T19:25:59 <jnewbery> oh, absolutely agree there - everyone should use named. But that's an orthogonal point
10302017-07-17T19:26:02 <sipa> ryanofsky: that seems orthogonal
10312017-07-17T19:26:05 <ryanofsky> i think changes to bitcoin-cli are simple but ugly
10322017-07-17T19:26:10 <ryanofsky> anyway these are minor points
10332017-07-17T19:26:30 <ryanofsky> i think we all understand tradeoffs between 1 / 2&3 at this point?
10342017-07-17T19:27:44 <ryanofsky> for those who have clear notions of what "conceptually a separate resource" means & who don't care about backwards compatibility, there is no difference between 2&3
10352017-07-17T19:29:12 <sipa> for those who don't have clear notions, you mean?
10362017-07-17T19:29:56 <ryanofsky> i'm saying i don't know clearly what "separate resource" means. it just seems like an arbitrary distinction
10372017-07-17T19:30:56 <ryanofsky> it just seems weird to me that you'd want to structure a path scheme around wallet filename, but if you're confident that this is the way to go, then great
10382017-07-17T19:31:04 <sipa> how about this: all parts of a request that are expected to be identical between all calls made by a single client application should go into a path
10392017-07-17T19:31:46 <jnewbery> Yes, I think we all understand the tradeoffs. My preference order is still 3 > 2 > 1, but I'm happy with any of them going in now, and then coming up with a stable design for all of this in time for v0.16.
10402017-07-17T19:32:18 *** promag has joined #bitcoin-core-dev
10412017-07-17T19:32:29 <ryanofsky> sipa, to me it's ugly you even want to make that distinction. aesthetically i prefer if all parts of request should just be treated as similarly as possible
10422017-07-17T19:33:26 <sipa> ryanofsky: actually, that sentence applies to (2) and (3) equally
10432017-07-17T19:33:42 <sipa> so replace 'path' with 'uri' in it
10442017-07-17T19:33:45 <ryanofsky> there are practical reasons (imo weak ones for wanting to make wallet special enough to go in url rather than json request). but making it root of brand new uri schema seems overboard to me
10452017-07-17T19:34:00 <jtimon> sipa: I guess we don't move "jsonrpc": "1.0" to the path because it's part of the rpc scheme specification or something?
10462017-07-17T19:34:27 <sipa> jtimon: no, it's part of the JSON-RPC spec
10472017-07-17T19:34:38 <sipa> oh, that's what you mean; yes
10482017-07-17T19:34:41 <ryanofsky> in (1) wallet is just one of many normal params. in (2) wallet is special enough to be a url param. in (3) wallet is root of a new uri-scheme
10492017-07-17T19:34:51 <jtimon> right, that's what I meant, but didn't rename json-rpc name
10502017-07-17T19:35:19 <ryanofsky> uri-path scheme i mean
10512017-07-17T19:35:22 <jtimon> wouldn't putting the wallet there violate the json-rpc spec ?
10522017-07-17T19:35:30 <sipa> no
10532017-07-17T19:35:40 <sipa> it's just a different URL you're using
10542017-07-17T19:37:55 <jtimon> no, I mean, putting wallet in the json data alongside method, jsonrpc, params and id
10552017-07-17T19:38:14 <ryanofsky> jtimon, that isn't (1) (2) or (3).
10562017-07-17T19:38:15 <sipa> that's not what's being suggested
10572017-07-17T19:38:26 <ryanofsky> (1) is just sticking it into params
10582017-07-17T19:38:28 <sipa> (1) suggests making the wallet part of 'params'
10592017-07-17T19:38:33 <jtimon> oh, I see
10602017-07-17T19:43:17 *** Chris_Stewart_5 has quit IRC
10612017-07-17T19:47:43 *** chjj has quit IRC
10622017-07-17T19:55:00 *** jamesob__ has joined #bitcoin-core-dev
10632017-07-17T19:55:35 *** jamesob has quit IRC
10642017-07-17T19:55:57 *** corebob has joined #bitcoin-core-dev
10652017-07-17T19:57:25 <promag> BlueMatt: saw some rpc functions, all have spaces in ()
10662017-07-17T19:57:46 *** Chris_Stewart_5 has joined #bitcoin-core-dev
10672017-07-17T19:58:45 <BlueMatt> promag: hmm, ok
10682017-07-17T20:00:17 <morcos> wumpus: not sure where we are with feature freeze, but i think #10707 is going to make it, looks ready for merge
10692017-07-17T20:00:19 <gribble> https://github.com/bitcoin/bitcoin/issues/10707 | Better API for estimatesmartfee RPC by morcos · Pull Request #10707 · bitcoin/bitcoin · GitHub
10702017-07-17T20:00:28 <morcos> #10672 can also be merged
10712017-07-17T20:00:30 <gribble> https://github.com/bitcoin/bitcoin/issues/10672 | Avoid division by zero in the case of a corrupt estimates file by practicalswift · Pull Request #10672 · bitcoin/bitcoin · GitHub
10722017-07-17T20:02:14 <BlueMatt> we should probably pull an 0.14 and just say that we're frozen with an exception for things already tagged that make it in the next day or two?
10732017-07-17T20:02:41 *** Aaronvan_ has joined #bitcoin-core-dev
10742017-07-17T20:03:52 <morcos> someone PLEASE tag #10830
10752017-07-17T20:03:54 <gribble> https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
10762017-07-17T20:03:59 <morcos> I keep forgetting about it b/c its not on the milestone list
10772017-07-17T20:04:25 <morcos> sipa: aren't you the one that was threatening to rip the entire wallet out if we didn't get a version of that in
10782017-07-17T20:04:37 <sipa> morcos: lol
10792017-07-17T20:04:58 <sipa> tagged
10802017-07-17T20:05:03 <morcos> thanks!
10812017-07-17T20:05:08 <jnewbery> I'm working on a cut-down version of #10830
10822017-07-17T20:05:09 <gribble> https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
10832017-07-17T20:05:19 <jnewbery> probably under a different PR number - sorry!
10842017-07-17T20:05:21 *** AaronvanW has quit IRC
10852017-07-17T20:05:42 <jnewbery> I think the prevailing view is we don't need the node sync pause stuff for 0.15
10862017-07-17T20:06:16 <sipa> jnewbery: you'd need to do something, though
10872017-07-17T20:06:20 <sipa> what is your suggestion?
10882017-07-17T20:07:10 <jnewbery> I'm trying to understand what's required. You've mentioned before that HD split makes things worse, but I can't understand why that would be true
10892017-07-17T20:07:34 <sipa> it doesn't; i was confused
10902017-07-17T20:07:46 <jtimon> so, regarding #9806 txoutsbyaddress, txoutsbyscript and all that...what are the plans for 0.15 again (if any), it seems I may have misunderstood things there
10912017-07-17T20:07:47 <jnewbery> ok, good
10922017-07-17T20:07:48 <gribble> https://github.com/bitcoin/bitcoin/issues/9806 | txoutsbyaddress index (take 3) by droark · Pull Request #9806 · bitcoin/bitcoin · GitHub
10932017-07-17T20:08:14 <jnewbery> so, I'll keep the stuff that marks all keys up to a used key as used
10942017-07-17T20:08:56 <jnewbery> gmaxwell says "we could couple that with something that prolongs keeping an encrypted wallet unlocked while syncing/rescanning is running". I'm just looking at how locking/unlocking works to understand what's required there
10952017-07-17T20:09:45 <sipa> i think you can just shutdown when the keypool goes below some threshold
10962017-07-17T20:10:10 <sipa> which can only happen when you're recovering from an old wallet backup anyway
10972017-07-17T20:10:51 <jnewbery> hows that? What if your wallet is encrypted and you can't top up?
10982017-07-17T20:11:11 <sipa> then you have a problem
10992017-07-17T20:11:21 *** Aaronvan_ has quit IRC
11002017-07-17T20:11:32 <sipa> because you're going to silently miss transactions
11012017-07-17T20:11:57 *** AaronvanW has joined #bitcoin-core-dev
11022017-07-17T20:12:54 <jnewbery> right. Sorry - I don't understand how your keypool can only go below a threshold if you're recovering from an old wallet backup
11032017-07-17T20:13:31 <sipa> during normal operation, you never see a key used on the network that you didn't create with getnewaddress, which will make you top up
11042017-07-17T20:16:02 *** AaronvanW has quit IRC
11052017-07-17T20:16:24 <jnewbery> ok, so I get a bunch of adddresses with getnewaddress, I hand them out, my wallet locks, and then I start seeing transactions to those addresses. What next? Isn't my unused keypool running down as I see those transactions?
11062017-07-17T20:16:44 <sipa> jnewbery: getnewaddress already marked those keys as used
11072017-07-17T20:16:59 <sipa> it can refuse to give you a new one before topping up
11082017-07-17T20:17:05 <jnewbery> ah, ok
11092017-07-17T20:17:13 <sipa> it does now, but the threshold is 0
11102017-07-17T20:17:18 <sipa> that's probably too low, but easy to change
11112017-07-17T20:17:50 <sipa> if you're able to avoid hitting a keypool of 0 when its maximum is 100, you're certainly able to avoid hitting 100 when the default is 1000
11122017-07-17T20:18:03 <jnewbery> ok, I'll take a look. Thanks
11132017-07-17T20:21:41 <promag> https://github.com/bitcoin/bitcoin/pull/9502 it's supposed to keep syncing headers? sipa?
11142017-07-17T20:22:22 <sipa> promag: i'm not sure what it's supposed to do, but i believe that is what it does yes
11152017-07-17T20:29:21 *** harrymm has quit IRC
11162017-07-17T20:30:58 <BlueMatt> sipa: can you update the state of #10526?
11172017-07-17T20:30:59 <gribble> https://github.com/bitcoin/bitcoin/issues/10526 | Force on-the-fly compaction during pertxout upgrade by sipa · Pull Request #10526 · bitcoin/bitcoin · GitHub
11182017-07-17T20:31:34 <BlueMatt> can we confirm if we need it for 15 or not?
11192017-07-17T20:34:57 <sipa> BlueMatt: willdo
11202017-07-17T20:35:15 *** chjj has joined #bitcoin-core-dev
11212017-07-17T20:38:53 <jnewbery> sipa: if we shutdown when the keypool goes below a threshold, how would I start with an old encrypted wallet that has fewer than the minimum threshold keypool? It'll shutdown before I have a change to unlock the wallet
11222017-07-17T20:40:32 <sipa> jnewbery: i don't know
11232017-07-17T20:45:41 *** harrymm has joined #bitcoin-core-dev
11242017-07-17T20:46:03 *** Dyaheon has quit IRC
11252017-07-17T20:46:33 *** Dyaheon has joined #bitcoin-core-dev
11262017-07-17T20:54:16 <jtimon> btw, with gettxout I think it should either include confirmed utxos when calling with include_mempool or have include_mempool renamed to mempool_only or something of the sort. At the very very least s/Whether to include the mempool/Only search in the mempool. Default: true/
11272017-07-17T20:54:19 <jtimon> thoughts ?
11282017-07-17T20:55:13 <sipa> jtimon: include_mempool is the wrong name
11292017-07-17T20:55:31 <sipa> it either presents the view of the utxo at the last block
11302017-07-17T20:55:43 <sipa> or the one as seen by the mempool
11312017-07-17T20:56:50 <jtimon> right, my point is that if we want to maintain the name, we can first consider the mempool, if not, the current utxo; and the caller can check whether "confirmations" == 0 or not
11322017-07-17T20:57:12 <jtimon> but I take it as you prefer just renaming, right?
11332017-07-17T20:57:23 <sipa> or properly documenting
11342017-07-17T20:57:43 <sipa> but the RPC is about viewing the utxo set... not random access to txouts
11352017-07-17T20:58:12 <sipa> we have two utxo sets... the one defined by the blockchain, and the one defined by the blockchain+mempool
11362017-07-17T20:58:14 *** jamesob has joined #bitcoin-core-dev
11372017-07-17T20:58:32 *** promag has quit IRC
11382017-07-17T20:59:14 *** nakaluna has quit IRC
11392017-07-17T21:00:16 <jnewbery> sipa: I can think of a couple of good solutions to the old encrypted wallet at start: 1. have a bitcoin-wallet util that can topup the keypool offline. 2. have a `loadwallet` RPC that can decrypt on load
11402017-07-17T21:00:16 *** jamesob__ has quit IRC
11412017-07-17T21:00:46 <jnewbery> obviously neither of those are in v0.15. It seems a shame to merge something that could make an encrypted wallet file unloadable
11422017-07-17T21:01:59 <jnewbery> there would be one way to recover a blocked wallet: invalidateblock at the wallet's best block, load and unlock the wallet, topup keypool, then reconsiderblock. But I don't think we should be telling users to do that!
11432017-07-17T21:02:37 *** AaronvanW has joined #bitcoin-core-dev
11442017-07-17T21:02:42 <jtimon> sipa: mhmm, but a confirmed utxo won't appear if you call with include_mempool
11452017-07-17T21:03:25 <sipa> jtimon: of course not
11462017-07-17T21:03:37 <sipa> it is not an unspent output, when looking at the mempool
11472017-07-17T21:03:48 <sipa> wait
11482017-07-17T21:03:54 <sipa> i'm not sure what you're saying
11492017-07-17T21:04:54 <sipa> jtimon: my answer is about a transaction that is unspent in the chain, but spent by a mempool tx
11502017-07-17T21:05:15 <jtimon> right, that won't appear if you chose include_mempoo=true
11512017-07-17T21:05:44 <sipa> yes, intentionally
11522017-07-17T21:05:49 <jtimon> ok
11532017-07-17T21:06:02 <sipa> because it is not an unspent output when looking at the mempool
11542017-07-17T21:06:52 *** laurentmt has joined #bitcoin-core-dev
11552017-07-17T21:07:09 <jtimon> oh, I see, but if it's both spent in the chain and the mempool it will appear, I was missing that, thanks
11562017-07-17T21:07:39 <sipa> unspent, you mean
11572017-07-17T21:07:59 <jtimon> perhaps we should test that too
11582017-07-17T21:08:06 <jtimon> yeah, unspent sorry
11592017-07-17T21:08:39 <jtimon> ok, so I think I'll write a PR correcting the documentation and testing that case
11602017-07-17T21:09:10 <sipa> cool
11612017-07-17T21:10:35 <jtimon> but before closing #10822, I would like to know more about what the general thoughts about gettxoutbyaddress, gettxoutbyscript and all that
11622017-07-17T21:10:36 <gribble> https://github.com/bitcoin/bitcoin/issues/10822 | RPC: Also serve txo from gettxout (not just utxo and mempool) by jtimon · Pull Request #10822 · bitcoin/bitcoin · GitHub
11632017-07-17T21:11:21 <sipa> jtimon: i believe other people have different opinions, but imho that does not belong in core
11642017-07-17T21:11:24 *** nakaluna has joined #bitcoin-core-dev
11652017-07-17T21:12:07 <sipa> or at least only if we modularize the code enough so that it's a totally separately pluggable thing
11662017-07-17T21:14:27 <jtimon> by thought was to have something like -scriptpubkeyindex analogous to txindex or something like that, but I see you don't like having -txindex already
11672017-07-17T21:14:46 <sipa> yes
11682017-07-17T21:15:11 <sipa> i'm very strongly opposed to any functionality that requires having a fully indexed blockchain
11692017-07-17T21:15:16 <jtimon> of course I don't need this to be in core for my purposes, but since people were talking about it I was trying to find synergies
11702017-07-17T21:15:34 <sipa> if it's just the utxo set... it's less bad, but i still think it's beyond our scope
11712017-07-17T21:16:00 <jtimon> but a -scriptpubkeyindex only for current utxo would be more acceptable?
11722017-07-17T21:16:07 <jtimon> ok
11732017-07-17T21:18:15 <jtimon> thanks, I will ask on the next meeting about if I haven't closed #10822 by the time if more people give me their opinion
11742017-07-17T21:18:17 <gribble> https://github.com/bitcoin/bitcoin/issues/10822 | RPC: Also serve txo from gettxout (not just utxo and mempool) by jtimon · Pull Request #10822 · bitcoin/bitcoin · GitHub
11752017-07-17T21:19:11 <sipa> i think we're a bit too busy with 0.15 :)
11762017-07-17T21:20:49 *** jamesob has quit IRC
11772017-07-17T21:21:22 *** jamesob has joined #bitcoin-core-dev
11782017-07-17T21:23:55 *** earlz has quit IRC
11792017-07-17T21:25:53 *** jamesob has quit IRC
11802017-07-17T21:29:08 *** earlz has joined #bitcoin-core-dev
11812017-07-17T21:33:36 *** earlz has quit IRC
11822017-07-17T21:33:48 *** earlz has joined #bitcoin-core-dev
11832017-07-17T21:35:03 *** Chris_Stewart_5 has quit IRC
11842017-07-17T21:38:53 <sipa> jnewbery: yeah, i don't know
11852017-07-17T21:39:11 <sipa> a cleaner solution is to actually support stopping a sync, and allowing it to continue with a keypool unlock
11862017-07-17T21:39:39 <sipa> but this is an unusual situation, which will only be reached when recovering from a backup anyway
11872017-07-17T21:41:20 <jcorgan> just fyi, we (gnuradio project) just got a github PR with a malware PDF attached to a comment, looked automated
11882017-07-17T21:41:54 <jcorgan> well, assuming it is malware, virustotal found 0 hits
11892017-07-17T21:44:33 <sipa> jcorgan: good to know, thanks
11902017-07-17T21:51:47 <jtimon> sipa: yeah, sure, no hurry on my part, I just thought something re gettxoutbyaddress for 0.15, I guess it was just for after 0.15
11912017-07-17T21:52:46 *** mmgen has quit IRC
11922017-07-17T21:56:32 *** earlz has quit IRC
11932017-07-17T21:56:43 *** earlz has joined #bitcoin-core-dev
11942017-07-17T21:57:14 *** jtimon has quit IRC
11952017-07-17T21:59:22 *** CubicEarth has joined #bitcoin-core-dev
11962017-07-17T22:01:02 *** d9b4bef9 has quit IRC
11972017-07-17T22:01:57 *** CubicEarth has quit IRC
11982017-07-17T22:02:08 *** d9b4bef9 has joined #bitcoin-core-dev
11992017-07-17T22:07:51 *** jannes has quit IRC
12002017-07-17T22:08:26 *** CubicEarth has joined #bitcoin-core-dev
12012017-07-17T22:13:03 <bitcoin-git> [bitcoin] sipa pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/0b019357ff09...fee0d803fb55
12022017-07-17T22:13:03 <bitcoin-git> bitcoin/master 8276e70 Chris Stewart: Adding assert to avoid a memory access violation inside of PartialMerkleTree::CalcHash()...
12032017-07-17T22:13:04 <bitcoin-git> bitcoin/master fee0d80 Pieter Wuille: Merge #9980: Fix mem access violation merkleblock...
12042017-07-17T22:13:17 <bitcoin-git> [bitcoin] sipa closed pull request #9980: Fix mem access violation merkleblock (master...fix_mem_access_violation_merkleblock) https://github.com/bitcoin/bitcoin/pull/9980
12052017-07-17T22:16:34 *** laurentmt has quit IRC
12062017-07-17T22:18:07 *** jcorgan has quit IRC
12072017-07-17T22:20:07 *** chjj has quit IRC
12082017-07-17T22:27:29 *** dermoth has quit IRC
12092017-07-17T22:27:37 *** spinza has quit IRC
12102017-07-17T22:27:55 *** dermoth has joined #bitcoin-core-dev
12112017-07-17T22:28:43 *** CubicEarth has quit IRC
12122017-07-17T22:30:20 *** CubicEarth has joined #bitcoin-core-dev
12132017-07-17T22:31:49 *** CubicEarth has quit IRC
12142017-07-17T22:32:39 *** chjj has joined #bitcoin-core-dev
12152017-07-17T22:32:41 *** Aaronvan_ has joined #bitcoin-core-dev
12162017-07-17T22:33:32 *** spinza has joined #bitcoin-core-dev
12172017-07-17T22:35:50 <bitcoin-git> [bitcoin] achow101 opened pull request #10857: [RPC] Add a deprecation warning to getinfo's output (master...deprecate-getinfo) https://github.com/bitcoin/bitcoin/pull/10857
12182017-07-17T22:36:04 *** AaronvanW has quit IRC
12192017-07-17T22:37:58 *** promag has joined #bitcoin-core-dev
12202017-07-17T22:42:03 *** discreteunit has joined #bitcoin-core-dev
12212017-07-17T22:43:46 *** tiagotrs has quit IRC
12222017-07-17T22:50:18 *** Dyaheon has quit IRC
12232017-07-17T22:53:16 *** Dyaheon has joined #bitcoin-core-dev
12242017-07-17T22:53:43 *** jcorgan has joined #bitcoin-core-dev
12252017-07-17T23:08:14 *** BashCo has quit IRC
12262017-07-17T23:09:04 *** BashCo has joined #bitcoin-core-dev
12272017-07-17T23:13:02 *** rockhouse has joined #bitcoin-core-dev
12282017-07-17T23:14:51 <bitcoin-git> [bitcoin] achow101 opened pull request #10858: [RPC] Add "errors" field to getblockchaininfo and unify "errors" field in get*info RPCs (master...getblockchaininfo-errors) https://github.com/bitcoin/bitcoin/pull/10858
12292017-07-17T23:26:10 *** discreteunit has quit IRC
12302017-07-17T23:27:49 *** chjj has quit IRC
12312017-07-17T23:34:46 *** justan0theruser has joined #bitcoin-core-dev
12322017-07-17T23:36:57 *** justanotheruser has quit IRC
12332017-07-17T23:37:36 *** coredump_ has joined #bitcoin-core-dev
12342017-07-17T23:41:11 *** chjj has joined #bitcoin-core-dev
12352017-07-17T23:41:22 *** CubicEarth has joined #bitcoin-core-dev
12362017-07-17T23:48:13 *** nakaluna has quit IRC
12372017-07-17T23:53:05 *** Aaronvan_ has quit IRC
12382017-07-17T23:54:51 <bitcoin-git> [bitcoin] sipa pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/fee0d803fb55...75b5643c47c3
12392017-07-17T23:54:51 <bitcoin-git> bitcoin/master 439c4e8 Alex Morcos: Improve api to estimatesmartfee...
12402017-07-17T23:54:52 <bitcoin-git> bitcoin/master 06bcdb8 Alex Morcos: Convert named argument from nblocks to conf_target...
12412017-07-17T23:54:52 <bitcoin-git> bitcoin/master 75b5643 Pieter Wuille: Merge #10707: Better API for estimatesmartfee RPC...
12422017-07-17T23:55:15 <bitcoin-git> [bitcoin] sipa closed pull request #10707: Better API for estimatesmartfee RPC (master...bettersmartfeeapi) https://github.com/bitcoin/bitcoin/pull/10707