 Show Posts
|
|
Pages: [1]
|
|
Since I had only compiled my cleaned bitcoin code with Mac OS X thus far, I thought it would be a good idea to fix the cleaned codebase for Windows compilation. Darn is it complicated to create a bitcoin VC++ build setup! Is anyone building bitcoin using VC++ or is everyone using MinGW? I can compile the "official" version of bitcoin, but whenever I run it, I get tons of runtime assertion errors. Anyone ever got that?
Moreover, it's much more complicated than I thought to make my cleaned code compile under Windows. I thought it was only a matter of correctly "ifdef-ing" a couple of includes I had moved around, but it's much more complicated than that. Where gcc compiles fine, MSVC just spit errors about ambiguous and re-defined symbols and stuff like that. I'm trying to remove the "using namespace X" statements to go around this, but then I get even weirder errors. I don't feel like fighting with MSVC anymore, so I'm giving up.
The positive side is that I got to learn a little bit of C++.
|
|
|
|
One concern I'd have with losing the inlinable .h functions is whether it could slow down either initial block load/verify/indexing, or mining (a bit moot with gpu miners these days).
inlining doesn't have to be in .h files. I removed them because I thought it was silly to put them everywhere without first identifying bottlenecks. At first, I wondered if maybe it was the result of thoughtful optimization, but this inlining was really put at silly places, so I doubt it was the case. They can easily be brought back, but I think that anyone bringing them back should read about it first (for example: http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.3 ). Misused inlining makes the code slower. EDIT: Thinking about it, I'm having doubt about my statement (that inlined function don't have to be in headers)... I'd have to verify that. After all, I'm only learning C++... But it still doesn't change the fact that optimization should be targeted at measured bottlenecks. |
|
|
|
To be able to create a library out of the BitCoin code (as I explain in http://bitcointalk.org/index.php?topic=2075.0 ) and eventually create bindings in other languages for it (Python!), I cleaned Bitcoin's source code. I'm pretty much finished and the result is at: http://bitbucket.org/hsoft/bitcoin-cleanSo the question I'm asking you guys is: Should this be included in the main repository? Personally, I find the code much more readable now, but then I guess opinions can diverge, so that why I'm asking. The list of things I've done is described there: http://bitbucket.org/hsoft/bitcoin-clean/src/f7995af49149/cleaning_process.txtIt's a huge change, but I doubt I introduced new bugs because I haven't changed a single line of actual code. I have just restructured it. If I had introduced bugs, my guess is that it would have been bugs causing the whole app not to work, but it builds and run fine (I get connections and I download blocks). The next step towards my ultimate goal (libBitCoin, python bindings, alternate UIs) involves decoupling classes, which is more bug prone (because I have to actually change the code), but I added a tag in the repo (before-decouple) to indicate at which point I started modifying the code itself. So, there I go: Do you find the cleaned code more readable and maintainable, or do you prefer the old one? Should it replace the official one on SVN? |
|
|
|
Anyway, IMO: there are more important things to worry about right now than Satoshi's code style. Such as building services that accept bitcoin.
This is precisely what I'm trying to do. My main goal is to create Python bindings to bitcoin. I've tried to extract a library out of it (see http://bitcointalk.org/index.php?topic=2075.0 ) but I failed. I'm cleaning the code now because I see it as the only way to achieve my main goal. |
|
|
|
I might be learning C++, but I'm not learning programming. I know code smell when I see it. But then, no need to argue, because you can see the result for yourself (I'm pretty far in the cleaning process). I've just pushed the "de-templatization" of the stream related functions (at http://bitbucket.org/hsoft/bitcoin-clean/changeset/ffcfc79e29f0 ). The code still works well (I haven't touched the code per se, just its structure, so I doubt I introduced bugs) and it compiles significantly faster (yeah yeah, templates makes compilation slow). |
|
|
|
Putting your code in headers is a valid code style in C++. It allows current compilers to do a better job in optimization, and for templates (in general) it's required as they can't be exported between compilation units.
Boost is almost entirely written this way, so are quite a few other modern C++ libraries. They also prefer the functional programming approach using templates to 'simple inheritance'.
I'm only learning C++, but from what I could read, the having "templated" code in headers is a last resort. The preferred way to go is to instanciate your templates in your cpp files (so that you don't get linking errors). I have already completed the move from headers to cpp files in my cleaned repo, and it works well. There is nothing "functional" about misusing templates. Functional programming is about passing functions around and having only "pure" (no side effects) functions. The Bitcoin code uses templates for function that takes a "Stream" type (something that has a read() and a write() method), which is implemented by 2 or 3 classes in the code. Take it however you like, templating is the wrong way to go. The correct way to go is to have an abstract stream class and to make the stream classes subclass and implement the virtual read() and write method. Using templates is just sloppy. Templates is for data structures (list of <whatever>), not for polymorphism (I used the wrong term earlier when I wrote "simple inheritance"). |
|
|
|
Another client is useful, especially since the current Bitcoin client is a big mess. I was *shocked* that cryptography code looked like this.
I'd like to hear some specific criticisms of the code. To me it looks like an impressive job, although I'd wish for more comments. Now I've mostly studied the init, main, script and a bit of net modules. This is some powerful machinery. Monolithic, code in header files, all units very tightly coupled, misuse of templates (when simple inheritance would do). I started cleaning up the code. The repo is at http://bitbucket.org/hsoft/bitcoin-clean |
|
|
|
I just read your description of the cleaning process. I think that instead of decoupling wxw from the code you should drop it altogether and just make it easy (JSON-RPC is easy  ) to implement one as an extra layer. Of course, that was the whole point of my "libBitCoin" initiative. But because I failed at that, what I want to do is decouple wx, one step at a time, and once this is done, extract libBitCoin out of bitcoin. |
|
|
|
|
For the record: I failed.
This code is way too coupled (with the UI as well as everything else) to extract a libBitCoin from it without deep knowledge of it.
I might try again, but then I'd start by cleaning the code from within the project, and only after that, try to extract a library out of it.
Am I the only one worrying about the structure of that code?
|
|
|
|
Well well, I gave it a try today (I want to learn C++, so I thought it would be a good opportunity). The (incomplete, of course) result is at: http://bitbucket.org/hsoft/libbitcoinI also cloned the bitcoin repository and changed it so that it uses libBitCoin: http://bitbucket.org/hsoft/bitcoin-reorgThe more I play with that code, the more I think some restructuring would be healthy. I don't know much about C++, but is it normal that this much code end up in header files instead of cpp files? |
|
|
|
|
One example I've been thinking about is that "pooled mining" thing. The proposed scheme of having a central authority push work to miners seems wrong to me. It should rather be miners that fetch work from the central authority. Doing so with the current code seems more complicated than it should.
Another one is to (I don't even know if it's possible) maybe develop a concept of "BitCoin authority", which would maintain that huge block database, and "lite clients" which would rely on central authorities. Would it be p2p anymore? no, you'd have to trust a central authority (a kind of bank), but because of the p2p protocol, you'd always have the choice to run a full client yourself if you want. But most people would run the "lite client" because the normal client is resource hungry.
Anyway, the whole thing is mainly for good software engineering practices, which increases maintainability and extensibility, which is a good idea in general. Also, the RPC API seems rather hackish to me.
It doesn't seem like that much work (ok, maybe I'm wrong there...), it's just a matter of re-organizing the existing code, which is already there.
|
|
|
|
|
Isn't bitcoind the exact same software, but with the "-server" flag?
What I mean to do with a libBitCoin is to make it easier to create bindings for it as well as alternate clients (instead of the wxWidgets one).
This could spur innovation within the BitCoin project (new stuff is easier to try in dynamic languages like Python). An example could be django bitcoin integration, or stuff like that. The last time I checked the RPC API, it seemed rather limited to me.
Moreover, implementing RPC stuff is much easier to do in Python than in C++. That's one of the specific things I was thinking about.
|
|
|
|
|
I find the idea of BitCoin interesting, and I looked into its source code a little bit. I find it a bit monolithic and I thought it would be a good idea to extract the non-ui (and by that, I mean every UI, including RPC and command line) code and create a new library out of it.
This way, it would be easier to create alternative clients as well as bindings to other languages (Python!!!).
As anyone thought of it yet? Is there a particular reason why this would be doomed to failure?
|
|
|
|
|
