1 2015-05-11 00:01:32 <andytoshi> hulkhogan_: you should be compiling with `-W -Wall -Wextra -pedantic` and fixing all the warnings
  2 2015-05-11 00:02:05 <andytoshi> and depending what you're doing and how far you're into it, rust might be a better choice .. one sec, i'll expose that function in rust-secp256k1
  3 2015-05-11 00:03:06 <hulkhogan_> sorry for the noise guys, was all my code there
  4 2015-05-11 00:03:52 <andytoshi> hulkhogan_: it's cool, thx for testing. in future please run `make check` on any pull requests as a sanity check before submitting them tho
  5 2015-05-11 00:04:20 <hulkhogan_> andytoshi: yea, i will do that from now on, thanks for the headsup
  6 2015-05-11 00:08:59 <hulkhogan_> andytoshi: i might want to do that( rust), is there a sensible way to call rust code from java (bindings)?
  7 2015-05-11 00:09:37 <andytoshi> hulkhogan_: if you can, connect to #rust on irc.mozilla.org and ask ... the answer is yes but idk how
  8 2015-05-11 00:10:02 <andytoshi> hulkhogan_: rust has no runtime, it's just as low-level as C, so you can tell the compiler to output the same style function interface
  9 2015-05-11 00:13:11 <hulkhogan_> interesting, it seems its possible using JNA, which i ought to be using anyway to avoid these low level errors
 10 2015-05-11 00:13:58 <hulkhogan_> it would make expanding the interface easier too
 11 2015-05-11 00:21:56 <andytoshi> hulkhogan_: let me know (probably on #rust is better, i am apoelstra there) if you are using rust and have any trouble with the lang or with the libsecp256k1 bindings
 12 2015-05-11 00:23:45 <hulkhogan_> andytoshi: yea will do, trying to sort out how to deal with the java bindings, couple of options but JNA seems the way to go...
 13 2015-05-11 00:24:06 <hulkhogan_> it requires a bit of a refactor, so i am just deciding a bit
 14 2015-05-11 00:24:48 <andytoshi> note that using rust-secp256k1 vs libsecp256k1 will also require a refactor since the rust-secp256k1 API is much better-typed
 15 2015-05-11 00:25:32 <hulkhogan_> see, the main thing with the java bindings are that they live inside the repo now
 16 2015-05-11 00:25:48 <hulkhogan_> pulling in your lib i'm not opposed to, but thinking about the build process
 17 2015-05-11 00:26:08 <hulkhogan_> it would be jna + your lib + dependencies from both
 18 2015-05-11 00:26:21 <hulkhogan_> pretty big just to keep it upstream
 19 2015-05-11 00:28:49 <andytoshi> you def wanna ask on #rust, doing manual dependency management in rust is not something i've ever heard of someone doing
 20 2015-05-11 00:29:55 <hulkhogan_> just a sec hehe, before we go down that path, travis works with rust pretty well right?
 21 2015-05-11 00:30:03 <hulkhogan_> well
 22 2015-05-11 00:30:15 <hulkhogan_> yea its OT, ill ask on rust then, thanks
 23 2015-05-11 00:30:16 <hulkhogan_> yea its OT, ill ask on rust then, thanks
 24 2015-05-11 00:31:11 <andytoshi> yes, travis works with rust
 25 2015-05-11 00:33:49 <andytoshi> sipa: gmaxwell: how do you feel about changing the secp256k1_ec_pubkey_decompress() API to take separate in and out pointers?
 26 2015-05-11 00:34:42 <gmaxwell> andytoshi: I think that isn't the normal C norm for functions that work in-place. the issue is that most callers would end up with two 'sizes' for the same buffer.
 27 2015-05-11 00:34:53 <gmaxwell> (it's not always clear when arguments to a function can alias!)
 28 2015-05-11 00:35:13 <andytoshi> yeah, we'd need to explicitly say its OK because i think the answer is almost always no
 29 2015-05-11 00:35:17 <andytoshi> except if both ptrs were const
 30 2015-05-11 00:35:35 <andytoshi> that's fair, i agree it'd be weird for C users
 31 2015-05-11 00:36:20 <gmaxwell> fortunately the failure if you get this wrong is reliable.. it'll fail every time unless 33 or 65 is mapped (and returns 33 or 65) or you have no mmu. :)
 32 2015-05-11 00:37:22 <andytoshi> oh, i meant for the pubkey, the length pointer is fine to be in/out
 33 2015-05-11 00:37:45 <gmaxwell> ohhh
 34 2015-05-11 00:37:50 <gmaxwell> you mean making it not 'in place.
 35 2015-05-11 00:37:55 <andytoshi> yeah
 36 2015-05-11 00:38:59 <gmaxwell> yea, in place functions kinda suck. Though for this application where the data is fixed maximum... it's a good fit, and as observed no one would use it in place if it had in/out since even if we document that they may alias people won't look or believe it.
 37 2015-05-11 00:39:14 <andytoshi> yeah, understood
 38 2015-05-11 00:39:23 <gmaxwell> would it help the rust stuff if it could run out of place?
 39 2015-05-11 00:39:43 <andytoshi> that's cool, i will do something evil on the rust end of things ... the problem is that in my code compressed/uncompressed are different enum variants and there is no way to directly fiddle with the discriminate to change them
 40 2015-05-11 00:40:01 <andytoshi> (rust enums work like unions in C)
 41 2015-05-11 00:40:28 <gmaxwell> I withhold opinion on out of place. Reservations about no one ever using it in place aside; it's a more flexible formulation.
 42 2015-05-11 00:41:06 <andytoshi> ok .. i also withhold opinion because i'm badly divided on it; i agree that nobody would believe that aliasing is ok and i think this is a problem
 43 2015-05-11 00:41:17 <andytoshi> but it would make my life much easier when binding :P
 44 2015-05-11 00:41:28 <andytoshi> which i don't feel is a good reason for changing the C API, at least in this case..
 45 2015-05-11 00:42:41 <gmaxwell> well we could expose two interfaces. e.g. the one parameter one just calls the other.
 46 2015-05-11 00:43:14 <gmaxwell> {return secp256k1_ec_pubkey_decompress_split(ctx,pubkey,pubkey,pubkeylen);}
 47 2015-05-11 00:43:32 <gmaxwell> I don't mind another interface when its as simple as that!
 48 2015-05-11 00:43:57 <andytoshi> ok, cool, i will make a PR as soon as i think of a name .. i dont think that _split sonuds right
 49 2015-05-11 00:44:15 <gmaxwell> no I think it doesn't sound good.
 50 2015-05-11 00:44:42 <gmaxwell> make sure you check to see if there are any other inplace public api functions that need the same transformation... though thats the only one thats coming to mind.
 51 2015-05-11 00:45:13 <gmaxwell> (and change some of the tests to use one vs the other)
 52 2015-05-11 00:45:32 <andytoshi> will do
 53 2015-05-11 00:46:30 <hulkhogan_> i made a comment on the pr i started, i'd like to make the right choice on how to do things so i left it as an open question for now
 54 2015-05-11 00:49:13 <hulkhogan_> writing the jni code was scary as-is, so having a way to abstract the details would be a plus, for implementing
 55 2015-05-11 02:29:19 <hulkhogan_> this is really strange, i'm getting an unaligned read segfault and i really do think i'm using the library correctly this time
 56 2015-05-11 02:35:03 <andytoshi> hulkhogan_: can you post code?
 57 2015-05-11 02:35:31 <hulkhogan_> yea
 58 2015-05-11 02:37:20 <hulkhogan_> pastie.org/pastes/10181897/text
 59 2015-05-11 02:37:39 <hulkhogan_> if i uncomment that line into secp256k1, i get the segfault under it
 60 2015-05-11 02:38:41 <hulkhogan_> im stumped because it looks like its happening in libc
 61 2015-05-11 02:39:40 <hulkhogan_> it possibly could be JNI-related, but i haven't gotten that far yet
 62 2015-05-11 02:42:03 <hulkhogan_> hm, doubt it actually, re-reading the trace suggests its a native code fault (another user error probably)
 63 2015-05-11 02:43:58 <hulkhogan_> (if i've made another user error, i'm taking the night off :P)
 64 2015-05-11 02:44:09 <hulkhogan_> thanks btw, for looking
 65 2015-05-11 02:57:41 <andytoshi> sorry, i don't know the JNI stuff well enough to know what to do about this .. i'd guess, just run it in gdb and see what it says
 66 2015-05-11 02:58:44 <hulkhogan_> no problem, thanks for looking, i'm going to have a fresh start in the morning, i think ill need to go a bit deeper to figure out whats wrong, thanks for your help!
 67 2015-05-11 03:37:44 <fanquake> ;;blocks
 68 2015-05-11 03:37:46 <gribble> 355862
 69 2015-05-11 08:22:03 <wumpus> a node on an ARM box crashed with a block corruption issue again "2015-05-10 21:52:41 ERROR: ReadBlockFromDisk: Deserialize or I/O error - ReadCompactSize(): size too large at CBlockDiskPos(nFile=127, nPos=15626348)", but I'm fairly sure this is caused by hardware or kernel issues - 45 bitflips in block 292699, nothing in surrounding areas. Not the overlap issue reported (and hopefully fixed) before.
 70 2015-05-11 08:23:13 <gmaxwell> hm.
 71 2015-05-11 08:23:55 <wumpus> looks like random bitflip spraying too, seemingly random distribution and each corrupted byte had only one flipped bit
 72 2015-05-11 08:24:21 <gmaxwell> wumpus: error in an address line perhaps.
 73 2015-05-11 08:24:31 <gmaxwell> causing it to read a whole incorrect dram row?
 74 2015-05-11 08:25:40 <wumpus> it's possible, though apart from that the system works stable, so it's at most intermittent (maybe caused by overheating heh..)
 75 2015-05-11 08:26:18 <wumpus> I've dropped on the original block in the right place, restarted the node
 76 2015-05-11 08:26:33 <wumpus> let's see what happens
 77 2015-05-11 08:44:59 <phantomcircuit> gmaxwell, i wonder how much of the issues on arm are just from arm chips being unstable in general
 78 2015-05-11 08:45:15 <phantomcircuit> and bitcoin being one of the only things that consistently catches virtually all errors
 79 2015-05-11 08:48:46 <wumpus> a cross-section of all consumer hardware is unstable in general, nothing ARM specific here
 80 2015-05-11 08:49:28 <gmaxwell> a lot of 'small' hardware is under cooled or has overly weak psus.
 81 2015-05-11 08:49:29 <wumpus> you are right though in that bitcoin core is very effective in bringing to light these kinds of issues
 82 2015-05-11 09:05:15 <wumpus> I want to do a 0.10.2 mini-release with backported fix for https://github.com/bitcoin/bitcoin/issues/6078 (regression in 0.10.1 re: for some windows users), anothing else that needs to be backported?
 83 2015-05-11 09:06:02 <wumpus> also has #6114
 84 2015-05-11 09:16:12 <gmaxwell> oh there was that thing where we fail on start with gen=1 and no initial blockchain
 85 2015-05-11 09:16:21 <gmaxwell> (hits people who copy config files from the internets)
 86 2015-05-11 09:20:16 <wumpus> but that isn't merged into master either yet
 87 2015-05-11 09:21:02 <wumpus> but arguably a simple change, yes
 88 2015-05-11 09:21:29 <gmaxwell> just trying to think about regressions people complained about.
 89 2015-05-11 09:22:32 <wumpus> I'm not sure I like having CreateNewBlock depend upon Checkpoints::GetTotalBlocksEstimate() though
 90 2015-05-11 09:22:33 <wumpus> I'm not sure I like having CreateNewBlock depend upon Checkpoints::GetTotalBlocksEstimate() though
 91 2015-05-11 09:25:21 <gmaxwell> oh did I really do that? :(
 92 2015-05-11 09:25:25 <gmaxwell> bleh.
 93 2015-05-11 09:25:34 <wumpus> the whole functionality around mining and checkpoints is messy; but that flow is kind of tricky, that's why I pointed to luke-jr's #5987 which attempts to solve a similar problem regarding mining and checkpoints
 94 2015-05-11 09:26:03 <wumpus> it's also an ugly fix
 95 2015-05-11 09:26:04 <gmaxwell> I could drop that part out; it's basically fixing the issue two different ways at once;  I was worried that if I didn't avoid the issue in both a later RPC change would reintroduce the issue.
 96 2015-05-11 09:26:28 <wumpus> he  removes the checkpoint for testnet in the process
 97 2015-05-11 09:27:15 <wumpus> yes, getblocktemplate RPC also has a check for IsInitialBlockDownload() and refuses to return a block to mine if that evaluates to true
 98 2015-05-11 09:28:27 <wumpus> getblocktemplate, on othe other hand reports an "out of memory" error when CreateNewBlock returns NULL
 99 2015-05-11 09:29:13 <wumpus> I agree that a ugly-but-working fix for 0.10 may be a good idea
100 2015-05-11 09:29:40 <gmaxwell> sorry I've been super tied up and hadn't cycled back to it. :(
101 2015-05-11 09:29:41 <gmaxwell> sorry I've been super tied up and hadn't cycled back to it. :(
102 2015-05-11 09:30:36 <gmaxwell> the easier 'workaround' is to rename and hide gen in 0.10.
103 2015-05-11 09:30:55 <wumpus> there's a "Busy-wait for the network to come online so we don't waste time mining on an obsolete chain." check at the top of BitcoinMiner, what about adding a IsInitialDownload check there too?
104 2015-05-11 09:31:18 <gmaxwell> I thought I did?  lemme go find that tree.
105 2015-05-11 09:31:38 <wumpus> ohh maybe you did and I got distracted by the deeper change in CreateNewBlock
106 2015-05-11 09:32:06 <wumpus> yes, you actually did
107 2015-05-11 09:32:09 <phantomcircuit> wumpus, btw that behavior is actually pretty annoying
108 2015-05-11 09:32:20 <phantomcircuit> in general you want mining hardware never to stop running
109 2015-05-11 09:32:25 <gmaxwell> yea, I changed it in both places; I feel like it's wrong to leave CreateNewBlock in a state where it will malfunction like that; and chasing around the codebase to make sure there is always a IsInitialDownload check seems wrong. BUT; it seemed right to also check in that loop.
110 2015-05-11 09:32:29 <wumpus> phantomcircuit: this has nothing to do with mining hardware
111 2015-05-11 09:32:30 <phantomcircuit> even if it spends some time working on useless nonsense
112 2015-05-11 09:32:37 <phantomcircuit> <wumpus> yes, getblocktemplate RPC also has a check for IsInitialBlockDownload() and refuses to return a block to mine if that evaluates to true
113 2015-05-11 09:32:40 <phantomcircuit> was talking about that
114 2015-05-11 09:32:41 <phantomcircuit> was talking about that
115 2015-05-11 09:33:01 <gmaxwell> phantomcircuit: it will _crash_ right now if it's not past the highest checkpoint when it creates a block template.
116 2015-05-11 09:33:09 <phantomcircuit> (and yeah i know you can give the hw random stuff but various buts of software on top of it will refuse to do that)
117 2015-05-11 09:33:14 <gmaxwell> because it tests if it would accept the block and it won't.
118 2015-05-11 09:33:17 <phantomcircuit> gmaxwell, oh really? ha
119 2015-05-11 09:33:38 <gmaxwell> it's prudent to shut down if it self-detects its creating blocks it wouldn't accept.
120 2015-05-11 09:33:52 <phantomcircuit> yeah
121 2015-05-11 09:34:14 <wumpus> phantomcircuit: I agree though that anything that relies on build-in checkpoints is annoying and creates all kinds of nasty edge cases
122 2015-05-11 09:34:19 <gmaxwell> so yea, that interacts poorly with syncing a new node and having gen=1 set.. it start trying to mine at block 10ish and shuts down right away.
123 2015-05-11 09:35:00 <jonasschnelli> wumpus: 0.10 branch misses: https://github.com/bitcoin/bitcoin/issues/6078.
124 2015-05-11 09:35:26 <wumpus> jonasschnelli: huh?
125 2015-05-11 09:35:47 <jonasschnelli> https://github.com/bitcoin/bitcoin/commits/0.10
126 2015-05-11 09:35:54 <wumpus> 424ae66 right?
127 2015-05-11 09:36:24 <wumpus> oh I see I hadn't pushed
128 2015-05-11 09:36:30 <jonasschnelli> it's not squashed, so it's probably two commits.
129 2015-05-11 09:36:37 <jonasschnelli> Should i squash first?
130 2015-05-11 09:36:46 <wumpus> no, check again
131 2015-05-11 09:38:20 <jonasschnelli> it's okay, 424ae66 is the squashed version of b3ffcdf and 3da7849.
132 2015-05-11 09:38:24 <wumpus> yes
133 2015-05-11 09:51:51 <wumpus> gmaxwell: what I don't get is why the entire program crashes when TestBlockValidity in CreateNewBlock fails
134 2015-05-11 09:52:09 <wumpus> that seems to be the real issue to me
135 2015-05-11 09:53:11 <wumpus> going to try to reproduce it
136 2015-05-11 09:55:54 <wumpus> (CreateNewBlock could fail on new blocks for other reason, e.g. clock problems, so we should try to actually handle its errors)
137 2015-05-11 09:56:32 <wumpus> eh, TestBlockValidity
138 2015-05-11 10:03:20 <wumpus> ok: the problem is that the internal miner doesn't catch the std::runtime error that can be raised by CreateNewBlock
139 2015-05-11 10:03:47 <wumpus> from RPC this is not an issue, RPC framework catches them in the outer error handler
140 2015-05-11 10:03:48 <wumpus> from RPC this is not an issue, RPC framework catches them in the outer error handler
141 2015-05-11 10:11:15 <wumpus> commented on #6079...
142 2015-05-11 12:09:52 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come. Click http://www.bukolay.com I Come. Click http://www.bukolay.com I Come.

143 2015-05-11 12:09:55 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come. Click http://www.bukolay.com I Come. Click http://www.bukolay.com I Come.

144 2015-05-11 12:10:57 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

145 2015-05-11 12:10:59 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

146 2015-05-11 12:11:02 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

147 2015-05-11 12:11:05 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

148 2015-05-11 12:11:07 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

149 2015-05-11 12:11:09 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

150 2015-05-11 12:11:10 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

151 2015-05-11 12:11:12 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

152 2015-05-11 12:11:14 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

153 2015-05-11 12:11:16 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

154 2015-05-11 12:11:19 <wwwBUKOLAYcom> 
ACTION Click http://www.bukolay.com I Come.

155 2015-05-11 12:12:40 <rabidus> :|
156 2015-05-11 12:44:08 <wumpus> wtf
157 2015-05-11 12:45:19 <jonasschnelli> I got some Abuse-Messages from my Datacenter-Operator... is that possible that my bitcoind node can receive (getaddr) private IPs (or other invalid ranges) and try connect to tem?
158 2015-05-11 12:45:24 <jonasschnelli> tem/them
159 2015-05-11 12:45:45 <jonasschnelli> but i guess its sipas dnsseeder
160 2015-05-11 12:46:55 <wumpus> what abuse do they complain about?
161 2015-05-11 12:48:20 <jonasschnelli> wumpus: NetscanOutLevel: Netscan detected from <ip>
162 2015-05-11 12:48:41 <jonasschnelli> things like: 176.9.45.239 41107 =>    9.45.203.212 9333
163 2015-05-11 12:53:16 <jonasschnelli> i assume i have to implement something like "bool CNetAddr::IsRoutable() const" for bitcoin-seeder
164 2015-05-11 12:58:36 <kinlo> those 2 ip's don't look like private ips
165 2015-05-11 12:59:14 <kinlo> seems like you just have an over-eager datacenter operator that doesn't fully understands it's own tools and logs
166 2015-05-11 13:03:44 <jonasschnelli> kinlo: seems like! Have to call them
167 2015-05-11 13:14:25 <wumpus> they see your dnsseeder's behavior as 'netscanning', which it is in a way, as it opens connections to see if a node is listening. Except that these nodes are *asking* to be connected to by advertising. So yes, over-eager detection...
168 2015-05-11 13:17:03 <jonasschnelli> hmm... bitcoin-seeder already checks against server IPv4 RFCs (IsRoutable()). Sent some emails to my op, let's see what they think.
169 2015-05-11 13:17:13 <jonasschnelli> s/server/serval
170 2015-05-11 14:10:43 <jonasschnelli> wumpus: https://github.com/bitcoin/bitcoin/pull/6058 is now squashed
171 2015-05-11 14:19:46 <jonasschnelli> FYI: my server op took back the network abuse report. He accepted bitcoin-seeders scanning behavior.
172 2015-05-11 14:21:25 <morcos> wumpus: are you still feature freezing 0.11 this week?
173 2015-05-11 14:22:00 <morcos> if there is anyone that wants to review #5159, i think it would be helpful to get that in for 0.11, so much of this discussion around blocksize revolves around estimating fees to be confirmed
174 2015-05-11 14:22:35 <morcos> i realize its pretty dense hard to read code, but it gives much better estimates than the existing code, and i think would be useful to get rolled out
175 2015-05-11 14:28:32 <wumpus> morcos: yes, that's still the plan
176 2015-05-11 14:31:35 <wumpus> jonasschnelli: thanks
177 2015-05-11 14:40:46 <wumpus> morcos: agree that it makes sense to merge the better fee estimation
178 2015-05-11 14:44:29 <Croves> So, I'm reading Bitcoin Developers Guide.. let's see if I get it right: If I'm going to build a bitcoin exchange, I need to generate a wallet address for each user in my website, right?
179 2015-05-11 14:45:24 <Croves> And, for example, if I have a bitcoin loan website, I need 3 wallet address: one for the user who's going to deposit the bitcoins, other for the user who's  going to withdraw this bitcoins and one for my website for charging fees?
180 2015-05-11 14:48:03 <akstunt600> sure, but you will probably want to use getnewaddress or something because you will want the user to be able to generate new addresses.
181 2015-05-11 15:10:36 <jonasschnelli> wumpus: i think it would be great to have https://github.com/bitcoin/bitcoin/pull/6110 in 0.11
182 2015-05-11 15:10:59 <jonasschnelli> 0.11 will introduce new icons and the coloring on windows looks bad
183 2015-05-11 15:11:21 <jonasschnelli> At least the last of the 4 commits from 6110 should go in.
184 2015-05-11 15:13:05 <jonasschnelli> I also try to do some pixel perfection regarding the new icons before this Friday.
185 2015-05-11 15:16:20 <wumpus> jonasschnelli: code changes look harmless enough at least :)
186 2015-05-11 15:18:41 <jonasschnelli> wumpus: let's hope. :-)
187 2015-05-11 16:36:13 <StephenM347> Is it possible to manually add peers on the regtest network?
188 2015-05-11 16:37:23 <StephenM347> (for testing applications where you want network connectivity and control over whether what's in the mempool.)
189 2015-05-11 17:06:41 <nickler> Speaking of testing... I am creating a set of scripts that trigger edge cases using fuzzing. Maybe that could be interesting to bitcoin-dev as well. https://github.com/jonasnick/bitcoinconsensus_testcases
190 2015-05-11 17:23:17 <Luke-Jr> 
ACTION wonders how long posts to Bitcoin-Development take these days

191 2015-05-11 17:37:35 <rnicoll> evening all
192 2015-05-11 17:41:01 <cfields> jonasschnelli: i hope you didn't redo the univalue stuff yourself? I fixed it up the same way a few weeks ago
193 2015-05-11 18:09:49 <jonasschnelli> cfields: whaaat? I was starting on top of jgarziks PR. Where are your changes?
194 2015-05-11 18:10:38 <cfields> jonasschnelli: i pushed em a few weeks ago. jgarzik said he'd take a look and i moved on to something else. Didn't realize you were looking at it too :(
195 2015-05-11 18:10:51 <cfields> from what i can tell, i like your changes better though
196 2015-05-11 18:11:07 <jonasschnelli> You open a PR on the main repo?
197 2015-05-11 18:11:19 <jonasschnelli> *You did open
198 2015-05-11 18:11:23 <cfields> jonasschnelli: https://github.com/theuni/bitcoin/tree/univalue
199 2015-05-11 18:11:48 <jonasschnelli> (phone) will check later. Funny. :-)
200 2015-05-11 18:11:58 <cfields> jonasschnelli: no, i was waiting for jgarzik to have a look, wasn't sure if he'd want to pull em into his pr or start fresh with a new one
201 2015-05-11 18:12:24 <jonasschnelli> While working on the new wallets RPC I stumbled over UniValue because JSON Spirit sucks.
202 2015-05-11 18:12:33 <cfields> heh
203 2015-05-11 18:13:03 <cfields> jonasschnelli: in particular, you fixed up the object init problem correctly. i added to the wrapper hack: https://github.com/theuni/bitcoin/commit/ef377057f3310c43fe2417dad1405e3c420e29ff
204 2015-05-11 18:13:24 <jonasschnelli> However. This should go in once. But is like to have a more/full coverage of all RPC Commands in Unittests.
205 2015-05-11 18:14:11 <jonasschnelli> Yeah. You commit looks good. I decided to go with search/replace :-)
206 2015-05-11 18:14:43 <cfields> yea, that's the correct approach for sure. Mine was just the easy hammer-fix to get the tests passing
207 2015-05-11 18:20:14 <jonasschnelli> cfields: it's really cool to see, that you added more or less the same things. Like the VREAL and isReal(). :)
208 2015-05-11 18:20:47 <jonasschnelli> but you did some changes to the Univaue::read() function...
209 2015-05-11 18:21:04 <cfields> jonasschnelli: heh, indeed. the main difference is that i taught read() how to understand an individual value...
210 2015-05-11 18:21:05 <cfields> jonasschnelli: heh, indeed. the main difference is that i taught read() how to understand an individual value...
211 2015-05-11 18:21:08 <jonasschnelli> cfields: did you change the behavior so it parses things like "string" and "null"?
212 2015-05-11 18:21:43 <cfields> yea. looking at yours now, my approach really doesn't make any sense
213 2015-05-11 18:22:14 <jonasschnelli> i was going the easy/lazy way (https://github.com/jonasschnelli/bitcoin/commit/a825589344a5b12f60930f8450860f3e8d44838c)
214 2015-05-11 18:22:37 <jonasschnelli> i kinda like that UniCode is strict in JSON parsing
215 2015-05-11 18:22:38 <jonasschnelli> i kinda like that UniCode is strict in JSON parsing
216 2015-05-11 18:23:01 <jonasschnelli> makes the interface more predictable
217 2015-05-11 18:23:15 <cfields> yep, i agree
218 2015-05-11 18:23:26 <cfields> iirc there was one actual bug there though, sec
219 2015-05-11 18:24:06 <cfields> https://github.com/theuni/bitcoin/commit/849b5e339d1d8eecae9b5a5e834a17c1a1f22120
220 2015-05-11 18:24:12 <cfields> did that not come up for you?
221 2015-05-11 18:24:28 <jonasschnelli> cfields: did you implementation succeeded all tests?
222 2015-05-11 18:24:37 <cfields> yes
223 2015-05-11 18:25:05 <jonasschnelli> no, never had to deal with L260+ in univalue_read.cpp
224 2015-05-11 18:25:38 <cfields> ok. probably had to do with me abusing read() to parse values
225 2015-05-11 18:26:25 <jonasschnelli> i think i solved this by that: https://github.com/jonasschnelli/bitcoin/commit/5079957ba5ef957e0976132ed97ddd2b2dc7a6f6#diff-6acaa876c8e0335a97282193c4cc0428R149
226 2015-05-11 18:26:53 <jonasschnelli> you solution to this could be more clean... have to doublecheck soon.
227 2015-05-11 18:27:23 <cfields> ah, maybe
228 2015-05-11 18:28:21 <cfields> i also kept precision the same everywhere so that the old tests passed. I wasn't sure which behaviors were necessary to keep, so i opted for 1:1
229 2015-05-11 18:28:22 <cfields> i also kept precision the same everywhere so that the old tests passed. I wasn't sure which behaviors were necessary to keep, so i opted for 1:1
230 2015-05-11 18:29:10 <jonasschnelli> cfields: but how did you manage to pass tests with json output like "1.1" and still having the JSON RPC API compatible?
231 2015-05-11 18:32:08 <cfields> jonasschnelli: iirc it's the difference between getValueStr() and get_real(). one truncates zeroes, one is fixed to 8 places
232 2015-05-11 18:32:46 <jonasschnelli> ah. Okay. I didn' fiddle with getValueStr(). But makes sense.
233 2015-05-11 18:32:47 <cfields> it was very subtle. I don't recall the exact details now, i'd have to take a closer look
234 2015-05-11 18:33:10 <cfields> again, i'm not sure that it's worth trying to maintain that old behavior
235 2015-05-11 18:33:43 <jonasschnelli> Yeah. And sorry interfere your work. Didn't knew that anyone else is interested in that. :)
236 2015-05-11 18:34:43 <jonasschnelli> If you had mentioned it in https://github.com/bitcoin/bitcoin/pull/4738 ... that probably would had made me not do it.
237 2015-05-11 18:34:45 <cfields> np, probably a good thing to have it done twice separately, really. good for comparison
238 2015-05-11 18:35:05 <jonasschnelli> Now we have a benchmark. :)
239 2015-05-11 18:35:26 <cfields> yea, my fault. I pinged jgarzik on irc and he said he'd take a look. Should've done it on the PR instead.
240 2015-05-11 18:36:18 <jonasschnelli> How ever i think the univalue transition is something for 0.12. 0.11 is probably to late.
241 2015-05-11 18:36:43 <cfields> yep. wumpus already nixed it for 0.11
242 2015-05-11 18:37:20 <jonasschnelli> / laanwj modified the milestone: 0.12.0, 0.11.0 10 days ago //
243 2015-05-11 18:37:34 <jonasschnelli> Looks like. :)
244 2015-05-11 18:39:06 <cfields> is the univalue wrapper still necessary? I'd think with your fixup of the objects/arrays, that shouldn't be needed?
245 2015-05-11 18:39:34 <cfields> unless there are still valid Value objects around?
246 2015-05-11 18:45:38 <jonasschnelli> cfields: i think i accidentally left some Object/Array around. But right, i need to get rid of them and the wrapper. This would be the right thing IMO.
247 2015-05-11 18:46:06 <cfields> jonasschnelli: no big deal, i only mentioned it because i figured it'd be really close after all of those changes
248 2015-05-11 18:47:01 <jonasschnelli> right. I first thought to do a slow transition. In the end it came out that its more or less a complete replacement.
249 2015-05-11 18:47:18 <jonasschnelli> (which i should complete by removing all json spirit zombies)
250 2015-05-11 18:48:35 <cfields> well if you'd prefer the other approach, you can take my inheritance change and avoid the huge replacement. But I still think your approach is the nicer one.
251 2015-05-11 18:49:54 <jonasschnelli> The inheritance change is for lowering the diff size and get quicker review IMO. If we have three people working on it or reviewing it (you, jgarzik and me) we can do the whole replacement IMO.
252 2015-05-11 18:50:11 <cfields> yep
253 2015-05-11 18:57:33 <jonasschnelli> cfields: You did add REAL type to the parser as well (JTOK_REAL). I was trying to avoid this because http://en.wikipedia.org/wiki/JSON#Data_types.2C_syntax_and_example said that there are only "NUMBERS".
254 2015-05-11 18:57:49 <jonasschnelli> But you validRealStr looks resonable
255 2015-05-11 18:59:00 <cfields> jonasschnelli: iirc i only did that because i re-used .read() as a tokenizer.
256 2015-05-11 18:59:19 <cfields> jonasschnelli: in case i haven't been clear, i think your changes are better than mine in every way
257 2015-05-11 18:59:55 <jonasschnelli> not everywhere: i think i have to pull in some stuff from your branch. Like https://github.com/theuni/bitcoin/commit/be426798ca8df2a12a8617431785f53cfeb04746
258 2015-05-11 19:00:01 <cfields> with the possible exception of the ERROR problem that i stumbled upon that might've been causing you issues without you knowing
259 2015-05-11 19:00:36 <jonasschnelli> right.
260 2015-05-11 19:01:22 <cfields> jonasschnelli: yea, i wasn't sure how to handle that one.
261 2015-05-11 19:01:26 <jonasschnelli> I'd like to extend the rpc unitests (rpc_tests.cpp, rpc_wallet_tests.cpp) os it coveres more of the rpc imput / output. This could detect possible API breakages.
262 2015-05-11 19:01:58 <jonasschnelli> (which is my main concern in this PR's case)
263 2015-05-11 19:02:33 <cfields> agreed
264 2015-05-11 19:23:17 <cfields> jonasschnelli: fwiw, i'm busy atm refactoring the net code and moving it towards something more modular. Just in case you were going to start some work there and overlap again :)
265 2015-05-11 20:40:06 <jonasschnelli> cfields: hah. Thanks for letting me know. No plans in this section. ;-)