1 2017-07-19 00:44:05	0|bitcoin-git|[13bitcoin] 15achow101 opened pull request #10874: [RPC] getblockchaininfo: Loop through the bip9 soft fork deployments instead of hard coding (06master...06getblockchaininfo-bip9-loop) 02https://github.com/bitcoin/bitcoin/pull/10874
  2 2017-07-19 01:26:35	0|bitcoin-git|[13bitcoin] 15achow101 opened pull request #10875: BIP 91 deployment parameters (06master...06bip91-dep-params) 02https://github.com/bitcoin/bitcoin/pull/10875
  3 2017-07-19 01:27:10	0|bitcoin-git|[13bitcoin] 15DJMorgan opened pull request #10876: What do you mean a bug in the latest version? (060.14...06master) 02https://github.com/bitcoin/bitcoin/pull/10876
  4 2017-07-19 01:31:18	0|gmaxwell|::sigh::
  5 2017-07-19 01:37:02	0|bitcoin-git|[13bitcoin] 15sipa closed pull request #10876: What do you mean a bug in the latest version? (060.14...06master) 02https://github.com/bitcoin/bitcoin/pull/10876
  6 2017-07-19 04:01:56	0|bitcoin-git|[13bitcoin] 15kallewoof opened pull request #10877: [rpc] Verbose flags for chaining and scripting (06master...06verbose-flagging) 02https://github.com/bitcoin/bitcoin/pull/10877
  7 2017-07-19 04:19:42	0|bitcoin-git|[13bitcoin] 15dongcarl opened pull request #10878: Docs: Fix markdown line breaks in init.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10878
  8 2017-07-19 04:52:38	0|bitcoin-git|[13bitcoin] 15kallewoof opened pull request #10879: Difficulty adjustment (06master...06difficulty-adjustment) 02https://github.com/bitcoin/bitcoin/pull/10879
  9 2017-07-19 04:52:45	0|bitcoin-git|[13bitcoin] 15kallewoof closed pull request #10879: Difficulty adjustment (06master...06difficulty-adjustment) 02https://github.com/bitcoin/bitcoin/pull/10879
 10 2017-07-19 06:50:07	0|bitcoin-git|[13bitcoin] 15eklitzke opened pull request #10880: Update .gitignore to ignore more test files (06master...06gitignore) 02https://github.com/bitcoin/bitcoin/pull/10880
 11 2017-07-19 07:22:49	0|eck|what's the recommended way for me to test a change before submitting a PR? is "make check" sufficient?
 12 2017-07-19 07:23:16	0|gmaxwell|eck: no, read test/README.md
 13 2017-07-19 07:24:02	0|eck|thanks
 14 2017-07-19 07:24:43	0|gmaxwell|test/functional/test_runner.py
 15 2017-07-19 07:24:53	0|gmaxwell|is what you want to run in addition to a make check.
 16 2017-07-19 07:25:12	0|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.
 17 2017-07-19 07:25:18	0|gmaxwell|but if you don't know, better to run them all.
 18 2017-07-19 07:25:45	0|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.
 19 2017-07-19 07:41:37	0|bitcoin-git|[13bitcoin] 15eklitzke opened pull request #10881: trivial: fix various pyflakes/vulture warnings (06master...06vulture) 02https://github.com/bitcoin/bitcoin/pull/10881
 20 2017-07-19 09:54:09	0|bitcoin-git|13bitcoin/06master 14e0d4592 15practicalswift: Avoid redundant redeclaration of GetWarnings(const string&)...
 21 2017-07-19 09:54:09	0|bitcoin-git|[13bitcoin] 15jonasschnelli pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9e8d6a3fb43a...a6ec5802b0a9
 22 2017-07-19 09:54:10	0|bitcoin-git|13bitcoin/06master 14a6ec580 15Jonas Schnelli: Merge #10864: Avoid redundant redeclaration of GetWarnings(const string&)...
 23 2017-07-19 09:54:41	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #10864: Avoid redundant redeclaration of GetWarnings(const string&) (06master...06GetWarnings-in-warnings.h) 02https://github.com/bitcoin/bitcoin/pull/10864
 24 2017-07-19 09:54:57	0|jonasschnelli|Oh. I guess I should not have merged this because of the freeze.
 25 2017-07-19 09:54:59	0|jonasschnelli|Sry
 26 2017-07-19 13:54:35	0|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
 27 2017-07-19 13:54:50	0|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."
 28 2017-07-19 14:10:21	0|cfields|jonasschnelli: that one uses everything built with extra debugging defines
 29 2017-07-19 14:10:41	0|jonasschnelli|cfields: D_GLIBCXX_DEBUG, right?
 30 2017-07-19 14:10:46	0|cfields|jonasschnelli: -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC
 31 2017-07-19 14:11:01	0|cfields|make -C depends DEBUG=1
 32 2017-07-19 14:11:13	0|jonasschnelli|Ah.. the dependencies...
 33 2017-07-19 14:11:43	0|jonasschnelli|cfields: Just passing -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC into ./configure won't be "enought"?
 34 2017-07-19 14:12:19	0|cfields|jonasschnelli: iirc all the deps have to be built with those flags otherwise wonky things happen
 35 2017-07-19 14:12:24	0|cfields|but i could be misremembering
 36 2017-07-19 14:12:35	0|jonasschnelli|okay. Let me try that...
 37 2017-07-19 14:12:37	0|jonasschnelli|(via depends)
 38 2017-07-19 14:13:04	0|cfields|jonasschnelli: those errors are almost always foo.empty() && foo[0]
 39 2017-07-19 14:13:29	0|jonasschnelli|cfields: accessing an index that is not available?
 40 2017-07-19 14:15:03	0|cfields|jonasschnelli: ah, looks like a "singular" iterator is one that can't be reached.
 41 2017-07-19 14:15:16	0|cfields|so probably more like *foo.end()
 42 2017-07-19 14:16:59	0|jonasschnelli|cfields: Thanks... I'll check the code.
 43 2017-07-19 14:25:12	0|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
 44 2017-07-19 14:25:14	0|gribble|https://github.com/bitcoin/bitcoin/issues/10873 | Use a condition variable for shutdown notifications by eklitzke · Pull Request #10873 · bitcoin/bitcoin · GitHub
 45 2017-07-19 14:25:41	0|wumpus|this won't work for handling UNIX signals, certainly not in a portable way
 46 2017-07-19 14:26:03	0|jonasschnelli|wumpus: Yes. How long was it ago since I tried it. :)
 47 2017-07-19 14:26:36	0|jonasschnelli|I didn't commented because I tried with a boost signal rather then a mutex
 48 2017-07-19 14:26:47	0|jonasschnelli|Wasn't sure if this is portable or not
 49 2017-07-19 14:27:16	0|cfields|jonasschnelli: the list of things you're actually allowed to use is remarkably small
 50 2017-07-19 14:27:42	0|jonasschnelli|cfields: Yes. I once checked... atomics should be fine... but I guess conditional vars aren't
 51 2017-07-19 14:27:54	0|wumpus|yes, setting a flag is fine
 52 2017-07-19 14:28:18	0|wumpus|I think you are allowed to read/write pipes as well
 53 2017-07-19 14:29:44	0|wumpus|there are some tricks that could be taken over from other daemons, but it might not be worth the trouble
 54 2017-07-19 14:37:49	0|wumpus|maybe there's even some obscure trick with sigwait() that would work
 55 2017-07-19 14:47:27	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/a6ec5802b0a9...9022aa37226b
 56 2017-07-19 14:47:28	0|bitcoin-git|13bitcoin/06master 14b138585 15Alex Morcos: Remove factor of 3 from definition of dust....
 57 2017-07-19 14:47:28	0|bitcoin-git|13bitcoin/06master 14f4d00e6 15Alex Morcos: Add a discard_rate...
 58 2017-07-19 14:47:29	0|bitcoin-git|13bitcoin/06master 149022aa3 15Wladimir J. van der Laan: Merge #10817: Redefine Dust and add a discard_rate...
 59 2017-07-19 14:47:57	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10817: Redefine Dust and add a discard_rate (06master...06discardmore) 02https://github.com/bitcoin/bitcoin/pull/10817
 60 2017-07-19 14:48:08	0|promag|is practicalswift around?
 61 2017-07-19 14:50:21	0|jonasschnelli|jnewbery, ryanofsky: continue the discussion about -wallet vs -usewallet here?
 62 2017-07-19 14:50:50	0|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).
 63 2017-07-19 14:51:13	0|jonasschnelli|Why would you want the use same argument for two different things?
 64 2017-07-19 14:51:56	0|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)
 65 2017-07-19 14:52:36	0|ryanofsky|jonasschnelli, maybe you can give a practical example of why having different argument names is useful
 66 2017-07-19 14:53:16	0|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
 67 2017-07-19 14:53:36	0|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)
 68 2017-07-19 14:53:47	0|jonasschnelli|"two
 69 2017-07-19 14:54:53	0|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
 70 2017-07-19 14:54:54	0|ryanofsky|can you explain qt use case a little more? qt runs a full node & wallet
 71 2017-07-19 14:55:06	0|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)
 72 2017-07-19 14:55:28	0|jonasschnelli|ryanofsky; Right now, Qt always uses the wallet at index 0
 73 2017-07-19 14:56:06	0|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
 74 2017-07-19 14:56:16	0|ryanofsky|right, so what do you need a -usewallet option for in context of qt?
 75 2017-07-19 14:56:55	0|jonasschnelli|ryanofsky: Qt: until we have a wallet selection via GUI, you could define which of those wallets is accessible over Qt.
 76 2017-07-19 14:57:59	0|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
 77 2017-07-19 14:58:21	0|ryanofsky|adding -usewallet for qt would again just be adding confusion & complexity for no reason
 78 2017-07-19 14:58:44	0|jonasschnelli|Yes. Agree that this would work, though do we really want to depend on orders of arguments?
 79 2017-07-19 14:58:54	0|ryanofsky|no, we want to add a dropdown or tab
 80 2017-07-19 14:59:00	0|jonasschnelli|Yes. Indeed.
 81 2017-07-19 14:59:18	0|ryanofsky|i'm saying adding "usewallet" is not clarifying, it is just throwing in more complexity
 82 2017-07-19 14:59:23	0|jonasschnelli|Yes. I'm not sure if the -usewallet case for Qt is valid... though I think it is for cli
 83 2017-07-19 14:59:40	0|jonasschnelli|It is already complex.
 84 2017-07-19 14:59:41	0|ryanofsky|-wallet=filename everywhere should just work
 85 2017-07-19 15:00:35	0|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?
 86 2017-07-19 15:00:38	0|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
 87 2017-07-19 15:01:15	0|jonasschnelli|--help is always help. Not once setting the -help information and once reading it
 88 2017-07-19 15:01:35	0|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
 89 2017-07-19 15:02:00	0|bitcoin-git|13bitcoin/06master 141c9b818 15Andrew Chow: getinfo deprecation warning
 90 2017-07-19 15:02:00	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9022aa37226b...d445a2c2eaa1
 91 2017-07-19 15:02:01	0|bitcoin-git|13bitcoin/06master 14d445a2c 15Wladimir J. van der Laan: Merge #10857: [RPC] Add a deprecation warning to getinfo's output...
 92 2017-07-19 15:02:19	0|ryanofsky|users should just be shown an error if they are expecting bitcoin-cli to send an single rpc command to multiple wallets
 93 2017-07-19 15:02:42	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10857: [RPC] Add a deprecation warning to getinfo's output (06master...06deprecate-getinfo) 02https://github.com/bitcoin/bitcoin/pull/10857
 94 2017-07-19 15:02:55	0|jonasschnelli|It's just my opinion. I can live with the pure -wallet for everything approach.
 95 2017-07-19 15:03:13	0|ryanofsky|sounds good to me
 96 2017-07-19 15:03:37	0|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
 97 2017-07-19 15:03:38	0|jonasschnelli|I wonder what other developers think...
 98 2017-07-19 15:04:11	0|jonasschnelli|jnewbery: but that is how it currently works (just with -usewallet), right?
 99 2017-07-19 15:04:29	0|ryanofsky|pr needs reviews, so i also want other developers to chime in :)
100 2017-07-19 15:04:41	0|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
101 2017-07-19 15:06:32	0|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
102 2017-07-19 15:07:45	0|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
103 2017-07-19 15:07:57	0|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
104 2017-07-19 15:15:07	0|jonasschnelli|Maybe something we should discuss at this weeks IRC meeting
105 2017-07-19 15:21:25	0|ryanofsky|i'd prefer pr reviews & acks but ok :)
106 2017-07-19 15:25:32	0|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
107 2017-07-19 15:39:19	0|sipa|jonasschnelli: i though the same way
108 2017-07-19 15:40:06	0|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"
109 2017-07-19 15:40:55	0|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
110 2017-07-19 15:41:05	0|sipa|evwn if it were implemwnted differentlt
111 2017-07-19 15:54:08	0|wumpus|I don't think re-using -wallet for bitcoin-cli is a good idea
112 2017-07-19 15:55:17	0|wumpus|re-using rpcport is clever because the setting on client and server side happens to match up w/ the interface in between
113 2017-07-19 15:56:16	0|wumpus|if it'd be possible to specify multiple rpcports in the server config, we'd have the same problem there
114 2017-07-19 15:56:41	0|wumpus|-rpcbind is allowed to be used multiple times, but isn't used by the client at all
115 2017-07-19 15:57:11	0|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
116 2017-07-19 15:59:13	0|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
117 2017-07-19 15:59:29	0|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
118 2017-07-19 15:59:56	0|wumpus|if bitconi-cli read a different configuration file I'd have agreed there was no conflict
119 2017-07-19 16:00:11	0|wumpus|but that's not where we're coming from, or even where we're going
120 2017-07-19 16:00:24	0|ryanofsky|wumpus, yes, the tradeoff for having bitcoin-cli accept a wallet argument is having to ignore wallet values from the config file
121 2017-07-19 16:00:32	0|wumpus|that sounds stupid
122 2017-07-19 16:00:49	0|wumpus|why would you be so insistent in using -wallet for the option to special case it in config file reading?
123 2017-07-19 16:00:57	0|wumpus|just use a different argument name and be done with it
124 2017-07-19 16:01:03	0|ryanofsky|ok, well that's the tradeoff, we've discussed what i think are better options for allowing default wallets
125 2017-07-19 16:01:17	0|ryanofsky|i'm not insistent on anything
126 2017-07-19 16:01:20	0|wumpus|what is the trade-off? why would you insist on using -wallet at all?
127 2017-07-19 16:01:32	0|wumpus|it causes a conflict so imo the option should not be named that
128 2017-07-19 16:01:50	0|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
129 2017-07-19 16:01:51	0|wumpus|making a special workarounds just to be able to call it -wallet is strange
130 2017-07-19 16:02:12	0|wumpus|if bitcoin-cli would not read the config file at all, or a different config file, I'd be ok with it
131 2017-07-19 16:02:13	0|ryanofsky|ok, well that's my strange argument :)
132 2017-07-19 16:02:25	0|wumpus|but we can't just do that, certainly not for 0.15
133 2017-07-19 16:02:30	0|ryanofsky|if you object, then go with usewallet and wallet
134 2017-07-19 16:02:40	0|wumpus|special-casing just seems weird
135 2017-07-19 16:02:44	0|wumpus|I really dislike it
136 2017-07-19 16:02:58	0|wumpus|'why would all options be read from the config file except for X?' that's just drunk
137 2017-07-19 16:03:19	0|jnewbery|having a `useargument` argument that is used by bitcoin-cli but is not used by bitcoind seems strange to me
138 2017-07-19 16:03:43	0|wumpus|why owuld that be strange? bitcoin-cli has lots of arguments that are not used by bitcoind
139 2017-07-19 16:03:48	0|wumpus|why would they need to overlap?
140 2017-07-19 16:03:57	0|ryanofsky|yeah, i just think this wallet config file thing is a strange corner case compared to everyday usage
141 2017-07-19 16:04:10	0|wumpus|what would bitcoind want to do with it ta all?
142 2017-07-19 16:04:20	0|jnewbery|Because of the arguments I give here: https://github.com/bitcoin/bitcoin/pull/10868#issuecomment-316409725
143 2017-07-19 16:04:22	0|sipa|if the '-use' sounds liie meaningless, use -rpcwallet maybe
144 2017-07-19 16:04:22	0|wumpus|it'd be the default wallet for the *client*
145 2017-07-19 16:04:30	0|wumpus|sipa: yes! sounds good to me
146 2017-07-19 16:04:43	0|ryanofsky|i'd be fine with rpcwallet, that's what i suggested when the arg was first added
147 2017-07-19 16:04:54	0|instagibbs|rpcwallet is nicer
148 2017-07-19 16:05:00	0|wumpus|I don't really care what name, it should just not be -wallet
149 2017-07-19 16:05:33	0|wumpus|though I guess rpcwallet would be more consistent with some other rpc options
150 2017-07-19 16:06:00	0|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
151 2017-07-19 16:06:10	0|ryanofsky|maybe also consider making it an error for user to specify -wallet on bitcoin-cli command line if it will be ignored
152 2017-07-19 16:06:19	0|wumpus|multiple -rpcwallet should also be an error
153 2017-07-19 16:06:27	0|sipa|ryanofsky: ?
154 2017-07-19 16:06:54	0|wumpus|ryanofsky: that'd result in an error as well  when it's specified in the config file
155 2017-07-19 16:07:14	0|ryanofsky|yes that's why i said "on bitcoin-cli command line"
156 2017-07-19 16:07:24	0|jnewbery|wumpus: depends where you put the check
157 2017-07-19 16:07:33	0|wumpus|then again, when bitcoind runs in multiwallet mode, it won't accept missing wallet
158 2017-07-19 16:07:41	0|ryanofsky|or don't, anyway i was just suggesting that because i think -wallet -rpcwallet are easy to confuse
159 2017-07-19 16:07:41	0|wumpus|so you *have* to run -cli with rpcwallet
160 2017-07-19 16:07:47	0|wumpus|right?
161 2017-07-19 16:08:20	0|ryanofsky|you wouldn't need -rpcwallet if you are making a non-wallet rpc call or your node only has one wallet loaded
162 2017-07-19 16:08:32	0|wumpus|yes, and that's fine
163 2017-07-19 16:08:58	0|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
164 2017-07-19 16:09:03	0|ryanofsky|yes, i'm just said that in response to "so you *have* to run -cli with rpcwallet
165 2017-07-19 16:09:46	0|wumpus|we don't distinguish command line and config options, and imo that should stay that way
166 2017-07-19 16:10:10	0|jnewbery|not true - rpcport is an example
167 2017-07-19 16:10:41	0|wumpus|how?
168 2017-07-19 16:10:51	0|sipa|it has a different meaning for client and server, sure
169 2017-07-19 16:11:07	0|sipa|but we don't complain in bitcoin-cli that -dbcache is specfied, right?
170 2017-07-19 16:11:16	0|jnewbery|oh sorry - yes you're right
171 2017-07-19 16:16:33	0|sipa|i guess there is one example of an option that is treated differently between cmdline and configfile... -conf
172 2017-07-19 16:18:11	0|jnewbery|and I'd expect #10267 to be treated differently on command-line versus .conf file, but again I may be in the minority
173 2017-07-19 16:18:13	0|gribble|https://github.com/bitcoin/bitcoin/issues/10267 | New -includeconf argument for including external configuration files by kallewoof · Pull Request #10267 · bitcoin/bitcoin · GitHub
174 2017-07-19 16:18:34	0|jnewbery|>we don't complain in bitcoin-cli that -dbcache is specfied
175 2017-07-19 16:18:59	0|jnewbery|but specifying the wrong -dbcache on the command line for bitcoin-cli cannot lead to loss of funds
176 2017-07-19 16:21:04	0|sipa|i think that's unlikely here
177 2017-07-19 16:21:27	0|gmaxwell|Wrong testnet parameter can, in the same way wallet parameters can. (much worse, in fact)
178 2017-07-19 16:21:55	0|gmaxwell|I've been saved multiple times from doing something phenominally dumb re testnet vs mainnet through using wallet encryption as an authorization step.
179 2017-07-19 16:23:42	0|wumpus|yes, having separate configuration files per network would make a lot of sense too
180 2017-07-19 16:24:48	0|gmaxwell|(I like sharing them for most parameters, in fact...)
181 2017-07-19 16:24:52	0|sipa|wumpus: oh tes
182 2017-07-19 16:24:54	0|sipa|yes
183 2017-07-19 16:24:54	0|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
184 2017-07-19 16:25:09	0|wumpus|gmaxwell: yes, keeping a shared one is ok too
185 2017-07-19 16:25:43	0|wumpus|doesn't have to be either or
186 2017-07-19 16:25:50	0|wumpus|as long as the precedence order is well-defined
187 2017-07-19 16:42:26	0|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
188 2017-07-19 18:20:49	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10882: [WIP] Keypool topup (06master...06keypool_topup) 02https://github.com/bitcoin/bitcoin/pull/10882
189 2017-07-19 18:22:02	0|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
190 2017-07-19 18:22:03	0|gribble|https://github.com/bitcoin/bitcoin/issues/10830 | [WIP] [wallet] keypool restore by jnewbery · Pull Request #10830 · bitcoin/bitcoin · GitHub
191 2017-07-19 18:22:11	0|jnewbery|I think people wanted this in 0.15 as a bugfix
192 2017-07-19 18:34:40	0|instagibbs|jnewbery, will review
193 2017-07-19 18:35:53	0|jnewbery|thanks instagibbs
194 2017-07-19 18:37:17	0|bitcoin-git|[13bitcoin] 15jnewbery closed pull request #10830: [WIP] [wallet] keypool restore (06master...06pr10240) 02https://github.com/bitcoin/bitcoin/pull/10830
195 2017-07-19 18:38:45	0|jonasschnelli|I just did a quick scrollback read. Is -rpcwallet still in talk?
196 2017-07-19 18:38:53	0|jonasschnelli|I'd prefere to keep rpc out of the naming...
197 2017-07-19 18:39:04	0|jonasschnelli|isn't bitcoin-cli transport agnostic?
198 2017-07-19 18:39:19	0|jonasschnelli|(from the users interface perspective)
199 2017-07-19 18:40:19	0|sipa|what do you mean transport agnostic/
200 2017-07-19 18:42:54	0|jonasschnelli|I probably over-engineer. But what if we once change bitcoin-cli's transport layer?
201 2017-07-19 18:43:04	0|jonasschnelli|the rpc layer is hidden from the user
202 2017-07-19 18:43:35	0|jonasschnelli|-rpcwallet includes the used transport layer in the naming scheme.
203 2017-07-19 18:43:35	0|sipa|so?
204 2017-07-19 18:43:43	0|sipa|so does -wallet
205 2017-07-19 18:43:49	0|jonasschnelli|?
206 2017-07-19 18:44:01	0|sipa|i may misunderstand what you're saying
207 2017-07-19 18:44:08	0|jonasschnelli|-(rpc)wallet
208 2017-07-19 18:44:18	0|sipa|bitcoin-cli is an rpc client
209 2017-07-19 18:44:30	0|sipa|the transport layer may change from TCP to Unix sockets or something
210 2017-07-19 18:44:35	0|sipa|but it'll still be an RPC client
211 2017-07-19 18:44:58	0|jonasschnelli|What if we change to a different IPC protocol?
212 2017-07-19 18:45:16	0|jonasschnelli|I probably over-enginee. Nm.
213 2017-07-19 18:45:25	0|sipa|then everything needs to change; including rpcport, rpcserver, ...
214 2017-07-19 18:45:34	0|jonasschnelli|I just though until now, I could not see any "rpc" in bitcoin-cli (from the users perspetive)
215 2017-07-19 18:45:45	0|jonasschnelli|Yeah. That.. so yes. nm then.
216 2017-07-19 18:46:18	0|jonasschnelli|Okay. I take back my argument. With -rpcserver, etc. -rpcwallet may make sense.
217 2017-07-19 18:47:07	0|gmaxwell|what is -rpcwallet precisely
218 2017-07-19 18:47:25	0|jonasschnelli|gmaxwell: I guess it would be the substitute for -usewallet
219 2017-07-19 18:47:28	0|gmaxwell|is it the setting that tells bitcoin-cli what wallet to talk to
220 2017-07-19 18:47:28	0|sipa|gmaxwell: suggested new name for -usewallet
221 2017-07-19 18:47:32	0|gmaxwell|okay
222 2017-07-19 18:47:32	0|sipa|yes
223 2017-07-19 18:47:42	0|jonasschnelli|What was the issue with -usewallet?
224 2017-07-19 18:47:56	0|sipa|people think use is a weasel word
225 2017-07-19 18:48:08	0|sipa|(i really don't care anymore)
226 2017-07-19 18:48:51	0|jonasschnelli|Yeah, I don't care. But please not -wallet
227 2017-07-19 18:49:59	0|gmaxwell|gowallet=foo :P
228 2017-07-19 18:50:41	0|sipa|./bitcoin-cli -thethingimean=foo
229 2017-07-19 18:51:09	0|gmaxwell|enablewallet=bob
230 2017-07-19 18:51:37	0|gmaxwell|walletify=bob
231 2017-07-19 18:51:57	0|sipa|ok...
232 2017-07-19 18:52:10	0|gmaxwell|openwallet=info
233 2017-07-19 18:56:07	0|jonasschnelli|Among those options, I would still prefere "usewallet"
234 2017-07-19 19:25:14	0|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)
235 2017-07-19 19:25:34	0|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
236 2017-07-19 19:26:18	0|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.
237 2017-07-19 19:26:51	0|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
238 2017-07-19 19:28:16	0|wumpus|if it wasn't for the authentication options we could stop parsing bitcoin.conf completely for the -cli
239 2017-07-19 19:28:43	0|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
240 2017-07-19 19:28:50	0|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
241 2017-07-19 19:29:18	0|wumpus|I'm not convinced it should be treated otherwise than other options
242 2017-07-19 19:29:30	0|morcos|Meaning the last value controls?
243 2017-07-19 19:29:34	0|wumpus|yes
244 2017-07-19 19:29:46	0|wumpus|if you use -rpcport you don't expect it to connect to all ports at once, do you?
245 2017-07-19 19:29:57	0|wumpus|not sure why -rpcwallet would be different
246 2017-07-19 19:30:03	0|morcos|no but i expect someone to tell me i'm an idiot
247 2017-07-19 19:30:37	0|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
248 2017-07-19 19:30:44	0|wumpus|however we're not there, and we're not going to be there for 0.15
249 2017-07-19 19:31:18	0|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
250 2017-07-19 19:31:26	0|morcos|hmm, is the code to figure out whether something is a multiarg vs a single arg shared btwn bitcoind and -cli
251 2017-07-19 19:31:51	0|morcos|i guess it must not be?
252 2017-07-19 19:31:55	0|morcos|oh
253 2017-07-19 19:31:56	0|morcos|never mind
254 2017-07-19 19:32:04	0|morcos|if its different names it doesnt matter
255 2017-07-19 19:32:11	0|wumpus|all the argument parsing code is shared
256 2017-07-19 19:32:14	0|wumpus|itis' in util.cpp
257 2017-07-19 19:32:17	0|morcos|(rpc/use)wallet is a single arg, wallet ais a multi arg
258 2017-07-19 19:32:20	0|wumpus|but just use a different name so it doesn't matter
259 2017-07-19 19:32:21	0|wumpus|right.
260 2017-07-19 19:32:23	0|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
261 2017-07-19 19:32:43	0|wumpus|I do think the special casing is a big deal
262 2017-07-19 19:32:49	0|wumpus|but I have to maintain the code over a long time
263 2017-07-19 19:33:00	0|morcos|well lets just pick and be done with it, any will be fine..
264 2017-07-19 19:33:01	0|wumpus|and I'm sick of all kind of weird special casing
265 2017-07-19 19:33:33	0|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
266 2017-07-19 19:33:38	0|wumpus|it might seem easier on the short term but on the long time, arbitrariness is going to confuse
267 2017-07-19 19:33:58	0|wumpus|both developers and users
268 2017-07-19 19:34:04	0|wumpus|but it's always 'hey, why not add another special case'
269 2017-07-19 19:34:15	0|jnewbery|I don't even consider it special casing. Command line overrides conf file, just as for other arguments
270 2017-07-19 19:34:21	0|wumpus|noo of course not
271 2017-07-19 19:34:23	0|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
272 2017-07-19 19:34:43	0|jnewbery|it's implemented differently because it's not possible to fully override mapmultiargs
273 2017-07-19 19:34:48	0|jnewbery|but that's the functional outcome
274 2017-07-19 19:34:48	0|wumpus|if you do it it's not a special case
275 2017-07-19 19:34:51	0|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
276 2017-07-19 19:35:19	0|wumpus|I don't feel like repeating myself
277 2017-07-19 19:35:32	0|wumpus|I already argued against reusing -wallet forbitcoin-cli specifically
278 2017-07-19 19:35:33	0|ryanofsky|happy to defer
279 2017-07-19 19:36:12	0|wumpus|it's strange to have a same argument name as single-option for one utility then for multi-option for another utility
280 2017-07-19 19:36:24	0|morcos|so arguably the only thing to change from the merged code is s/usewallet/rpcwallet/ if that would overall be preferred?
281 2017-07-19 19:36:24	0|wumpus|*if* htey parse the same configuration file
282 2017-07-19 19:36:30	0|wumpus|yes
283 2017-07-19 19:36:34	0|wumpus|rpcwallet is a better name
284 2017-07-19 19:37:34	0|jonasschnelli|Should we still revoke (halt bitcoin-cli) if one provides multiple -rpc/-usewallet arguments?
285 2017-07-19 19:37:39	0|wumpus|and I'm ok with making providing -wallet on the command line for -cli an error, though I think it's overthinking it
286 2017-07-19 19:37:53	0|wumpus|jonasschnelli: no, I don't think so
287 2017-07-19 19:38:07	0|jonasschnelli|So just use the last one?
288 2017-07-19 19:38:09	0|wumpus|jonasschnelli: just like multiple -rpcport is not an error
289 2017-07-19 19:38:19	0|wumpus|unless we change that behavior on a global level
290 2017-07-19 19:38:20	0|jonasschnelli|wumpus: Okay. Fine by me
291 2017-07-19 19:38:34	0|wumpus|so if multiple of any non-multi argument is an error, it should be an error, obviously
292 2017-07-19 19:38:47	0|wumpus|but there's no need to suddenly do all kinds of crazy stuff for rpcwallet
293 2017-07-19 19:38:49	0|jonasschnelli|Yes. No special case for every argument type
294 2017-07-19 19:39:26	0|jonasschnelli|But I guess what we should merge for 0.15 is the Qt console support: https://github.com/bitcoin/bitcoin/pull/10870
295 2017-07-19 19:39:33	0|jonasschnelli|Right now, Qt console is useless in multiwallet
296 2017-07-19 19:39:38	0|wumpus|yes
297 2017-07-19 19:40:05	0|wumpus|I certainly think making the functionality work in the first place is important :)
298 2017-07-19 19:41:21	0|wumpus|tagged it
299 2017-07-19 19:42:29	0|jonasschnelli|thanks
300 2017-07-19 19:44:47	0|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
301 2017-07-19 19:45:51	0|eck|I am trying to understand the BIP process, and I'm confused how I can tell which BIPs were actually deployed
302 2017-07-19 19:45:54	0|eck|e.g. is BIP 62 deployed?
303 2017-07-19 19:46:03	0|wumpus|it can even be useful in scripts etc, to be able to override something that is set before
304 2017-07-19 19:46:05	0|ryanofsky|GetArg implement first value holds, right?
305 2017-07-19 19:46:21	0|wumpus|ryanofsky: ugh, okay
306 2017-07-19 19:46:25	0|instagibbs|eck, #bitcoin (can answer there)
307 2017-07-19 19:46:32	0|jonasschnelli|eck: look at the overview. Bip62 is "Withdrawn"
308 2017-07-19 19:46:34	0|wumpus|ryanofsky: are you sure?
309 2017-07-19 19:46:41	0|ryanofsky|no
310 2017-07-19 19:46:51	0|ryanofsky|which is why i like these things to be errors :)
311 2017-07-19 19:47:10	0|jonasschnelli|But then it should be globally?
312 2017-07-19 19:47:10	0|wumpus|but every other comamnd line utiltiy I know uses 'last setting holds'
313 2017-07-19 19:47:20	0|wumpus|gcc, ld, ls, etc
314 2017-07-19 19:47:47	0|bitcoin-git|[13bitcoin] 15morcos opened pull request #10883: Rename -usewallet to -rpcwallet (06master...06rpcwallet) 02https://github.com/bitcoin/bitcoin/pull/10883
315 2017-07-19 19:50:25	0|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
316 2017-07-19 19:50:50	0|jonasschnelli|:/
317 2017-07-19 19:50:51	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #10867: Allow only a single -usewallet argument (06master...062017/07/multiwallet_bitcoincli) 02https://github.com/bitcoin/bitcoin/pull/10867
318 2017-07-19 19:51:01	0|wumpus|yes, it seems to correctly chose the latest from the command line
319 2017-07-19 19:51:10	0|wumpus|.conf semantics are strange
320 2017-07-19 19:51:13	0|jonasschnelli|but why the first from the conf?
321 2017-07-19 19:51:21	0|jonasschnelli|that makes no sense..
322 2017-07-19 19:51:24	0|wumpus|no, it doesn't
323 2017-07-19 19:51:48	0|wumpus|it should be the other way aorund
324 2017-07-19 19:52:22	0|wumpus|for all options, not special casing one :)
325 2017-07-19 19:52:29	0|jonasschnelli|yes.
326 2017-07-19 19:52:50	0|jonasschnelli|Though changing it now could have unexpected consequences. :/
327 2017-07-19 19:53:07	0|morcos|that behavior is what preseves command line overriding right?
328 2017-07-19 19:53:41	0|wumpus|morcos: yes, indeed, in the silly way it works currently
329 2017-07-19 19:53:57	0|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
330 2017-07-19 19:54:34	0|jonasschnelli|however, we should not change that.
331 2017-07-19 19:55:04	0|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
332 2017-07-19 19:56:07	0|wumpus|especially with things like includeconf=..., people are going to expect later config options are overriding previous ones
333 2017-07-19 19:56:36	0|wumpus|like it is for any other config file parser in the world
334 2017-07-19 19:56:50	0|jonasschnelli|heh. Indeed
335 2017-07-19 19:58:21	0|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
336 2017-07-19 19:59:19	0|wumpus|so if the config file sets bind=127.0.0.1:1234, the command line can never override that, only add to it
337 2017-07-19 20:00:17	0|jonasschnelli|I'm not sure, but isn't comma separation a thing? -wallet=w1.dat,w2.dat?
338 2017-07-19 20:00:29	0|wumpus|yeah
339 2017-07-19 20:00:49	0|wumpus|though that always leaves 'what if we have a comma in our value'
340 2017-07-19 20:01:01	0|jonasschnelli|Use ;
341 2017-07-19 20:01:13	0|wumpus|but that conflicts with the shell :-)
342 2017-07-19 20:01:33	0|jonasschnelli|heh.. yes. use -wallet="a;b;c"?
343 2017-07-19 20:01:38	0|jonasschnelli|But meh
344 2017-07-19 20:01:41	0|wumpus|hehe
345 2017-07-19 20:01:53	0|wumpus|I was just illustrating a point, I know it's hard to work around
346 2017-07-19 20:02:15	0|wumpus|json syntax config file would work
347 2017-07-19 20:02:22	0|wumpus|for command line it's harder
348 2017-07-19 20:02:34	0|jonasschnelli|Yes
349 2017-07-19 20:03:42	0|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
350 2017-07-19 20:05:26	0|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
351 2017-07-19 20:06:22	0|wumpus|cool, it warns at least now: Warning: Unsupported logging category -debug=rpc,net,tor.
352 2017-07-19 20:06:35	0|wumpus|suppose that is since the debug categories are an enum
353 2017-07-19 20:06:52	0|wumpus|it used to be silent about it
354 2017-07-19 20:09:33	0|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
355 2017-07-19 20:09:36	0|jonasschnelli|BTW: github recommends to add a COC to the bitcoin core project: https://opensource.guide/code-of-conduct/
356 2017-07-19 20:11:50	0|jnewbery|I suppose we'll generally need to give bitcoin-cli more smarts if we do wallet process separation
357 2017-07-19 20:11:50	0|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
358 2017-07-19 20:12:45	0|wumpus|well ... we'll worry about that, then
359 2017-07-19 20:12:49	0|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
360 2017-07-19 20:13:05	0|sipa|so there's not much reason why you couldn't send a getpeerinfo to a wallet endpoint
361 2017-07-19 20:13:05	0|wumpus|there's no way to specify an alternative endpoint at all at the moment
362 2017-07-19 20:13:15	0|sipa|which is exactly what it does now
363 2017-07-19 20:13:25	0|sipa|for a few things, like mempool or fee estimation something else may be needed
364 2017-07-19 20:13:56	0|sipa|but that's a worry for later indeed
365 2017-07-19 20:13:58	0|wumpus|heh yes if a wallet endpoints getpeerinfo you can send getpeerinfo to it
366 2017-07-19 20:14:35	0|wumpus|if it doesn't, you'll get an error that it's not implemented
367 2017-07-19 20:15:01	0|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?
368 2017-07-19 20:15:24	0|jonasschnelli|Or do you see a different channel (RPC, etc,) for that?
369 2017-07-19 20:15:59	0|sipa|jonasschnelli: maybe... that may be controversial
370 2017-07-19 20:16:45	0|sipa|but i don't see much problem in having private extensions available only for known peers
371 2017-07-19 20:16:49	0|jonasschnelli|I'd say that would simplify connection management massively. But I'd also say some people would dislike that.
372 2017-07-19 20:17:06	0|jonasschnelli|Yes. me two
373 2017-07-19 20:17:11	0|sipa|actually, i think that was one of the uses for -whitelist when i suggested :)
374 2017-07-19 20:17:20	0|jonasschnelli|*too
375 2017-07-19 20:17:54	0|jonasschnelli|But thats far in future (net refactor, Bip151, Bip150 and maybe then).
376 2017-07-19 20:18:00	0|sipa|yah
377 2017-07-19 20:18:05	0|wumpus|I agree authenticated extensions could make sense, please don't overload -whitelist with more things
378 2017-07-19 20:18:18	0|sipa|wumpus: oh, absolutely - i agree with that now
379 2017-07-19 20:21:16	0|sipa|just giving some historical background
380 2017-07-19 20:25:13	0|wumpus|oh sure, it's one kind of whitelisting option
381 2017-07-19 20:37:09	0|bitcoin-git|[13bitcoin] 15eliahuhorwitz opened pull request #10884: Update build-windows.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10884
382 2017-07-19 20:48:23	0|bitcoin-git|13bitcoin/06master 147ec3343 15Gregory Sanders: add gdb attach process to test README
383 2017-07-19 20:48:23	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/d445a2c2eaa1...df0793f324e3
384 2017-07-19 20:48:24	0|bitcoin-git|13bitcoin/06master 14df0793f 15MarcoFalke: Merge #10681: add gdb attach process to test README...
385 2017-07-19 20:48:48	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #10681: add gdb attach process to test README (06master...06gdbattach) 02https://github.com/bitcoin/bitcoin/pull/10681
386 2017-07-19 21:18:08	0|promag|is this correct bitcoin-qt -wallet=foo.dat -wallet=wallet.dat -regtest ?
387 2017-07-19 21:18:28	0|promag|to run multiwallet in qt?
388 2017-07-19 21:28:20	0|sipa|yes
389 2017-07-19 22:08:08	0|promag|sipa: what should be the load order when -wallet=foo -wallet=bar -wallet=foo
390 2017-07-19 22:10:14	0|sipa|ha, no clue
391 2017-07-19 22:10:43	0|sipa|does that even work?
392 2017-07-19 22:11:07	0|promag|yeah
393 2017-07-19 22:11:09	0|promag|:P
394 2017-07-19 22:11:25	0|promag|I'm submitting a PR to handle that
395 2017-07-19 22:11:55	0|sipa|cool
396 2017-07-19 22:13:12	0|promag|anyway, I have 2 ideas: 1st add second argumento unique = false to ArgsManager::GetArgs
397 2017-07-19 22:13:40	0|promag|the other is to add GetUniqueArgs
398 2017-07-19 22:14:26	0|promag|which preserves order (first takes effect)
399 2017-07-19 22:14:35	0|cfields|i think the arg manager itself should just sort that out
400 2017-07-19 22:14:50	0|cfields|would we ever care about dupes post-init?
401 2017-07-19 22:15:16	0|promag|however ParseParameters explicit says that the last takes effect when one only wants the arg as a single value
402 2017-07-19 22:15:50	0|cfields|promag: yes, that's the common convention. so that you can add things later in the line to disable earlier ones
403 2017-07-19 22:16:06	0|cfields|s/disable/override/
404 2017-07-19 22:16:36	0|promag|cfields: right so it's config < arg[n] < arg[m] where m > n
405 2017-07-19 22:18:05	0|promag|but in the case of multi value then the order is? config[0], config[1], arg[0], arg[1] right?
406 2017-07-19 22:18:33	0|cfields|that's what i'd expect, yes
407 2017-07-19 22:18:50	0|promag|would we ever care about dupes post-init? -> dunno, wdyt?
408 2017-07-19 22:19:00	0|sipa|i thinkit should just error if you soecify the same one twice
409 2017-07-19 22:19:09	0|sipa|sorry, ohone typing
410 2017-07-19 22:19:28	0|promag|sipa: and if you config[1] = arg[0] ?
411 2017-07-19 22:20:06	0|promag|I mean, if you pass a value in the command line which happens to be in config?
412 2017-07-19 22:20:12	0|promag|is that a error?
413 2017-07-19 22:20:13	0|cfields|right, i would expect it to just quietly drop the dupe
414 2017-07-19 22:20:39	0|promag|the error/warn should be just for the command line args right?
415 2017-07-19 22:21:00	0|promag|so it's just a matter of fixing ParseParameters
416 2017-07-19 22:21:00	0|sipa|ok, merging them is also a reasonable thing to fo
417 2017-07-19 22:21:17	0|sipa|is it *always* the right thing?
418 2017-07-19 22:21:32	0|cfields|sipa: you mean for all args? or for wallet in particular?
419 2017-07-19 22:21:34	0|promag|dunno, didn't go that far
420 2017-07-19 22:21:47	0|sipa|cfields: for all args
421 2017-07-19 22:21:57	0|sipa|if you're talking about changing ParseParameters
422 2017-07-19 22:22:04	0|promag|for instance, -connect
423 2017-07-19 22:22:40	0|cfields|sipa: well at minimum, i think we always want to be able to override the config
424 2017-07-19 22:22:59	0|cfields|and i think that always implies merging?
425 2017-07-19 22:23:33	0|sipa|cfields: multiargs don't get merged, they get appended
426 2017-07-19 22:23:34	0|promag|if it's multiarg then imo it's unique stable merge
427 2017-07-19 22:24:07	0|sipa|i'm not convinced that for all options uniquifying is the right thing to do
428 2017-07-19 22:24:48	0|cfields|yea, ok...
429 2017-07-19 22:25:00	0|cfields|-addnode=127.0.0.1 -addnode=127.0.0.1
430 2017-07-19 22:25:04	0|cfields|i want 2 connections there
431 2017-07-19 22:25:23	0|promag|does that make sense?
432 2017-07-19 22:25:57	0|promag|if that's the case, GetUniqueArgs is the way to go?
433 2017-07-19 22:26:32	0|sipa|or just unique it in the wallet loading code
434 2017-07-19 22:26:41	0|sipa|at least if you're talking about 0.15
435 2017-07-19 22:27:32	0|promag|it's in 3 different places
436 2017-07-19 22:29:08	0|promag|the PR adds GetUniqueArgs and changes 3 occurencies of gArgs.GetArgs("-wallet")
437 2017-07-19 22:29:29	0|promag|and adds tests for GetUniqueArgs
438 2017-07-19 23:38:33	0|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)
439 2017-07-19 23:38:54	0|sipa|yes, that's intentional
440 2017-07-19 23:39:02	0|sipa|last option takes effect
441 2017-07-19 23:39:49	0|promag|but GetUniqueArgs("-foo") gives {"a", "b"} sgty?
442 2017-07-19 23:42:30	0|sipa|sure