1 2017-05-05 00:12:57	0|luke-jr|it seems that my system univalue returns a temporary, and for some reason Core is including that instead of the embedded univalue :|
  2 2017-05-05 00:13:34	0|praxeology|Not sure how "garbage collection" works with std::string.  Does that line make a string array on the stack or is it allocated in heap?
  3 2017-05-05 00:14:23	0|sipa|praxeology: c++ has no garbage collection
  4 2017-05-05 00:14:41	0|sipa|when the variable goes out of scope, it is destroyed
  5 2017-05-05 00:14:46	0|luke-jr|what is supposed to add src/univalue/include/ to the include path? it's not there..
  6 2017-05-05 00:15:25	0|sipa|praxeology: and strings are always allocated on the heap (in practice), but that's not relevant
  7 2017-05-05 00:15:40	0|sipa|praxeology: there is no refcounting for references
  8 2017-05-05 00:16:30	0|sipa|some c++ standard libraries implement refcounting for strings internally, but that still only works with different _string_ objects with the same content, not multiple references to the same one
  9 2017-05-05 00:16:43	0|luke-jr|oh, system univalue is used by default, as expected, ok
 10 2017-05-05 00:17:04	0|luke-jr|so I guess options are: 1) require newer univalue to use system, or 2) rewrite this code to work either way
 11 2017-05-05 00:18:13	0|luke-jr|it's probably slightly optimised to do num 2
 12 2017-05-05 00:18:50	0|luke-jr|is it valid to assign the return std::string temporary to a reference? or is G++ just extra tolerant?
 13 2017-05-05 00:18:57	0|gmaxwell|'system univalue' dear lord, why would that exist?
 14 2017-05-05 00:18:58	0|luke-jr|const std::string& account_param = request.params[0].get_str();
 15 2017-05-05 00:19:16	0|luke-jr|gmaxwell: because that's the right way to do libraries
 16 2017-05-05 00:19:16	0|sipa|luke-jr: it is valid
 17 2017-05-05 00:19:24	0|sipa|(i commented on the PR)
 18 2017-05-05 00:20:34	0|praxeology|sipa: yea thought so... "it's taking a pointer to a temporary", not sure why you used the word "temporary" there because if allocated on the heap, its not temporary in any way.
 19 2017-05-05 00:20:35	0|luke-jr|which PR?
 20 2017-05-05 00:20:51	0|luke-jr|praxeology: it's not. if there's no "new", it's not on the heap
 21 2017-05-05 00:20:57	0|gmaxwell|luke-jr: if it were actually a library and not some unmaintained code dump that only we use...
 22 2017-05-05 00:21:24	0|praxeology|luke-jr: are you sure there is no "new" deeper in the library?
 23 2017-05-05 00:21:25	0|sipa|praxeology: 'heap' and 'stack' don't exist in the C++ spec, they're implementation details
 24 2017-05-05 00:21:35	0|sipa|praxeology: 'temporary' however exists in the spec
 25 2017-05-05 00:22:05	0|sipa|praxeology: a temporary is the result of an expression that returns something by-value, and isn't assigned to a variable
 26 2017-05-05 00:22:30	0|bitcoin-git|[13bitcoin] 15luke-jr opened pull request #10341: rpc/wallet: Workaround older UniValue which returns a std::string temporary for get_str (06master...06rpcwallet_uv_workaround) 02https://github.com/bitcoin/bitcoin/pull/10341
 27 2017-05-05 00:23:14	0|sipa|praxeology: in practice, the std::string is allocated on the stack, but the string object itself allocates the string/length data on the heap
 28 2017-05-05 00:24:43	0|sipa|but again, that doesn't matter
 29 2017-05-05 00:24:56	0|sipa|the compiler may even optimize part out and not have such objects hit memory at all
 30 2017-05-05 00:25:15	0|sipa|what matters is that it's returning an object and not storing it in a variable
 31 2017-05-05 00:26:17	0|praxeology|Hm, yea I guess somehow with the automatic out-of scope clearing/freeing/deleting of the actual data... you need to assign the std::string to a stack variable for the duration you want it to exist
 32 2017-05-05 00:26:53	0|sipa|no, you don't
 33 2017-05-05 00:26:58	0|sipa|that's what i just pointed out
 34 2017-05-05 00:27:25	0|sipa|you can take a reference to a temporary, and in that case, the lifetime of the temporary is automatically extended as long as the reference exists
 35 2017-05-05 00:27:46	0|sipa|but that's done by static analysis by the compiler, not by reference counting
 36 2017-05-05 00:28:17	0|praxeology|its returning an object that becomes part of stack, but in order for that to happen you need to assign the object to a variable that exists in the stack
 37 2017-05-05 00:28:35	0|sipa|stop talking about stacks, that doesn't matter
 38 2017-05-05 00:30:00	0|praxeology|ok, scope somehow.  but that is less clear to me how such works...  I guess we don't have to discuss it here, I can look into it  myself
 39 2017-05-05 00:36:56	0|praxeology|const std::string* account = nullptr;
 40 2017-05-05 00:37:38	0|praxeology|woops sorry I'll leave you guys alone now
 41 2017-05-05 01:34:40	0|dcousens|sipa: "its modifying rng_state, protected by the lock" --- I suppose, for an RNG, racing memcpy of random data isn't as much an issue? :D haha
 42 2017-05-05 01:36:38	0|gmaxwell|it's very much an issue, as it could cause arbritary corruption.
 43 2017-05-05 01:36:55	0|dcousens|gmaxwell: would it could corruption outside of rng_state though?
 44 2017-05-05 01:36:59	0|gmaxwell|Yes.
 45 2017-05-05 01:37:04	0|dcousens|OK, then yes, issue
 46 2017-05-05 01:37:20	0|gmaxwell|yea, I was just being stupid and thought that was the copy into the output too.
 47 2017-05-05 01:39:00	0|dcousens|gmaxwell: ooi, what could I lookup to understand the implications of that, I wasn't aware it could cause arbitrary corruption
 48 2017-05-05 01:39:02	0|gmaxwell|dcousens: in C/C++ the compiler is allowed to assume that it has exclusive access to any variable it writes to in a scope for the duration of it (the technical guarentee is more complicated and I don't fully understand it, but this approximation is enough).  So the compiler could do optimizations like temporarily store the stack pointer into memory that it knows it's going to later overwrite.  t
 49 2017-05-05 01:39:08	0|gmaxwell|hen copy it back out again before overwriting it.
 50 2017-05-05 01:39:10	0|dcousens|gmaxwell: answered as I asked
 51 2017-05-05 01:40:03	0|dcousens|cheers
 52 2017-05-05 01:40:11	0|gmaxwell|Its very rare for it to _in practice_ cause arbritary corruption, but it's not unheared of.. intel's compiler is a little more prone to crazy stunts that expose things like that than GCC is, but presumably future versions of GCC will keep getting smarter until nothing runs. :)
 53 2017-05-05 01:40:27	0|dcousens|ha
 54 2017-05-05 06:00:43	0|jonasschnelli|<*highlight>	[22:47:52] <BlueMatt:#bitcoin-core-dev> jonasschnelli: you might kill me for #10340, sorry :/
 55 2017-05-05 06:00:44	0|gribble|https://github.com/bitcoin/bitcoin/issues/10340 | Add harmless missing cs_wallet lock in qt CoinControlDialog by TheBlueMatt · Pull Request #10340 · bitcoin/bitcoin · GitHub
 56 2017-05-05 06:02:02	0|jonasschnelli|BlueMatt: seems reasonable to me (10340)
 57 2017-05-05 07:49:04	0|jonasschnelli|BlueMatt: Ah.. know I now why you mentioned " you might kill me". Massive layer violation.
 58 2017-05-05 13:11:58	0|bitcoin-git|[13bitcoin] 15jtimon closed pull request #10119: Util: Remove ArgsManager wrappers: (06master...060.14-args-wrappers) 02https://github.com/bitcoin/bitcoin/pull/10119
 59 2017-05-05 13:26:28	0|jonasschnelli|Collecting stats over CScheduler with a interval of 2000ms (TBD), would it still make sense to collect a timestamp per sample of is CScheduler in general precise enough so we can calculate (index-of-sample * 2000ms) == timeoffset?
 60 2017-05-05 13:31:54	0|rafalcpp|dcousens: with any UB in c++, anything can happen. In theory a race condition in some debug code, could make your node start mining dogecoin instead
 61 2017-05-05 13:32:49	0|dcousens|rafalcpp: haha, I suppose I was simply affirming it was U/B,  not just a product of something else I'd missed
 62 2017-05-05 13:35:38	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10342: [tests] Improve mempool_persist test (06master...06improve_wait_until_test) 02https://github.com/bitcoin/bitcoin/pull/10342
 63 2017-05-05 13:36:15	0|Chris_Stewart_5|Is there a way to have two local regtest node share the same chain, while having different wallets?
 64 2017-05-05 13:40:00	0|instagibbs|Chris_Stewart_5, this is #bitcoin question, but yes
 65 2017-05-05 14:25:26	0|BlueMatt|jonasschnelli: ehh, I didnt feel like making needless copies of everything 3 times
 66 2017-05-05 14:25:36	0|BlueMatt|but i can do that if you prefer
 67 2017-05-05 14:26:39	0|jonasschnelli|BlueMatt: I don't like accessing the wallet's insides from outside of walletmodel. We can make an exception,... but I guess ryanofsky has to do more work in case we want process sep.
 68 2017-05-05 14:27:07	0|BlueMatt|well we can merge ryanofsky's pr first and then maybe extend it even further and never expose COutput outside of wallet
 69 2017-05-05 14:27:12	0|BlueMatt|ultimately we should move towards ^
 70 2017-05-05 14:27:33	0|jonasschnelli|Yes. Thats a good point. For intermediate steps, I think it's fine.
 71 2017-05-05 14:28:49	0|BlueMatt|I mean I'm more than happy to not take 10340 yet and do the real fix, as long as we get it in for 0.15
 72 2017-05-05 14:28:52	0|BlueMatt|it shouldnt be /too/ hard
 73 2017-05-05 14:29:02	0|ryanofsky|my pr that stops using coutput/cwallettx (#10244) is ready, just needs review
 74 2017-05-05 14:29:04	0|gribble|https://github.com/bitcoin/bitcoin/issues/10244 | [qt] Add abstraction layer for accessing node and wallet functionality from gui by ryanofsky · Pull Request #10244 · bitcoin/bitcoin · GitHub
 75 2017-05-05 14:45:04	0|BlueMatt|oh, heh, missed that
 76 2017-05-05 14:45:06	0|BlueMatt|k, will review
 77 2017-05-05 14:46:46	0|bitcoin-git|[13bitcoin] 15TheBlueMatt closed pull request #10340: Add harmless missing cs_wallet lock in qt CoinControlDialog (06master...062017-05-fix-mapwallet-zap-runtime) 02https://github.com/bitcoin/bitcoin/pull/10340
 78 2017-05-05 15:59:01	0|bitcoin-git|[13bitcoin] 15practicalswift opened pull request #10343: Remove redundant on-the-same-line-repetition of type names (DRY): RepeatedTypeName foo = static_cast<RepeatedTypeName>(bar) (06master...06auto) 02https://github.com/bitcoin/bitcoin/pull/10343
 79 2017-05-05 17:06:42	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10344: [tests] Fix abandonconflict.py intermittency (06master...06fix_abandon_conflict) 02https://github.com/bitcoin/bitcoin/pull/10344
 80 2017-05-05 17:32:22	0|BlueMatt|sipa: really? per-txout ends with a smaller dbcache? that's (marginally) surprising to me
 81 2017-05-05 17:33:21	0|sipa|BlueMatt: likewise
 82 2017-05-05 17:34:00	0|sipa|BlueMatt: in my memory the master dbcache usage for a full reindex was around 4GB or so, but apparently i haven't done one in a very long time...
 83 2017-05-05 17:34:17	0|BlueMatt|heh
 84 2017-05-05 17:39:23	0|BlueMatt|jtimon: do you feel so inclined to redo #9494 with scripted-diff?
 85 2017-05-05 17:39:24	0|gribble|https://github.com/bitcoin/bitcoin/issues/9494 | Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs by jtimon · Pull Request #9494 · bitcoin/bitcoin · GitHub
 86 2017-05-05 17:39:33	0|BlueMatt|it would be kinda nice, though I'd understand if you dont want to redo it at this point
 87 2017-05-05 17:51:05	0|jtimon|BlueMatt: yeah, for some parts of it it would make a lot of sense. I think I may have broke something in the last edit from what travis said. I will fix that and will s/argsGlobal/gArgs/ and see if I can move some parts to scripte-diff without much effort
 88 2017-05-05 17:52:14	0|BlueMatt|sounds good
 89 2017-05-05 17:52:16	0|BlueMatt|cool
 90 2017-05-05 17:53:40	0|jtimon|BlueMatt: in the meantime #8855 remains very easy to review :p
 91 2017-05-05 17:53:43	0|gribble|https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
 92 2017-05-05 18:37:45	0|BlueMatt|heh, sipa, I was about to PR https://github.com/bitcoin/bitcoin/commit/64d45d0fc3b6f7669d0f2c24bca3beaa37cec0b3
 93 2017-05-05 18:37:54	0|BlueMatt|(and https://github.com/bitcoin/bitcoin/commit/5e03232a5dcc4674c620187f67163dc462fbafd1)
 94 2017-05-05 18:39:17	0|sipa|BlueMatt: ha
 95 2017-05-05 18:39:25	0|sipa|RandAddSeedSleepSanity has no return statement
 96 2017-05-05 18:39:43	0|luke-jr|sounds very random.
 97 2017-05-05 18:39:44	0|BlueMatt|oh, well, i never said i had gotten a chance to test it yet
 98 2017-05-05 18:40:55	0|BlueMatt|sipa: it seems a waste to call GetPerformanceCounter and *not* go ahead and add it to our random state
 99 2017-05-05 18:41:51	0|sipa|BlueMatt: how about we add that after #10338 ?
100 2017-05-05 18:41:52	0|gribble|https://github.com/bitcoin/bitcoin/issues/10338 | Maintain state across GetStrongRandBytes calls by sipa · Pull Request #10338 · bitcoin/bitcoin · GitHub
101 2017-05-05 18:42:13	0|BlueMatt|sipa: initially it was built on both, but right now everything is pushed into openssl's prng state, so...meh?
102 2017-05-05 18:46:04	0|sipa|BlueMatt: like this? https://github.com/bitcoin/bitcoin/pull/10322/commits/999021ea0c6fce6dcef1ff05514c983e60ddc7eb
103 2017-05-05 18:47:02	0|BlueMatt|sipa: sure! RandAddSeed() also memory_cleanse()s, but I dont think thats actually useful
104 2017-05-05 19:15:40	0|jonasschnelli|hmm... CScheduler seems to be innacurate. I've set up scheduler.scheduleEvery(fn,2000) (every 2000ms)... I often get a callback with a delta of 10 seconds between each other...
105 2017-05-05 19:15:50	0|BlueMatt|huh?
106 2017-05-05 19:15:53	0|BlueMatt|thats...not good
107 2017-05-05 19:16:43	0|jonasschnelli|I need to look into it
108 2017-05-05 19:17:04	0|BlueMatt|jonasschnelli: which version? cfields fixed something with sub-second precision not too long ago
109 2017-05-05 19:17:10	0|BlueMatt|but i believe the symptom was high cpu usage
110 2017-05-05 19:17:41	0|jonasschnelli|Hmm.. my Laptop runs pretty quite... i'll investigate
111 2017-05-05 19:18:08	0|jonasschnelli|running in LLDB shoudln't cause such high offsets (i'm not halting the process)¨
112 2017-05-05 19:20:05	0|jonasschnelli|BlueMatt: What if another Scheduler task requires a couple of seconds to complete? (not familiar with the internals of the Scheduler)
113 2017-05-05 19:20:30	0|BlueMatt|sure, that could cause it, but the only thing in scheduler is wallet flush, really
114 2017-05-05 19:21:21	0|jonasschnelli|hmm... I'm not sure if that is the right approach for collecting stats... well,.. at least I have to add a timestamp then to each sample
115 2017-05-05 19:21:43	0|BlueMatt|hmm?
116 2017-05-05 19:22:07	0|sipa|jonasschnelli: you'll have a timestamp on each sample regardless
117 2017-05-05 19:22:34	0|jonasschnelli|sipa: You mean I should have one per sample?
118 2017-05-05 19:22:37	0|sipa|yes
119 2017-05-05 19:22:49	0|sipa|you can't guarantee that you'll always be able to gather any statistic at any given point in time at that accuracy
120 2017-05-05 19:22:54	0|jonasschnelli|Yes. I just removed it when I switch to useing CScheduler.. but I guess your right
121 2017-05-05 19:23:02	0|sipa|BlueMatt: jonas is gather statistics
122 2017-05-05 19:23:07	0|BlueMatt|stats for what?
123 2017-05-05 19:23:08	0|sipa|every 2 seconds
124 2017-05-05 19:23:23	0|jonasschnelli|BlueMatt: my stats collector PR (== mempool graph)
125 2017-05-05 19:23:25	0|sipa|i assume things like memory usage, bandwidth, ...
126 2017-05-05 19:23:29	0|BlueMatt|ohoh, that one
127 2017-05-05 19:23:36	0|jonasschnelli|For now it does only collect mempool data..
128 2017-05-05 19:23:47	0|jonasschnelli|but it would be trivial to expand it for bandwidth, etc.
129 2017-05-05 19:24:27	0|jonasschnelli|sipa: you think it's worth keeping a int64 for the starttime and only uint32 deltas per sample?
130 2017-05-05 19:24:30	0|BlueMatt|well if people want to start seriously using the scheduler we can give it another thread
131 2017-05-05 19:25:06	0|sipa|jonasschnelli: i assume your resolution is never less than a second
132 2017-05-05 19:25:17	0|sipa|jonasschnelli: in that case you can use int32 for the starttime and int16 for the deltas :)
133 2017-05-05 19:26:36	0|jonasschnelli|sipa: I have now different precision levels and I wanted to keep it absolute per sample... but,... I can't remember why I have damned the idea of deltaes between samples..
134 2017-05-05 19:27:13	0|jonasschnelli|Yes. I guess this would work now that I run over the Schedular (rather then collect "on event")
135 2017-05-05 19:27:48	0|sipa|using absolute time and data is certainly easier... you can just delete entries to reduce resolution over time
136 2017-05-05 19:27:56	0|jonasschnelli|But still strange that sometimes the Scheduler fires the 2000ms interval after 10000ms (its an inaccuracy of x5)
137 2017-05-05 19:27:56	0|sipa|but uses more memory
138 2017-05-05 19:28:17	0|sipa|that does not sound normal and should be investigated regardless
139 2017-05-05 19:28:51	0|sipa|you're sure this is not due to lock contention on main or mempool, right?
140 2017-05-05 19:29:22	0|jonasschnelli|https://0bin.net/paste/HXqOTrnFkRzwECHA#8D9wGGWTFJwNZ1vkcQqarVAawSDdZZtENrMBPKeQcYu
141 2017-05-05 19:29:22	0|jonasschnelli|Some sample output of a 2000ms scheduler:
142 2017-05-05 19:29:23	0|gribble|https://github.com/bitcoin/bitcoin/issues/8 | RPC command to sign text with wallet private key · Issue #8 · bitcoin/bitcoin · GitHub
143 2017-05-05 19:29:37	0|jonasschnelli|No,... it does only lock a new cs_stats
144 2017-05-05 19:29:57	0|jonasschnelli|The mempool obtain it's own atomic caches in my current working branch
145 2017-05-05 19:31:01	0|jonasschnelli|Although I run with the GUI... but shoudln't make any different. It's a calm regtest peer anyways
146 2017-05-05 19:55:23	0|bitcoin-git|[13bitcoin] 15sdaftuar opened pull request #10345: [P2P] Timeout for headers sync (06master...062017-05-timeout-headers-sync) 02https://github.com/bitcoin/bitcoin/pull/10345
147 2017-05-05 20:03:05	0|paveljanik|BTW mempool - "standard" nodes are now running with ~300MB mempools...
148 2017-05-05 20:03:50	0|gmaxwell|paveljanik: yep,   "mempoolminfee": 0.00002822
149 2017-05-05 20:04:06	0|gmaxwell|hurray, system is back to a healthy state.
150 2017-05-05 20:04:27	0|paveljanik|not so often tested/run code parts are now being tested :-)
151 2017-05-05 20:04:29	0|BlueMatt|gmaxwell: +1
152 2017-05-05 20:04:30	0|instagibbs|what was happening before?
153 2017-05-05 20:04:37	0|instagibbs|I missed all the excitement it seems
154 2017-05-05 20:04:58	0|gmaxwell|instagibbs: nah just for the last couple months we were riding at the minimum mempoolminfee... everything was getting confirmed.
155 2017-05-05 20:05:37	0|instagibbs|RIP sub-sat/byte confirms
156 2017-05-05 21:06:42	0|bitcoin-git|[13bitcoin] 15practicalswift opened pull request #10346: Use range-based for loops (C++11) when looping over vector elements (06master...06range-based-for-loops) 02https://github.com/bitcoin/bitcoin/pull/10346
157 2017-05-05 21:22:12	0|bitcoin-git|[13bitcoin] 15practicalswift closed pull request #10346: Use range-based for loops (C++11) when looping over vector elements (06master...06range-based-for-loops) 02https://github.com/bitcoin/bitcoin/pull/10346
158 2017-05-05 21:37:31	0|bitcoin-git|[13bitcoin] 15practicalswift opened pull request #10347: Use range-based for loops (C++11) when looping over vector elements (06master...06range-based-for-loops) 02https://github.com/bitcoin/bitcoin/pull/10347
159 2017-05-05 22:04:03	0|phantomcircuit|gmaxwell, odd
160 2017-05-05 22:04:15	0|phantomcircuit|my bitcoind running patches to accept anything
161 2017-05-05 22:04:19	0|phantomcircuit|is still   "mempoolminfee": 0.00000000
162 2017-05-05 22:04:43	0|sipa|"usage": 294825248,
163 2017-05-05 22:04:47	0|sipa|"mempoolminfee": 0.00001801
164 2017-05-05 22:05:02	0|phantomcircuit|"usage": 298733328,
165 2017-05-05 22:05:07	0|phantomcircuit|oh
166 2017-05-05 22:05:16	0|phantomcircuit|it's because i have maxmempool set to like infinity also
167 2017-05-05 22:05:17	0|phantomcircuit|derp
168 2017-05-05 22:06:26	0|luke-jr|lol
169 2017-05-05 22:07:34	0|phantomcircuit|so it's just on the edge
170 2017-05-05 22:21:10	0|BlueMatt|phantomcircuit: yea, well because most of your peers are limiting to 3MB its rare that your mempool ever gets much above 3mb
171 2017-05-05 22:21:16	0|BlueMatt|whether you limit or not
172 2017-05-05 22:21:24	0|sipa|3MB ??
173 2017-05-05 22:21:32	0|sipa|300?
174 2017-05-05 22:21:53	0|BlueMatt|oh, yes, 300
175 2017-05-05 22:22:33	0|phantomcircuit|BlueMatt, i have peers that are before any kind of mempool limiting
176 2017-05-05 22:22:34	0|phantomcircuit|but yeah
177 2017-05-05 22:22:50	0|BlueMatt|well their peers :p
178 2017-05-05 22:23:04	0|BlueMatt|even my 200-connection bitcoind seednode has only 307MB of txn