1 2017-07-11 01:35:16	0|bitcoin-git|[13bitcoin] 15achow101 opened pull request #10788: Replace ismine with producesignature check in witnessifier (06master...06fix-addwitnessaddress) 02https://github.com/bitcoin/bitcoin/pull/10788
  2 2017-07-11 02:05:57	0|bitcoin-git|[13bitcoin] 15stevendlander opened pull request #10789: Punctuation/grammer fixes in rpcwallet.cpp (06master...06cli-punctuation-standardization) 02https://github.com/bitcoin/bitcoin/pull/10789
  3 2017-07-11 02:42:16	0|jtimon|mhm, can't I use something I have in /etc/host with -rpcallowip ? only hardcoded ips ?
  4 2017-07-11 02:42:25	0|jtimon|not even localhost it seems :(
  5 2017-07-11 02:49:18	0|sipa|jtimon: it has to be a netmask, i think
  6 2017-07-11 02:49:20	0|sipa|so no
  7 2017-07-11 02:51:35	0|jtimon|found getent hosts myhostname | awk '{ print $1 }', hope it solves my issue
  8 2017-07-11 02:52:04	0|jtimon|thanks for confirming
  9 2017-07-11 03:24:12	0|bitcoin-git|[13bitcoin] 15MeshCollider opened pull request #10790: Use vector::data() instead of &vch[0] in base58.cpp (06master...06prefer-vector-data) 02https://github.com/bitcoin/bitcoin/pull/10790
 10 2017-07-11 05:17:50	0|gmaxwell|hurray for protocol improvements, you can see the inv rate on this chart approve as people upgrade to 0.13+ https://statoshi.info/dashboard/db/p2p-messages?from=1453110746831&to=1499750141000
 11 2017-07-11 07:23:23	0|wumpus|right, some of the options support host lookup as of master, but rpcallowip certainly doesn't
 12 2017-07-11 07:24:05	0|wumpus|(e.g. #9774)
 13 2017-07-11 07:24:07	0|gribble|https://github.com/bitcoin/bitcoin/issues/9774 | Enable host lookups for -proxy and -onion parameters by jmcorgan · Pull Request #9774 · bitcoin/bitcoin · GitHub
 14 2017-07-11 07:26:45	0|wumpus|oh yes, inv rate looks a lot more calm recently
 15 2017-07-11 07:28:51	0|gmaxwell|the changes for inv batching and timing make it basically constant.
 16 2017-07-11 07:39:18	0|bitcoin-git|[13bitcoin] 15laanwj pushed 8 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9edda0c5f5f2...21ed30a314cf
 17 2017-07-11 07:39:19	0|bitcoin-git|13bitcoin/06master 14ff6a834 15Matt Corallo: Use TestingSetup to DRY qt rpcnestedtests
 18 2017-07-11 07:39:20	0|bitcoin-git|13bitcoin/06master 143a19fed 15Matt Corallo: Make ValidationInterface signals-type-agnostic...
 19 2017-07-11 07:39:21	0|bitcoin-git|13bitcoin/06master 14cda1429 15Matt Corallo: Give CMainSignals a reference to the global scheduler...
 20 2017-07-11 07:39:34	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10179: Give CValidationInterface Support for calling notifications on the CScheduler Thread (06master...062017-01-wallet-cache-inmempool-3) 02https://github.com/bitcoin/bitcoin/pull/10179
 21 2017-07-11 07:46:41	0|bitcoin-git|[13bitcoin] 15maaku opened pull request #10792: Replace MAX_OPCODE for OP_NOP10. (06master...06max-opcode-over-nop10) 02https://github.com/bitcoin/bitcoin/pull/10792
 22 2017-07-11 07:54:14	0|gmaxwell|you mean it doesn't compute the max of the last two stack variables!?!?
 23 2017-07-11 07:54:17	0|gmaxwell|:P
 24 2017-07-11 07:56:46	0|gmaxwell|[OT-ish] https://pax.grsecurity.net/docs/PaXTeam-H2HC15-RAP-RIP-ROP.pdf  < the ictp described here is neat. I wish pax stuff was open.
 25 2017-07-11 07:57:05	0|sipa|gmaxwell: ???
 26 2017-07-11 07:57:13	0|sipa|(re: last two stack variables)
 27 2017-07-11 08:00:55	0|gmaxwell|sipa: joke on mark's MAX_OPCODE
 28 2017-07-11 08:01:12	0|gmaxwell|that it doesn't compute max()
 29 2017-07-11 08:02:08	0|sipa|ah
 30 2017-07-11 08:16:35	0|bitcoin-git|[13bitcoin] 15MeshCollider closed pull request #10790: Use vector::data() instead of &vch[0] in base58.cpp (06master...06prefer-vector-data) 02https://github.com/bitcoin/bitcoin/pull/10790
 31 2017-07-11 09:44:06	0|bitcoin-git|13bitcoin/06master 14f2f1d0a 15Gregory Sanders: document script-based return fields for validateaddress
 32 2017-07-11 09:44:06	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/21ed30a314cf...379aed0e53a6
 33 2017-07-11 09:44:07	0|bitcoin-git|13bitcoin/06master 14379aed0 15Wladimir J. van der Laan: Merge #10676: document script-based return fields for validateaddress...
 34 2017-07-11 09:44:32	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10676: document script-based return fields for validateaddress (06master...06validatata) 02https://github.com/bitcoin/bitcoin/pull/10676
 35 2017-07-11 09:45:44	0|bitcoin-git|[13bitcoin] 15MeshCollider opened pull request #10793: Changing &var[0] to var.data() (06master...06prefer-vector-data) 02https://github.com/bitcoin/bitcoin/pull/10793
 36 2017-07-11 09:58:27	0|bitcoin-git|[13bitcoin] 15laanwj pushed 6 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/379aed0e53a6...104f5f21dc24
 37 2017-07-11 09:58:28	0|bitcoin-git|13bitcoin/06master 14cfaef69 15Alex Morcos: remove default argument from GetMinimumFee
 38 2017-07-11 09:58:28	0|bitcoin-git|13bitcoin/06master 14d507c30 15Alex Morcos: Introduce a fee estimate mode....
 39 2017-07-11 09:58:29	0|bitcoin-git|13bitcoin/06master 14e0738e3 15Alex Morcos: remove default argument from estimateSmartFee
 40 2017-07-11 09:58:47	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10589: More economical fee estimates for RBF and RPC options to control (06master...06rpcestimatechoice) 02https://github.com/bitcoin/bitcoin/pull/10589
 41 2017-07-11 12:31:59	0|PlaneteNamek|Hello world 😊
 42 2017-07-11 12:34:08	0|jonasschnelli|Beg for review on: https://github.com/bitcoin/bitcoin/pull/9502
 43 2017-07-11 13:12:43	0|wumpus|#9502
 44 2017-07-11 13:12:45	0|gribble|https://github.com/bitcoin/bitcoin/issues/9502 | [Qt] Add option to pause/resume block downloads by jonasschnelli · Pull Request #9502 · bitcoin/bitcoin · GitHub
 45 2017-07-11 13:21:52	0|bitcoin-git|[13bitcoin] 15laanwj pushed 10 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/104f5f21dc24...e4f226a133d0
 46 2017-07-11 13:21:53	0|bitcoin-git|13bitcoin/06master 140a3a5ff 15John Newbery: [tests] Fix flake8 warnings in getblocktemplate tests
 47 2017-07-11 13:21:53	0|bitcoin-git|13bitcoin/06master 1432cffe6 15John Newbery: [tests] Fix import order in getblocktemplate test
 48 2017-07-11 13:21:54	0|bitcoin-git|13bitcoin/06master 1438b38cd 15John Newbery: [tests] getblocktemplate_proposals.py: add logging
 49 2017-07-11 13:22:14	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10190: [tests] mining functional tests (including regression test for submitblock) (06master...06mining_functional_test) 02https://github.com/bitcoin/bitcoin/pull/10190
 50 2017-07-11 13:24:34	0|bitcoin-git|13bitcoin/06master 14c8e29d7 15Mark Friedenbach: Replace MAX_OPCODE for OP_NOP10....
 51 2017-07-11 13:24:34	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/e4f226a133d0...badd81bd3185
 52 2017-07-11 13:24:35	0|bitcoin-git|13bitcoin/06master 14badd81b 15Wladimir J. van der Laan: Merge #10792: Replace MAX_OPCODE for OP_NOP10....
 53 2017-07-11 13:25:08	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10792: Replace MAX_OPCODE for OP_NOP10. (06master...06max-opcode-over-nop10) 02https://github.com/bitcoin/bitcoin/pull/10792
 54 2017-07-11 13:25:34	0|bitcoin-git|13bitcoin/06master 146270d62 15Matt Corallo: Verify binaries from bitcoincore.org and bitcoin.org
 55 2017-07-11 13:25:34	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/badd81bd3185...cef4b5ccaa37
 56 2017-07-11 13:25:35	0|bitcoin-git|13bitcoin/06master 14cef4b5c 15Wladimir J. van der Laan: Merge #10651: Verify binaries from bitcoincore.org and bitcoin.org...
 57 2017-07-11 13:25:58	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10651: Verify binaries from bitcoincore.org and bitcoin.org (06master...062017-06-verify-coreorg) 02https://github.com/bitcoin/bitcoin/pull/10651
 58 2017-07-11 13:37:21	0|bitcoin-git|[13bitcoin] 15laanwj pushed 4 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/cef4b5ccaa37...b27b004532db
 59 2017-07-11 13:37:22	0|bitcoin-git|13bitcoin/06master 141fafd70 15Alex Morcos: Add function to report highest estimate target tracked per horizon
 60 2017-07-11 13:37:22	0|bitcoin-git|13bitcoin/06master 149c85b91 15Alex Morcos: Change API to estimaterawfee...
 61 2017-07-11 13:37:23	0|bitcoin-git|13bitcoin/06master 145e3b7b5 15Alex Morcos: Improve error reporting for estimaterawfee
 62 2017-07-11 13:37:41	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10543: Change API to estimaterawfee (06master...06estimaterawapi) 02https://github.com/bitcoin/bitcoin/pull/10543
 63 2017-07-11 13:40:36	0|bitcoin-git|13bitcoin/06master 14475c08c 15Pieter Wuille: Add PR description to merge commit in github-merge.py
 64 2017-07-11 13:40:36	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/b27b004532db...ca4c545cc7e8
 65 2017-07-11 13:40:37	0|bitcoin-git|13bitcoin/06master 14ca4c545 15Wladimir J. van der Laan: Merge #10786: Add PR description to merge commit in github-merge.py...
 66 2017-07-11 13:41:13	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10786: Add PR description to merge commit in github-merge.py (06master...0620170710_prbody) 02https://github.com/bitcoin/bitcoin/pull/10786
 67 2017-07-11 14:12:08	0|ryanofsky|can #10235 be merged?
 68 2017-07-11 14:12:09	0|gribble|https://github.com/bitcoin/bitcoin/issues/10235 | Track keypool entries as internal vs external in memory by TheBlueMatt · Pull Request #10235 · bitcoin/bitcoin · GitHub
 69 2017-07-11 14:53:35	0|morcos|I think #10712 can also be merged.  sipa: you left just a nit, were you still reviewing?  I'll skip the nit since the current commit has 3 ack's
 70 2017-07-11 14:53:37	0|gribble|https://github.com/bitcoin/bitcoin/issues/10712 | Add change output if necessary to reduce excess fee by morcos · Pull Request #10712 · bitcoin/bitcoin · GitHub
 71 2017-07-11 15:01:45	0|bitcoin-git|[13bitcoin] 15jonasschnelli opened pull request #10794: Add simple light-client mode (RPC only) (06master...062017/07/spv) 02https://github.com/bitcoin/bitcoin/pull/10794
 72 2017-07-11 15:02:26	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #9171: Introduce auxiliary block requests (06master...062016/11/spv_step_1) 02https://github.com/bitcoin/bitcoin/pull/9171
 73 2017-07-11 15:12:29	0|BlueMatt|wumpus: would be nice to get a quick glance and merge on #10235
 74 2017-07-11 15:12:31	0|gribble|https://github.com/bitcoin/bitcoin/issues/10235 | Track keypool entries as internal vs external in memory by TheBlueMatt · Pull Request #10235 · bitcoin/bitcoin · GitHub
 75 2017-07-11 15:35:07	0|morcos|BlueMatt: How does it know what index to create key at if the keypool is empty? (same question before your PR)
 76 2017-07-11 15:36:26	0|BlueMatt|hmm?
 77 2017-07-11 15:36:42	0|BlueMatt|it doesnt matter if the pool is empty, use index 0?
 78 2017-07-11 15:37:08	0|BlueMatt|the indexes dont really have a purpose, its more of a set
 79 2017-07-11 15:37:13	0|wumpus|looking at 10235
 80 2017-07-11 15:37:25	0|morcos|yeah i just figured that out, that the indices are only for keypool management
 81 2017-07-11 15:37:30	0|BlueMatt|yea
 82 2017-07-11 15:37:59	0|morcos|but are you sure there is not a bug now that you have two keypools
 83 2017-07-11 15:38:28	0|BlueMatt|well afterwards you should still make sure they keyids are unique, ie creating at the max of both sets
 84 2017-07-11 15:38:28	0|morcos|if external runs out, couldn't you create a new key in the external pool with the same index as one in the internal pool
 85 2017-07-11 15:38:39	0|BlueMatt|i hope that pr doesnt do that
 86 2017-07-11 15:39:59	0|morcos|i'm not sure if there is a bug, i'm making my way through all this logic for the first time, but i think it would be a lot clearer if you figured out nInternalEnd and nExternalEnd and maxed them for nEnd
 87 2017-07-11 15:40:52	0|morcos|actually it seems even worse than that
 88 2017-07-11 15:40:59	0|morcos|i just don't think this logic works with two pools
 89 2017-07-11 15:41:14	0|BlueMatt|hmm?
 90 2017-07-11 15:41:14	0|morcos|imagine internal = {1,3} and external = {2,4}
 91 2017-07-11 15:41:18	0|morcos|then external runs out
 92 2017-07-11 15:41:30	0|BlueMatt|you can reuse indexes
 93 2017-07-11 15:41:32	0|morcos|you'll now generate 4 as the next internal key or external key
 94 2017-07-11 15:41:33	0|BlueMatt|thats no problem
 95 2017-07-11 15:41:36	0|morcos|oh yeah, i guess thats ok
 96 2017-07-11 15:42:20	0|morcos|ok, so ignore me
 97 2017-07-11 15:42:28	0|morcos|maybe it's pretty clear.
 98 2017-07-11 15:43:44	0|morcos|what about the pool on disk though?  it seems like you shouldn't start reusining indices until you're sure they aren't in there?
 99 2017-07-11 15:44:30	0|BlueMatt|hmmmmmm, yea, there is a bug there, but I didnt introduce it
100 2017-07-11 15:44:43	0|morcos|yeah, but made it much more likely to get hit i think
101 2017-07-11 15:44:51	0|BlueMatt|dont think so?
102 2017-07-11 15:45:03	0|BlueMatt|ReserveKeyFromKeyPool, (keypool now empty), TopUpKeyPool, fucked
103 2017-07-11 15:45:14	0|BlueMatt|I dont think i made a real difference to the likelyhood there
104 2017-07-11 15:45:25	0|morcos|well before you start over again at 1, so it's unlikely the initial indices are still in disk pool
105 2017-07-11 15:45:41	0|morcos|but with yours you could easily reuse a recent one as per my {1,3} {2,4} example above
106 2017-07-11 15:45:44	0|BlueMatt|depends on your keypool size
107 2017-07-11 15:45:52	0|BlueMatt|true
108 2017-07-11 15:45:53	0|morcos|sure, seems like a bug either way
109 2017-07-11 15:46:04	0|BlueMatt|anyway, its only really likely if you have a small pool, but still not good
110 2017-07-11 15:46:17	0|BlueMatt|unless you object I'll fix it in a separate pr?
111 2017-07-11 15:47:31	0|wumpus|what is the worst that can happen if an index is inadvertently reused?
112 2017-07-11 15:47:36	0|morcos|so what happens, do you overwrite the old keypool entry with the new one on disk?  and seems like you could either return the reserved key or use it, and either case might cause problems
113 2017-07-11 15:47:43	0|BlueMatt|jonasschnelli: where are you on #10650? We
114 2017-07-11 15:47:45	0|gribble|https://github.com/bitcoin/bitcoin/issues/10650 | Multiwallet: add RPC endpoint support by jonasschnelli · Pull Request #10650 · bitcoin/bitcoin · GitHub
115 2017-07-11 15:47:48	0|morcos|wumpus: not clear to me
116 2017-07-11 15:47:51	0|BlueMatt|re dangerously close to having multiwallet slip to 16
117 2017-07-11 15:48:28	0|BlueMatt|wumpus: yea, not clear to me either, intuitively it should be rather minimal, not like we're clearing private keys or anything, but I could see it hitting an assert somewhere
118 2017-07-11 15:48:34	0|morcos|BlueMatt: I suppose I have a slight preference for you adding another commit to that PR to fix it.
119 2017-07-11 15:48:36	0|wumpus|is everything necessary for multiwallet tagged as 0.15?
120 2017-07-11 15:48:50	0|BlueMatt|i think its just 10650 at this point, no?
121 2017-07-11 15:49:14	0|wumpus|yes, I think so, and that one is tagged, good
122 2017-07-11 15:50:07	0|BlueMatt|morcos: heh, ok, I was trying to avoid polluting the bugfix-vs-not distinction, but I suppose the performance regression is somewhat bugfix-y anyway
123 2017-07-11 15:50:50	0|morcos|64-bits is a big number.. seems pretty safe to just have an in-memory nextIndex that is initialized by taking max of the on-disk last entry in the set?
124 2017-07-11 15:50:58	0|morcos|sets
125 2017-07-11 15:51:27	0|wumpus|if you do that, please do make it per-wallet, not global
126 2017-07-11 15:51:44	0|BlueMatt|yes, that was my preferred fix
127 2017-07-11 15:51:54	0|wumpus|but if it's 64-bit I don't think we have to be worried about reusing
128 2017-07-11 15:52:15	0|wumpus|unless small numbers are preferable because they are more efficient for some reason
129 2017-07-11 15:52:21	0|wumpus|but I don't think so in this case
130 2017-07-11 15:52:37	0|BlueMatt|ugh, no, bigger numbers better
131 2017-07-11 15:53:23	0|wumpus|ok, so hold off on merging #10235?
132 2017-07-11 15:53:25	0|gribble|https://github.com/bitcoin/bitcoin/issues/10235 | Track keypool entries as internal vs external in memory by TheBlueMatt · Pull Request #10235 · bitcoin/bitcoin · GitHub
133 2017-07-11 15:53:32	0|BlueMatt|i guess
134 2017-07-11 15:53:46	0|wumpus|I'm not sure that's the right thing to do if that bug was not introduced here
135 2017-07-11 15:54:03	0|morcos|wumpus: don't hold off for my sake
136 2017-07-11 15:54:17	0|morcos|i trust we'll merge the fix before 0.15
137 2017-07-11 15:54:30	0|BlueMatt|heh, ok, how bout i just open a pr right now to fix it
138 2017-07-11 15:54:35	0|BlueMatt|and take 10235
139 2017-07-11 15:54:48	0|morcos|BlueMatt: stop talking and start coding
140 2017-07-11 15:55:59	0|wumpus|std::max(nEnd, *(--setExternalKeyPool.end()) + 1);
141 2017-07-11 15:56:06	0|wumpus|whaaaa
142 2017-07-11 15:56:20	0|BlueMatt|lol, i didnt introduce that
143 2017-07-11 15:56:23	0|wumpus|what is *that*
144 2017-07-11 15:56:58	0|BlueMatt|there's no back() in set :(
145 2017-07-11 15:57:10	0|wumpus|I'm not blaming you, but that's not acceptable, certainly not without a comment
146 2017-07-11 15:57:26	0|wumpus|then add a back(set) helper function maybe
147 2017-07-11 15:57:30	0|BlueMatt|lol, ok, sec
148 2017-07-11 15:57:33	0|wumpus|factor this out to a sensible something
149 2017-07-11 15:58:01	0|BlueMatt|fwiw, I find that pretty readable, but ok
150 2017-07-11 15:58:08	0|wumpus|no, that's not readable
151 2017-07-11 15:58:09	0|wumpus|not at all
152 2017-07-11 15:58:25	0|wumpus|why would you want to infix-decrease end()?
153 2017-07-11 15:58:30	0|wumpus|what does that even do?
154 2017-07-11 15:58:40	0|wumpus|does it move the end of the set?
155 2017-07-11 15:59:47	0|BlueMatt|but, then, i write iterator garbage that looks like https://github.com/bitcoinfibre/bitcoinfibre/blob/master/src/blockencodings.cpp#L281
156 2017-07-11 16:00:20	0|wumpus|people have to be able to maintain that code, also people that are not you
157 2017-07-11 16:00:35	0|BlueMatt|i didnt write it
158 2017-07-11 16:00:45	0|BlueMatt|I'm fixing...
159 2017-07-11 16:02:00	0|wumpus|that function in fibre looks more understandable
160 2017-07-11 16:02:11	0|wumpus|it doesn't do anything really crazy and it does it step by step
161 2017-07-11 16:03:03	0|BlueMatt|ok, fixed on 10235
162 2017-07-11 16:03:32	0|BlueMatt|lol, clearly I need to try harder to write garbage iterator code in fibre then
163 2017-07-11 16:04:05	0|wumpus|well as long as you don't inadvertently add UB
164 2017-07-11 16:04:33	0|wumpus|but if you really want to, make sure it's all in one expression and the order of operations, as well as what applies to what isn't clear
165 2017-07-11 16:05:17	0|wumpus|yes, this is much better, thanks
166 2017-07-11 16:05:25	0|BlueMatt|heh, I was joking, that stuff is hard enough to maintain when I go back once every three months and rewrite big chunks to get another 2 milliseconds out of it :p
167 2017-07-11 16:05:41	0|BlueMatt|without remembering the threading model that will cause a segfault-in-a-million if you break it
168 2017-07-11 16:07:37	0|wumpus|I can imagine
169 2017-07-11 16:07:39	0|cfields|BlueMatt: fwiw, you can use *.rend() if !empty()
170 2017-07-11 16:07:49	0|cfields|er, rbegin
171 2017-07-11 16:07:58	0|BlueMatt|yea, that too
172 2017-07-11 16:13:00	0|BlueMatt|wumpus: err, nvm, easier to just fix the first bug than bother with simplifying that code
173 2017-07-11 16:13:30	0|wumpus|I'm afraid #10712 doesn't build on top of current master
174 2017-07-11 16:13:32	0|gribble|https://github.com/bitcoin/bitcoin/issues/10712 | Add change output if necessary to reduce excess fee by morcos · Pull Request #10712 · bitcoin/bitcoin · GitHub
175 2017-07-11 16:13:37	0|wumpus|BlueMatt: well you had simplified that code just fine imo
176 2017-07-11 16:13:45	0|BlueMatt|yes, but the next pr is to remove it :p
177 2017-07-11 16:13:59	0|morcos|ok will look
178 2017-07-11 16:14:02	0|BlueMatt|lol
179 2017-07-11 16:14:03	0|wumpus|BlueMatt: lol, oops, that happens to me all the time
180 2017-07-11 16:14:17	0|wumpus|spend optimizing or improving some part of the code, then it's no longer necessary :p
181 2017-07-11 16:15:31	0|wumpus|CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, currentConfirmationTarget, ::mempool, ::feeEstimator, nullptr);
182 2017-07-11 16:15:31	0|wumpus|re: #10712: src/wallet/wallet.cpp:2763:151: error: too few arguments to function call, expected 7, have 5
183 2017-07-11 16:15:33	0|gribble|https://github.com/bitcoin/bitcoin/issues/10712 | Add change output if necessary to reduce excess fee by morcos · Pull Request #10712 · bitcoin/bitcoin · GitHub
184 2017-07-11 16:15:56	0|BlueMatt|well, I'll lave 10235 with the fix for now, since it should be an easy merge
185 2017-07-11 16:16:07	0|BlueMatt|and then it'll get removed in the next pr, which will need a whole review cycle
186 2017-07-11 16:17:11	0|morcos|yeah conflict with my other PR you just merged but in a newly added line, will fix 10712
187 2017-07-11 16:18:38	0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #10795: No longer ever reuse keypool indexes (06master...062017-07-wallet-keypool-overwrite) 02https://github.com/bitcoin/bitcoin/pull/10795
188 2017-07-11 17:02:59	0|sipa|these are some preliminary benchmarks for a reindex-chainstate up to height 450000 (before assumevalid, so no signature checking), with infinity dbcache, for various sha256 implementations:
189 2017-07-11 17:03:04	0|sipa|bitcoind-rorx: 4835
190 2017-07-11 17:03:06	0|sipa|bitcoind-shani: 4297
191 2017-07-11 17:03:09	0|sipa|bitcoind-basic: 5134
192 2017-07-11 17:03:11	0|sipa|bitcoind-sse: 4875
193 2017-07-11 17:04:01	0|sipa|basic is master
194 2017-07-11 17:04:11	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/ca4c545cc7e8...e8b95239eeb0
195 2017-07-11 17:04:12	0|bitcoin-git|13bitcoin/06master 140f402b9 15Alex Morcos: Fix rare edge case of paying too many fees when transaction has no change....
196 2017-07-11 17:04:12	0|bitcoin-git|13bitcoin/06master 14253cd7e 15Alex Morcos: Only reserve key for scriptChange once in CreateTransaction...
197 2017-07-11 17:04:13	0|bitcoin-git|13bitcoin/06master 14e8b9523 15Wladimir J. van der Laan: Merge #10712: Add change output if necessary to reduce excess fee...
198 2017-07-11 17:04:23	0|sipa|rorx and sse are from wumpus' work to include some asm optimized versions
199 2017-07-11 17:04:41	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10712: Add change output if necessary to reduce excess fee (06master...06addchangehwenoverpaying) 02https://github.com/bitcoin/bitcoin/pull/10712
200 2017-07-11 17:04:48	0|sipa|the shani version is using code from cryptopp
201 2017-07-11 17:07:35	0|BlueMatt|lol, Make Bitcoin CryptoPP Again
202 2017-07-11 17:08:15	0|BlueMatt|thats a pretty impressive gain
203 2017-07-11 17:08:34	0|sipa|unfortunately, very few systems right now have sha-ni instructions
204 2017-07-11 17:09:04	0|BlueMatt|yes, but that should change over time
205 2017-07-11 17:09:11	0|BlueMatt|the sha-ni in intel and amd are identical, no?
206 2017-07-11 17:09:35	0|sipa|afaik, there is exists no "sha-ni in intel" yet :)
207 2017-07-11 17:10:34	0|wumpus|that's always the dilemma with new instructions, the systems that have that instruction probably least need the optimization :)
208 2017-07-11 17:11:08	0|sipa|sse is present in nearly every x86_64 cpu, i think
209 2017-07-11 17:12:01	0|wumpus|yes, sse is present in all of them
210 2017-07-11 17:12:41	0|wumpus|even sse2 should be in all of them
211 2017-07-11 17:13:30	0|BlueMatt|sipa: hmm? I was under the impression they had announced something like the next arch will get it
212 2017-07-11 17:13:32	0|gmaxwell|sse2 is required by x86_64.
213 2017-07-11 17:13:35	0|BlueMatt|no shipping ones, obviously
214 2017-07-11 17:14:15	0|wumpus|those are not exactly new instructions though :) anyhow supporting shani as well is nice, just takes time and it will be in near everything...
215 2017-07-11 17:14:55	0|gmaxwell|here is something funny: compilng bitcoin with -march=native makes the sha2 in C most of the speed of the sse one. (compiler manages to use rorx)
216 2017-07-11 17:15:33	0|gmaxwell|Also the x86 simd support will make it easier to plug in arm versions; which will be more important for speed.
217 2017-07-11 17:15:56	0|BlueMatt|wow, yay compiler smarts
218 2017-07-11 17:16:03	0|gmaxwell|sipa: there is an intel chip with sha-ni, but it's some crappy atom processor.
219 2017-07-11 17:16:24	0|BlueMatt|<wumpus> that's always the dilemma with new instructions, the systems that have that instruction probably least need the optimization :)
220 2017-07-11 17:16:28	0|BlueMatt|heh, guess not, then
221 2017-07-11 17:17:24	0|sipa|gmaxwell: compiling all of bitcoind with -march=native actually makes it overall another 100s faster to sync
222 2017-07-11 17:17:30	0|gmaxwell|well aformentioned atom is some server atom that isn't very widely deployed from what I can tell.
223 2017-07-11 17:17:46	0|BlueMatt|do you have lto benchmarks?
224 2017-07-11 17:17:53	0|sipa|not yet, but i can create
225 2017-07-11 17:18:06	0|wumpus|yes, march=native is nice, everyone compiling bitcoind locally should use it
226 2017-07-11 17:18:26	0|gmaxwell|#9804 looks like a good change that is getting forgotten. (I say that as someone whos only concept acked so far...)
227 2017-07-11 17:18:28	0|gribble|https://github.com/bitcoin/bitcoin/issues/9804 | Fixes subscript 0 ([0]) where should use (var.data()) instead. by JeremyRubin · Pull Request #9804 · bitcoin/bitcoin · GitHub
228 2017-07-11 17:18:57	0|wumpus|with so many open PRs it's unavoidable that some things get forgotten, unfortunately
229 2017-07-11 17:19:08	0|wumpus|but no, that one isn't
230 2017-07-11 17:19:50	0|wumpus|I just referred people to review it today (because they opened a PR that was a strict subset of it)
231 2017-07-11 17:20:44	0|sipa|wumpus: thanks for that, i had almost forgotten about jeremy's version
232 2017-07-11 17:30:32	0|BlueMatt|lolwut
233 2017-07-11 17:30:40	0|BlueMatt|ehh, wrong window
234 2017-07-11 17:31:45	0|sipa|seems totally appropriate here
235 2017-07-11 17:32:05	0|BlueMatt|ok, then I'll pretend I meant to
236 2017-07-11 17:58:27	0|sipa|running benchmarks for lto and shani+lto as well now
237 2017-07-11 17:58:40	0|sipa|will have numbers in a day...
238 2017-07-11 17:59:34	0|sipa|interestingly, the shani+lto does not even contain any non-shani sha code, even though it's dispatched to a modifiable pointer
239 2017-07-11 17:59:40	0|BlueMatt|heh
240 2017-07-11 17:59:44	0|BlueMatt|oh nice
241 2017-07-11 17:59:49	0|sipa|the compiler must notice there are no assignments to that pointer, and treats it as a constant
242 2017-07-11 18:00:06	0|BlueMatt|yea, lto can be clever sometimes
243 2017-07-11 18:08:37	0|gmaxwell|if compiler versions are still keeping up from ltoing by default perhaps we could add a --gofast configure flag to do -march=native -O3 -lto
244 2017-07-11 18:09:06	0|BlueMatt|is O3 a win in bitcoin?
245 2017-07-11 18:09:39	0|sipa|i guess i'll benchmark that too :)
246 2017-07-11 18:09:54	0|BlueMatt|heh
247 2017-07-11 18:13:51	0|wumpus|I don't see why something as basic as that would need a configure fla
248 2017-07-11 18:14:01	0|wumpus|people that want to supply their own CXXFLAGS can do so already
249 2017-07-11 18:14:30	0|wumpus|could just mention it in the build document
250 2017-07-11 18:14:55	0|wumpus|a dedicated configure flag only makes sense if we use something for the gitian build, so people can do their own build with the same settings
251 2017-07-11 18:15:08	0|gmaxwell|wumpus: fair enough
252 2017-07-11 18:15:24	0|wumpus|but using -march=native would be a bad idea there, -flto might be a good one
253 2017-07-11 18:15:52	0|BlueMatt|yea, flto could use its own configure flat...doing it manually is like 6 variables long
254 2017-07-11 18:17:39	0|gmaxwell|well I think we should lto by default, but there was still some question of compiler support. (which I think was mooted by C++11, but perhaps I'm wrong, since we still haven't done it.)
255 2017-07-11 18:18:42	0|wumpus|I'm not convinced anything but -O2 optimization should be default
256 2017-07-11 18:19:14	0|gmaxwell|wumpus: why shouldn't it be lto-ing by default?
257 2017-07-11 18:19:17	0|wumpus|cfields might have more of an opinion, but in my experience buildling open source software for lots of platforms, I've had lots of annoyances with software that tried to forcibly injects certain sets of optimizations flags
258 2017-07-11 18:19:34	0|wumpus|it should be left up to the distribution ideally to set optimization flags
259 2017-07-11 18:19:37	0|BlueMatt|gmaxwell: lto does not work on moderately-recent compilers
260 2017-07-11 18:19:44	0|wumpus|well even if it did
261 2017-07-11 18:19:49	0|BlueMatt|eg debian jessie
262 2017-07-11 18:19:58	0|sipa|works fine here, and for release builds we control the environment entirely anyway
263 2017-07-11 18:20:06	0|BlueMatt|well, i guess 4.9 isnt moderately-recent
264 2017-07-11 18:20:07	0|wumpus|yes, for release builds flto would be good
265 2017-07-11 18:20:12	0|BlueMatt|anyway, we support it, and lto doesnt work on it
266 2017-07-11 18:20:14	0|cfields|wumpus: agreed. "-02 -g" is expected to be the default, I get really irritated when that's not the case
267 2017-07-11 18:20:21	0|wumpus|cfields: exactly
268 2017-07-11 18:20:35	0|sipa|benchmarking reindex with "-O2", "-O3", "-flto -O2",  and "-flto -O3"
269 2017-07-11 18:20:40	0|BlueMatt|nice
270 2017-07-11 18:21:13	0|wumpus|don't get me wrong, flto would be great for gitian builds, and we could have a configure option for that, but adding non-standard optimization flags by default on configure tends to be really annoying in edge cases
271 2017-07-11 18:21:51	0|sipa|cfields was concerned about having release builds deviate too much from developers-tested builds
272 2017-07-11 18:21:53	0|cfields|I get the impression i should go ahead and wire up --enable-lto before someone sneaks in something unexpected :)
273 2017-07-11 18:22:06	0|gmaxwell|BlueMatt: I do not believe that LTO fails to work on anything that can compile Bitcoin Core.   It's been supported since GCC 4.7
274 2017-07-11 18:22:20	0|wumpus|if you want to encourage people to build with optimization flags then just document it in e.g. build-unix.md
275 2017-07-11 18:22:24	0|wumpus|don't do anything sneaky
276 2017-07-11 18:22:35	0|gmaxwell|Our cflags are far from -O2 -g, though, we add about two dozen warning flags.
277 2017-07-11 18:22:47	0|wumpus|warning flags don't do aything funny to the code
278 2017-07-11 18:22:57	0|cfields|sipa: i think an --enable-lto alleviates that somewhat, since it makes it easy for devs to get the same results
279 2017-07-11 18:23:04	0|BlueMatt|gmaxwell: I dunno, I've been building on lto for a long time and every debian jessie server I've ever installed has always failed to build lto
280 2017-07-11 18:23:17	0|sipa|BlueMatt: due to some boost interaction, right?
281 2017-07-11 18:23:20	0|BlueMatt|its an easy fix - just recompile util.cpp without lto and then it works, but its broken
282 2017-07-11 18:23:27	0|BlueMatt|sipa: its not boost-specific, but thats where it fails
283 2017-07-11 18:23:38	0|BlueMatt|it appears to be some general issue with destructors defined in headers
284 2017-07-11 18:23:41	0|cfields|sipa: iirc i hit a boost::filesystem problem last i tried
285 2017-07-11 18:23:49	0|gmaxwell|BlueMatt: good datapoint then.
286 2017-07-11 18:23:53	0|BlueMatt|I've seen it fail on other things, but normally the thing you see is ~boost::filesystem()
287 2017-07-11 18:23:59	0|cfields|but i assumed it was local
288 2017-07-11 18:24:01	0|BlueMatt|ehh, sorry, not filesystem
289 2017-07-11 18:24:06	0|BlueMatt|some boost::filesystem path ~ thing
290 2017-07-11 18:25:20	0|cfields|sipa: ooh i know, we can wire it up with depends builds
291 2017-07-11 18:25:41	0|BlueMatt|cfields: didnt you tell me you were gonna wire up a "trusting-trust" gcc build in depends for 15? :P
292 2017-07-11 18:25:44	0|BlueMatt|I still dont see a PR
293 2017-07-11 18:26:12	0|cfields|that way anyone using depends gets lto by default, and we also know what dep versions we're dealing with
294 2017-07-11 18:26:21	0|wumpus|our good friend ubuntu trusty has gcc 4.8.4, hopefully that can do flto without probems (especially worried about windows! but we'll see)
295 2017-07-11 18:26:35	0|cfields|BlueMatt: i don't think it's going to make it :(
296 2017-07-11 18:26:37	0|BlueMatt|wumpus: I'd be kinda surprised if it did
297 2017-07-11 18:26:52	0|BlueMatt|cfields: what? by this coming sunday? yea, somehow i doubt it
298 2017-07-11 18:27:12	0|wumpus|can someone please just try instead of guess :-)
299 2017-07-11 18:27:26	0|BlueMatt|+1
300 2017-07-11 18:27:29	0|BlueMatt|go cfields, go
301 2017-07-11 18:27:39	0|wumpus|I can do it, np, but thought cfields was going to
302 2017-07-11 18:28:34	0|cfields|add lto to configure (and see if it works) ?
303 2017-07-11 18:28:59	0|wumpus|cfields: yes, doing a gitian build with lto (for all platforms)
304 2017-07-11 18:29:11	0|BlueMatt|oh, i thought we were talking about a "trusting trust" copy of gcc
305 2017-07-11 18:29:11	0|wumpus|then seeing where/if the result crashes
306 2017-07-11 18:29:12	0|cfields|sure, i can do that
307 2017-07-11 18:29:19	0|BlueMatt|yea, ok, adding lto is much easier, that could make it
308 2017-07-11 18:29:48	0|wumpus|BlueMatt: I thoughtwe were talking about optimizations
309 2017-07-11 18:30:27	0|cfields|adding lto to configure now, then seeing where it works.
310 2017-07-11 18:32:48	0|wumpus|cool
311 2017-07-11 18:34:54	0|cluelessperson|Hey guys, I'm looking at buying some new server hardware.
312 2017-07-11 18:34:56	0|cluelessperson|https://www.asus.com/us/Commercial-Servers-Workstations/RS100-E9-PI2/specifications/
313 2017-07-11 18:35:19	0|cluelessperson|is currently on my plate, but I was wondering if you might suggest something else that might be more beneficial towards bitcoin development, testing/QA?
314 2017-07-11 18:38:11	0|wumpus|this really isn't the channel for server hw suggestions
315 2017-07-11 18:38:17	0|midnightmagic|I sent him here.
316 2017-07-11 18:39:09	0|BlueMatt|cluelessperson: anything with fast disk and lots of memory
317 2017-07-11 18:39:13	0|BlueMatt|otherwise, O/T
318 2017-07-11 18:40:03	0|cluelessperson|BlueMatt: I could do PCI-SSD, but what type of ram requirements are we talking?
319 2017-07-11 18:40:13	0|midnightmagic|-- since he was hoping to help in some capacity with the development itself, and I know occasionally guidance in terms of e.g. differing hardware or stuff that would be more helpful than generic hardware is sometimes in demand.
320 2017-07-11 18:40:25	0|BlueMatt|ok, no big deal, lets move to #bitcoin
321 2017-07-11 18:43:21	0|bitcoin-git|[13bitcoin] 15jonasschnelli closed pull request #8369: [FOR LATER USE][WIP][Wallet] add support for a flexible "set of features" (06master...062016/07/wallet_features) 02https://github.com/bitcoin/bitcoin/pull/8369
322 2017-07-11 19:00:07	0|morcos|ryanofsky: #10244 is marked high-priority, but seems to me from reading the conversation that although it is now concept acked it's targeted for post-0.15?
323 2017-07-11 19:00:09	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
324 2017-07-11 19:00:31	0|morcos|just want to direct my review appropriately with 0.15 coming up
325 2017-07-11 19:01:55	0|jonasschnelli|morcos: Yes. Agree. 10244 should probably removed until there is conceptual consensus (not cleat to me if we have that or not, which probably means we have not :) )
326 2017-07-11 19:02:20	0|ryanofsky|i thought i had two concept acks
327 2017-07-11 19:02:29	0|jonasschnelli|Oh. Yes. Right..
328 2017-07-11 19:02:48	0|ryanofsky|but it's not intended for 15. maybe search for milestone:0.15.0
329 2017-07-11 19:02:54	0|jonasschnelli|I take back my last comment then...
330 2017-07-11 19:04:22	0|jonasschnelli|I think for 10244, it makes sense to rebase it after 0.15 has been split off and concentrate review (and hope for a merge)
331 2017-07-11 19:04:35	0|jonasschnelli|It's a large PR though
332 2017-07-11 19:04:44	0|jonasschnelli|+2,262 −1,152
333 2017-07-11 19:05:00	0|ryanofsky|it's pretty much a mechanical change
334 2017-07-11 19:06:36	0|wumpus|agreed
335 2017-07-11 19:06:51	0|wumpus|it's not an end-user visible change, we should prioritize those for 0.15
336 2017-07-11 19:07:12	0|wumpus|all the code cleanups pure refactorings can wait for 0.16
337 2017-07-11 19:11:12	0|morcos|ok, i was viewing high priority as a subset of milestone:0.15, but perhaps that isn't correct
338 2017-07-11 19:12:38	0|jonasschnelli|wumpus, ryanofsky: Argee about it beeing mechanical, though it still require line by line review by at least 2-3 guys.
339 2017-07-11 19:12:43	0|wumpus|well maybe it is now, but in general it's just something that people can request review if it blocks future PRs from themselves
340 2017-07-11 19:12:59	0|wumpus|but I agree ahving something high-prio that won't make 0.15 anyway is a bad idea
341 2017-07-11 19:13:05	0|wumpus|right now
342 2017-07-11 19:13:22	0|jonasschnelli|Yes. I had the same feeling. High-Prio in context of holding back the individual developer / focus of the individual developer.
343 2017-07-11 19:14:47	0|jonasschnelli|About sipa's ser. changes (#10785), is it worth to benchmark the differences or do we expect non to little impact on performance?
344 2017-07-11 19:14:48	0|gribble|https://github.com/bitcoin/bitcoin/issues/10785 | Serialization improvements by sipa · Pull Request #10785 · bitcoin/bitcoin · GitHub
345 2017-07-11 19:15:12	0|jonasschnelli|(otherwise I can add a bench-package for the serialization code
346 2017-07-11 19:15:13	0|jonasschnelli|)
347 2017-07-11 19:17:59	0|jonasschnelli|ryanofsky about the CAuxiliaryRequest, where do you think we have duplicated code? Also, what do you think about the points I wrote (https://github.com/bitcoin/bitcoin/pull/10794#issuecomment-314534086)?
348 2017-07-11 19:18:49	0|jonasschnelli|(maybe its better to discuss some thing here instead of have to much discussion on the PR before we actually get reviews)
349 2017-07-11 19:18:55	0|jonasschnelli|things*
350 2017-07-11 19:19:11	0|sipa|jonasschnelli: i expect 0 impact on performance
351 2017-07-11 19:19:29	0|jonasschnelli|sipa: Okay. Then it's probably not worth...
352 2017-07-11 19:19:29	0|ryanofsky|it's been months since i've looked at it but, i remember duplicated logic in the aux request class & normal network download code
353 2017-07-11 19:20:38	0|jonasschnelli|ryanofsky: I can't see any real duplication, maybe some lines that take care of passing to the instance and for the callback approach...
354 2017-07-11 19:20:54	0|ryanofsky|i'm looking up my old comments
355 2017-07-11 19:21:43	0|jonasschnelli|Great. Thanks..
356 2017-07-11 19:22:24	0|sipa|jonasschnelli: it could be considered a bugfix though (but almost certainly not one that matters)
357 2017-07-11 19:22:54	0|ryanofsky|here is the comment where i point out the duplication: https://github.com/bitcoin/bitcoin/pull/9483#discussion_r100077677
358 2017-07-11 19:22:57	0|jonasschnelli|sipa: you mean the "cast ways the const"?
359 2017-07-11 19:23:06	0|jonasschnelli|"cast away"
360 2017-07-11 19:24:07	0|ryanofsky|jonasschnelli, my broader point is that i would like you to get some concept ack on your networking design from some other developers who know more about the networking code than me, before i spend more time reviewing it
361 2017-07-11 19:24:31	0|jonasschnelli|ryanofsky: Yes. Sure. That makes sense.
362 2017-07-11 19:25:37	0|jonasschnelli|About your comment, I saw that and I think its then not about duplication, it's a slightly different approach (which seems also to make sense).
363 2017-07-11 19:26:05	0|jonasschnelli|I just like the have-its-own file approach for the reasons I wrote.. but lets get other opinions
364 2017-07-11 19:26:33	0|ryanofsky|fillInNextBlocks is duplicative of FindNextBlocksToDownload, processWithPossibleBlock is duplicative of ProcessNewBlock was my point
365 2017-07-11 19:27:26	0|ryanofsky|i don't think you are going to convince me that having two different control flows for regular downloads vs priority downloads is the way to go, but you don't have to convince me. i'm happy to accept that if others think it is a good idea
366 2017-07-11 19:27:44	0|jonasschnelli|ryanofsky: I don't think its duplicative, if we would eliminate the class, that logic has just to move into net_processing.cpp/validation.cpp
367 2017-07-11 19:28:09	0|gmaxwell|ryanofsky: I think others do not think it's a good idea, and already raised the concern.
368 2017-07-11 19:28:18	0|ryanofsky|iirc, more logic could be shared if they were in the same place
369 2017-07-11 19:28:32	0|gmaxwell|When that code was first implemented; ... made sense to do for a demo or whatnot.
370 2017-07-11 19:28:48	0|sipa|just have a means to tell the network layer "i need block X, and here is callback for when you have it"
371 2017-07-11 19:29:38	0|jonasschnelli|Okay. But the network layer hasn't to be stuffed into a single file/class?
372 2017-07-11 19:29:39	0|ryanofsky|sipa, yes that is what i requested. i think the auxilliary class is too low level
373 2017-07-11 19:30:23	0|jonasschnelli|But if the general opinion goes direction of having the logic in net_processing.cpp directly, then I'm fine with moving it there...
374 2017-07-11 19:30:24	0|sipa|jonasschnelli: net_processing is already a single file
375 2017-07-11 19:30:27	0|ryanofsky|jonasschnelli, i think agree networking code is monolithic. but that you have to find the right way to break it up and this does not seem like the right way
376 2017-07-11 19:31:10	0|jonasschnelli|sipa: Yes. And I think having specialized functions (like the light client block rquests) in dedicated files makes sense.. rebasing, patches, code-overview
377 2017-07-11 19:32:16	0|sipa|jonasschnelli: i really don't think block fetching code should be in more than 1 place
378 2017-07-11 19:32:36	0|gmaxwell|we have a hard enough time keeping the two existing block requesting state machine functioning and maintained... really pretty ugly to add another seperate one for the wallet in another place.
379 2017-07-11 19:32:43	0|sipa|it's arguably already too spread out in net_processing (dealing with invs, direct fetching, headers fetching, async fetching ...)
380 2017-07-11 19:32:54	0|jonasschnelli|sipa: it's not fetching,.. is an object that hold information about what it should fetch and if it's done or not and eventuall the process flow
381 2017-07-11 19:33:11	0|sipa|jonasschnelli: yes, that's probably how all the fetching code should work
382 2017-07-11 19:33:33	0|sipa|keep information on what blocks are to be downloaded
383 2017-07-11 19:34:09	0|jonasschnelli|ryanofsky gmaxwell: I think the out-of-band (auxiliary) requests need state, and therefore a class makes sense (when was the request initiated, how much is done, etc.) would you add that class into net_processing rathern then creating a new file?
384 2017-07-11 19:34:24	0|ryanofsky|i think if you wanted to move code *out* of net_processing into your new download implementation, that would be nice. but just adding the new download implementation alongside does not seem seem so nice
385 2017-07-11 19:34:32	0|gmaxwell|jonasschnelli: all block requests need state.
386 2017-07-11 19:34:52	0|gmaxwell|jonasschnelli: we have to track what we've asked for, what we need, whats been recieved... and so on.
387 2017-07-11 19:34:55	0|jonasschnelli|gmaxwell: yes. But here it is a user-requested range
388 2017-07-11 19:35:11	0|sipa|jonasschnelli: in the current case, the user is the block validation logic
389 2017-07-11 19:35:20	0|sipa|you just want to add another user
390 2017-07-11 19:35:27	0|gmaxwell|^
391 2017-07-11 19:35:42	0|jonasschnelli|sipa: can't follow the "user is the block validation logic" :/
392 2017-07-11 19:35:43	0|ryanofsky|to make discussion more concrete, what do you think of my blocksToDownloadFirst suggestion?
393 2017-07-11 19:36:39	0|jonasschnelli|ryanofsky: Yes. Make sense. But still, the requester (assume wallet) will need to blocks in a clear sequence (assume A, B, C, D and not C, D, A, B).
394 2017-07-11 19:37:21	0|jonasschnelli|Therefore there must be something that takes care of the state of a wallet/user initiated out-of-band block request and make sure, it will be passed in in sequence
395 2017-07-11 19:38:22	0|jonasschnelli|IMO its about "requesting n amount of "range of blocks" rather then just requesting blocks
396 2017-07-11 19:40:12	0|gmaxwell|jonasschnelli: right now the consensus logic requests the block download machinery to download blocks, keep track of the requests and what not.
397 2017-07-11 19:40:17	0|ryanofsky|i'm not sure i see the problem. that state could be tracked inside the wallet. wallet just needs to request blocks, get notifications when they come in, then request more blocks
398 2017-07-11 19:40:30	0|gmaxwell|jonasschnelli: sipa is pointing out that what you're doing is just adding an addition set of callers.
399 2017-07-11 19:41:00	0|gmaxwell|jonasschnelli: why does the wallet need blocks in order?
400 2017-07-11 19:42:03	0|jonasschnelli|gmaxwell: if we assume the wallet does show to the user what it had processed it would probably look strange if the wallet would update the balance, tx-list of block that come in out of order
401 2017-07-11 19:42:39	0|jonasschnelli|Also, detecting re-orgs would be difficult?
402 2017-07-11 19:42:41	0|ryanofsky|can't wallet keep track of it's own requests and ignore anything it wants to ignore?
403 2017-07-11 19:43:13	0|gmaxwell|if the wallet is just saying how far it had processed, couldn't it just show the minimum height processed so far?  showing things out of order would certantly allow things faster.
404 2017-07-11 19:43:17	0|jonasschnelli|ryanofsky: Yes. If we assume only wallets want that. What about the other user cases, ZMQ, light-client "middleware" (Oh I had this term)
405 2017-07-11 19:44:53	0|ryanofsky|jonasschnelli, that's why i was asking for some kind of design doc, going into the what specifically you envision there
406 2017-07-11 19:45:18	0|jonasschnelli|gmaxwell: hmm.. I guess the user would look at the transaction list and see a possible incoming transaction while a older expected incoming transaction is missing, regardless of how the wallet communicates "minimum process height"
407 2017-07-11 19:45:49	0|sipa|jonasschnelli: right now, the only thing that "needs" blocks is the block validation
408 2017-07-11 19:46:00	0|sipa|you're adding something else that needs blocks
409 2017-07-11 19:46:05	0|jonasschnelli|ryanofsky: Not only, #10794 is purely RPC/ZMQ
410 2017-07-11 19:46:07	0|gribble|https://github.com/bitcoin/bitcoin/issues/10794 | Add simple light-client mode (RPC only) by jonasschnelli · Pull Request #10794 · bitcoin/bitcoin · GitHub
411 2017-07-11 19:46:13	0|sipa|both of them should be seen as users of the block fetching logic
412 2017-07-11 19:46:25	0|jonasschnelli|sipa: Ah. I understand now
413 2017-07-11 19:48:25	0|jonasschnelli|sipa, ryanofsky: The idea in 10794 is to have the validation triggered download logic in tact (minimal impact) while preferring out-of-band requests. And I tried to keep the logic, state-machine outside of net_processing.cpp to allow later code splits
414 2017-07-11 19:49:12	0|sipa|i think that's the wrong approach (though admittedly i haven't reviewed the code)
415 2017-07-11 19:49:29	0|jonasschnelli|(It's good you haven't so I can change it before you do. :) )
416 2017-07-11 19:49:50	0|jonasschnelli|sipa: what would you prefer then?
417 2017-07-11 19:49:55	0|sipa|jonasschnelli: as i explained
418 2017-07-11 19:50:14	0|sipa|refactor the current block downloading logic to support multiple users
419 2017-07-11 19:50:25	0|sipa|make it keep track of what blocks to download
420 2017-07-11 19:50:44	0|sipa|and what to do when they arrive
421 2017-07-11 19:50:45	0|jonasschnelli|sipa: I thought you are going to say that... I just think this is way larger but probably cleaner
422 2017-07-11 19:50:57	0|sipa|it's much more refactoring, yes
423 2017-07-11 19:51:06	0|sipa|but it'll be tiny to add light client fetching on top :)
424 2017-07-11 19:51:29	0|jonasschnelli|heh.. Yes. After that one or two year for the block download refactoring.. :)
425 2017-07-11 19:52:48	0|ryanofsky|am i missing something? https://github.com/bitcoin/bitcoin/pull/9483#discussion_r100077677 seems pretty straightforward and not a large amount of refactoring
426 2017-07-11 19:53:03	0|jonasschnelli|I guess I focus to much on the minimal.impact solution rather then on the clean solution because I'm too much feature driven
427 2017-07-11 19:54:23	0|jonasschnelli|ryanofsky: Not sure if the idea in that comment fulfils sipa idea of having "multiple users for the download logic"
428 2017-07-11 19:55:30	0|morcos|sipa: i also like that idea, but i wonder if there is a priority difference there...  like how quickly you time out, or find someone else to deliver or penalize for bad behavior or etc...
429 2017-07-11 19:55:46	0|ryanofsky|because blocksToDownloadFirst is not sufficient for multiple users?
430 2017-07-11 19:55:56	0|morcos|who me?
431 2017-07-11 19:56:04	0|morcos|oh him
432 2017-07-11 19:58:06	0|jonasschnelli|ryanofsky: No I think because we would only partially (out of band request) allow multiple users for the block download. But not exactly sure to what extend sipas solution should go to
433 2017-07-11 19:58:24	0|ryanofsky|oh i think i see the difference. sipa's idea is to keep track of blocks to download and "what to do when they arrive". my idea would just do the first and let client figure out what to do when blocks come
434 2017-07-11 19:58:25	0|sipa|what do you mean with 'out of band' ?
435 2017-07-11 19:59:20	0|jonasschnelli|sipa: out-of-band is a poor multi-user approach for block download (the in-band is validation, the rest is out-of-band)
436 2017-07-11 19:59:46	0|sipa|i don't understand what you mean with 'out of band' in this context
437 2017-07-11 19:59:56	0|jonasschnelli|sipa: i guess what you meant is that the validation part uses the same block download interface then the light-client wallet? right?
438 2017-07-11 20:00:17	0|sipa|yes
439 2017-07-11 20:01:04	0|jonasschnelli|sipa: the validation requests block after it's own logic (not to far away, etc.), out-of-band requests can request a range of blocks that make no sense for the validation (in terms of time prioritation)
440 2017-07-11 20:01:52	0|jonasschnelli|sipa: I guess ryanofsky terms `blocksToDownloadFirst` is much better
441 2017-07-11 20:01:53	0|sipa|i still don't understand what you mean with out of band
442 2017-07-11 20:02:32	0|sipa|there are blocks to download, there is logic to determine which ones to download in what order, they're processed when they arrive
443 2017-07-11 20:02:40	0|jonasschnelli|I think i should call it "priorizedBlockRequests"
444 2017-07-11 20:03:39	0|jonasschnelli|sipa: Yes. But the light client (wallet) wants to break that oder by a) preferring other blocks first, b) pass them through BlockConnected() before they are connected to the tip
445 2017-07-11 20:04:06	0|jonasschnelli|(connected to the tip == validated, not connected == spv)
446 2017-07-11 20:05:44	0|sipa|sure
447 2017-07-11 20:05:46	0|jonasschnelli|ryanofsky: I think sipa's idea makes sense in the long run (don't distinct between wallet spv block requests and the validation triggered block request), ideally the validation would then use the same interface then the (light-client) wallet would use
448 2017-07-11 20:06:09	0|jonasschnelli|but this would be a lot of work
449 2017-07-11 20:06:41	0|sipa|sure, not everything has to happen in one go
450 2017-07-11 20:06:48	0|ryanofsky|yeah, i get that now. i think that refactoring is basically orthogonal though
451 2017-07-11 20:07:07	0|jonasschnelli|I agree...
452 2017-07-11 20:07:08	0|ryanofsky|you could do that refactoring with or without prioritized downloads, or vice versa
453 2017-07-11 20:07:25	0|sipa|ryanofsky: agree
454 2017-07-11 20:07:36	0|jonasschnelli|I even think implementing the prioritized downloads gives a better feeling on how-to-refactor
455 2017-07-11 20:07:54	0|jonasschnelli|because you refactor with a clear interface use-case
456 2017-07-11 20:08:09	0|sipa|yes, fair enough
457 2017-07-11 20:08:54	0|jonasschnelli|But I'm still not sure if it makes then sense to implement it within net_processing or leave it in a designated file (not very different IMO)
458 2017-07-11 20:09:13	0|jonasschnelli|Probably most devs things within net_processing.cpp makes more sense...
459 2017-07-11 20:09:18	0|jonasschnelli|*think
460 2017-07-11 20:09:45	0|jonasschnelli|Then let me do that and let me rename it from auxiliary (out-of-band) to prioritizedBlockRequests
461 2017-07-11 20:31:55	0|cfields|ok, i managed to patch boost and fix the lto linking error...
462 2017-07-11 20:32:24	0|cfields|anyone feel like looking at a head-scratcher?
463 2017-07-11 20:32:39	0|BlueMatt|really? I failed to do so previously
464 2017-07-11 20:33:24	0|cfields|BlueMatt: was this your problem as well? https://pastebin.com/raw/kmqJVmfq
465 2017-07-11 20:33:46	0|BlueMatt|oh, no
466 2017-07-11 20:34:02	0|cfields|oh. what's yours?
467 2017-07-11 20:34:03	0|BlueMatt|i always got a link-time "~boost::filesystem::path() undefined" error
468 2017-07-11 20:34:24	0|BlueMatt|and occasionally for other destructors that were implicitly-defined in headers or explicitly defined in headers
469 2017-07-11 20:34:31	0|cfields|oh, same thing
470 2017-07-11 20:34:41	0|cfields|ZN5boost10filesystem4pathD1Ev is mangled destructor
471 2017-07-11 20:35:15	0|cfields|right, so the fix was to give it an explicit destructor in the .cpp
472 2017-07-11 20:35:19	0|BlueMatt|heh, your linker has garbage error messages
473 2017-07-11 20:35:31	0|BlueMatt|yes, but boost::filesystem::path has no .cpp
474 2017-07-11 20:35:32	0|cfields|but i'm not understanding why
475 2017-07-11 20:35:34	0|BlueMatt|or, i dont recall one
476 2017-07-11 20:35:37	0|BlueMatt|oh, i have no idea why
477 2017-07-11 20:35:39	0|cfields|it does
478 2017-07-11 20:35:43	0|BlueMatt|oh, ok
479 2017-07-11 20:35:45	0|sipa|BlueMatt: go test cfields's fix :p
480 2017-07-11 20:36:04	0|cfields|heh, I'll PR as soon as I can understand why it's a fix :)
481 2017-07-11 20:36:17	0|BlueMatt|and it mostly only happens to that one place, not on other destructors-defined-in-headers
482 2017-07-11 20:36:25	0|BlueMatt|but i have seen it on others before
483 2017-07-11 20:36:31	0|cfields|BlueMatt: right, there's a particular reason for these, sec
484 2017-07-11 20:36:55	0|sipa|there are fairly complicated rules in C++ about which object the implementation of some implicit methods goes into
485 2017-07-11 20:37:11	0|cfields|https://github.com/boostorg/filesystem/blob/develop/src/path.cpp#L703
486 2017-07-11 20:37:34	0|cfields|they're static vars there
487 2017-07-11 20:38:21	0|cfields|(and statics for each of the failures in the link)
488 2017-07-11 20:38:28	0|BlueMatt|ah, is that why? static objects with implicitly-defined destructors?
489 2017-07-11 20:39:18	0|cfields|so my best guess is... the static vars are created there when the lib is built, so an entry for the dtor is added. But we never use those functions...
490 2017-07-11 20:39:56	0|cfields|so i think it's either: a gcc/lto bug, or some crazy ODR-like issue
491 2017-07-11 20:40:38	0|cfields|oh, also, we build with fvisibility=hidden. when that's off for boost _and_ our build, it links fine
492 2017-07-11 20:41:56	0|BlueMatt|lol
493 2017-07-11 21:16:35	0|gmaxwell|Did Paul Sztorc talk to anyone here before publishing his thing? (except luke-jr) I want to remark on his "
494 2017-07-11 21:16:58	0|gmaxwell|I wrote the roadmap to try to be representative of a Core / developer position." -- that if so he might have wanted to talk to some of the more frequent ones, it doesn't appear that he did.
495 2017-07-11 21:17:10	0|gmaxwell|But I want a fact-check before saying htat.
496 2017-07-11 21:17:15	0|gmaxwell|s/htat/that/
497 2017-07-11 21:17:23	0|BlueMatt|i havent heard from him
498 2017-07-11 21:17:38	0|BlueMatt|(nor speak to him specifically about that issue at any point i can recall)
499 2017-07-11 21:50:17	0|jtimon|images... https://github.com/bitcoin/bitcoin/pull/10757#issuecomment-314581913
500 2017-07-11 21:53:13	0|luke-jr|wumpus: https://github.com/UASF/bitcoin/releases/tag/v0.14.2-uasfsegwit1.0
501 2017-07-11 22:25:29	0|cfields|sipa: aha, you were right
502 2017-07-11 22:27:04	0|cfields|i traced the symbol references in linker dumps. as-is, with lto, the destructor gets stuffed into util.o (because it's one of the files where there's a static reference)
503 2017-07-11 22:27:51	0|cfields|but when i comment out the static references in util.cpp, it links fine, and the destructor goes into a boost object (operations.o)
504 2017-07-11 22:28:41	0|cfields|IIRC when it's ambiguous, the linker is free to choose where to put the symbol, as long as it's just added to one object
505 2017-07-11 22:29:39	0|cfields|but with lto, it gets put into util.o. then, the link-order causes boost_filesystem to be unable to find it
506 2017-07-11 22:29:48	0|sipa|cfields: so, you fixed it?
507 2017-07-11 22:30:06	0|cfields|i believe so
508 2017-07-11 22:30:53	0|cfields|2 fixes: 1 quick hack to use a static string rather than a fs::path, to avoid the issue entirely. 2. the upstream boost fix
509 2017-07-11 22:31:26	0|promag|jtimon: please rename #10757
510 2017-07-11 22:31:28	0|gribble|https://github.com/bitcoin/bitcoin/issues/10757 | RPC: Introduce getperblockstats to plot things by jtimon · Pull Request #10757 · bitcoin/bitcoin · GitHub
511 2017-07-11 22:35:58	0|jtimon|oh, right, I did s/getperblockstats/getblockstats/ as you suggested...thanks!
512 2017-07-11 22:36:17	0|bitcoin-git|[13bitcoin] 15jnewbery opened pull request #10798: test bitcoin-cli (06master...06cli_tests) 02https://github.com/bitcoin/bitcoin/pull/10798
513 2017-07-11 22:55:10	0|cluelessperson|BlueMatt: So I'm looking at a 3.8ghz Xeon Quad Core,  256 GB PCI SSD 3200MBps Read, 1500MBps Write,   64 GB RAM DDR4, thoughts?
514 2017-07-11 22:55:18	0|cluelessperson|$1400
515 2017-07-11 22:55:23	0|cluelessperson|(rounded up)
516 2017-07-11 22:55:43	0|BlueMatt|-> #bitcoin