1 2017-06-05 01:23:46	0|NicolasDorier|spudowiar: Awesome. I think you should share that as well with Ledger (btchip), they would probably review and make a plugin. Also it might move forward the conversation about HW standard
  2 2017-06-05 01:36:56	0|bitcoin-git|[13bitcoin] 15fanquake closed pull request #10513: Trivial: grammar fix to CONTRIBUTING.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10513
  3 2017-06-05 06:17:44	0|spudowiar|NicolasDorier: Will do. I think I'll make a new RPC method (signhwwtransaction) first (because this isn't entirely compatible with signrawtransaction) and then ask him to review it
  4 2017-06-05 06:44:53	0|bitcoin-git|13bitcoin/060.14 1421e1ed4 15Wladimir J. van der Laan: doc: Preliminary release notes 0.14.2
  5 2017-06-05 06:44:53	0|bitcoin-git|[13bitcoin] 15laanwj pushed 1 new commit to 060.14: 02https://github.com/bitcoin/bitcoin/commit/21e1ed48989e1d150e9f85770fd098534e0f81f1
  6 2017-06-05 06:58:34	0|NicolasDorier|spudowiar: what? signrawtransaction should just work. I mean the wallet already have all the info needed for a hardware wallet to sign
  7 2017-06-05 06:58:49	0|NicolasDorier|so why a new rpc method needed ? oO
  8 2017-06-05 06:59:44	0|NicolasDorier|the cool thing if we can reuse signrawtransaction is that moving from software hotwallet to hardware wallet is a configuration change, not a code change for users
  9 2017-06-05 07:05:44	0|NicolasDorier|Confirmed devs right now are
 10 2017-06-05 08:57:19	0|bitcoin-git|[13bitcoin] 15Flowdalic opened pull request #10529: Improve bitcoind systemd service file (06master...06systemd-service) 02https://github.com/bitcoin/bitcoin/pull/10529
 11 2017-06-05 09:01:03	0|_flow_|#10529 was me, in case you have any questions
 12 2017-06-05 09:01:04	0|gribble|https://github.com/bitcoin/bitcoin/issues/10529 | Improve bitcoind systemd service file by Flowdalic · Pull Request #10529 · bitcoin/bitcoin · GitHub
 13 2017-06-05 09:49:08	0|spudowiar|NicolasDorier: I meant for the communication with the hardware wallet
 14 2017-06-05 09:49:14	0|spudowiar|The hardware wallet plugins use JSON-RPC
 15 2017-06-05 09:49:21	0|spudowiar|https://github.com/saleemrashid/bitcoin/blob/hardware-wallet/src/wallet/wallet.cpp#L1526
 16 2017-06-05 09:49:46	0|spudowiar|I called the method signrawtransaction, but it's not compatible (as it sends hdKeypaths instead of privkeys)
 17 2017-06-05 09:50:14	0|spudowiar|Also, I want to add an identifier, so we can support multiple wallets attached to the computer
 18 2017-06-05 09:50:19	0|spudowiar|e.g. a serial number
 19 2017-06-05 09:54:25	0|bitcoin-git|[13bitcoin] 15pavlosantoniou opened pull request #10530: Fix possibly unsafe accesses of array in class base_uint<BITS>. (06master...06master) 02https://github.com/bitcoin/bitcoin/pull/10530
 20 2017-06-05 09:54:48	0|spudowiar|I'm also going to add a listhwwdevices which could be used on the first run to list all the devices so Bitcoin Core can ask the user to select one
 21 2017-06-05 09:54:58	0|spudowiar|And a gethwwinfo to get the xpub for the selected device
 22 2017-06-05 09:55:30	0|spudowiar|Then on first run, Bitcoin Core could allow you to select a vendor and then a device
 23 2017-06-05 09:55:33	0|spudowiar|And store this in the wallet
 24 2017-06-05 11:37:02	0|NicolasDorier|spudowiar: for multiple wallet, you don't need serial number. There is the multi wallet PR which use RPC User/Password as a way to selection a wallet
 25 2017-06-05 11:37:27	0|NicolasDorier|and the wallet already know the keypath given the address
 26 2017-06-05 11:37:36	0|NicolasDorier|so you don't need to pass it to the RPC method
 27 2017-06-05 12:02:01	0|spudowiar|NicolasDorier: How does the wallet plugin know which hardware device to connect to?
 28 2017-06-05 12:02:31	0|spudowiar|NicolasDorier: You might be misunderstanding
 29 2017-06-05 12:02:33	0|spudowiar|I'm not talking about the RPC interface Bitcoin Core normally uses
 30 2017-06-05 12:02:48	0|spudowiar|I'm talking about JSON-RPC that is sent over stdin to the hardware wallet plugin
 31 2017-06-05 12:03:02	0|NicolasDorier|oh
 32 2017-06-05 12:03:12	0|NicolasDorier|yes I was misunderstanding
 33 2017-06-05 12:03:28	0|spudowiar|Check out https://github.com/saleemrashid/bitcoin/blob/hardware-wallet/src/wallet/wallet.cpp#L1527
 34 2017-06-05 12:03:32	0|NicolasDorier|so it will be possible to use BTC signrawtransaction as if it was keys in software ?
 35 2017-06-05 12:03:39	0|spudowiar|Oh yeah
 36 2017-06-05 12:03:49	0|spudowiar|The normal RPC interface won't change
 37 2017-06-05 12:03:56	0|NicolasDorier|ha awesome
 38 2017-06-05 12:04:00	0|spudowiar|Although I might make it compatible as a hardware wallet plugin
 39 2017-06-05 12:04:03	0|NicolasDorier|that is what I meant
 40 2017-06-05 12:04:06	0|spudowiar|So you could connect one instance to another :)
 41 2017-06-05 12:04:14	0|spudowiar|And it thinks the other instance is a hardware wallet :)
 42 2017-06-05 12:05:37	0|NicolasDorier|spudowiar: very cool. Would you mind once you tested a bit to ACK my PR about hdwatchonly ? This would make your PR a bit smaller if you plan to include what you are doing into core
 43 2017-06-05 12:06:26	0|NicolasDorier|my PR is hanging around for a while, I am using it in production. But was lacking reviews
 44 2017-06-05 12:06:27	0|spudowiar|If you check that branch, the first few commits are your PR
 45 2017-06-05 12:06:42	0|spudowiar|I think I did ACK it, if not I'll do that when I can
 46 2017-06-05 12:07:31	0|NicolasDorier|yes I saw. Just a way to make your PR smaller, so it will be easier to merge yours later
 47 2017-06-05 12:10:07	0|spudowiar|The other PR we need is making signing asynchronous in Bitcoin Qt
 48 2017-06-05 12:10:36	0|spudowiar|At the moment, while it is talking to the hardware wallet the GUI freezes
 49 2017-06-05 12:10:54	0|spudowiar|Because the io_service is running on the UI thread
 50 2017-06-05 12:12:59	0|NicolasDorier|rebasing my PR.
 51 2017-06-05 12:13:22	0|spudowiar|Thanks, I'll ACK and rebase ASAP
 52 2017-06-05 12:13:35	0|NicolasDorier|actually you already rebased right ?
 53 2017-06-05 12:13:45	0|NicolasDorier|I can just take your branch up to my commits I think
 54 2017-06-05 12:14:51	0|spudowiar|Yeah, sure but you might want to check (I had to make a few changes for merge conflicts)
 55 2017-06-05 12:15:10	0|NicolasDorier|ok
 56 2017-06-05 12:15:34	0|spudowiar|A three way diff should let you see what I had to change
 57 2017-06-05 12:27:41	0|spudowiar|Do you think I should add a scan in the PATH for plugins? Or a scan in a folder?
 58 2017-06-05 12:27:49	0|spudowiar|e.g. bitcoin-hww-*
 59 2017-06-05 12:28:12	0|spudowiar|Then, on startup, the GUI wallet could display all the vendors
 60 2017-06-05 12:45:18	0|bitcoin-git|[13bitcoin] 15colgreen opened pull request #10531: Increased startup timeout. (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/10531
 61 2017-06-05 13:07:06	0|NicolasDorier|spudowiar: unsure, I like your current approach to just delegate to a child process specified by command line.
 62 2017-06-05 13:12:37	0|wumpus|I also like the external process approach
 63 2017-06-05 13:13:27	0|wumpus|loading plugins into bitcoind/-qt 's address space seem like the wrong way, for me, and is hard harmonize with the static linking we do for distributed executables
 64 2017-06-05 13:40:40	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/400fdd08cc95...296928eb38a4
 65 2017-06-05 13:40:41	0|bitcoin-git|13bitcoin/06master 143457331 15Wladimir J. van der Laan: test: Add test for `getpeerinfo` `bindaddr` field
 66 2017-06-05 13:40:41	0|bitcoin-git|13bitcoin/06master 14a7e3c28 15Wladimir J. van der Laan: rpc: Add listen address to incoming connections in `getpeerinfo`...
 67 2017-06-05 13:40:42	0|bitcoin-git|13bitcoin/06master 14296928e 15Wladimir J. van der Laan: Merge #10478: rpc: Add listen address to incoming connections in `getpeerinfo`...
 68 2017-06-05 13:41:20	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10478: rpc: Add listen address to incoming connections in `getpeerinfo` (06master...062017_05_peer_listenaddr) 02https://github.com/bitcoin/bitcoin/pull/10478
 69 2017-06-05 14:10:25	0|bitcoin-git|13bitcoin/06master 149aa215b 15Pieter Wuille: Bugfixes: missing == 0 after randrange
 70 2017-06-05 14:10:25	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/296928eb38a4...e103b3ff1e4f
 71 2017-06-05 14:10:26	0|bitcoin-git|13bitcoin/06master 14e103b3f 15Wladimir J. van der Laan: Merge #10514: Bugfix: missing == 0 after randrange...
 72 2017-06-05 14:10:59	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10514: Bugfix: missing == 0 after randrange (06master...06fixtests) 02https://github.com/bitcoin/bitcoin/pull/10514
 73 2017-06-05 14:24:49	0|bitcoin-git|13bitcoin/06master 148d4f401 15Alex Morcos: Fix timestamp in fee estimate debug message
 74 2017-06-05 14:24:49	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/e103b3ff1e4f...cec9e1ea6174
 75 2017-06-05 14:24:50	0|bitcoin-git|13bitcoin/06master 14cec9e1e 15Wladimir J. van der Laan: Merge #10422: Fix timestamp in fee estimate debug message...
 76 2017-06-05 14:25:26	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10422: Fix timestamp in fee estimate debug message (06master...06fixtimeunits) 02https://github.com/bitcoin/bitcoin/pull/10422
 77 2017-06-05 14:28:04	0|bitcoin-git|13bitcoin/06master 14cc36b5e 15Jimmy Song: [test] Add test for getchaintxstats
 78 2017-06-05 14:28:04	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/cec9e1ea6174...c871f323b418
 79 2017-06-05 14:28:05	0|bitcoin-git|13bitcoin/06master 14c871f32 15MarcoFalke: Merge #10515: [test] Add test for getchaintxstats...
 80 2017-06-05 14:28:39	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #10515: [test] Add test for getchaintxstats (06master...06test_getchaintxstats) 02https://github.com/bitcoin/bitcoin/pull/10515
 81 2017-06-05 14:38:29	0|bitcoin-git|13bitcoin/06master 1488b8f0b 15Russell Yanofsky: Simplify feebumper minimum fee code slightly...
 82 2017-06-05 14:38:29	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/c871f323b418...0747d3349817
 83 2017-06-05 14:38:30	0|bitcoin-git|13bitcoin/06master 140747d33 15Wladimir J. van der Laan: Merge #10455: Simplify feebumper minimum fee code slightly...
 84 2017-06-05 14:39:10	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10455: Simplify feebumper minimum fee code slightly (06master...06pr/bumpmin) 02https://github.com/bitcoin/bitcoin/pull/10455
 85 2017-06-05 14:41:25	0|bitcoin-git|13bitcoin/06master 144d2d604 15Russell Yanofsky: Fix importmulti failure to return rescan errors...
 86 2017-06-05 14:41:25	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/0747d3349817...08d0390a5fea
 87 2017-06-05 14:41:26	0|bitcoin-git|13bitcoin/06master 1408d0390 15Wladimir J. van der Laan: Merge #10403: Fix importmulti failure to return rescan errors...
 88 2017-06-05 14:41:57	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10403: Fix importmulti failure to return rescan errors (06master...06pr/scansame) 02https://github.com/bitcoin/bitcoin/pull/10403
 89 2017-06-05 15:00:22	0|bitcoin-git|13bitcoin/06master 14cf390df 15Cory Fields: build: silence gcc7's implicit fallthrough warning...
 90 2017-06-05 15:00:22	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/08d0390a5fea...e654d61d94ee
 91 2017-06-05 15:00:23	0|bitcoin-git|13bitcoin/06master 14e654d61 15Wladimir J. van der Laan: Merge #10489: build: silence gcc7's implicit fallthrough warning...
 92 2017-06-05 15:00:57	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #10489: build: silence gcc7's implicit fallthrough warning (06master...06wimplicit-fallthrough) 02https://github.com/bitcoin/bitcoin/pull/10489
 93 2017-06-05 15:21:44	0|ivo___|\disconnect
 94 2017-06-05 15:50:06	0|bitcoin-git|13bitcoin/06master 14e4bc19f 15Russell Yanofsky: Remove xvfb configuration from travis...
 95 2017-06-05 15:50:06	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/e654d61d94ee...bea5b00cfe95
 96 2017-06-05 15:50:07	0|bitcoin-git|13bitcoin/06master 14bea5b00 15MarcoFalke: Merge #10509: Remove xvfb configuration from travis...
 97 2017-06-05 15:50:41	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #10509: Remove xvfb configuration from travis (06master...06pr/rmfb) 02https://github.com/bitcoin/bitcoin/pull/10509
 98 2017-06-05 15:59:39	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9745: [RPC] Getting confirmations command (06master...06add-getconfirmations-to-rpc) 02https://github.com/bitcoin/bitcoin/pull/9745
 99 2017-06-05 16:06:56	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/bea5b00cfe95...9fec4da0bec9
100 2017-06-05 16:06:57	0|bitcoin-git|13bitcoin/06master 14164019d 15aideca: Add dumpwallet output test
101 2017-06-05 16:06:57	0|bitcoin-git|13bitcoin/06master 149f82134 15aideca: Add friendly output to dumpwallet refs #9564
102 2017-06-05 16:06:58	0|bitcoin-git|13bitcoin/06master 149fec4da 15Wladimir J. van der Laan: Merge #9740: Add friendly output to dumpwallet...
103 2017-06-05 16:07:16	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9740: Add friendly output to dumpwallet (06master...06dumpwallet) 02https://github.com/bitcoin/bitcoin/pull/9740
104 2017-06-05 16:09:40	0|spudowiar|NicolasDorier, wumpus: I didn't mean changing it from an external command
105 2017-06-05 16:09:50	0|spudowiar|Just a method of detecting the available external commands
106 2017-06-05 16:10:10	0|spudowiar|But I could add it to the wallet file or something
107 2017-06-05 16:10:24	0|spudowiar|And provide an option at startup of Bitcoin-Qt to enter the command
108 2017-06-05 16:24:38	0|bitcoin-git|[13bitcoin] 15earonesty opened pull request #10532: -bip148 option (06master...06bip148) 02https://github.com/bitcoin/bitcoin/pull/10532
109 2017-06-05 16:40:48	0|btcdrak|wumpus: could you restart the two failing jobs here https://travis-ci.org/bitcoin/bitcoin/builds/238932256
110 2017-06-05 16:58:46	0|spudowiar|sipa__: Is there some good reading about the polymod function Bech32 uses?
111 2017-06-05 17:10:36	0|BlueMatt|;;later tell sipa sipa__ is it just my C++11-lack-of-sanity, or am I suppose to find https://github.com/bitcoin/bitcoin/blob/master/src/coins.cpp#L99 gross as fuck? std::move(it->second.coin) then a few lines later it->second.coin.function_call()?
112 2017-06-05 17:10:36	0|gribble|The operation succeeded.
113 2017-06-05 17:13:07	0|cfields|BlueMatt: after a move, contents are usually undefined, but valid
114 2017-06-05 17:13:17	0|cfields|BlueMatt: a clear after a move makes them defined and valid :)
115 2017-06-05 17:13:27	0|BlueMatt|cfields: ugh, that just feels super gros
116 2017-06-05 17:13:28	0|BlueMatt|s
117 2017-06-05 17:13:51	0|BlueMatt|since that line depends on the exact behavior of CScript::Clear(), prevector, and CTxOut
118 2017-06-05 17:15:01	0|cfields|BlueMatt: the alternative would be defining an explicit move ctor which does the clear and leaves it in a valid state
119 2017-06-05 17:15:13	0|cfields|(or, i suppose, you'd rather just have it not used after the move)
120 2017-06-05 17:15:36	0|BlueMatt|yea, I mean I generally read std::move(X) as "X will not be touched again"
121 2017-06-05 17:15:44	0|BlueMatt|maybe thats just my naivete on C++11
122 2017-06-05 17:15:54	0|cfields|BlueMatt: well, that's not really feasible with a container :\
123 2017-06-05 17:16:21	0|cfields|std::move(foo[0]), foo still exists just fine.
124 2017-06-05 17:16:46	0|cfields|In many cases, this one included i think, it's the only way to avoid incurring the copy when moving the contents of a member
125 2017-06-05 17:17:51	0|BlueMatt|cfields: well obv destructor must run fine after std::move(), but to me I read that as, "I wont touch the 0th element of foo anymore after this, except to destruct"
126 2017-06-05 17:18:25	0|sipa__|BlueMatt: typically the definition os that you're allowed to do anything that foesn't depend on otd prior state
127 2017-06-05 17:18:33	0|sipa__|so you can assign, clear, ...
128 2017-06-05 17:19:00	0|sipa__|in a vector, a move-from + .clear() is well specified
129 2017-06-05 17:19:20	0|sipa__|doesn't
130 2017-06-05 17:21:26	0|MarcoFalke|btcdrak: done. Looks like they hit the 50 min timeout :(
131 2017-06-05 17:25:02	0|sipa__|BlueMatt: https://stackoverflow.com/a/7028318
132 2017-06-05 17:25:36	0|BlueMatt|sipa__: yea, but coin.Clear() is super strange, it calls CScript::clear() which doesnot prevector::clear(), instead it prevector().swap(*this), essentially
133 2017-06-05 17:25:36	0|BlueMatt|which is...wat
134 2017-06-05 17:25:54	0|BlueMatt|sipa: can we make it it->second.coin = CCoin() instead?
135 2017-06-05 17:26:06	0|BlueMatt|should be +/- identical performance, but without feeling so yucky
136 2017-06-05 17:27:12	0|sipa|i don't see the difference
137 2017-06-05 17:27:32	0|sipa|both the Clear method and assignment operator can be user defined
138 2017-06-05 17:28:02	0|BlueMatt|sure, swap just feels worse to me...I'll shut up if no one else thinks it feels strange, though :)
139 2017-06-05 17:29:45	0|cfields|BlueMatt: the swap idiom comes from pre-c++11 when there was no shrink_to_fit().
140 2017-06-05 17:30:16	0|cfields|imo the clear shouldn't be forcing mem dealloc, it should just be clearing, and we should be using shrink_to_fit() more liberally at higher layers.
141 2017-06-05 17:30:31	0|sipa|right
142 2017-06-05 17:30:44	0|BlueMatt|yes, prevector clear() doesnt, script clear() does
143 2017-06-05 17:31:10	0|BlueMatt|could make script more vector-like by pushing it up to CCoin Clear()
144 2017-06-05 17:31:17	0|BlueMatt|or CTxOut
145 2017-06-05 17:31:31	0|sipa|script clear could be changed into prevector clear + shrink to fit
146 2017-06-05 17:31:53	0|sipa|though i think prevector clear automatocally shrinks
147 2017-06-05 17:32:00	0|BlueMatt|std::move() + clear() + shrink_to_fit()?
148 2017-06-05 17:32:03	0|BlueMatt|i do not believe so
149 2017-06-05 17:32:23	0|cfields|oh right. prevector essentially has no shrink_to_fit because it never over-allocs
150 2017-06-05 17:32:44	0|BlueMatt|ohoh, true
151 2017-06-05 17:33:40	0|BlueMatt|ehh, no, wait, i dont think so?
152 2017-06-05 17:34:49	0|BlueMatt|erase(it, it) doesnt do any shrinking, i believe
153 2017-06-05 17:37:29	0|sipa|when usimg direct allocation, the capacity is always equal to the object size
154 2017-06-05 17:37:46	0|sipa|when using indirect allocation, the capacity is variable
155 2017-06-05 17:37:53	0|BlueMatt|yes, but if its grown, clear() doesnt shrink_to_fit
156 2017-06-05 17:38:22	0|sipa|so resizing to something less than the inline capacity always has a deallocation as a result
157 2017-06-05 17:38:27	0|sipa|or at least should have
158 2017-06-05 17:38:30	0|MarcoFalke|cfields: I had the impression that the wallet-block_tip race is still a thing. Otherwise we could get rid of waitforblockheight as well
159 2017-06-05 17:38:35	0|sipa|as there is no way to represent the alternative
160 2017-06-05 17:40:16	0|cfields|MarcoFalke: yes, that's right. But we don't have any tests that rely on the other 2 things being atomic.
161 2017-06-05 17:40:16	0|sipa|i'm now questioning whether the prevector code is correct...
162 2017-06-05 17:40:28	0|cfields|sipa: i was just thinking the same. that seems leaky :\
163 2017-06-05 17:41:27	0|BlueMatt|sipa: yes, there is a bug on erase(begin(), end()), but not on clear()/resize(0)
164 2017-06-05 17:42:16	0|sipa|clear() just calls erase all the thing
165 2017-06-05 17:42:27	0|sipa|erase has no change capacity call at all
166 2017-06-05 18:16:23	0|jtimon|https://github.com/bitcoin/bitcoin/pull/10502 was created 4 days ago and laready needed rebase twice
167 2017-06-05 18:16:28	0|jtimon|already
168 2017-06-05 18:17:11	0|sipa|jtimon: thankfully, it's a scripted diff mostly, so should be easy
169 2017-06-05 18:17:25	0|jtimon|it is disruptive (+296 −300 at this point), so it is likely to require more
170 2017-06-05 18:17:46	0|jtimon|sipa: yeah, I think we can get it reviewed fast too
171 2017-06-05 18:21:11	0|jtimon|#10463 has some utACKs already and is easy to review as well
172 2017-06-05 18:21:12	0|gribble|https://github.com/bitcoin/bitcoin/issues/10463 | Names: BIP9 vs versionbits by jtimon · Pull Request #10463 · bitcoin/bitcoin · GitHub
173 2017-06-05 18:22:33	0|bitcoin-git|[13bitcoin] 15achow101 opened pull request #10533: [tests] Use cookie auth instead of rpcuser and rpcpassword (06master...06tests-use-cookie-auth) 02https://github.com/bitcoin/bitcoin/pull/10533
174 2017-06-05 19:06:54	0|BlueMatt|sipa: ah, indeed, no bug in prevector, I was misreading the meaning of _size
175 2017-06-05 19:06:54	0|jtimon|sipa: regarding https://github.com/bitcoin/bitcoin/pull/10502#issuecomment-306275178 perhaps you can help me find out what's wrong with https://github.com/bitcoin/bitcoin/pull/10193/commits/0f151d6a1b5b048ca78879e367f7a18f91126066#diff-c295474658e4ef7a9f5324199375edaf
176 2017-06-05 19:07:33	0|sipa|BlueMatt: oh, good
177 2017-06-05 19:10:45	0|jtimon|well, except for the reverse lop thing breaking the unittests...
178 2017-06-05 19:11:01	0|jtimon|loop
179 2017-06-05 19:12:39	0|sipa|ah i see
180 2017-06-05 19:13:12	0|jtimon|I haven't been able to understand what's going on there
181 2017-06-05 19:13:12	0|sipa|https://status.github.com/
182 2017-06-05 19:13:52	0|jtimon|oh, http://downforeveryoneorjustme.com/https://github.com/bitcoin/bitcoin/ said it was just me
183 2017-06-05 19:14:47	0|sipa|it's up again
184 2017-06-05 19:15:03	0|jtimon|nice, thanks
185 2017-06-05 19:18:15	0|bitcoin-git|[13bitcoin] 15sipa opened pull request #10534: Clarify prevector::erase and avoid swap-to-clear (06master...06clarify_erase) 02https://github.com/bitcoin/bitcoin/pull/10534
186 2017-06-05 19:28:29	0|cfields|jtimon: i think your expression goes out of scope after the first iteration
187 2017-06-05 19:31:11	0|jtimon|cfields: mhmm, I don't follow
188 2017-06-05 19:33:56	0|cfields|jtimon: try: https://pastebin.com/raw/USdQhE36
189 2017-06-05 19:45:24	0|spudowiar|I added some stuff to the Intro Qt form for hardware wallet support... Qt Designer made me cry :(
190 2017-06-05 19:46:40	0|jtimon|cfields: awesome, I'll have a look, thanks
191 2017-06-05 19:49:00	0|BlueMatt|sipa: would you be upset if I PRd making AccessCoin return a CoinAccessor which just asserts if you modify the coinscache while holding a reference? I know its not illegal
192 2017-06-05 19:49:15	0|BlueMatt|cause map, but I'm afraid jeremy or I will at some point suggest using a different map
193 2017-06-05 19:49:19	0|BlueMatt|and then introduce a bug
194 2017-06-05 19:49:30	0|BlueMatt|should be +/- free performance wise
195 2017-06-05 19:49:55	0|sipa|BlueMatt: that seems more something for a sanity checker
196 2017-06-05 19:50:07	0|sipa|why specifically for CCoinsViewCache and not every other map?
197 2017-06-05 19:50:57	0|BlueMatt|sipa: cause I have a strong suspicion over the next release or two someone is gonna realize std::unordered_map is not the ideal map and decide to suggest a hand-written one
198 2017-06-05 19:51:12	0|sipa|yes, so?
199 2017-06-05 19:51:29	0|BlueMatt|and then suddenly using the std sanity-check mode wont help?
200 2017-06-05 19:51:55	0|sipa|yes, so?
201 2017-06-05 19:52:05	0|sipa|i mean things like valgrind exist to find bugs like that
202 2017-06-05 19:52:39	0|sipa|i plan to work on replacing the std::unordered_map myself there, btw
203 2017-06-05 19:52:43	0|BlueMatt|cause I'd be not at all surprised if it were very difficult to tickle that issue during valgrind
204 2017-06-05 19:52:46	0|BlueMatt|lol, figured someone would
205 2017-06-05 19:52:46	0|jeremyrubin|Can also use a weak reference or something
206 2017-06-05 19:53:11	0|sipa|i don't understand why you're specifically worried about this map
207 2017-06-05 19:53:12	0|BlueMatt|jeremyrubin: that may be incompatible with a smarter map, sadly :(
208 2017-06-05 19:53:29	0|jeremyrubin|Can just add an "updates" counter to the map
209 2017-06-05 19:53:50	0|jeremyrubin|and the weak ref is invalid if counter != weakref.counter
210 2017-06-05 19:53:51	0|sipa|it's a big and important one, but hardly the only complex data structure around
211 2017-06-05 19:54:17	0|gmaxwell|BlueMatt: please rebase #10192; review #10148
212 2017-06-05 19:54:20	0|BlueMatt|sipa: well we hold references into that map in rpc functions...I'm not aware of any other map where we do anything like that?
213 2017-06-05 19:54:20	0|gribble|https://github.com/bitcoin/bitcoin/issues/10192 | Cache full script execution results in addition to signatures by TheBlueMatt · Pull Request #10192 · bitcoin/bitcoin · GitHub
214 2017-06-05 19:54:22	0|gribble|https://github.com/bitcoin/bitcoin/issues/10148 | [WIP] Use non-atomic flushing with block replay by sipa · Pull Request #10148 · bitcoin/bitcoin · GitHub
215 2017-06-05 19:54:32	0|sipa|BlueMatt: example?
216 2017-06-05 19:54:34	0|BlueMatt|gmaxwell: yesyes, I'm working on a few small things i wanted in per-utxo first
217 2017-06-05 19:54:35	0|jeremyrubin|anyways, I'm generally sympathetic to Matt's request; long held iterators to maps -> bugs
218 2017-06-05 19:54:47	0|BlueMatt|well, luckily for now we have no "long held" ones
219 2017-06-05 19:54:51	0|BlueMatt|but we do have ones in RPC
220 2017-06-05 19:54:53	0|sipa|jtimon: not long held iterators; references to elements
221 2017-06-05 19:54:58	0|sipa|eh, jeremyrubin
222 2017-06-05 19:55:04	0|sipa|iterators are encapsulated
223 2017-06-05 19:55:09	0|BlueMatt|sipa: AccessCoin is called in rpc/rawtransaction
224 2017-06-05 19:55:11	0|BlueMatt|in several places
225 2017-06-05 19:55:33	0|sipa|in many places we also hold an iterator that comes out of mapBlockIndex.find
226 2017-06-05 19:55:55	0|BlueMatt|sipa: I'm sure this comes back to your "objects are transparent holders of data" vs my "objects should be opaque, where possible"
227 2017-06-05 19:56:02	0|gmaxwell|BlueMatt: k. other people's review is blocked on that rebase.
228 2017-06-05 19:56:24	0|BlueMatt|gmaxwell: hey, you're the one who got me to focus on fibre for two weeks :p
229 2017-06-05 19:56:32	0|BlueMatt|gmaxwell: but will try to circle around today
230 2017-06-05 19:57:16	0|BlueMatt|sipa: hmm, somehow i feel better about that cause its clear its a map, unlike CCoinsViewCache, where its an encapsulated datastructure that is actually private
231 2017-06-05 19:57:47	0|sipa|BlueMatt: it does return a reference, which is always a worry
232 2017-06-05 19:57:58	0|gmaxwell|BlueMatt: I know, I know.
233 2017-06-05 19:58:09	0|jeremyrubin|BlueMatt: at a higher level, I see what you're trying to do, trying to get me to write a better map and I'm maybe falling for it
234 2017-06-05 19:58:24	0|BlueMatt|jeremyrubin: sipa already said he was working on it :p
235 2017-06-05 19:58:31	0|sipa|if you're worried about particular call sites, you can always use Coin coin = view.Accessor(outpoint) ... instead of Coin& coin = ...
236 2017-06-05 19:58:49	0|BlueMatt|sipa: our disagreement is clearly a disagreement about encapsulation
237 2017-06-05 19:59:14	0|sipa|BlueMatt: i'm just so glad i got rid of that CCoinsModifier... and now you want to introduce it again :)
238 2017-06-05 19:59:22	0|sipa|the interface got so much easier
239 2017-06-05 19:59:31	0|BlueMatt|sipa: CCoinsViewModifier *did* things, there I agree that is gross
240 2017-06-05 19:59:45	0|BlueMatt|I'm proposing something who's only value is to assert with litte/no performance impact
241 2017-06-05 20:00:03	0|BlueMatt|so as to act as a sanity checker on callsites to a function I cant say I'm sold on
242 2017-06-05 20:00:22	0|gmaxwell|BlueMatt: what sanity check will it do?
243 2017-06-05 20:00:45	0|sipa|gmaxwell: check that no references are held while a modify happens
244 2017-06-05 20:01:16	0|BlueMatt|gmaxwell: technically assert something that is currently allowed, but something I assume will be come unallowed when sipa (or someone) writes a custom map for mapCoins
245 2017-06-05 20:01:40	0|gmaxwell|that doesn't sound free to verify.
246 2017-06-05 20:02:04	0|sipa|BlueMatt: also, i don't think it's technically illegal to hold a reference to a deleted object
247 2017-06-05 20:02:05	0|BlueMatt|hmm? just a boolean/int to set when you AccessCoins and assert() on in function calls
248 2017-06-05 20:02:16	0|sipa|only using it is
249 2017-06-05 20:02:23	0|jeremyrubin|what if you have a templated thing that doesn't check that property if the type of map used is the current one ;p
250 2017-06-05 20:02:44	0|BlueMatt|gmaxwell: compared to looking up in the map it is free :)
251 2017-06-05 20:03:14	0|gmaxwell|BlueMatt: not necessarily compared to an alternative data structure (also, I doubt it's free even compared to a map)
252 2017-06-05 20:03:18	0|BlueMatt|sipa: sure, but cutting hairs, I'm asking if you're ok adding a class which is more strict than it needs to be, cause its easy to write and, imo, a useful sanity check
253 2017-06-05 20:03:19	0|ryanofsky|sipa, not directly related but #9384 also adds back a version of the modifier
254 2017-06-05 20:03:27	0|gribble|https://github.com/bitcoin/bitcoin/issues/9384 | CCoinsViewCache code cleanup & deduplication by ryanofsky · Pull Request #9384 · bitcoin/bitcoin · GitHub
255 2017-06-05 20:03:32	0|BlueMatt|gmaxwell: assert(nAccessors == 0); ?
256 2017-06-05 20:03:33	0|sipa|ryanofsky: yes... but not an exposed one
257 2017-06-05 20:03:53	0|ryanofsky|yeah
258 2017-06-05 20:03:59	0|BlueMatt|gmaxwell: that sounds like something that the cpu will run without any attached preconditions and will easily run free since you're informing the branch predictor what is unlikely
259 2017-06-05 20:04:19	0|sipa|ryanofsky: i like that 9384 reduces logic duplication, i dislike that it does not actually reduce the amount of code :)
260 2017-06-05 20:05:41	0|ryanofsky|i think that could be because i wrote it in an intentionally verbose style with lots of comments and named variables
261 2017-06-05 20:06:02	0|sipa|ryanofsky: yeah, will review
262 2017-06-05 20:06:07	0|sipa|that was just a quick comment
263 2017-06-05 20:07:01	0|sipa|BlueMatt: so, an accessor object may have another advantage at some point... if we'd want to make CCoinsViewCache have split storage for txids and CCoins
264 2017-06-05 20:07:05	0|sipa|eh, Coins
265 2017-06-05 20:07:27	0|sipa|i don't know if that's a good idea or not, but it may reduce memory usage a fair bit (or not)
266 2017-06-05 20:07:37	0|BlueMatt|you mean to hold a ptr to uint256?
267 2017-06-05 20:08:10	0|sipa|well the point is that encapsulating it may give freedom to change the representation more liberally
268 2017-06-05 20:08:14	0|BlueMatt|sipa: if you really dont want an accessor object, I'ma add a nice long comment to AccessCoin describing the preciditions...which I'm ok with, but not as much a fan
269 2017-06-05 20:08:16	0|BlueMatt|hence the q
270 2017-06-05 20:08:20	0|BlueMatt|fair
271 2017-06-05 20:08:37	0|sipa|so how about we benchmark whether it has an effect?
272 2017-06-05 20:08:46	0|BlueMatt|do we have a decent way to benchmark?
273 2017-06-05 20:09:00	0|BlueMatt|i mean could reindex
274 2017-06-05 20:09:08	0|BlueMatt|but that nearly definitely wont show it
275 2017-06-05 20:09:08	0|sipa|reindex chainstate on a -connect=0 node with fixed CPU clockrate
276 2017-06-05 20:09:12	0|sipa|works very accurately
277 2017-06-05 20:09:21	0|BlueMatt|I so dont want to do that....I can haz ssh?
278 2017-06-05 20:10:20	0|sipa|i can set things up
279 2017-06-05 20:29:38	0|BlueMatt|sipa: well, went and did it and no clear way to enforce semantics cause you can take reference and then drop the accessor too easily when writing cleints. it is nice cause it forces back to a ptr and restores a bunch of effectively-assert-cause-nullptrs that you removed in per-utxo
280 2017-06-05 20:29:48	0|BlueMatt|maybe I'll just re-add asserts and a long comment
281 2017-06-05 20:30:41	0|bitcoin-git|[13bitcoin] 15practicalswift closed pull request #10527: Use parentheses to clarify intended precedence when using bitwise operations (06master...06clarify-precedence) 02https://github.com/bitcoin/bitcoin/pull/10527
282 2017-06-05 20:35:52	0|Chris_Stewart_5|FYI: You can install bitcoind as a dependency now on travis ci, previously it had been black listed.
283 2017-06-05 20:47:05	0|jtimon|cfields: thanks a lot! with your suggestion and another small change it seems the problem is gone
284 2017-06-05 20:47:10	0|jtimon|re #10193
285 2017-06-05 20:47:12	0|gribble|https://github.com/bitcoin/bitcoin/issues/10193 | scripted-diff: Remove #include foreach.hpp> by jtimon · Pull Request #10193 · bitcoin bitcoin · GitHub
286 2017-06-05 20:47:16	0|jtimon|will update
287 2017-06-05 20:53:12	0|jtimon|done
288 2017-06-05 21:48:09	0|cfields|jtimon: great :)
289 2017-06-05 21:49:21	0|jtimon|:)
290 2017-06-05 21:50:04	0|cfields|jtimon: err, you need to use the reverser thing everywhere, not just in tests
291 2017-06-05 21:50:30	0|jtimon|maybe now it doesn't make sense for them to be separated, but maybe if I hadn't separated them I would still be stuck, so no regrets
292 2017-06-05 21:50:47	0|jtimon|oh, I see
293 2017-06-05 21:52:20	0|jtimon|I didn't run the python tests, but they should fail in that case, I'll wait for travis to fail
294 2017-06-05 21:52:43	0|jtimon|then edit the scripted-diff commit to do the same thing everywhere
295 2017-06-05 21:53:12	0|jtimon|although prevector tests will still need something different for the const reverse loop
296 2017-06-05 21:53:18	0|jtimon|or I think so
297 2017-06-05 21:54:04	0|jtimon|thanks for the heads up, I really thought it was just there because of how templates are used there
298 2017-06-05 21:55:05	0|cfields|np. no, it's because of how a range-based-for binds to the expression
299 2017-06-05 21:55:07	0|jtimon|then the new class sucks a little bit, perhaps I can do the same with auto and rbegin rend inlined
300 2017-06-05 21:55:23	0|cfields|though I can't say that I 100% understand the semantics
301 2017-06-05 21:55:24	0|jtimon|without needing a new class
302 2017-06-05 21:55:47	0|jtimon|alright, let's wait for travis
303 2017-06-05 21:56:09	0|jtimon|if it passes and you are still right maybe we can spot some missing tests
304 2017-06-05 21:56:48	0|jtimon|I can run the python tests locally too, I'll do that
305 2017-06-05 22:05:02	0|jtimon|jnewbery: what do you think about changing /tmp/bitcoin_test_runner_20170605_235927 to /tmp/bitcoin_test_runner/20170605_235927 ?
306 2017-06-05 22:06:01	0|jtimon|not a big deal, just slightly easier to remove the whole folder, kudos for the clearer prefix
307 2017-06-05 22:06:12	0|jnewbery|mildly against. Why add an extra directory layer? It just means I need to press the tab key one more time :)
308 2017-06-05 22:06:33	0|jnewbery|why easier to remove the whole folder?
309 2017-06-05 22:06:40	0|jtimon|fair enough, that was the question
310 2017-06-05 22:07:18	0|jtimon|it wasn't unlikely that it was only more convenient for me
311 2017-06-05 22:07:38	0|jtimon|the current prefix is good enough anyway
312 2017-06-05 22:09:54	0|jtimon|cfields: I don't know about travis, but this passes python3 ./test/functional/test_runner.py -j56 --extended -x pruning,smartfees,maxuploadtarget
313 2017-06-05 22:10:08	0|jnewbery|jtimon: cool. Thanks
314 2017-06-05 22:10:55	0|cfields|jtimon: I think you need a new set of tests for reverse_iterator. I don't trust it with the simple for construct :(
315 2017-06-05 22:15:06	0|jtimon|cfields: yep, more reason to simply rewrite the scripted-diff into something that uses rbegin/rend direclty instead of adding the new template
316 2017-06-05 22:15:21	0|cfields|agreed
317 2017-06-05 22:15:45	0|jtimon|but, still, if this shouldn't pass tests and it passes, perhaps we should open an issue asking for more tests
318 2017-06-05 22:17:02	0|cfields|agree with that too :)
319 2017-06-05 22:17:44	0|jtimon|I will try the version without the new class either way, it'll probably look smaller, just a more complex script to read if you use scripted-diff for review
320 2017-06-05 22:18:21	0|jtimon|but the diff should look very simple as well
321 2017-06-05 22:19:26	0|jtimon|cool
322 2017-06-05 23:26:06	0|bitcoin-git|[13bitcoin] 15MarcoFalke opened pull request #10535: [qa] fundrawtx: Fix shutdown race (06master...06Mf1706-qaFundrawRace) 02https://github.com/bitcoin/bitcoin/pull/10535
323 2017-06-05 23:29:45	0|midnightmagic|wait was there an rc that I missed a gitian for..?
324 2017-06-05 23:29:53	0|BlueMatt|14.2? maybe?
325 2017-06-05 23:30:09	0|midnightmagic|argh
326 2017-06-05 23:32:37	0|achow101|midnightmagic: might as well not build it now since an rc2 is guaranteed and the rc1 codesigned bins won't be made
327 2017-06-05 23:32:57	0|achow101|the version number wasn't bumped
328 2017-06-05 23:35:11	0|midnightmagic|achow101: I have the time, and I am one of those completionists you see sometimes who opens every crate in the game just because there might be a coupla gold in there..
329 2017-06-05 23:35:46	0|luke-jr|but there is no chance of gold in this case? :p
330 2017-06-05 23:36:02	0|midnightmagic|The gold is figurative!
331 2017-06-05 23:46:39	0|luke-jr|midnightmagic: what does it represent in this case?
332 2017-06-05 23:56:27	0|bitcoin-git|[13bitcoin] 15practicalswift opened pull request #10536: Remove unreachable or otherwise redundant code (06master...06unreachable) 02https://github.com/bitcoin/bitcoin/pull/10536
333 2017-06-05 23:56:40	0|midnightmagic|luke-jr: The reward of having opened every case. :) Also, I can say to people that I've built almost every gitian and therefore personally verified that almost every build is represented by source code as it is in the repo.