1 2016-03-09 00:48:27 0|GitHub82|[13bitcoin] 15promag opened pull request #7656: Improve EncodeBase58 performance (06master...06enhancement/speedup-encodebase58) 02https://github.com/bitcoin/bitcoin/pull/7656
2 2016-03-09 00:50:27 0|GitHub171|[13bitcoin] 15Lewuathe closed pull request #7653: Suppress unused variable warning in automated test (06master...06suppress-unused-variable-warn) 02https://github.com/bitcoin/bitcoin/pull/7653
3 2016-03-09 01:39:01 0|sdaftuar|sipa: around?
4 2016-03-09 09:22:07 0|GitHub57|[13bitcoin] 15btcdrak opened pull request #7658: Add curl to Gitian setup instrustions (06master...06curl) 02https://github.com/bitcoin/bitcoin/pull/7658
5 2016-03-09 09:27:51 0|phantomcircuit|are we using git submodule or git subtree ?
6 2016-03-09 09:35:07 0|jonasschnelli|phantomcircuit: subtree
7 2016-03-09 09:35:19 0|jonasschnelli|secp256k1, univalue, leveldb
8 2016-03-09 09:40:02 0|nsh|jgarzik, is univalue de novo code or based on another library?
9 2016-03-09 09:40:30 0|nsh|(parsers scare me from a software security perspective. i wonder if it should be fuzzed or otherwise audited)
10 2016-03-09 09:43:16 0|gmaxwell|it's denovo. it was originally vulnerable when it was merged in bitcoin core.
11 2016-03-09 09:43:36 0|gmaxwell|after the first vulnerability was found extensive fuzz testing was done.
12 2016-03-09 09:44:18 0|gmaxwell|Andytoshi implemented a functional equivilent in rust so we could do cross implementation round-trip/agreement tests, and a number of non-security bugs were found in it which haven't been fixed yet.
13 2016-03-09 09:44:29 0|nsh|ah, great
14 2016-03-09 09:44:52 0|nsh|i scanned the respository for the words 'grammar' 'lexer' and 'parser' and was disappointface but not surpriseface :)
15 2016-03-09 09:45:05 0|jonasschnelli|univalue was used <0.12 for bitcoin-tx ... and its the default json parser/encode since 0.12
16 2016-03-09 09:46:18 0|jonasschnelli|And by default I think there are no attack vectors for the JSON parser/encode...because you can't parse/encode unless you can do RPC auth or unless you enabled -rest=1
17 2016-03-09 09:46:29 0|gmaxwell|nsh: fortunately we also do not take json from untrusted inputs.
18 2016-03-09 09:46:39 0|nsh|right
19 2016-03-09 09:46:53 0|gmaxwell|(even with rest=1)
20 2016-03-09 09:47:01 0|gmaxwell|so that helps too.
21 2016-03-09 09:47:09 0|nsh|there may be some instances where people are running services that expose the RPC to unvalidated input
22 2016-03-09 09:47:32 0|nsh|even though this would be inadvisable. we can't really see how people use bitcoind in their services easily
23 2016-03-09 09:47:35 0|gmaxwell|yea, maybe but I caution against that.
24 2016-03-09 09:48:08 0|gmaxwell|I would be kind of surprised if there werent vulnerabilities there beyond the obvious.
25 2016-03-09 09:48:15 0|nsh|likewise
26 2016-03-09 09:48:28 0|gmaxwell|(obvious being that the walletexport/backup rpcs that take files can overwrite anything bitcoind can write to.)
27 2016-03-09 09:49:00 0|gmaxwell|there are almost certantly memory exhaustion attacks one can perform from the RPC.
28 2016-03-09 09:49:03 0|nsh|in principle bitcoind could be containerised to only have write access to the leveldb and config files it needs to overwrite
29 2016-03-09 09:49:12 0|nsh|but that might not be proportionate
30 2016-03-09 09:49:27 0|nsh|(and container solutions are not mature yet)
31 2016-03-09 09:49:28 0|gmaxwell|it doesn't overwrite its own config files.
32 2016-03-09 09:49:33 0|nsh|ah, my bad
33 2016-03-09 09:50:37 0|phantomcircuit|nsh, backupwallet can write anywhere
34 2016-03-09 09:51:12 0|phantomcircuit|which makes building a selinux profile mostly useless
35 2016-03-09 09:51:22 0|phantomcircuit|etc etc for containerization
36 2016-03-09 09:51:58 0|nsh|what's a sensible way forward in terms of least privilegeing [if that can be granted as a verb]
37 2016-03-09 09:52:00 0|nsh|?
38 2016-03-09 09:53:20 0|jonasschnelli|phantomcircuit: In case you want to check my first logdb implementation, check: https://github.com/libbtc/libbtc/pull/41 and feel free to feedback
39 2016-03-09 09:53:25 0|gmaxwell|phantomcircuit: warren has been working on selinux policy that just denies that external FS access, and there is an selinux bool to reenable it if you really want it to be able to access outside of its directory.
40 2016-03-09 09:53:42 0|jonasschnelli|format is explained here: https://github.com/jonasschnelli/libbtc/commit/319cce54c34f6c91a4f58d4bd4823c177f206579#diff-73f280355a901d2fc049af287aed9e90R29
41 2016-03-09 09:54:30 0|gmaxwell|phantomcircuit: ultimately I think we should add replacements for those rpcs that send the data over the rpc... and depricate the ones that write locally.
42 2016-03-09 09:55:01 0|phantomcircuit|gmaxwell, that sounds like a sound plan
43 2016-03-09 10:30:27 0|GitHub190|[13bitcoin] 15btcdrak opened pull request #7659: gitian: Add suport for deterministic armhf builds (06master...06arm) 02https://github.com/bitcoin/bitcoin/pull/7659
44 2016-03-09 15:23:23 0|GitHub137|[13bitcoin] 15MarcoFalke opened pull request #7660: [amount] Extend GetFee() by optional flag ceil (06master...06Mf1603-amountCeil) 02https://github.com/bitcoin/bitcoin/pull/7660
45 2016-03-09 15:56:27 0|instagibbs|what does this mean:
46 2016-03-09 15:56:28 0|instagibbs|The command "if [ "$RUN_TESTS" = "true" ]; then qa/pull-tester/rpc-tests.py --coverage; fi" exited with 1.
47 2016-03-09 15:57:03 0|MarcoFalke|Some issue in the rpc test suite?
48 2016-03-09 15:57:17 0|MarcoFalke|Likely the well know wallet issue?
49 2016-03-09 15:58:15 0|instagibbs|hmm, my pull is failing this for one travis build, i barely changed the test too, before it was fine. Is it likely my fault?
50 2016-03-09 15:59:40 0|MarcoFalke|Just look for the line with "AssertionError" or something like this, to see where it fails. (Or send us the link)
51 2016-03-09 15:59:56 0|warren|gmaxwell: oops, I forgot about the FS access part, only the exec bool, I'll have both.
52 2016-03-09 16:04:57 0|instagibbs|MarcoFalke, nevermind, I'll do more investigation first thanks
53 2016-03-09 16:06:25 0|instagibbs|https://travis-ci.org/bitcoin/bitcoin/jobs/114248476#L4131
54 2016-03-09 16:17:29 0|GitHub107|[13bitcoin] 15MarcoFalke opened pull request #7661: [wallet] Round up to the next satoshi on odd fee rates (06master...06Mf1603-walletCeil) 02https://github.com/bitcoin/bitcoin/pull/7661
55 2016-03-09 16:36:40 0|GitHub100|[13bitcoin] 15rat4 opened pull request #7662: remove unused NOBLKS_VERSION_{START,END} constants (06master...06patch) 02https://github.com/bitcoin/bitcoin/pull/7662
56 2016-03-09 16:50:07 0|MarcoFalke|morcos, IsStandard depends on IsDust which depends on GetFee()
57 2016-03-09 16:50:24 0|MarcoFalke|so making GetFee() do a ceil all the time would cause unwanted side effects prob.
58 2016-03-09 16:51:08 0|morcos|MarcoFalke: eh.. then make it truncate all the time
59 2016-03-09 16:51:25 0|MarcoFalke|... would introduce the regression for small fee rates
60 2016-03-09 16:51:40 0|MarcoFalke|https://github.com/bitcoin/bitcoin/pull/4465#issue-37199263
61 2016-03-09 16:51:44 0|MarcoFalke|second bullet point
62 2016-03-09 16:51:49 0|morcos|I'm not sure that really counts though, who cares if the defniition of IsDust changes microscopically. It already changed drastically from 0.11.1 to 0.11.2 and back for 0.12.0
63 2016-03-09 16:52:59 0|morcos|MarcoFalke: yeah i read that bullet point... is that even how the code works now?
64 2016-03-09 16:53:28 0|MarcoFalke|yup, I guess so: https://github.com/MarcoFalke/bitcoin/commit/1923bb13de2bb45646b3437ecfa1781df15f88e9
65 2016-03-09 16:53:43 0|MarcoFalke|if (nFeeNeeded == 0) check is still there
66 2016-03-09 16:53:58 0|morcos|yeah
67 2016-03-09 16:54:11 0|morcos|but that seems intended behavior right
68 2016-03-09 16:54:24 0|morcos|at least if you change it round (neither ceil nor trunc)
69 2016-03-09 16:55:08 0|MarcoFalke|If I `settxfee 1e-8`, I expect the wallet to go into the paytxfee-branch
70 2016-03-09 16:55:13 0|morcos|if you say you want to pay 1 sat per 1000 bytes, and you put something of 499 bytes in there. why should we intrepet that as you intending to pay 1 satoshi and not 0
71 2016-03-09 16:55:46 0|morcos|we already don't let you pay 0... so if you specify 0 another way, why is that different
72 2016-03-09 16:56:09 0|morcos|i don't really care, but i just think the CFeeRate class needs to not get more convoluted
73 2016-03-09 16:56:13 0|morcos|its already an annoying class
74 2016-03-09 16:56:22 0|MarcoFalke|In the first case you pay fallbackfee, in the other case you pay 1 sat
75 2016-03-09 16:56:30 0|MarcoFalke|It's mostly not relevant for mainnet
76 2016-03-09 16:56:32 0|morcos|but you asked to pay 0
77 2016-03-09 16:56:56 0|morcos|make it do ceiling then, i agree we need to just look through the follow on effects
78 2016-03-09 16:57:07 0|morcos|but i don't see a priori why they would be a problem
79 2016-03-09 16:57:47 0|MarcoFalke|You asked to pay 1 sat per kB and the wallet should always pay that fee rate (or slightly more)
80 2016-03-09 16:58:40 0|MarcoFalke|Ok, I had the ceil-only patch somwhere laying around this mornig but I deleted it in the meantime
81 2016-03-09 16:58:45 0|morcos|so are you ok with just making it always ceiling then?
82 2016-03-09 16:58:56 0|MarcoFalke|I will just create a pull and see what others think
83 2016-03-09 16:59:08 0|morcos|anyway, got to run, i don't like making that silly class more complicated though
84 2016-03-09 17:27:27 0|wumpus|back in NL
85 2016-03-09 17:28:39 0|sipa|congrats!
86 2016-03-09 17:29:31 0|btcdrak|wumpus: \o/ now for jetlag sleep :)
87 2016-03-09 17:33:17 0|Bootvis_|welcome back
88 2016-03-09 17:48:02 0|wumpus|thanks :)
89 2016-03-09 18:14:35 0|jonasschnelli|wumpus: welcome back, hope you had a good flight!
90 2016-03-09 18:25:03 0|sipa|i'll be back in europe on friday
91 2016-03-09 18:25:33 0|jonasschnelli|sipa: still in Boston?
92 2016-03-09 18:26:30 0|sipa|sf now
93 2016-03-09 18:27:01 0|jonasschnelli|hah. Globetrotter.
94 2016-03-09 18:28:57 0|morcos|jonasschnelli: missed you in boston, we were about to split the wallet out from Core, but decided to push it off another 5 years since you weren't there
95 2016-03-09 18:29:10 0|sipa|haha
96 2016-03-09 18:36:47 0|Luke-Jr|lol
97 2016-03-09 18:52:10 0|morcos|sipa: did you see my other comment on versionbits. about setting block version = 4?
98 2016-03-09 18:52:42 0|morcos|i think it would make sense to also include in the first version bits soft fork a requirement that block version > 4
99 2016-03-09 18:53:15 0|morcos|then the warning code can ignore blocks <= 4 because they will be invalid once the soft fork activates. this is analogous to how ISM soft forks have worked
100 2016-03-09 18:54:02 0|morcos|we should also start setting TOP_BITS immediately... yes that means that old code will be warned to upgrade potentially before any soft fork is being rolled out, but that to me only serves to be more warning time.
101 2016-03-09 18:54:43 0|morcos|if we were going to roll out verison bits code without the soft fork included, then i could see the argument against, b/c they'd get tricked into upgrading twice. but as long as they only need to upgrade that first time, its only a good thing that they are warned about it earlier
102 2016-03-09 19:22:03 0|jonasschnelli|morcos: haha!
103 2016-03-09 19:22:40 0|jonasschnelli|morcos: You and Suhas where there?
104 2016-03-09 19:25:07 0|morcos|yeah, just came for the day monday
105 2016-03-09 19:31:00 0|jonasschnelli|To bad I had to fly home on sunday.
106 2016-03-09 19:31:44 0|cfields|jonasschnelli: you didn't come home and find a new baby, i hope? :)
107 2016-03-09 19:32:04 0|jonasschnelli|cfields: Not yet. Still a couple of month to go...
108 2016-03-09 19:32:10 0|jonasschnelli|(buhh...)
109 2016-03-09 19:32:27 0|cfields|heh, good
110 2016-03-09 20:12:42 0|GitHub69|[13bitcoin] 15btcdrak closed pull request #7659: gitian: Add suport for deterministic armhf builds (06master...06arm) 02https://github.com/bitcoin/bitcoin/pull/7659
111 2016-03-09 20:40:17 0|GitHub124|13bitcoin/06master 147d2f84c 15Pavel Vasin: remove unused NOBLKS_VERSION_{START,END} constants
112 2016-03-09 20:40:17 0|GitHub124|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/87d65622996d...386f4385ab04
113 2016-03-09 20:40:18 0|GitHub124|13bitcoin/06master 14386f438 15Pieter Wuille: Merge #7662: remove unused NOBLKS_VERSION_{START,END} constants...
114 2016-03-09 20:40:27 0|GitHub8|[13bitcoin] 15sipa closed pull request #7662: remove unused NOBLKS_VERSION_{START,END} constants (06master...06patch) 02https://github.com/bitcoin/bitcoin/pull/7662
115 2016-03-09 20:43:44 0|jonasschnelli|sipa: BTW, I have written a C implementation of "logdb". If you want to rain down your criticism (which is appreciated!).
116 2016-03-09 20:43:53 0|jonasschnelli|https://github.com/libbtc/libbtc/pull/41
117 2016-03-09 20:44:23 0|jonasschnelli|It will be used as wallet data-format in the SPV implementation I'm writing.
118 2016-03-09 20:46:08 0|cfields|jonasschnelli: nice
119 2016-03-09 20:46:51 0|jonasschnelli|cfields: a design decision would be, to use your libbtcnet implementation for the network layer.
120 2016-03-09 20:47:01 0|jonasschnelli|Though, would be C++ then.
121 2016-03-09 20:47:09 0|jonasschnelli|I guess even C++11.
122 2016-03-09 20:47:23 0|cfields|jonasschnelli: It was designed so that a C api should be simple enough
123 2016-03-09 20:47:35 0|jonasschnelli|But,... I don't expect people using the net layer on a MCU device. C++ could be fine for that case...
124 2016-03-09 20:48:12 0|cfields|jonasschnelli: so you'd end up with the c++ runtime, but it wouldn't require a c++ compiler for your stuff
125 2016-03-09 20:48:58 0|jonasschnelli|cfields: Hmm.. but I guess the c++ runtime is to big for atmel cortex chips...
126 2016-03-09 20:50:13 0|cfields|jonasschnelli: unsure what spec you're dealing with, but yes, i can see how that'd be much heavier than what you're currently working with
127 2016-03-09 20:50:46 0|jonasschnelli|I just hope I made the right design choices... always unsure about C vs C++
128 2016-03-09 20:51:13 0|sipa|jonasschnelli: you're not alone :)
129 2016-03-09 20:51:20 0|sipa|libsecp256k1 started as C++, btw :p
130 2016-03-09 20:51:38 0|sipa|then C99, then C89
131 2016-03-09 20:51:39 0|cfields|jonasschnelli: well c++11 adds even more into the mix. It hugely reduces allocations
132 2016-03-09 20:51:52 0|jonasschnelli|sipa: ah. Didn't know that. But C makes certainly sense for a crypto library... not sure for a SPV library..
133 2016-03-09 20:52:39 0|jonasschnelli|But I kinda like the simpleness and portability of C
134 2016-03-09 20:52:45 0|GitHub95|13bitcoin/06master 149988554 15R E Broadley: No "Unknown command" for getaddr command.
135 2016-03-09 20:52:45 0|GitHub95|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/386f4385ab04...c8d2473e6cb0
136 2016-03-09 20:52:46 0|GitHub95|13bitcoin/06master 14c8d2473 15Pieter Wuille: Merge #7642: Avoid "Unknown command" messages when receiving getaddr on outbound câ¦...
137 2016-03-09 20:53:05 0|GitHub152|[13bitcoin] 15sipa closed pull request #7642: Avoid "Unknown command" messages when receiving getaddr on outbound c⦠(06master...06GetAddrUnknownCommand) 02https://github.com/bitcoin/bitcoin/pull/7642
138 2016-03-09 20:55:07 0|cfields|sipa: i managed to process where we were mis-aligned wrt threading models. I was far too close to the code to see what you were asking. I'll add some notes to the design doc to explain.
139 2016-03-09 20:56:20 0|sipa|cfields: please don't let my comments cause you extra work
140 2016-03-09 20:56:41 0|sipa|i think the model you used is perfect as a lowest-layer replacement for the current network code, and we should work to get it in
141 2016-03-09 20:56:44 0|cfields|sipa: tl;dr: the event loop is essentially a large wrapper around select/epoll/etc, so thread management must be done a layer above
142 2016-03-09 20:58:17 0|cfields|sipa: no problem, i just want to be able to explain the design choices, so that discussion was helpful.
143 2016-03-09 21:32:00 0|GitHub45|[13bitcoin] 15sipa opened pull request #7663: Make the generate RPC call function for non-regtest (06master...06generatenonreg) 02https://github.com/bitcoin/bitcoin/pull/7663
144 2016-03-09 21:44:45 0|gmaxwell|I'm concerned about this: https://github.com/bitcoin/bitcoin/issues/7657
145 2016-03-09 21:48:44 0|gmaxwell|My analysis on #1643 was that the change suggested there would potentially cause wallets to be more ground up into small coins; the author there did some simulation and and it least some models saw that effect. (doubling the utxo set size in test case 7, and increasing it by 40% in test case 8).
146 2016-03-09 21:48:55 0|gmaxwell|And now we have a user complaining about the effect.
147 2016-03-09 21:51:25 0|gmaxwell|I missed that we merged this.... I think immediate action may be needed to improve the situation, explosion in the utxo set size may take a long time to reverse.
148 2016-03-09 21:51:35 0|sipa|which PR changed it?
149 2016-03-09 21:51:42 0|gmaxwell|https://github.com/bitcoin/bitcoin/pull/4906
150 2016-03-09 21:52:11 0|gmaxwell|sipa: basically the old behavior didn't do what it said on the tin, the coin selection would sometimes select extra coins that it technically didn't have to select.
151 2016-03-09 21:52:18 0|gmaxwell|So the change fixed that to prune them out.
152 2016-03-09 21:54:03 0|gmaxwell|and I'm wondering if this wasn't responsible for the increase we've seen in the utxo growth rate even after filtering out some of the obvious dust-spam.
153 2016-03-09 21:54:19 0|sipa|perhaps we should make the wallet produce 2 change outputs, where one mimicks the amount sent (mimcks, not equals)
154 2016-03-09 21:55:09 0|sipa|that's for privacy, not for utxo optimization, but i expect it to lead to a better distribution of coin sizes to pick from
155 2016-03-09 22:00:34 0|gmaxwell|sipa: I've suggested a scheme before where you do two change outputs and flip a coin: either you split the change amount, OR you copy the output amount.
156 2016-03-09 22:01:22 0|gmaxwell|(though I suggested doing this only when the change would be fairly large)
157 2016-03-09 22:02:08 0|gmaxwell|in general I think we should have a scheme which does not select coins but selects 'taint groups' (maybe a taint group is just a particular scriptpubkey) and spends all the coins in it, subject to some sanity limitation.
158 2016-03-09 22:24:52 0|cfields|morcos: https://github.com/theuni/bitcoin/commit/eeda3643766b2110ecf6596d7fb24718412a043a
159 2016-03-09 23:17:09 0|cfields|sipa: re our aes impl. Would you prefer to remove the tables before merge? Or ok to do it as a next step?
160 2016-03-09 23:17:52 0|cfields|(assuming it wouldn't be worse than what we currently have with openssl)
161 2016-03-09 23:23:02 0|cfields|sipa: by which i mean "would you prefer that i remove the tables" ofc :)
162 2016-03-09 23:24:15 0|sipa|cfields: i'm now thinking about how you would implement all of the AES steps without tables
163 2016-03-09 23:25:07 0|gmaxwell|AES is notoriously painful to do both sidechannel free and fast.
164 2016-03-09 23:25:19 0|gmaxwell|The tables could be converted into CMOV oblivious lookups, for a big speed hit.
165 2016-03-09 23:26:28 0|cfields|sipa: ah, our discussion had me believe that there was a standard alternate approach
166 2016-03-09 23:27:00 0|sipa|cfields: hmm, no; i think you'll find plenty of academic work for fast & sidechannel free AES, but nothing standard
167 2016-03-09 23:27:15 0|sipa|*but* i think AES is trivial to do slow & sidechannel free
168 2016-03-09 23:27:20 0|sipa|well, constant-time
169 2016-03-09 23:29:16 0|sipa|hmm, the S-box is nontrivial
170 2016-03-09 23:39:44 0|sipa|i guess you'll need a cmov for the sbox
171 2016-03-09 23:40:13 0|sipa|everything else can be done algebraically... the first thing besides the S-box is a GF(2^8) multiplication by 3
172 2016-03-09 23:40:37 0|sipa|s/first thing/worst thing/
173 2016-03-09 23:42:46 0|morcos|gmaxwell: re: #4906, i flagged that shortly after it got merged echoing your concerns. wumpus asked us if we should revert it and you and i both told him no.
174 2016-03-09 23:43:38 0|gmaxwell|lol
175 2016-03-09 23:43:43 0|morcos|gmaxwell: i think its unlikely that its had much of an effect on utxo set already, b/c its only been included since 0.12 right? but this is an example of something that we'll all forget about revisiting in time for 0.13 if we don't put it on a list somewhere
176 2016-03-09 23:44:08 0|gmaxwell|morcos: it's been merged for a while, so it may be the case that people running pre-releases have had an effect.
177 2016-03-09 23:44:26 0|gmaxwell|certantly if it is the cause of this user's complaint it's a cause for concern.
178 2016-03-09 23:44:40 0|gmaxwell|But I think we wouldn't revert regardless, we would move forward.
179 2016-03-09 23:45:47 0|morcos|Should we maintain an issue that includes goals for 0.13, or open a new issue and tag it with milestone 0.13 or something.. it'd be nice if we were more organized about this stuff
180 2016-03-09 23:46:36 0|gmaxwell|I think perhaps one reason I also might have said to not revert it was a belief that we'd move forward and further fix the issue before the release but then it fell off the radar. A milestoned issue would probably be fine.
181 2016-03-09 23:53:27 0|morcos|ttps://github.com/bitcoin/bitcoin/issues/7664
182 2016-03-09 23:53:30 0|morcos|oops