1 2017-12-04 02:35:33	0|xRavenheart|anyone able to assist me in my queries about a school project please?
  2 2017-12-04 02:47:32	0|Randolf|xRavenheart:  What's your school project about?  Bitcoin development?
  3 2017-12-04 02:50:39	0|xRavenheart|hi anyone able to assist me in a school project related question?
  4 2017-12-04 02:51:42	0|sipa|02:47:32 < Randolf> xRavenheart:  What's your school project about?  Bitcoin development?
  5 2017-12-04 02:51:44	0|luke-jr|xRavenheart: not here
  6 2017-12-04 02:55:28	0|Randolf|luke-jr:  You're right.  However, they just contacted me via PM in response to my response, so they probably have a secondary account watching this channel (I suspect).
  7 2017-12-04 02:58:48	0|Dizzle|Randolf: he's here, rejoined. I think luke-jr was just suggesting he ask elsewhere, e.g. #bitcoin
  8 2017-12-04 02:58:58	0|sipa|i'm talking in PM to him
  9 2017-12-04 03:05:01	0|xRavenheart|Ok guys, it seems like i should raise the question here instead of PM-ing
 10 2017-12-04 03:06:01	0|xRavenheart|Basically, im a final year student doing FYP and my project scope is to modify on an existing lightweight client by bringing in optimizations from other clients (full/lightweight).
 11 2017-12-04 03:06:19	0|Randolf|Dizzle:  Ah, yes.  I suggested that asking here may be appropriate since it might involve core development.  I also suggested that they ask in the #bitcoin and ##altcoins channel.
 12 2017-12-04 03:06:30	0|xRavenheart|This optimization may be in any form such as block headers download, transacting speed, retrieving of wallet addresses, etc etc.
 13 2017-12-04 03:07:03	0|sipa|xRavenheart: you're welcome here to ask questions about bitcoin's design and operation
 14 2017-12-04 03:07:08	0|xRavenheart|E.g. For instance Bitcoin Core has optimizations 1,2 and Electrum has optimizations 3,4
 15 2017-12-04 03:07:22	0|xRavenheart|I'm required to bring together these optimizations into one application
 16 2017-12-04 03:07:32	0|sipa|though as i told, most of that will not be applicable to other types of clients
 17 2017-12-04 03:07:45	0|xRavenheart|Is anyone able to first provide me with a clear architecture diagram of bitcoin?
 18 2017-12-04 03:08:04	0|xRavenheart|I've been trying to understand the system but it is way too broad and i have no clear idea of where to start
 19 2017-12-04 03:08:09	0|sipa|ii don't think that exists
 20 2017-12-04 05:36:42	0|achow101|is there any particular reason that Core is using 26 GB of RAM during a reindex?
 21 2017-12-04 05:36:52	0|achow101|(out of 32 available on my machine)
 22 2017-12-04 05:37:04	0|gmaxwell|sipa: ^
 23 2017-12-04 05:37:05	0|achow101|my dbcache is only set to 6000
 24 2017-12-04 05:37:10	0|gmaxwell|achow101: master, right?
 25 2017-12-04 05:37:14	0|achow101|gmaxwell: yeah
 26 2017-12-04 05:37:35	0|gmaxwell|achow101: sipa saw this on his odroid too, thought it was a bug in master, but I think he had some reproduction problems.
 27 2017-12-04 05:37:45	0|gmaxwell|It may be a leak that depends on particular dependency versions.
 28 2017-12-04 05:38:17	0|achow101|I can consistently reproduce it by starting it, letting it go, and then waiting for my computer to freeze up.
 29 2017-12-04 05:38:27	0|achow101|then reboot and repeat :/
 30 2017-12-04 05:41:49	0|gmaxwell|any chance you could run in valgrind, let it obviously bloat up a bit then issue a clean stop?  if it's a true leak valgrind will report it.
 31 2017-12-04 05:42:15	0|achow101|Yeah, I'll do that once I reboot it again
 32 2017-12-04 05:42:19	0|gmaxwell|(I'd bet it's not actually a true leak... we have far too little manual memory management for that to be likely... but worth checking)
 33 2017-12-04 05:44:01	0|Randolf|I could try running bitcoind to confirm memory usage too -- I've got bitcoind v0.15.1 on Windows 10 (64-bit) handy.
 34 2017-12-04 05:44:11	0|Randolf|Which command-line parameters are you using?
 35 2017-12-04 05:44:35	0|gmaxwell|0.15.1 doesn't exhibit this behavior in any testing we've seen so far.
 36 2017-12-04 05:44:43	0|Randolf|Oh.  Which version are you using?
 37 2017-12-04 05:45:01	0|gmaxwell|git master... the current in-development stuff.
 38 2017-12-04 05:45:06	0|Randolf|Oh.  I see.
 39 2017-12-04 05:45:22	0|achow101|Some build of master from sometime in the last few days
 40 2017-12-04 05:46:00	0|achow101|This is fairly recent, I think it's from yesterday or the day before
 41 2017-12-04 05:46:17	0|achow101|Although maybe I only noticed it now because I had to reindex
 42 2017-12-04 05:46:34	0|Randolf|Which OS are you running on?
 43 2017-12-04 05:46:42	0|achow101|Ubuntu 17.10
 44 2017-12-04 05:47:08	0|Randolf|Oh, I don't have that available.  I have a NetBSD server I could run it on if you need an extra test run.
 45 2017-12-04 05:48:05	0|Randolf|I'd have to get sync'd though because I don't have bitcoind running there now.  Maybe I should see if I can get it going and get the sync started for now.
 46 2017-12-04 05:50:25	0|achow101|also, i'm running bitcoin-qt right now, not bitcoind
 47 2017-12-04 05:57:34	0|Randolf|These servers don't have any GUI stuff installed.  So I won't be able to try bitcoin-qt on them.  But I could at least try bitcoin-qt on Windows 10 to confirm whether the same problem occurs cross-platform.
 48 2017-12-04 06:03:06	0|achow101|btw I'm using commit 00d25e90db06149fa456b0a8f15b7b68005ff9c5 which is the current head of master
 49 2017-12-04 06:09:50	0|achow101|gmaxwell, sipa: when did you observe this issue on the odroid?
 50 2017-12-04 06:17:35	0|Randolf|Hmm, I'm running into problems getting bitcoind compiled on NetBSD, but I've had problems getting other things to compile due to a learning curve on my part so I'll have to work on this.  In the meantime, I'll look into commit 00d25e90db06149fa456b0a8f15b7b68005ff9c5 (as I continue to learn more
 51 2017-12-04 06:19:25	0|achow101|it might be useful to walk back through the commit history testing each merge until we get to one that doesn't exhibit the leak. that would then tell us what commit introduced the problem, in theory.
 52 2017-12-04 06:19:34	0|achow101|although that will be very tedious to do
 53 2017-12-04 06:19:46	0|sipa|achow101: i have a difficult time reproducing it; i just saw it twice
 54 2017-12-04 06:19:47	0|Randolf|about GitHub) to see if there's something in there that I can test out on Windows.
 55 2017-12-04 06:19:56	0|sipa|it would be asome if you can bisect
 56 2017-12-04 06:20:41	0|achow101|sipa: I'm going to try that once I finish this valgrind test
 57 2017-12-04 06:20:46	0|sipa|great
 58 2017-12-04 06:20:53	0|achow101|but because it's valgrind, it's going slowly
 59 2017-12-04 06:20:55	0|sipa|i do know 0.15.1 is unaffected
 60 2017-12-04 06:21:31	0|achow101|will bisect work with 0.15.1 even though it is not part of master's commit history
 61 2017-12-04 06:22:24	0|sipa|if not, you can always use the last common forking point between master and 0.15.1 is goof
 62 2017-12-04 06:22:37	0|achow101|indeed
 63 2017-12-04 06:23:01	0|sipa|*good, not goof
 64 2017-12-04 06:38:23	0|Varunram|achow101: If you set the dbcache in the GUI, you might be affected by this: #11788
 65 2017-12-04 06:38:24	0|gribble|https://github.com/bitcoin/bitcoin/issues/11788 | Bitcoin-Qt (Settings) reports incorrect "Size of database cache" · Issue #11788 · bitcoin/bitcoin · GitHub
 66 2017-12-04 06:38:34	0|achow101|Varunram: bitcoin.conf, not gui
 67 2017-12-04 06:38:44	0|achow101|I don't use the gui for any settings at all
 68 2017-12-04 06:39:32	0|Varunram|phew, along those messages, I read somewhere you were running qt, so..
 69 2017-12-04 06:39:58	0|achow101|I do run qt for testing things, and I noticed this issue while running qt
 70 2017-12-04 06:40:06	0|sipa|that's very unlikely to be related
 71 2017-12-04 06:40:17	0|achow101|but I'm just running bitcoind in valgrind right now
 72 2017-12-04 06:40:22	0|Varunram|yep, mostly not :) just wondered
 73 2017-12-04 07:01:11	0|achow101|damn valgrind is slow
 74 2017-12-04 07:03:14	0|eck|should be about 10x slower
 75 2017-12-04 07:03:53	0|sipa|much worse for cryptographic code
 76 2017-12-04 07:04:08	0|achow101|It's still in the rescanning part of init
 77 2017-12-04 07:04:32	0|eck|the actual numbers depend on how many memory accesses you are doing, all of the instructions that hit main memory are "emulated"
 78 2017-12-04 07:04:53	0|sipa|all instructions are emulated
 79 2017-12-04 07:05:01	0|sipa|it's not like it's running in a VM
 80 2017-12-04 07:05:21	0|sipa|but memory access get additional instrumentation
 81 2017-12-04 07:06:12	0|eck|i thought it worked by rewriting all of the instructions that access main memory, and then sort of fusing the non-memory instructions, i read julian seward's paper on it a long time ago
 82 2017-12-04 07:06:28	0|eck|but i certainly could be wrong
 83 2017-12-04 07:06:41	0|sipa|hmm, so could i
 84 2017-12-04 07:08:47	0|sipa|i didn't know it was that advanced, actually but it certainly could be
 85 2017-12-04 07:13:49	0|eck|in some cases it has to emulate cpu registers, in other cases the emulated cpu register may be the actual host cpu registers, section 3.4 http://www.valgrind.org/docs/valgrind2007.pdf
 86 2017-12-04 07:14:20	0|eck|clearly if the registers need to be emulated the overhead will be quite high
 87 2017-12-04 07:48:10	0|Randolf|Well, I was unsuccessful at getting bitcoind to compile on NetBSD because of problems with Berkeley DB API function naming, and Boost incompatibility problems.  I'll have to try again when get some more time (and I will try again).
 88 2017-12-04 08:07:54	0|eck|fyi if you run configure with --disable-wallet you don't need berkeleydb, which would just leave you to figure out boost
 89 2017-12-04 08:18:30	0|achow101|I gave up on valgrind. It was doing the rescanning thing and moving at one percent per hour. I'm going to do the git bisect stuff tomorrow
 90 2017-12-04 08:27:29	0|eck|were you using massif or memcheck?
 91 2017-12-04 08:27:35	0|achow101|memcheck
 92 2017-12-04 08:27:51	0|eck|massif will just straight up tell you at any given point of time, where all the allocations came from
 93 2017-12-04 08:28:26	0|eck|which is useful if you can't wait for the program to run to completion
 94 2017-12-04 08:28:57	0|eck|iirc it's even slower than memcheck though
 95 2017-12-04 08:28:58	0|achow101|the problem was that it wasn't getting passed the init part. the leak is definitely coming from somwehere past the init part
 96 2017-12-04 08:30:08	0|eck|i have heard that ASAN is faster than valgrind but i haven't used it
 97 2017-12-04 08:30:18	0|eck|http://www.chromium.org/developers/testing/addresssanitizer http://www.chromium.org/developers/testing/leaksanitizer
 98 2017-12-04 08:30:20	0|eck|it's built into vlang
 99 2017-12-04 08:30:22	0|eck|*clang
100 2017-12-04 08:31:37	0|eck|this is for a rescan?
101 2017-12-04 08:32:44	0|achow101|reindex
102 2017-12-04 08:33:07	0|achow101|the rescan I think is part of the reindex because the reindex was killed before it flushed
103 2017-12-04 08:35:01	0|eck|i might take a look at that this week (if i can reproduce it), i was interested in generally profiling bitocind -reindex using systemtap
104 2017-12-04 08:37:25	0|achow101|made an issue for this #11822
105 2017-12-04 08:37:26	0|gribble|https://github.com/bitcoin/bitcoin/issues/11822 | Severe memory leak on current master · Issue #11822 · bitcoin/bitcoin · GitHub
106 2017-12-04 08:59:25	0|Randolf|eck:  Thanks.  There's an incompatibility because I have the wrong version of Boost.
107 2017-12-04 09:00:09	0|eck|it is pretty straightforward to build, if you have the patience
108 2017-12-04 09:01:32	0|Randolf|eck:  I have plenty of patience, but my problem is meeting other deadlines at the moment so I've put this on hold for now.  I also need to learn more about compiling.
109 2017-12-04 09:17:08	0|achow101|I fairly quick and rather unscientific git bisect says that e545dedf72bff2bd41c93c93eb576929fce37112 is the commit that introduces the problem.
110 2017-12-04 09:17:40	0|achow101|so if you want to investigate, that commit and commits in its vicinity should be looked at. But I need to sleep, so I'll look at it tomorrow.
111 2017-12-04 11:32:22	0|mmhhtt|Hello, Is there anyone knows to how can build a exchange market for other country? Is there any request from bitcoin company to me for making exc market?
112 2017-12-04 11:33:13	0|mmhhtt|also how can work payment for buy/sell btc for other country local money?
113 2017-12-04 12:13:44	0|sipa|achow101: that's what i feared...
114 2017-12-04 13:11:01	0|RK_|Hello
115 2017-12-04 13:11:48	0|RK_|I want to know about bitcoin
116 2017-12-04 13:28:11	0|mmhhtt|#bitcoin-core-dev
117 2017-12-04 13:28:16	0|mmhhtt|hello
118 2017-12-04 13:28:39	0|mmhhtt|Is there any one?
119 2017-12-04 14:16:45	0|notabot_|mmhhtt: try #bitcoin channel or reddit
120 2017-12-04 14:17:37	0|notabot_|RK_: bitcoin.org is the best place to start.
121 2017-12-04 16:04:10	0|BlueMatt|sipa: #11403 looks to have failed travis to to a bad iterator deref ( https://travis-ci.org/bitcoin/bitcoin/jobs/309801879#L2838 )
122 2017-12-04 16:04:14	0|gribble|https://github.com/bitcoin/bitcoin/issues/11403 | SegWit wallet support by sipa · Pull Request #11403 · bitcoin/bitcoin · GitHub
123 2017-12-04 18:26:59	0|BlueMatt|wumpus: I believe #10773 can be merge
124 2017-12-04 18:27:00	0|BlueMatt|d
125 2017-12-04 18:27:02	0|gribble|https://github.com/bitcoin/bitcoin/issues/10773 | Shell script cleanups by practicalswift · Pull Request #10773 · bitcoin/bitcoin · GitHub
126 2017-12-04 18:47:11	0|BlueMatt|achow101: ping
127 2017-12-04 18:47:20	0|BlueMatt|can you reliably reproduce #11822 ?
128 2017-12-04 18:47:21	0|gribble|https://github.com/bitcoin/bitcoin/issues/11822 | Severe memory leak on current master · Issue #11822 · bitcoin/bitcoin · GitHub
129 2017-12-04 18:47:25	0|achow101|BlueMatt: pong-ish
130 2017-12-04 18:47:27	0|achow101|yes
131 2017-12-04 18:48:17	0|achow101|I'm trying it in valgrind again, it's still going very slowly
132 2017-12-04 18:50:19	0|BlueMatt|sec, I doubt its actually a leak, maybe just validation running ahead of wallet
133 2017-12-04 18:50:22	0|BlueMatt|do you have a big(ish) wallet?
134 2017-12-04 18:51:06	0|BlueMatt|achow101: can you try https://github.com/TheBlueMatt/bitcoin/tree/2017-12-11822-debug and see what debug log looks like?
135 2017-12-04 18:51:16	0|BlueMatt|do you get a ton of events generated and then not executed?
136 2017-12-04 19:02:41	0|achow101|BlueMatt: the wallet is literally empty
137 2017-12-04 19:02:50	0|achow101|I haven't used it; it's completely fresh
138 2017-12-04 19:02:56	0|BlueMatt|but it has a wallet?
139 2017-12-04 19:03:01	0|achow101|yes
140 2017-12-04 19:03:05	0|BlueMatt|can you test with that patch? may be a lock issue or so
141 2017-12-04 19:03:36	0|achow101|I'll try as soon as it responds to the stop command
142 2017-12-04 19:06:20	0|GAit|am I correct if I say that the rpc doesn't support socket file?
143 2017-12-04 19:06:48	0|sipa|GAit: what rpc?
144 2017-12-04 19:07:11	0|GAit|bitcoin core rpc. Using a socket file rather than a port
145 2017-12-04 19:07:46	0|achow101|GAit: as in unix sockets?
146 2017-12-04 19:07:49	0|sipa|iirc there was a PR for that, but it needed a new version of libevent at the time
147 2017-12-04 19:07:55	0|BlueMatt|it does not (currently), I believe wumpus had some stuff to get it to work, but ...yea, libevent
148 2017-12-04 19:08:04	0|GAit|yes. Thinking because of Abcore. Other apps  could connect to it.
149 2017-12-04 19:08:17	0|GAit|with a socket file i can prevent that. android app have a different user per app
150 2017-12-04 19:08:23	0|achow101|#9979
151 2017-12-04 19:08:25	0|gribble|https://github.com/bitcoin/bitcoin/issues/9979 | p2p: Bare minimum to support UNIX sockets by laanwj · Pull Request #9979 · bitcoin/bitcoin · GitHub
152 2017-12-04 19:08:44	0|achow101|and #9919
153 2017-12-04 19:08:46	0|gribble|https://github.com/bitcoin/bitcoin/issues/9919 | UNIX sockets support for RPC by laanwj · Pull Request #9919 · bitcoin/bitcoin · GitHub
154 2017-12-04 19:08:47	0|GAit|I managed to compile core for android with ndk by the way, which means i don't need the glibc hack + ld library path with archlinux/debian glibc
155 2017-12-04 19:09:49	0|GAit|thanks sipa & achow101
156 2017-12-04 19:12:07	0|sipa|GAit: cool!
157 2017-12-04 19:29:34	0|achow101|BlueMatt: running your patch now. what exactly do you want?
158 2017-12-04 19:30:56	0|BlueMatt|achow101: looking for a pattern of debug.log entries like a bunch of "X event queued!" but not (enough) corresponding "X event executing..."
159 2017-12-04 19:30:56	0|BlueMatt|s
160 2017-12-04 19:31:03	0|BlueMatt|(or just paste debug log
161 2017-12-04 19:31:47	0|achow101|eyballing this debug.log tail looks like each one is properly paired
162 2017-12-04 19:31:58	0|BlueMatt|hmmm, ok, so not the issue i assumed :(
163 2017-12-04 19:32:01	0|BlueMatt|but the memory is growing?
164 2017-12-04 19:32:08	0|achow101|yes
165 2017-12-04 19:32:14	0|BlueMatt|hmmmm
166 2017-12-04 19:32:25	0|achow101|not as quickly as yesterday since I dropped the dbcache, but growing much faster than it did with 0.15.1
167 2017-12-04 19:32:25	0|BlueMatt|ok, then its news to me, have you checked a heap profile?
168 2017-12-04 19:32:39	0|achow101|no
169 2017-12-04 19:33:30	0|BlueMatt|well i guess thats next? (did valgrind give you any actual leaks?)
170 2017-12-04 19:33:36	0|BlueMatt|i assuming circular shared_ptr refs
171 2017-12-04 19:36:16	0|achow101|valgrind... died
172 2017-12-04 19:36:21	0|achow101|(I killed it)
173 2017-12-04 19:37:12	0|BlueMatt|ah :/
174 2017-12-04 19:38:02	0|achow101|but while it was running it did not output anything
175 2017-12-04 19:39:12	0|achow101|I'll try it again and be more patient this time
176 2017-12-04 19:51:58	0|ryanofsky|it's pretty use address sanitizer too. just: "./configure CXX=clang++ CXXFLAGS=-fsanitize=address LDFLAGS=-fsanitize=address" and run normally. it prints leaks at the end
177 2017-12-04 19:54:22	0|BlueMatt|ugh, fuck you cfields, you sent me down a rabbit hole to replace cvBlockChanged with CValidationInterface, but I think I decided that has to wait on replacing the signals backend with our own thing
178 2017-12-04 19:55:14	0|BlueMatt|cause you really end up wanting the validation interface stuff to have eg two threads where the ordering guarantees are only made per-client, and each new client that gets connected only receives events generated *after* when it was connected
179 2017-12-04 19:55:15	0|cfields|BlueMatt: heh, sorry
180 2017-12-04 19:55:38	0|BlueMatt|(pretty easy to do, I think, you just make events eg a struct and put shared_ptrs to it in each listener's queue)
181 2017-12-04 19:55:56	0|BlueMatt|but its way more work than just fixing a fucking missing cs_main
182 2017-12-04 19:56:08	0|BlueMatt|and is gonna have to wait for 20 other higher-priority things
183 2017-12-04 19:57:23	0|achow101|ryanofsky: will try that at some point
184 2017-12-04 19:57:30	0|BlueMatt|the cvBlockChanged/csBestBlock garbage is....garbage now that we have "real interfaces" to get notifications of validation events
185 2017-12-04 19:59:45	0|cfields|BlueMatt: fwiw, i was picturing just adding a cached uint256 and a function to update it rather than notifying the cv directly
186 2017-12-04 20:00:19	0|BlueMatt|i know you were, but lets *please* not add more in-thread callbacks out of validation :(
187 2017-12-04 20:00:28	0|BlueMatt|I've been working too hard to get rid of those things
188 2017-12-04 20:00:28	0|cfields|i understand your point about using our interfaces. but weighing quick fix against quick fix, that seems better to me
189 2017-12-04 20:00:46	0|BlueMatt|meh, its a short cs_main, why not just take the cs_main
190 2017-12-04 20:02:47	0|cfields|i'm worried that we'll end up locked there for milliseconds where we wouldn't have otherwise. Granted, the re-locking of cs_main at the bottom kinda nullifies that argument.
191 2017-12-04 20:03:03	0|BlueMatt|yea, we re-lock anyway
192 2017-12-04 20:03:22	0|BlueMatt|afaiu
193 2017-12-04 20:03:22	0|BlueMatt|and, more importantly no one uses longpolling anyway
194 2017-12-04 20:06:07	0|BlueMatt|achow101: you said you can repro by just -reindex'ing?
195 2017-12-04 20:06:14	0|achow101|BlueMatt: yes
196 2017-12-04 20:25:32	0|BlueMatt|concept review on #11799 would be appreciated - ignore the code thats there, its useless, but the question is do we want negative-lock-annotations (ie you may not hold cs_main when calling function x) when they are only enforced if the lock is taken directly in the calling function (unlike the requires-lock annotations, these ones dont require you pollute your entire codebase to fully document locking states)
197 2017-12-04 20:25:33	0|gribble|https://github.com/bitcoin/bitcoin/issues/11799 | wallet: Add compile-time checking of (non-)locking assumptions for BlockUntilSyncedToCurrentChain() [wip] by practicalswift · Pull Request #11799 · bitcoin/bitcoin · GitHub
198 2017-12-04 20:26:37	0|BlueMatt|someone want to close #11823 as NOTABUG
199 2017-12-04 20:26:38	0|gribble|https://github.com/bitcoin/bitcoin/issues/11823 | Shouldnt this be nOut? · Issue #11823 · bitcoin/bitcoin · GitHub
200 2017-12-04 20:29:30	0|bitcoin-git|[13bitcoin] 15practicalswift closed pull request #11799: wallet: Add compile-time checking of (non-)locking assumptions for BlockUntilSyncedToCurrentChain() [wip] (06master...06BlockUntilSyncedToCurrentChain-compile-time-warnings) 02https://github.com/bitcoin/bitcoin/pull/11799
201 2017-12-04 20:40:20	0|achow101|hmm. valgrind didn't turn up with anything too frightetning, nor did the address sanitizer
202 2017-12-04 20:40:47	0|achow101|only 160 bytes leaked (granted I only ran then for ~10 minutes each)
203 2017-12-04 20:42:36	0|BlueMatt|achow101: nvm, i got it
204 2017-12-04 20:42:48	0|achow101|.. ok?
205 2017-12-04 20:44:00	0|achow101|BlueMatt: got it as in reproduced or found the leak?
206 2017-12-04 20:44:20	0|BlueMatt|working on fix
207 2017-12-04 20:44:21	0|BlueMatt|sec
208 2017-12-04 20:54:41	0|BlueMatt|achow101: heh, well maybe I didnt have it, can repro and ran into a seemingly-related bug, however
209 2017-12-04 21:05:09	0|BlueMatt|achow101: nope, nvm, my first guess was right
210 2017-12-04 21:05:49	0|sipa|BlueMatt: care to elaborate?
211 2017-12-04 21:06:20	0|BlueMatt|validation running ahead of validationinterface, you end up with a deep queue of shared_ptr<CBlock>s of all the shit you connected
212 2017-12-04 21:06:36	0|BlueMatt|not normally an issue, but reindex is particularly easy to hit there
213 2017-12-04 21:07:09	0|sipa|i would guess the solution is to block progress if the queue gets too long?
214 2017-12-04 21:07:31	0|BlueMatt|indeed
215 2017-12-04 21:10:25	0|achow101|BlueMatt: oh, I see now. so that's probably why valgrind and address sanitizer wasn't catching it
216 2017-12-04 21:11:02	0|BlueMatt|its not a leak, thats why :p
217 2017-12-04 21:11:09	0|BlueMatt|just an unbounded queue
218 2017-12-04 21:12:16	0|achow101|was my git bisect correct?
219 2017-12-04 21:12:21	0|BlueMatt|yes
220 2017-12-04 21:12:28	0|achow101|:D
221 2017-12-04 21:41:42	0|cfields|BlueMatt: Hmm. doesn't that imply that at the time something (say wallet) is running a callback, chainActive's tip could've progressed significantly past the payload block?
222 2017-12-04 21:42:07	0|BlueMatt|cfields: yes, but that was always assumed to be allowed?
223 2017-12-04 21:43:28	0|BlueMatt|cfields: heh, you're gonna murder me...the "quick fix" for this requires a net_processing-specific CNode boolean to be added :/
224 2017-12-04 21:43:37	0|cfields|BlueMatt: how would that have manifested before moving to the scheduler thread?
225 2017-12-04 21:43:51	0|BlueMatt|well, the "super quick fix" is to add a bunch of std::this_thread::yield()s, butttt....
226 2017-12-04 21:44:08	0|BlueMatt|cfields: it wouldnt have, cause the calls are all on the thread that is generating the events
227 2017-12-04 21:44:46	0|cfields|BlueMatt: right. I'm trying to reconsile that with "yes, but that was always assumed to be allowed?"
228 2017-12-04 21:45:17	0|BlueMatt|oh, sorry, yes, just meant that that is the design of validationinterface now
229 2017-12-04 21:45:31	0|BlueMatt|ie shouldnt be news if you reviewed the last 5 prs :p
230 2017-12-04 21:46:21	0|cfields|well i didn't have this issue in mind, as I was still thinking too serially. Now I need to go re-review :(
231 2017-12-04 21:46:40	0|BlueMatt|heh, sorry
232 2017-12-04 21:46:44	0|cfields|*synchronously
233 2017-12-04 22:51:15	0|cfields|BlueMatt: see: SyncTransaction -> AddToWalletIfInvolvingMe -> MarkConflicted. That ends up testing against chainActive for conflicts rather than the view from the callback :(
234 2017-12-04 22:52:37	0|BlueMatt|cfields: heh, I know :(
235 2017-12-04 22:53:04	0|BlueMatt|oh, wait, sec
236 2017-12-04 22:53:09	0|BlueMatt|missed your point, sorry
237 2017-12-04 22:54:59	0|cfields|BlueMatt: assuming the callbacks are lagging and chainActive is significantly ahead, MarkConflicted could be looking for conflicts on some future (or reorg'd) chain
238 2017-12-04 22:56:15	0|cfields|(i'm just looking around for things that could accidentally be using chainActive on the callback thread)
239 2017-12-04 23:14:36	0|jamesob|cfields: you're looking into the shared_ptr<CBlock> backup?
240 2017-12-04 23:15:58	0|cfields|just having another look over the change that moves the validatioininterface callbacks to the scheduler thread. I guess that's what you mean?
241 2017-12-04 23:16:07	0|jamesob|yeah
242 2017-12-04 23:17:40	0|jamesob|didn't know if there was a particular fix in mind for that yet
243 2017-12-04 23:30:37	0|sipa|BlueMatt, cfields: if the wallet notifications can lag behind chainActive, the wallet needs its own "tip" to test against, no?
244 2017-12-04 23:34:50	0|BlueMatt|cfields: sorry, was just finishing the queue-drain branch...I do not believe that to be an issue? Specifically, in the conflicted case you mention here, the only way to hit it is if the wallet lags behind and the block that made a transaction conflict is already reorg'd off the main chain, in which case you dont want to mark it conflicted, and if its still gonna be conflicted once you get to the best chain a later callback will mark
245 2017-12-04 23:34:50	0|BlueMatt|it conflicted appropriately
246 2017-12-04 23:34:55	0|BlueMatt|jamesob: yes, I have a branch
247 2017-12-04 23:35:31	0|BlueMatt|sipa: I do not believe that to be the case (currently), please prove me wrong!
248 2017-12-04 23:36:11	0|achow101|BlueMatt: cfields: are you working on the fix for the queue thing or something else?
249 2017-12-04 23:36:37	0|BlueMatt|achow101: yes
250 2017-12-04 23:36:48	0|BlueMatt|though cfields is trying to find other issues in the original pr
251 2017-12-04 23:36:52	0|achow101|ok
252 2017-12-04 23:38:22	0|sipa|BlueMatt: it certainly sounds safer...
253 2017-12-04 23:38:46	0|cfields|sipa: agreed
254 2017-12-04 23:39:23	0|BlueMatt|sipa: I agree, but to do that you'd rewrite about another 10 functions in the wallet and probably end up refactoring half of it
255 2017-12-04 23:39:28	0|BlueMatt|(again, please prove me wrong :p)
256 2017-12-04 23:39:49	0|cfields|even if the conflicts aren't an issue, i still really don't like that we're testing against some seemingly random chain
257 2017-12-04 23:40:12	0|BlueMatt|well in that one specific case its fine (and kinda the way the function was written to begin with, strangely)
258 2017-12-04 23:40:12	0|sipa|BlueMatt: hmm, really? I would just replace every call to chainActive in the wallet with a local variable, which gets updated through the callbac
259 2017-12-04 23:40:23	0|cfields|BlueMatt: at least for the stuff affected by callbacks, can't we just pass a pindex around instead?
260 2017-12-04 23:41:26	0|BlueMatt|cfields: isnt that like the one place that matters from the callbacks (its been like 8 months since I've looked though)
261 2017-12-04 23:41:58	0|BlueMatt|sipa: the number of places I'd have to go read to make sure GetDepthInMainChain being disconnected from main chain is fine to be convinced of that is....high
262 2017-12-04 23:41:58	0|cfields|BlueMatt: unsure. I'll take a closer look before commenting further
263 2017-12-04 23:42:07	0|BlueMatt|sipa: though I'm sure ryanofsky would appreciate it greatly
264 2017-12-04 23:44:01	0|BlueMatt|sipa: the obvious issue is that gui would have to be aware of the wallet chain state
265 2017-12-04 23:44:24	0|sipa|right
266 2017-12-04 23:44:45	0|sipa|and if there's anything that looks at pcoinsTip from the wallet (is there?) that would also be affected
267 2017-12-04 23:45:01	0|BlueMatt|I dont think there is
268 2017-12-04 23:48:32	0|BlueMatt|sipa: ReacceptWalletTransactions/RelayWalletTransactions would need a only-if-caught-up-to-main-chain condition, but thats the only obvious ones in wallet.cpp...it may be much easier than I thought, but I already have too many open prs to want to do it any time soon
269 2017-12-04 23:52:29	0|D__|Dd