12017-07-19T00:06:34 *** jamesob_ has quit IRC
22017-07-19T00:15:00 *** kexkey has joined #bitcoin-core-dev
32017-07-19T00:18:23 *** kexkey has quit IRC
42017-07-19T00:19:02 *** kexkey has joined #bitcoin-core-dev
52017-07-19T00:19:14 *** darawk has joined #bitcoin-core-dev
62017-07-19T00:22:04 *** chjj has quit IRC
72017-07-19T00:22:06 *** talmai has joined #bitcoin-core-dev
82017-07-19T00:31:49 *** Murch has quit IRC
92017-07-19T00:35:19 *** chjj has joined #bitcoin-core-dev
102017-07-19T00:36:37 *** discreteunit has joined #bitcoin-core-dev
112017-07-19T00:44:04 <bitcoin-git> [bitcoin] achow101 opened pull request #10874: [RPC] getblockchaininfo: Loop through the bip9 soft fork deployments instead of hard coding (master...getblockchaininfo-bip9-loop) https://github.com/bitcoin/bitcoin/pull/10874
122017-07-19T00:48:45 *** ivan has quit IRC
132017-07-19T00:51:36 *** ayy1337|2 has quit IRC
142017-07-19T00:54:59 *** jnewshoes has joined #bitcoin-core-dev
152017-07-19T00:59:16 *** Guest33550 has quit IRC
162017-07-19T01:01:03 *** Ylbam has quit IRC
172017-07-19T01:02:04 *** discreteunit has quit IRC
182017-07-19T01:07:11 *** dabura667 has joined #bitcoin-core-dev
192017-07-19T01:13:52 *** talmai has quit IRC
202017-07-19T01:17:15 *** sdfgsdfg has joined #bitcoin-core-dev
212017-07-19T01:17:21 *** Dyaheon has quit IRC
222017-07-19T01:17:38 *** ill has joined #bitcoin-core-dev
232017-07-19T01:19:46 *** Dyaheon has joined #bitcoin-core-dev
242017-07-19T01:26:34 <bitcoin-git> [bitcoin] achow101 opened pull request #10875: BIP 91 deployment parameters (master...bip91-dep-params) https://github.com/bitcoin/bitcoin/pull/10875
252017-07-19T01:27:09 <bitcoin-git> [bitcoin] DJMorgan opened pull request #10876: What do you mean a bug in the latest version? (0.14...master) https://github.com/bitcoin/bitcoin/pull/10876
262017-07-19T01:27:32 *** chjj has quit IRC
272017-07-19T01:30:16 *** sdfgsdfg has quit IRC
282017-07-19T01:31:17 <gmaxwell> ::sigh::
292017-07-19T01:37:01 <bitcoin-git> [bitcoin] sipa closed pull request #10876: What do you mean a bug in the latest version? (0.14...master) https://github.com/bitcoin/bitcoin/pull/10876
302017-07-19T01:38:07 *** ayy1337|2 has joined #bitcoin-core-dev
312017-07-19T01:40:24 *** chjj has joined #bitcoin-core-dev
322017-07-19T01:42:43 *** goatpig has quit IRC
332017-07-19T01:43:04 *** sdfgsdfg has joined #bitcoin-core-dev
342017-07-19T01:49:50 *** coredump_ has quit IRC
352017-07-19T01:52:47 *** ivan has joined #bitcoin-core-dev
362017-07-19T01:57:16 *** AsadSalman has joined #bitcoin-core-dev
372017-07-19T01:57:43 *** kexkey_ has joined #bitcoin-core-dev
382017-07-19T01:59:29 *** kexkey has quit IRC
392017-07-19T01:59:45 *** sdfgsdfg has quit IRC
402017-07-19T02:00:13 *** sdfgsdfg has joined #bitcoin-core-dev
412017-07-19T02:00:15 *** kexkey_ is now known as kexkey
422017-07-19T02:05:38 *** AsadSalman has quit IRC
432017-07-19T02:17:54 *** arowser has quit IRC
442017-07-19T02:18:53 *** darawk has quit IRC
452017-07-19T02:24:01 *** arowser has joined #bitcoin-core-dev
462017-07-19T02:33:37 *** sdfgsdfg has quit IRC
472017-07-19T02:33:40 *** chjj has quit IRC
482017-07-19T02:56:47 *** ayy1337|2 has quit IRC
492017-07-19T02:57:22 *** jamesob_ has joined #bitcoin-core-dev
502017-07-19T03:03:27 *** jnewshoes has quit IRC
512017-07-19T03:06:14 *** coredump_ has joined #bitcoin-core-dev
522017-07-19T03:12:06 *** BashCo has quit IRC
532017-07-19T03:13:16 *** BashCo has joined #bitcoin-core-dev
542017-07-19T03:22:56 *** coredump_ has quit IRC
552017-07-19T03:23:38 *** Dyaheon has quit IRC
562017-07-19T03:24:28 *** Dyaheon has joined #bitcoin-core-dev
572017-07-19T03:30:45 *** sdfgsdfg has joined #bitcoin-core-dev
582017-07-19T03:35:18 *** Giszmo has quit IRC
592017-07-19T03:50:05 *** ill has quit IRC
602017-07-19T04:01:55 <bitcoin-git> [bitcoin] kallewoof opened pull request #10877: [rpc] Verbose flags for chaining and scripting (master...verbose-flagging) https://github.com/bitcoin/bitcoin/pull/10877
612017-07-19T04:13:02 *** corebob has quit IRC
622017-07-19T04:19:41 <bitcoin-git> [bitcoin] dongcarl opened pull request #10878: Docs: Fix markdown line breaks in init.md (master...patch-1) https://github.com/bitcoin/bitcoin/pull/10878
632017-07-19T04:26:20 *** sdfgsdfg has quit IRC
642017-07-19T04:26:42 *** sdfgsdfg has joined #bitcoin-core-dev
652017-07-19T04:28:20 *** sdfgsdfg has quit IRC
662017-07-19T04:28:53 *** sdfgsdfg has joined #bitcoin-core-dev
672017-07-19T04:42:20 *** sdfgsdfg has quit IRC
682017-07-19T04:42:43 *** sdfgsdfg has joined #bitcoin-core-dev
692017-07-19T04:44:20 *** sdfgsdfg has quit IRC
702017-07-19T04:44:42 *** sdfgsdfg has joined #bitcoin-core-dev
712017-07-19T04:50:20 *** sdfgsdfg has quit IRC
722017-07-19T04:50:43 *** sdfgsdfg has joined #bitcoin-core-dev
732017-07-19T04:52:20 *** sdfgsdfg has quit IRC
742017-07-19T04:52:38 <bitcoin-git> [bitcoin] kallewoof opened pull request #10879: Difficulty adjustment (master...difficulty-adjustment) https://github.com/bitcoin/bitcoin/pull/10879
752017-07-19T04:52:42 *** sdfgsdfg has joined #bitcoin-core-dev
762017-07-19T04:52:44 <bitcoin-git> [bitcoin] kallewoof closed pull request #10879: Difficulty adjustment (master...difficulty-adjustment) https://github.com/bitcoin/bitcoin/pull/10879
772017-07-19T04:58:20 *** sdfgsdfg has quit IRC
782017-07-19T04:58:42 *** sdfgsdfg has joined #bitcoin-core-dev
792017-07-19T05:00:20 *** sdfgsdfg has quit IRC
802017-07-19T05:00:43 *** sdfgsdfg has joined #bitcoin-core-dev
812017-07-19T05:12:59 *** jamesob_ has quit IRC
822017-07-19T05:21:58 *** kallewoof has quit IRC
832017-07-19T05:25:20 *** schnerchi has quit IRC
842017-07-19T05:26:20 *** sdfgsdfg has quit IRC
852017-07-19T05:26:43 *** sdfgsdfg has joined #bitcoin-core-dev
862017-07-19T05:28:41 *** schnerchi has joined #bitcoin-core-dev
872017-07-19T05:32:20 *** sdfgsdfg has quit IRC
882017-07-19T05:32:39 *** sdfgsdfg has joined #bitcoin-core-dev
892017-07-19T05:36:01 *** kallewoof has joined #bitcoin-core-dev
902017-07-19T05:36:04 *** d_t has quit IRC
912017-07-19T05:38:20 *** sdfgsdfg has quit IRC
922017-07-19T05:38:45 *** sdfgsdfg has joined #bitcoin-core-dev
932017-07-19T05:40:20 *** sdfgsdfg has quit IRC
942017-07-19T05:40:44 *** sdfgsdfg has joined #bitcoin-core-dev
952017-07-19T05:42:21 *** goatpig has joined #bitcoin-core-dev
962017-07-19T05:43:38 *** kallewoof has quit IRC
972017-07-19T05:51:03 *** kallewoof has joined #bitcoin-core-dev
982017-07-19T06:01:35 *** kexkey has quit IRC
992017-07-19T06:12:48 *** Ylbam has joined #bitcoin-core-dev
1002017-07-19T06:30:34 *** kallewoof has quit IRC
1012017-07-19T06:33:47 *** kallewoof has joined #bitcoin-core-dev
1022017-07-19T06:41:06 *** d_t has joined #bitcoin-core-dev
1032017-07-19T06:50:07 <bitcoin-git> [bitcoin] eklitzke opened pull request #10880: Update .gitignore to ignore more test files (master...gitignore) https://github.com/bitcoin/bitcoin/pull/10880
1042017-07-19T07:12:13 *** BashCo has quit IRC
1052017-07-19T07:12:52 *** BashCo has joined #bitcoin-core-dev
1062017-07-19T07:13:34 *** eck has joined #bitcoin-core-dev
1072017-07-19T07:22:39 *** paveljanik has quit IRC
1082017-07-19T07:22:48 <eck> what's the recommended way for me to test a change before submitting a PR? is "make check" sufficient?
1092017-07-19T07:23:16 <gmaxwell> eck: no, read test/README.md
1102017-07-19T07:24:02 <eck> thanks
1112017-07-19T07:24:42 <gmaxwell> test/functional/test_runner.py
1122017-07-19T07:24:52 <gmaxwell> is what you want to run in addition to a make check.
1132017-07-19T07:25:11 <gmaxwell> of course it depends on what you're doing, for some things make check is sufficient... or just a particular one of the functional tests.
1142017-07-19T07:25:17 <gmaxwell> but if you don't know, better to run them all.
1152017-07-19T07:25:45 <gmaxwell> Our CI setup will run them on whatever you submit, but if it fails you'll still need to be able to run them locally to figure out what happened, so its best to get in the habbit of it.
1162017-07-19T07:41:36 <bitcoin-git> [bitcoin] eklitzke opened pull request #10881: trivial: fix various pyflakes/vulture warnings (master...vulture) https://github.com/bitcoin/bitcoin/pull/10881
1172017-07-19T07:48:49 *** d_t has quit IRC
1182017-07-19T08:07:42 *** ADheaume has joined #bitcoin-core-dev
1192017-07-19T08:08:50 *** ADheaume has quit IRC
1202017-07-19T08:12:32 *** Dyaheon has quit IRC
1212017-07-19T08:13:13 *** Dyaheon has joined #bitcoin-core-dev
1222017-07-19T08:37:13 *** AaronvanW has joined #bitcoin-core-dev
1232017-07-19T08:42:31 *** vicenteH has joined #bitcoin-core-dev
1242017-07-19T08:47:56 *** darawk has joined #bitcoin-core-dev
1252017-07-19T08:50:22 *** timothy has joined #bitcoin-core-dev
1262017-07-19T08:52:58 *** riemann has joined #bitcoin-core-dev
1272017-07-19T09:01:34 *** Aaronvan_ has joined #bitcoin-core-dev
1282017-07-19T09:04:27 *** AaronvanW has quit IRC
1292017-07-19T09:17:23 *** darawk has quit IRC
1302017-07-19T09:29:56 *** chjj has joined #bitcoin-core-dev
1312017-07-19T09:46:24 *** laurentmt has joined #bitcoin-core-dev
1322017-07-19T09:46:38 *** laurentmt has quit IRC
1332017-07-19T09:54:08 <bitcoin-git> [bitcoin] jonasschnelli pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9e8d6a3fb43a...a6ec5802b0a9
1342017-07-19T09:54:08 <bitcoin-git> bitcoin/master e0d4592 practicalswift: Avoid redundant redeclaration of GetWarnings(const string&)...
1352017-07-19T09:54:09 <bitcoin-git> bitcoin/master a6ec580 Jonas Schnelli: Merge #10864: Avoid redundant redeclaration of GetWarnings(const string&)...
1362017-07-19T09:54:41 <bitcoin-git> [bitcoin] jonasschnelli closed pull request #10864: Avoid redundant redeclaration of GetWarnings(const string&) (master...GetWarnings-in-warnings.h) https://github.com/bitcoin/bitcoin/pull/10864
1372017-07-19T09:54:56 <jonasschnelli> Oh. I guess I should not have merged this because of the freeze.
1382017-07-19T09:54:58 <jonasschnelli> Sry
1392017-07-19T10:03:04 *** AaronvanW has joined #bitcoin-core-dev
1402017-07-19T10:03:23 *** dabura667 has quit IRC
1412017-07-19T10:04:35 *** Aaronvan_ has quit IRC
1422017-07-19T11:00:30 *** chjj has quit IRC
1432017-07-19T11:10:22 *** BashCo has quit IRC
1442017-07-19T11:11:10 *** BashCo has joined #bitcoin-core-dev
1452017-07-19T11:13:35 *** paveljanik has joined #bitcoin-core-dev
1462017-07-19T11:36:25 *** jtimon has quit IRC
1472017-07-19T11:39:07 *** Dyaheon has quit IRC
1482017-07-19T11:40:25 *** Dyaheon has joined #bitcoin-core-dev
1492017-07-19T11:53:07 *** JackH has joined #bitcoin-core-dev
1502017-07-19T12:57:04 *** vicenteH has quit IRC
1512017-07-19T13:07:33 *** talmai has joined #bitcoin-core-dev
1522017-07-19T13:11:20 *** vicenteH has joined #bitcoin-core-dev
1532017-07-19T13:12:14 *** riemann has quit IRC
1542017-07-19T13:17:10 *** goatpig has quit IRC
1552017-07-19T13:20:00 *** Giszmo has joined #bitcoin-core-dev
1562017-07-19T13:21:08 *** riemann has joined #bitcoin-core-dev
1572017-07-19T13:25:17 *** arowser has quit IRC
1582017-07-19T13:31:32 *** arowser has joined #bitcoin-core-dev
1592017-07-19T13:31:53 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1602017-07-19T13:32:05 *** Giszmo has quit IRC
1612017-07-19T13:44:54 *** ayy1337|2 has joined #bitcoin-core-dev
1622017-07-19T13:46:32 *** Giszmo has joined #bitcoin-core-dev
1632017-07-19T13:54:34 <jonasschnelli> I can't reproduce the following travis issue locally (by using Ubuntu 14.04 and the same build configuration): https://travis-ci.org/bitcoin/bitcoin/jobs/254826520#L2299
1642017-07-19T13:54:50 <jonasschnelli> Maybe someone has an idea why I get a "/usr/include/c++/4.8/debug/safe_iterator.h:279:error: attempt to dereference a singular iterator."
1652017-07-19T13:55:29 *** Giszmo has quit IRC
1662017-07-19T13:59:32 *** talmai has quit IRC
1672017-07-19T14:02:00 *** Guest32541 has quit IRC
1682017-07-19T14:05:12 *** Aaronvan_ has joined #bitcoin-core-dev
1692017-07-19T14:06:41 *** AaronvanW has quit IRC
1702017-07-19T14:10:21 <cfields> jonasschnelli: that one uses everything built with extra debugging defines
1712017-07-19T14:10:40 <jonasschnelli> cfields: D_GLIBCXX_DEBUG, right?
1722017-07-19T14:10:46 <cfields> jonasschnelli: -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC
1732017-07-19T14:11:00 <cfields> make -C depends DEBUG=1
1742017-07-19T14:11:12 <jonasschnelli> Ah.. the dependencies...
1752017-07-19T14:11:43 <jonasschnelli> cfields: Just passing -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC into ./configure won't be "enought"?
1762017-07-19T14:12:19 <cfields> jonasschnelli: iirc all the deps have to be built with those flags otherwise wonky things happen
1772017-07-19T14:12:23 <cfields> but i could be misremembering
1782017-07-19T14:12:35 <jonasschnelli> okay. Let me try that...
1792017-07-19T14:12:36 <jonasschnelli> (via depends)
1802017-07-19T14:13:03 <cfields> jonasschnelli: those errors are almost always foo.empty() && foo[0]
1812017-07-19T14:13:28 <jonasschnelli> cfields: accessing an index that is not available?
1822017-07-19T14:15:02 *** cheese_ has joined #bitcoin-core-dev
1832017-07-19T14:15:02 <cfields> jonasschnelli: ah, looks like a "singular" iterator is one that can't be reached.
1842017-07-19T14:15:15 <cfields> so probably more like *foo.end()
1852017-07-19T14:15:23 *** Aaronvan_ is now known as AaronvanW
1862017-07-19T14:16:59 <jonasschnelli> cfields: Thanks... I'll check the code.
1872017-07-19T14:18:30 *** Cheeseo has quit IRC
1882017-07-19T14:25:11 <wumpus> re: #10873 we really need to add a comment, this isn't the first time someone has tried to replace the signal handling with a mutex
1892017-07-19T14:25:13 <gribble> https://github.com/bitcoin/bitcoin/issues/10873 | Use a condition variable for shutdown notifications by eklitzke · Pull Request #10873 · bitcoin/bitcoin · GitHub
1902017-07-19T14:25:40 <wumpus> this won't work for handling UNIX signals, certainly not in a portable way
1912017-07-19T14:26:02 <jonasschnelli> wumpus: Yes. How long was it ago since I tried it. :)
1922017-07-19T14:26:36 <jonasschnelli> I didn't commented because I tried with a boost signal rather then a mutex
1932017-07-19T14:26:46 <jonasschnelli> Wasn't sure if this is portable or not
1942017-07-19T14:27:15 <cfields> jonasschnelli: the list of things you're actually allowed to use is remarkably small
1952017-07-19T14:27:41 <jonasschnelli> cfields: Yes. I once checked... atomics should be fine... but I guess conditional vars aren't
1962017-07-19T14:27:54 <wumpus> yes, setting a flag is fine
1972017-07-19T14:28:17 <wumpus> I think you are allowed to read/write pipes as well
1982017-07-19T14:29:43 <wumpus> there are some tricks that could be taken over from other daemons, but it might not be worth the trouble
1992017-07-19T14:30:08 *** Gnof has joined #bitcoin-core-dev
2002017-07-19T14:30:46 *** sdfgsdfg has quit IRC
2012017-07-19T14:37:48 <wumpus> maybe there's even some obscure trick with sigwait() that would work
2022017-07-19T14:45:04 *** Murch has joined #bitcoin-core-dev
2032017-07-19T14:47:09 *** promag has joined #bitcoin-core-dev
2042017-07-19T14:47:14 *** promag has left #bitcoin-core-dev
2052017-07-19T14:47:18 *** promag has joined #bitcoin-core-dev
2062017-07-19T14:47:26 <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/a6ec5802b0a9...9022aa37226b
2072017-07-19T14:47:28 <bitcoin-git> bitcoin/master b138585 Alex Morcos: Remove factor of 3 from definition of dust....
2082017-07-19T14:47:28 <bitcoin-git> bitcoin/master f4d00e6 Alex Morcos: Add a discard_rate...
2092017-07-19T14:47:29 <bitcoin-git> bitcoin/master 9022aa3 Wladimir J. van der Laan: Merge #10817: Redefine Dust and add a discard_rate...
2102017-07-19T14:47:57 <bitcoin-git> [bitcoin] laanwj closed pull request #10817: Redefine Dust and add a discard_rate (master...discardmore) https://github.com/bitcoin/bitcoin/pull/10817
2112017-07-19T14:47:59 *** riemann has quit IRC
2122017-07-19T14:48:08 <promag> is practicalswift around?
2132017-07-19T14:50:20 <jonasschnelli> jnewbery, ryanofsky: continue the discussion about -wallet vs -usewallet here?
2142017-07-19T14:50:50 <jonasschnelli> I can't follow the argument of having -wallet for both use-cases (define which wallet to use and for adding a wallet to the daemon).
2152017-07-19T14:51:12 <jonasschnelli> Why would you want the use same argument for two different things?
2162017-07-19T14:51:55 <jonasschnelli> In the daemon case, you define a wallet to add to the sync process. For cli, you want to define which of those availavle wallets you want to use (1:n)
2172017-07-19T14:52:35 <ryanofsky> jonasschnelli, maybe you can give a practical example of why having different argument names is useful
2182017-07-19T14:53:15 <ryanofsky> obviously command line arguments can have different meanings on different commands. if you add --help or --verbose to a unix command it will do different things depending on the command
2192017-07-19T14:53:36 <jonasschnelli> ryanofsky: a) because its to different things (and users should learn that from the beginning), b) -usewallet could be reused for bitcoin-Qt (until there is a GUI way to select the wallet)
2202017-07-19T14:53:46 <jonasschnelli> "two
2212017-07-19T14:54:53 <jnewbery> jonasschnelli, for the reasons I gave in https://github.com/bitcoin/bitcoin/pull/10868#issuecomment-316409725 - I think the name usewallet is meaningless and confusing
2222017-07-19T14:54:53 <ryanofsky> can you explain qt use case a little more? qt runs a full node & wallet
2232017-07-19T14:55:05 <jonasschnelli> Users now need to learn that they can run multiple wallets by providing multiple of the same -wallet arguments (it's already kind of confusing, most users would expect a comma separator argument)
2242017-07-19T14:55:28 <jonasschnelli> ryanofsky; Right now, Qt always uses the wallet at index 0
2252017-07-19T14:56:05 <jonasschnelli> I think having then -wallet for CLI, they would need to learn that in one case they specify multiple wallets while in the other case they specify a single wallet... seems confusing to me
2262017-07-19T14:56:16 <ryanofsky> right, so what do you need a -usewallet option for in context of qt?
2272017-07-19T14:56:55 <jonasschnelli> ryanofsky: Qt: until we have a wallet selection via GUI, you could define which of those wallets is accessible over Qt.
2282017-07-19T14:57:58 <ryanofsky> jonasschnelli, right now with qt, you already have it serve multiple wallets, and you can choose which wallet will be shown in the display by putting the wallet first. there is no need for -usewallet
2292017-07-19T14:58:21 <ryanofsky> adding -usewallet for qt would again just be adding confusion & complexity for no reason
2302017-07-19T14:58:44 <jonasschnelli> Yes. Agree that this would work, though do we really want to depend on orders of arguments?
2312017-07-19T14:58:53 <ryanofsky> no, we want to add a dropdown or tab
2322017-07-19T14:59:00 <jonasschnelli> Yes. Indeed.
2332017-07-19T14:59:17 <ryanofsky> i'm saying adding "usewallet" is not clarifying, it is just throwing in more complexity
2342017-07-19T14:59:22 <jonasschnelli> Yes. I'm not sure if the -usewallet case for Qt is valid... though I think it is for cli
2352017-07-19T14:59:39 <jonasschnelli> It is already complex.
2362017-07-19T14:59:40 <ryanofsky> -wallet=filename everywhere should just work
2372017-07-19T15:00:35 <jonasschnelli> So users need to know that in one case (daemon) it's the key to "have multiple" while in the other case (cli) is the key to reduce it from multiple to one?
2382017-07-19T15:00:37 <ryanofsky> nobody objects that "cp" and "mv" both take "--help" or "--verbose" arguments because verbose output is different in both cases, it's just a weird argument
2392017-07-19T15:01:15 <jonasschnelli> --help is always help. Not once setting the -help information and once reading it
2402017-07-19T15:01:34 <ryanofsky> users do need to know that daemon accepts multiple -wallet= arguments to load multiple wallets. i'm not sure why that is a problem
2412017-07-19T15:01:59 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9022aa37226b...d445a2c2eaa1
2422017-07-19T15:02:00 <bitcoin-git> bitcoin/master 1c9b818 Andrew Chow: getinfo deprecation warning
2432017-07-19T15:02:00 <bitcoin-git> bitcoin/master d445a2c Wladimir J. van der Laan: Merge #10857: [RPC] Add a deprecation warning to getinfo's output...
2442017-07-19T15:02:18 <ryanofsky> users should just be shown an error if they are expecting bitcoin-cli to send an single rpc command to multiple wallets
2452017-07-19T15:02:41 <bitcoin-git> [bitcoin] laanwj closed pull request #10857: [RPC] Add a deprecation warning to getinfo's output (master...deprecate-getinfo) https://github.com/bitcoin/bitcoin/pull/10857
2462017-07-19T15:02:54 <jonasschnelli> It's just my opinion. I can live with the pure -wallet for everything approach.
2472017-07-19T15:03:12 <ryanofsky> sounds good to me
2482017-07-19T15:03:36 <jnewbery> I think the error message in my PR can be improved, but russ is correct - the error should inform the user that they need to specify -wallet on the command line
2492017-07-19T15:03:38 <jonasschnelli> I wonder what other developers think...
2502017-07-19T15:04:10 <jonasschnelli> jnewbery: but that is how it currently works (just with -usewallet), right?
2512017-07-19T15:04:29 <ryanofsky> pr needs reviews, so i also want other developers to chime in :)
2522017-07-19T15:04:40 <jonasschnelli> I just can't follow why you would want to add code to differenciate -wallet in bitcoin conf to -wallet passed into bitcoin-cli
2532017-07-19T15:06:32 <ryanofsky> that is a drawback. if you want bitcoin-cli to be able to use wallet filename from a config file, you have to call the option something other than -wallet
2542017-07-19T15:07:44 <jnewbery> but how -usewallet is interpreted is differentiated as well - acted on by bitcoin-cli, ignored by bitcoind, and you haven't decided on whether it should be ignored or not by bitcoin-qt
2552017-07-19T15:07:56 <ryanofsky> reason i think tradeoff is worth is is that -wallet=filename means our tools do not silently ignore bad wallet filenames, and there is weirdness and option interactions to have to think about
2562017-07-19T15:10:53 *** BashCo has quit IRC
2572017-07-19T15:11:32 *** BashCo has joined #bitcoin-core-dev
2582017-07-19T15:15:06 <jonasschnelli> Maybe something we should discuss at this weeks IRC meeting
2592017-07-19T15:21:25 <ryanofsky> i'd prefer pr reviews & acks but ok :)
2602017-07-19T15:25:31 <jnewbery> I agree. I think Russ is the only one who's reviewed/tested the code so far. But happy to discuss further at the meeting
2612017-07-19T15:32:27 *** Dyaheon has quit IRC
2622017-07-19T15:34:12 *** Dyaheon has joined #bitcoin-core-dev
2632017-07-19T15:38:04 *** Guyver2 has joined #bitcoin-core-dev
2642017-07-19T15:39:18 <sipa> jonasschnelli: i though the same way
2652017-07-19T15:39:50 *** mmgen has joined #bitcoin-core-dev
2662017-07-19T15:40:05 <sipa> jonasschnelli: but it's not so different from how -rpcport for bitcoind means "the port to listen on" nd for bitcoin-cli means "the port to connect to"
2672017-07-19T15:40:55 <sipa> the ignoring of arguments in the config file is weird... but it is (almost) just an implementation detail... in practice you always get the expected behaviour
2682017-07-19T15:41:05 <sipa> evwn if it were implemwnted differentlt
2692017-07-19T15:50:40 *** mmgen has quit IRC
2702017-07-19T15:53:05 *** promag has quit IRC
2712017-07-19T15:54:08 <wumpus> I don't think re-using -wallet for bitcoin-cli is a good idea
2722017-07-19T15:55:17 <wumpus> re-using rpcport is clever because the setting on client and server side happens to match up w/ the interface in between
2732017-07-19T15:56:16 <wumpus> if it'd be possible to specify multiple rpcports in the server config, we'd have the same problem there
2742017-07-19T15:56:40 <wumpus> -rpcbind is allowed to be used multiple times, but isn't used by the client at all
2752017-07-19T15:57:10 <wumpus> e.g. a multi-option on the server side shouldn't be a single option on the client side, given we parse the same configuration file
2762017-07-19T15:59:12 <wumpus> <ryanofsky> nobody objects that "cp" and "mv" both take "--help" or "--verbose" arguments because verbose output is different in both cases, it's just a weird argument <- yes, but cp and mv don't read their arguments from a configuration file
2772017-07-19T15:59:28 <wumpus> e.g. you might want to set a default wallet for bitcoin-cli to use in the configuration file, you can't use -wallet for that
2782017-07-19T15:59:55 <wumpus> if bitconi-cli read a different configuration file I'd have agreed there was no conflict
2792017-07-19T16:00:11 <wumpus> but that's not where we're coming from, or even where we're going
2802017-07-19T16:00:23 <ryanofsky> wumpus, yes, the tradeoff for having bitcoin-cli accept a wallet argument is having to ignore wallet values from the config file
2812017-07-19T16:00:31 <wumpus> that sounds stupid
2822017-07-19T16:00:48 <wumpus> why would you be so insistent in using -wallet for the option to special case it in config file reading?
2832017-07-19T16:00:57 <wumpus> just use a different argument name and be done with it
2842017-07-19T16:01:03 <ryanofsky> ok, well that's the tradeoff, we've discussed what i think are better options for allowing default wallets
2852017-07-19T16:01:16 <ryanofsky> i'm not insistent on anything
2862017-07-19T16:01:20 <wumpus> what is the trade-off? why would you insist on using -wallet at all?
2872017-07-19T16:01:32 <wumpus> it causes a conflict so imo the option should not be named that
2882017-07-19T16:01:49 <ryanofsky> i'm just saying what i think, which is that using different arguments is confusing, and being able to read wallet from config file is not actually useful compared to better options
2892017-07-19T16:01:50 <wumpus> making a special workarounds just to be able to call it -wallet is strange
2902017-07-19T16:02:11 <wumpus> if bitcoin-cli would not read the config file at all, or a different config file, I'd be ok with it
2912017-07-19T16:02:12 <ryanofsky> ok, well that's my strange argument :)
2922017-07-19T16:02:25 <wumpus> but we can't just do that, certainly not for 0.15
2932017-07-19T16:02:30 <ryanofsky> if you object, then go with usewallet and wallet
2942017-07-19T16:02:39 <wumpus> special-casing just seems weird
2952017-07-19T16:02:43 <wumpus> I really dislike it
2962017-07-19T16:02:57 <wumpus> 'why would all options be read from the config file except for X?' that's just drunk
2972017-07-19T16:03:18 <jnewbery> having a `useargument` argument that is used by bitcoin-cli but is not used by bitcoind seems strange to me
2982017-07-19T16:03:43 <wumpus> why owuld that be strange? bitcoin-cli has lots of arguments that are not used by bitcoind
2992017-07-19T16:03:48 <wumpus> why would they need to overlap?
3002017-07-19T16:03:57 <ryanofsky> yeah, i just think this wallet config file thing is a strange corner case compared to everyday usage
3012017-07-19T16:04:10 <wumpus> what would bitcoind want to do with it ta all?
3022017-07-19T16:04:19 <jnewbery> Because of the arguments I give here: https://github.com/bitcoin/bitcoin/pull/10868#issuecomment-316409725
3032017-07-19T16:04:22 <wumpus> it'd be the default wallet for the *client*
3042017-07-19T16:04:22 <sipa> if the '-use' sounds liie meaningless, use -rpcwallet maybe
3052017-07-19T16:04:29 <wumpus> sipa: yes! sounds good to me
3062017-07-19T16:04:42 <ryanofsky> i'd be fine with rpcwallet, that's what i suggested when the arg was first added
3072017-07-19T16:04:53 <instagibbs> rpcwallet is nicer
3082017-07-19T16:04:59 <wumpus> I don't really care what name, it should just not be -wallet
3092017-07-19T16:05:32 <wumpus> though I guess rpcwallet would be more consistent with some other rpc options
3102017-07-19T16:05:59 <jnewbery> I still think ryanofksy's point about protecting against users putting in bad -wallet arguments is valid, but -rpcwallet is definitely a step up from -usewallet
3112017-07-19T16:06:09 <ryanofsky> maybe also consider making it an error for user to specify -wallet on bitcoin-cli command line if it will be ignored
3122017-07-19T16:06:18 <wumpus> multiple -rpcwallet should also be an error
3132017-07-19T16:06:26 <sipa> ryanofsky: ?
3142017-07-19T16:06:53 <wumpus> ryanofsky: that'd result in an error as well when it's specified in the config file
3152017-07-19T16:07:13 <ryanofsky> yes that's why i said "on bitcoin-cli command line"
3162017-07-19T16:07:24 <jnewbery> wumpus: depends where you put the check
3172017-07-19T16:07:32 <wumpus> then again, when bitcoind runs in multiwallet mode, it won't accept missing wallet
3182017-07-19T16:07:40 <ryanofsky> or don't, anyway i was just suggesting that because i think -wallet -rpcwallet are easy to confuse
3192017-07-19T16:07:40 <wumpus> so you *have* to run -cli with rpcwallet
3202017-07-19T16:07:46 <wumpus> right?
3212017-07-19T16:08:19 <ryanofsky> you wouldn't need -rpcwallet if you are making a non-wallet rpc call or your node only has one wallet loaded
3222017-07-19T16:08:31 <wumpus> yes, and that's fine
3232017-07-19T16:08:58 <jnewbery> I think Russ is thinking about the case where someone starts bitcoind with -wallet=mybusinesswallet.dat in the conf file, then calls `bitcoin-cli -wallet=mypersonalwallet.dat send ...` and is confused that money is sent from their business wallet even though they specified a different wallet
3242017-07-19T16:09:02 <ryanofsky> yes, i'm just said that in response to "so you *have* to run -cli with rpcwallet
3252017-07-19T16:09:45 <wumpus> we don't distinguish command line and config options, and imo that should stay that way
3262017-07-19T16:10:09 <jnewbery> not true - rpcport is an example
3272017-07-19T16:10:40 <wumpus> how?
3282017-07-19T16:10:50 <sipa> it has a different meaning for client and server, sure
3292017-07-19T16:11:06 <sipa> but we don't complain in bitcoin-cli that -dbcache is specfied, right?
3302017-07-19T16:11:16 <jnewbery> oh sorry - yes you're right
3312017-07-19T16:13:00 *** jnewbery has left #bitcoin-core-dev
3322017-07-19T16:16:32 <sipa> i guess there is one example of an option that is treated differently between cmdline and configfile... -conf
3332017-07-19T16:16:39 *** jnewbery has joined #bitcoin-core-dev
3342017-07-19T16:18:11 <jnewbery> and I'd expect #10267 to be treated differently on command-line versus .conf file, but again I may be in the minority
3352017-07-19T16:18:12 <gribble> https://github.com/bitcoin/bitcoin/issues/10267 | New -includeconf argument for including external configuration files by kallewoof · Pull Request #10267 · bitcoin/bitcoin · GitHub
3362017-07-19T16:18:33 <jnewbery> >we don't complain in bitcoin-cli that -dbcache is specfied
3372017-07-19T16:18:58 <jnewbery> but specifying the wrong -dbcache on the command line for bitcoin-cli cannot lead to loss of funds
3382017-07-19T16:21:03 <sipa> i think that's unlikely here
3392017-07-19T16:21:26 <gmaxwell> Wrong testnet parameter can, in the same way wallet parameters can. (much worse, in fact)
3402017-07-19T16:21:54 <gmaxwell> I've been saved multiple times from doing something phenominally dumb re testnet vs mainnet through using wallet encryption as an authorization step.
3412017-07-19T16:23:41 <wumpus> yes, having separate configuration files per network would make a lot of sense too
3422017-07-19T16:24:47 <gmaxwell> (I like sharing them for most parameters, in fact...)
3432017-07-19T16:24:52 <sipa> wumpus: oh tes
3442017-07-19T16:24:53 <sipa> yes
3452017-07-19T16:24:54 <wumpus> jnewbery: yes, -conf is somewhat of an exception too, it's not explicitly treated differently but in effect it is because -conf in a configuration file does nothing, it's parsed before
3462017-07-19T16:25:09 <wumpus> gmaxwell: yes, keeping a shared one is ok too
3472017-07-19T16:25:42 <wumpus> doesn't have to be either or
3482017-07-19T16:25:49 <wumpus> as long as the precedence order is well-defined
3492017-07-19T16:26:44 *** JackH has quit IRC
3502017-07-19T16:32:19 *** timothy has quit IRC
3512017-07-19T16:34:33 *** jamesob_ has joined #bitcoin-core-dev
3522017-07-19T16:42:26 <wumpus> especially for things like network configuration it's almost essential have different config files per network, e.g. as soon as you specify rpcbind or bind with explicit port you'll run into port conflicts otherwise
3532017-07-19T17:10:28 *** jtimon has joined #bitcoin-core-dev
3542017-07-19T17:38:08 *** Dyaheon has quit IRC
3552017-07-19T17:39:19 *** Dyaheon has joined #bitcoin-core-dev
3562017-07-19T17:39:59 *** vicenteH has quit IRC
3572017-07-19T17:51:39 *** THoVer has joined #bitcoin-core-dev
3582017-07-19T17:56:11 *** laurentmt has joined #bitcoin-core-dev
3592017-07-19T17:56:30 *** CubicEarth has joined #bitcoin-core-dev
3602017-07-19T18:00:36 *** newbie-- has joined #bitcoin-core-dev
3612017-07-19T18:03:20 *** laurentmt has quit IRC
3622017-07-19T18:05:39 *** laurentmt has joined #bitcoin-core-dev
3632017-07-19T18:06:49 *** darawk has joined #bitcoin-core-dev
3642017-07-19T18:11:10 *** ovovo has joined #bitcoin-core-dev
3652017-07-19T18:15:00 *** owowo has quit IRC
3662017-07-19T18:17:12 *** Dizzle has joined #bitcoin-core-dev
3672017-07-19T18:20:48 <bitcoin-git> [bitcoin] jnewbery opened pull request #10882: [WIP] Keypool topup (master...keypool_topup) https://github.com/bitcoin/bitcoin/pull/10882
3682017-07-19T18:22:01 <jnewbery> 10882 is the cut-down version of #10830 which sipa and gmaxwell were asking for in the last meeting. I'm still fixing the testcase, but I'd appreciate some code review if possible
3692017-07-19T18:22:03 <gribble> https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
3702017-07-19T18:22:11 <jnewbery> I think people wanted this in 0.15 as a bugfix
3712017-07-19T18:24:28 *** u_nuSLASHkm8 has joined #bitcoin-core-dev
3722017-07-19T18:25:50 *** laurentmt has quit IRC
3732017-07-19T18:25:57 *** Guyver2_ has joined #bitcoin-core-dev
3742017-07-19T18:27:03 *** u_nuSLASHkm8 has left #bitcoin-core-dev
3752017-07-19T18:27:43 *** Guyver2 has quit IRC
3762017-07-19T18:27:50 *** Guyver2_ is now known as Guyver2
3772017-07-19T18:34:07 *** laurentmt has joined #bitcoin-core-dev
3782017-07-19T18:34:39 <instagibbs> jnewbery, will review
3792017-07-19T18:35:52 <jnewbery> thanks instagibbs
3802017-07-19T18:37:17 <bitcoin-git> [bitcoin] jnewbery closed pull request #10830: [WIP] [wallet] keypool restore (master...pr10240) https://github.com/bitcoin/bitcoin/pull/10830
3812017-07-19T18:38:44 <jonasschnelli> I just did a quick scrollback read. Is -rpcwallet still in talk?
3822017-07-19T18:38:52 <jonasschnelli> I'd prefere to keep rpc out of the naming...
3832017-07-19T18:39:03 <jonasschnelli> isn't bitcoin-cli transport agnostic?
3842017-07-19T18:39:18 <jonasschnelli> (from the users interface perspective)
3852017-07-19T18:40:18 <sipa> what do you mean transport agnostic/
3862017-07-19T18:42:53 <jonasschnelli> I probably over-engineer. But what if we once change bitcoin-cli's transport layer?
3872017-07-19T18:43:04 <jonasschnelli> the rpc layer is hidden from the user
3882017-07-19T18:43:34 <jonasschnelli> -rpcwallet includes the used transport layer in the naming scheme.
3892017-07-19T18:43:34 <sipa> so?
3902017-07-19T18:43:42 <sipa> so does -wallet
3912017-07-19T18:43:48 <jonasschnelli> ?
3922017-07-19T18:44:00 <sipa> i may misunderstand what you're saying
3932017-07-19T18:44:08 <jonasschnelli> -(rpc)wallet
3942017-07-19T18:44:18 <sipa> bitcoin-cli is an rpc client
3952017-07-19T18:44:30 <sipa> the transport layer may change from TCP to Unix sockets or something
3962017-07-19T18:44:35 <sipa> but it'll still be an RPC client
3972017-07-19T18:44:57 <jonasschnelli> What if we change to a different IPC protocol?
3982017-07-19T18:45:15 <jonasschnelli> I probably over-enginee. Nm.
3992017-07-19T18:45:24 <sipa> then everything needs to change; including rpcport, rpcserver, ...
4002017-07-19T18:45:33 <jonasschnelli> I just though until now, I could not see any "rpc" in bitcoin-cli (from the users perspetive)
4012017-07-19T18:45:44 <jonasschnelli> Yeah. That.. so yes. nm then.
4022017-07-19T18:46:17 <jonasschnelli> Okay. I take back my argument. With -rpcserver, etc. -rpcwallet may make sense.
4032017-07-19T18:47:07 <gmaxwell> what is -rpcwallet precisely
4042017-07-19T18:47:24 <jonasschnelli> gmaxwell: I guess it would be the substitute for -usewallet
4052017-07-19T18:47:27 <gmaxwell> is it the setting that tells bitcoin-cli what wallet to talk to
4062017-07-19T18:47:28 <sipa> gmaxwell: suggested new name for -usewallet
4072017-07-19T18:47:31 <gmaxwell> okay
4082017-07-19T18:47:32 <sipa> yes
4092017-07-19T18:47:41 <jonasschnelli> What was the issue with -usewallet?
4102017-07-19T18:47:56 <sipa> people think use is a weasel word
4112017-07-19T18:48:08 <sipa> (i really don't care anymore)
4122017-07-19T18:48:51 <jonasschnelli> Yeah, I don't care. But please not -wallet
4132017-07-19T18:49:58 <gmaxwell> gowallet=foo :P
4142017-07-19T18:50:41 <sipa> ./bitcoin-cli -thethingimean=foo
4152017-07-19T18:51:08 <gmaxwell> enablewallet=bob
4162017-07-19T18:51:36 <gmaxwell> walletify=bob
4172017-07-19T18:51:57 <sipa> ok...
4182017-07-19T18:52:09 <gmaxwell> openwallet=info
4192017-07-19T18:52:26 *** laurentmt has quit IRC
4202017-07-19T18:56:06 <jonasschnelli> Among those options, I would still prefere "usewallet"
4212017-07-19T18:56:35 *** CubicEarth has quit IRC
4222017-07-19T19:08:23 *** THoVer has quit IRC
4232017-07-19T19:08:33 *** Chris_Stewart_5 has quit IRC
4242017-07-19T19:10:53 *** BashCo has quit IRC
4252017-07-19T19:11:20 *** CubicEarth has joined #bitcoin-core-dev
4262017-07-19T19:11:29 *** BashCo has joined #bitcoin-core-dev
4272017-07-19T19:24:50 *** Chris_Stewart_5 has joined #bitcoin-core-dev
4282017-07-19T19:25:13 <morcos> wumpus: I don't care too much what we call the argument name, and I think we should just decide so we can make the behavior nice around it (my vote would be wallet, rpcwallet, usewallet in that order, but I don't care)
4292017-07-19T19:25:33 <morcos> I would say though that I think we should break the conf file and command line option thing being treated equally no matter what
4302017-07-19T19:26:17 <morcos> If we use wallet as the -cli option, then we break it as per jnewbery's pull. If we use rpcwallet or usewallet, then i think it makes sense to flag an error if -wallet is given on the command line.
4312017-07-19T19:26:50 <morcos> I agree with ryanofsky that it is possible people will make that mistake, and I don't think we should let them semi-reasonably think they are specifying one wallet and have things happen to another
4322017-07-19T19:28:15 <wumpus> if it wasn't for the authentication options we could stop parsing bitcoin.conf completely for the -cli
4332017-07-19T19:28:43 <wumpus> that would be fine with me, I don't want to parse one option from .conf but not another though, exceptions like that always result in confusion and wtf in the longer run
4342017-07-19T19:28:49 <morcos> We should also let it be an error if there are multiple instances of (rpc/use)wallet which begs the question of what do we do if you put (rpc/use)wallet in the conf file because clearly you need to be able to override from the command line
4352017-07-19T19:29:18 <wumpus> I'm not convinced it should be treated otherwise than other options
4362017-07-19T19:29:30 <morcos> Meaning the last value controls?
4372017-07-19T19:29:33 <wumpus> yes
4382017-07-19T19:29:45 <wumpus> if you use -rpcport you don't expect it to connect to all ports at once, do you?
4392017-07-19T19:29:56 <wumpus> not sure why -rpcwallet would be different
4402017-07-19T19:30:02 <morcos> no but i expect someone to tell me i'm an idiot
4412017-07-19T19:30:37 <wumpus> if so,the whole command line/config parsing should be refactored, whih should probably happen at some time, including errors if you provide an unrecognized option
4422017-07-19T19:30:43 <wumpus> however we're not there, and we're not going to be there for 0.15
4432017-07-19T19:31:18 <wumpus> I think any special-casing we introduce now for rpcwallet is a bad idea, especially as the whole API is still supposed to be experimental in the first place
4442017-07-19T19:31:25 <morcos> hmm, is the code to figure out whether something is a multiarg vs a single arg shared btwn bitcoind and -cli
4452017-07-19T19:31:51 <morcos> i guess it must not be?
4462017-07-19T19:31:54 <morcos> oh
4472017-07-19T19:31:56 <morcos> never mind
4482017-07-19T19:32:03 <morcos> if its different names it doesnt matter
4492017-07-19T19:32:11 <wumpus> all the argument parsing code is shared
4502017-07-19T19:32:13 <wumpus> itis' in util.cpp
4512017-07-19T19:32:16 <morcos> (rpc/use)wallet is a single arg, wallet ais a multi arg
4522017-07-19T19:32:19 <wumpus> but just use a different name so it doesn't matter
4532017-07-19T19:32:20 <wumpus> right.
4542017-07-19T19:32:22 <ryanofsky> i mostly like john's pr because i think it gets rid of all the ways to shoot yourself in the foot. and i don't think the special casing is a big deal. but if it is, then obvs different approach is needed
4552017-07-19T19:32:42 <wumpus> I do think the special casing is a big deal
4562017-07-19T19:32:49 <wumpus> but I have to maintain the code over a long time
4572017-07-19T19:32:59 <morcos> well lets just pick and be done with it, any will be fine..
4582017-07-19T19:33:00 <wumpus> and I'm sick of all kind of weird special casing
4592017-07-19T19:33:33 <morcos> rpcwallet it is.. although i admit i'm hesitant about specifying multiple rpcwallets and having the last one control.. but i don't know an easy solution to that
4602017-07-19T19:33:38 *** Guyver2 has quit IRC
4612017-07-19T19:33:38 <wumpus> it might seem easier on the short term but on the long time, arbitrariness is going to confuse
4622017-07-19T19:33:57 <wumpus> both developers and users
4632017-07-19T19:34:03 <wumpus> but it's always 'hey, why not add another special case'
4642017-07-19T19:34:14 <jnewbery> I don't even consider it special casing. Command line overrides conf file, just as for other arguments
4652017-07-19T19:34:21 <wumpus> noo of course not
4662017-07-19T19:34:23 <morcos> but you're right a later clean up could allow for all rpc options, you can only have a second single arg if its the command line overriding the conf file
4672017-07-19T19:34:42 <jnewbery> it's implemented differently because it's not possible to fully override mapmultiargs
4682017-07-19T19:34:47 <wumpus> if you do it it's not a special case
4692017-07-19T19:34:48 <jnewbery> but that's the functional outcome
4702017-07-19T19:34:50 <ryanofsky> wumpus, these arguments just seem pretty abstract to me, and i wish you'd bring them down to concrete examples of how the actual code in the pr could cause problems
4712017-07-19T19:35:18 <wumpus> I don't feel like repeating myself
4722017-07-19T19:35:31 <wumpus> I already argued against reusing -wallet forbitcoin-cli specifically
4732017-07-19T19:35:32 <ryanofsky> happy to defer
4742017-07-19T19:36:11 <wumpus> it's strange to have a same argument name as single-option for one utility then for multi-option for another utility
4752017-07-19T19:36:20 *** tinyurl_comSLASH has joined #bitcoin-core-dev
4762017-07-19T19:36:23 <morcos> so arguably the only thing to change from the merged code is s/usewallet/rpcwallet/ if that would overall be preferred?
4772017-07-19T19:36:24 <wumpus> *if* htey parse the same configuration file
4782017-07-19T19:36:29 <wumpus> yes
4792017-07-19T19:36:33 <wumpus> rpcwallet is a better name
4802017-07-19T19:36:42 *** vicenteH has joined #bitcoin-core-dev
4812017-07-19T19:37:33 <jonasschnelli> Should we still revoke (halt bitcoin-cli) if one provides multiple -rpc/-usewallet arguments?
4822017-07-19T19:37:39 <wumpus> and I'm ok with making providing -wallet on the command line for -cli an error, though I think it's overthinking it
4832017-07-19T19:37:52 <wumpus> jonasschnelli: no, I don't think so
4842017-07-19T19:38:06 <jonasschnelli> So just use the last one?
4852017-07-19T19:38:08 <wumpus> jonasschnelli: just like multiple -rpcport is not an error
4862017-07-19T19:38:11 *** tinyurl_comSLASH has left #bitcoin-core-dev
4872017-07-19T19:38:19 <wumpus> unless we change that behavior on a global level
4882017-07-19T19:38:19 <jonasschnelli> wumpus: Okay. Fine by me
4892017-07-19T19:38:34 <wumpus> so if multiple of any non-multi argument is an error, it should be an error, obviously
4902017-07-19T19:38:46 <wumpus> but there's no need to suddenly do all kinds of crazy stuff for rpcwallet
4912017-07-19T19:38:49 <jonasschnelli> Yes. No special case for every argument type
4922017-07-19T19:39:25 <jonasschnelli> But I guess what we should merge for 0.15 is the Qt console support: https://github.com/bitcoin/bitcoin/pull/10870
4932017-07-19T19:39:32 <jonasschnelli> Right now, Qt console is useless in multiwallet
4942017-07-19T19:39:37 <wumpus> yes
4952017-07-19T19:40:04 <wumpus> I certainly think making the functionality work in the first place is important :)
4962017-07-19T19:40:34 *** laurentmt has joined #bitcoin-core-dev
4972017-07-19T19:41:12 *** treebeardd has joined #bitcoin-core-dev
4982017-07-19T19:41:21 <wumpus> tagged it
4992017-07-19T19:42:29 <jonasschnelli> thanks
5002017-07-19T19:43:29 *** Dyaheon has quit IRC
5012017-07-19T19:44:47 <wumpus> from what I see "if an argument is specified multiple times, the last value holds" seems to be pretty standard behavior, I don't know of many command line utilities that reject if you specify a flag multiple times
5022017-07-19T19:45:28 *** Dyaheon has joined #bitcoin-core-dev
5032017-07-19T19:45:50 <eck> I am trying to understand the BIP process, and I'm confused how I can tell which BIPs were actually deployed
5042017-07-19T19:45:53 <eck> e.g. is BIP 62 deployed?
5052017-07-19T19:46:02 <wumpus> it can even be useful in scripts etc, to be able to override something that is set before
5062017-07-19T19:46:05 <ryanofsky> GetArg implement first value holds, right?
5072017-07-19T19:46:21 <wumpus> ryanofsky: ugh, okay
5082017-07-19T19:46:25 <instagibbs> eck, #bitcoin (can answer there)
5092017-07-19T19:46:32 <jonasschnelli> eck: look at the overview. Bip62 is "Withdrawn"
5102017-07-19T19:46:34 <wumpus> ryanofsky: are you sure?
5112017-07-19T19:46:40 <ryanofsky> no
5122017-07-19T19:46:51 <ryanofsky> which is why i like these things to be errors :)
5132017-07-19T19:47:09 <wumpus> but every other comamnd line utiltiy I know uses 'last setting holds'
5142017-07-19T19:47:10 <jonasschnelli> But then it should be globally?
5152017-07-19T19:47:20 <wumpus> gcc, ld, ls, etc
5162017-07-19T19:47:46 <bitcoin-git> [bitcoin] morcos opened pull request #10883: Rename -usewallet to -rpcwallet (master...rpcwallet) https://github.com/bitcoin/bitcoin/pull/10883
5172017-07-19T19:50:25 <morcos> wumpus: sigh.. it chooses the first option given in the conf file unless you give it on the command line and then its the last one from the command line
5182017-07-19T19:50:50 <jonasschnelli> :/
5192017-07-19T19:50:50 <bitcoin-git> [bitcoin] jonasschnelli closed pull request #10867: Allow only a single -usewallet argument (master...2017/07/multiwallet_bitcoincli) https://github.com/bitcoin/bitcoin/pull/10867
5202017-07-19T19:51:01 <wumpus> yes, it seems to correctly chose the latest from the command line
5212017-07-19T19:51:09 <wumpus> .conf semantics are strange
5222017-07-19T19:51:12 <jonasschnelli> but why the first from the conf?
5232017-07-19T19:51:20 <jonasschnelli> that makes no sense..
5242017-07-19T19:51:23 <wumpus> no, it doesn't
5252017-07-19T19:51:45 *** arubi has quit IRC
5262017-07-19T19:51:47 <wumpus> it should be the other way aorund
5272017-07-19T19:52:05 *** laurentmt has quit IRC
5282017-07-19T19:52:22 <wumpus> for all options, not special casing one :)
5292017-07-19T19:52:29 <jonasschnelli> yes.
5302017-07-19T19:52:49 <jonasschnelli> Though changing it now could have unexpected consequences. :/
5312017-07-19T19:53:06 <morcos> that behavior is what preseves command line overriding right?
5322017-07-19T19:53:41 <wumpus> morcos: yes, indeed, in the silly way it works currently
5332017-07-19T19:53:56 <jonasschnelli> morcos: IMO the command line overriding is good. But the last config value should be takend before we go to the optional command line overriding
5342017-07-19T19:54:33 <jonasschnelli> however, we should not change that.
5352017-07-19T19:55:04 <wumpus> in any case there are probably a zillion things broken in command line / config file handling, nothing we should do about that for 0.15, but for later on it makes sense to actually think about it
5362017-07-19T19:56:06 <wumpus> especially with things like includeconf=..., people are going to expect later config options are overriding previous ones
5372017-07-19T19:56:36 <wumpus> like it is for any other config file parser in the world
5382017-07-19T19:56:49 <jonasschnelli> heh. Indeed
5392017-07-19T19:57:13 *** arubi has joined #bitcoin-core-dev
5402017-07-19T19:57:14 *** Guest24650 has joined #bitcoin-core-dev
5412017-07-19T19:58:20 <wumpus> how do other programs handle multi-args? I think our way is strange, in that we can never clear something that has been set, things are only added
5422017-07-19T19:59:18 <wumpus> so if the config file sets bind=127.0.0.1:1234, the command line can never override that, only add to it
5432017-07-19T20:00:16 <jonasschnelli> I'm not sure, but isn't comma separation a thing? -wallet=w1.dat,w2.dat?
5442017-07-19T20:00:28 <wumpus> yeah
5452017-07-19T20:00:48 <wumpus> though that always leaves 'what if we have a comma in our value'
5462017-07-19T20:00:59 *** Chris_Stewart_5 has quit IRC
5472017-07-19T20:01:00 <jonasschnelli> Use ;
5482017-07-19T20:01:12 <wumpus> but that conflicts with the shell :-)
5492017-07-19T20:01:33 <jonasschnelli> heh.. yes. use -wallet="a;b;c"?
5502017-07-19T20:01:37 <jonasschnelli> But meh
5512017-07-19T20:01:40 <wumpus> hehe
5522017-07-19T20:01:52 <wumpus> I was just illustrating a point, I know it's hard to work around
5532017-07-19T20:02:14 <wumpus> json syntax config file would work
5542017-07-19T20:02:21 <wumpus> for command line it's harder
5552017-07-19T20:02:33 <jonasschnelli> Yes
5562017-07-19T20:03:42 <wumpus> I must say it wouldn't be the first time I typed -debug=tor,bench though, comma-separated is kind of natural, when it works
5572017-07-19T20:04:38 *** laurentmt has joined #bitcoin-core-dev
5582017-07-19T20:05:25 <jonasschnelli> Yes. For the wallet it would feel more natural and there is no need for a comma in the filename, though there could be other use cases where you may want/need it
5592017-07-19T20:06:21 <wumpus> cool, it warns at least now: Warning: Unsupported logging category -debug=rpc,net,tor.
5602017-07-19T20:06:34 <wumpus> suppose that is since the debug categories are an enum
5612017-07-19T20:06:51 <wumpus> it used to be silent about it
5622017-07-19T20:09:33 <jnewbery> quick question about -rpcwallet in .conf: does that mean that server RPCs are also sent to the /wallet/<rpcwallet> endpoint? That's allowed in the current implementation, but it'll break when we do wallet process separation
5632017-07-19T20:09:35 <jonasschnelli> BTW: github recommends to add a COC to the bitcoin core project: https://opensource.guide/code-of-conduct/
5642017-07-19T20:11:19 *** laurentmt has quit IRC
5652017-07-19T20:11:49 <wumpus> jnewbery: I'd expect -rpcwallet would mean commands are sent to a wallet endpoint, yes, -cli doesn't have the knowledge whether something is a wallet call or not
5662017-07-19T20:11:50 <jnewbery> I suppose we'll generally need to give bitcoin-cli more smarts if we do wallet process separation
5672017-07-19T20:12:45 <wumpus> well ... we'll worry about that, then
5682017-07-19T20:12:49 <sipa> well my view is that the 'wallet process' would still be a full bitcoind-like thing communicating over SPV... so it would still have P2P connections (typically just one), etc
5692017-07-19T20:13:05 <wumpus> there's no way to specify an alternative endpoint at all at the moment
5702017-07-19T20:13:05 <sipa> so there's not much reason why you couldn't send a getpeerinfo to a wallet endpoint
5712017-07-19T20:13:14 <sipa> which is exactly what it does now
5722017-07-19T20:13:25 <sipa> for a few things, like mempool or fee estimation something else may be needed
5732017-07-19T20:13:55 <sipa> but that's a worry for later indeed
5742017-07-19T20:13:58 <wumpus> heh yes if a wallet endpoints getpeerinfo you can send getpeerinfo to it
5752017-07-19T20:14:35 <wumpus> if it doesn't, you'll get an error that it's not implemented
5762017-07-19T20:14:57 *** Chris_Stewart_5 has joined #bitcoin-core-dev
5772017-07-19T20:15:01 <jonasschnelli> sipa: Curious, could you think that â if we have p2p enc/auth â mempool and fee est would be something to serve over p2p to auth. peers?
5782017-07-19T20:15:23 <jonasschnelli> Or do you see a different channel (RPC, etc,) for that?
5792017-07-19T20:15:58 <sipa> jonasschnelli: maybe... that may be controversial
5802017-07-19T20:16:44 <sipa> but i don't see much problem in having private extensions available only for known peers
5812017-07-19T20:16:48 <jonasschnelli> I'd say that would simplify connection management massively. But I'd also say some people would dislike that.
5822017-07-19T20:17:06 <jonasschnelli> Yes. me two
5832017-07-19T20:17:11 <sipa> actually, i think that was one of the uses for -whitelist when i suggested :)
5842017-07-19T20:17:20 <jonasschnelli> *too
5852017-07-19T20:17:54 <jonasschnelli> But thats far in future (net refactor, Bip151, Bip150 and maybe then).
5862017-07-19T20:18:00 <sipa> yah
5872017-07-19T20:18:04 <wumpus> I agree authenticated extensions could make sense, please don't overload -whitelist with more things
5882017-07-19T20:18:18 <sipa> wumpus: oh, absolutely - i agree with that now
5892017-07-19T20:21:16 <sipa> just giving some historical background
5902017-07-19T20:23:15 *** promag has joined #bitcoin-core-dev
5912017-07-19T20:25:12 <wumpus> oh sure, it's one kind of whitelisting option
5922017-07-19T20:26:59 *** roidster has joined #bitcoin-core-dev
5932017-07-19T20:27:02 *** roidster is now known as Guest86981
5942017-07-19T20:28:17 *** Chris_Stewart_5 has quit IRC
5952017-07-19T20:30:57 *** Gnof has quit IRC
5962017-07-19T20:37:08 <bitcoin-git> [bitcoin] eliahuhorwitz opened pull request #10884: Update build-windows.md (master...patch-1) https://github.com/bitcoin/bitcoin/pull/10884
5972017-07-19T20:47:01 *** treebeardd has quit IRC
5982017-07-19T20:48:22 <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d445a2c2eaa1...df0793f324e3
5992017-07-19T20:48:22 <bitcoin-git> bitcoin/master 7ec3343 Gregory Sanders: add gdb attach process to test README
6002017-07-19T20:48:23 <bitcoin-git> bitcoin/master df0793f MarcoFalke: Merge #10681: add gdb attach process to test README...
6012017-07-19T20:48:47 <bitcoin-git> [bitcoin] MarcoFalke closed pull request #10681: add gdb attach process to test README (master...gdbattach) https://github.com/bitcoin/bitcoin/pull/10681
6022017-07-19T20:57:32 *** laurentmt has joined #bitcoin-core-dev
6032017-07-19T20:57:33 *** laurentmt has quit IRC
6042017-07-19T21:02:57 *** Dizzle has quit IRC
6052017-07-19T21:13:34 *** CubicEarth has quit IRC
6062017-07-19T21:18:07 <promag> is this correct bitcoin-qt -wallet=foo.dat -wallet=wallet.dat -regtest ?
6072017-07-19T21:18:27 <promag> to run multiwallet in qt?
6082017-07-19T21:28:20 <sipa> yes
6092017-07-19T21:30:50 *** CubicEarth has joined #bitcoin-core-dev
6102017-07-19T21:40:44 *** CubicEarth has quit IRC
6112017-07-19T21:49:17 *** Dyaheon has quit IRC
6122017-07-19T21:53:08 *** Dyaheon has joined #bitcoin-core-dev
6132017-07-19T22:08:08 <promag> sipa: what should be the load order when -wallet=foo -wallet=bar -wallet=foo
6142017-07-19T22:10:14 <sipa> ha, no clue
6152017-07-19T22:10:42 <sipa> does that even work?
6162017-07-19T22:11:06 <promag> yeah
6172017-07-19T22:11:08 <promag> :P
6182017-07-19T22:11:25 <promag> I'm submitting a PR to handle that
6192017-07-19T22:11:55 <sipa> cool
6202017-07-19T22:13:11 <promag> anyway, I have 2 ideas: 1st add second argumento unique = false to ArgsManager::GetArgs
6212017-07-19T22:13:40 <promag> the other is to add GetUniqueArgs
6222017-07-19T22:14:25 <promag> which preserves order (first takes effect)
6232017-07-19T22:14:34 <cfields> i think the arg manager itself should just sort that out
6242017-07-19T22:14:50 <cfields> would we ever care about dupes post-init?
6252017-07-19T22:15:15 <promag> however ParseParameters explicit says that the last takes effect when one only wants the arg as a single value
6262017-07-19T22:15:50 <cfields> promag: yes, that's the common convention. so that you can add things later in the line to disable earlier ones
6272017-07-19T22:16:05 <cfields> s/disable/override/
6282017-07-19T22:16:36 <promag> cfields: right so it's config < arg[n] < arg[m] where m > n
6292017-07-19T22:17:47 *** Chris_Stewart_5 has joined #bitcoin-core-dev
6302017-07-19T22:18:05 <promag> but in the case of multi value then the order is? config[0], config[1], arg[0], arg[1] right?
6312017-07-19T22:18:32 <cfields> that's what i'd expect, yes
6322017-07-19T22:18:50 <promag> would we ever care about dupes post-init? -> dunno, wdyt?
6332017-07-19T22:19:00 <sipa> i thinkit should just error if you soecify the same one twice
6342017-07-19T22:19:08 <sipa> sorry, ohone typing
6352017-07-19T22:19:28 <promag> sipa: and if you config[1] = arg[0] ?
6362017-07-19T22:20:05 <promag> I mean, if you pass a value in the command line which happens to be in config?
6372017-07-19T22:20:11 <promag> is that a error?
6382017-07-19T22:20:13 <cfields> right, i would expect it to just quietly drop the dupe
6392017-07-19T22:20:38 <promag> the error/warn should be just for the command line args right?
6402017-07-19T22:20:59 <promag> so it's just a matter of fixing ParseParameters
6412017-07-19T22:21:00 <sipa> ok, merging them is also a reasonable thing to fo
6422017-07-19T22:21:16 <sipa> is it *always* the right thing?
6432017-07-19T22:21:32 <cfields> sipa: you mean for all args? or for wallet in particular?
6442017-07-19T22:21:33 <promag> dunno, didn't go that far
6452017-07-19T22:21:47 <sipa> cfields: for all args
6462017-07-19T22:21:56 <sipa> if you're talking about changing ParseParameters
6472017-07-19T22:22:04 <promag> for instance, -connect
6482017-07-19T22:22:40 <cfields> sipa: well at minimum, i think we always want to be able to override the config
6492017-07-19T22:22:58 <cfields> and i think that always implies merging?
6502017-07-19T22:23:33 <sipa> cfields: multiargs don't get merged, they get appended
6512017-07-19T22:23:34 <promag> if it's multiarg then imo it's unique stable merge
6522017-07-19T22:24:04 *** Guest86981 has quit IRC
6532017-07-19T22:24:06 <sipa> i'm not convinced that for all options uniquifying is the right thing to do
6542017-07-19T22:24:47 <cfields> yea, ok...
6552017-07-19T22:24:59 <cfields> -addnode=127.0.0.1 -addnode=127.0.0.1
6562017-07-19T22:25:03 <cfields> i want 2 connections there
6572017-07-19T22:25:22 <promag> does that make sense?
6582017-07-19T22:25:41 *** chjj has joined #bitcoin-core-dev
6592017-07-19T22:25:56 <promag> if that's the case, GetUniqueArgs is the way to go?
6602017-07-19T22:26:32 <sipa> or just unique it in the wallet loading code
6612017-07-19T22:26:40 <sipa> at least if you're talking about 0.15
6622017-07-19T22:27:31 <promag> it's in 3 different places
6632017-07-19T22:29:08 <promag> the PR adds GetUniqueArgs and changes 3 occurencies of gArgs.GetArgs("-wallet")
6642017-07-19T22:29:28 <promag> and adds tests for GetUniqueArgs
6652017-07-19T23:00:49 *** MarcoFalke has quit IRC
6662017-07-19T23:04:21 *** MarcoFalke has joined #bitcoin-core-dev
6672017-07-19T23:11:28 *** BashCo has quit IRC
6682017-07-19T23:12:07 *** BashCo has joined #bitcoin-core-dev
6692017-07-19T23:30:23 *** roidster has joined #bitcoin-core-dev
6702017-07-19T23:31:34 *** vicenteH has quit IRC
6712017-07-19T23:38:00 *** vicenteH has joined #bitcoin-core-dev
6722017-07-19T23:38:33 <promag> sipa, cfields: what should GetArg("-foo") give when -foo=a -foo=b -foo=a? (for me last "a" takes effect even though it's duplicate)
6732017-07-19T23:38:53 <sipa> yes, that's intentional
6742017-07-19T23:39:01 <sipa> last option takes effect
6752017-07-19T23:39:48 <promag> but GetUniqueArgs("-foo") gives {"a", "b"} sgty?
6762017-07-19T23:42:29 <sipa> sure