minimalistic bitcoin client on D language?

[grondilu]

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.

I agree.  Actually I'm amazed this is the product of only one person's work.

[ribuck]

It's easy to complain about code. But unless a person can show that they did something better, they have no moral authority to complain.

[satoshi]

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.

That means a lot coming from you, Hal.  Thanks.

[farmer_boy]

Another client is useful, especially since the current Bitcoin client is a big mess. I was *shocked* that cryptography code looked like this.

Can you be more specific? What exactly makes it a 'mess'?

There is no object oriented design. Basically there is one big global state and that's it.

Very specifically line 24 in my SVN chechout contains the following number, but there is no explanation as to why this number is there; I can guess why it is there, but it shouldn't be my task to figure out why something is there. A comment above this constant should say why exactly it is that value and not some other value, why it is important for the system, etc.

Additionally a comment should state which hashing function is being used. E.g. it should say "The magic constant below is equal to the sha256sum (See, <reference to original definition of this algorithm>) of value <myvalue>. This is done, because <insert reasons>. "
 

Code:

uint256 hashGenesisBlock("0x000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f");

There is also no need to create a 3200 lines long source file. There is just no reason to do that other than to discourage people from making the effort to understand it.

I am sure it is possible to reverse-engineer it in a few months to get it into a state in which one can just read through the whole program in an hour or so, but that's not the same as it already being in such a state.

Anyway, I hope you see the difference in approach.

[grondilu]

Very specifically line 24 in my SVN chechout contains the following number, but there is no explanation as to why this number is there; I can guess why it is there, but it shouldn't be my task to figure out why something is there. A comment above this constant should say why exactly it is that value and not some other value, why it is important for the system, etc.
 

Code:

uint256 hashGenesisBlock("0x000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f");

Lol

As the name of the variable indicates it, it's the genesis block.   I guess it's the public key of the first ever bitcoins.

This is certainly not the best critic of Satoshi's code you can make.

[hsoft]

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

[farmer_boy]

Very specifically line 24 in my SVN chechout contains the following number, but there is no explanation as to why this number is there; I can guess why it is there, but it shouldn't be my task to figure out why something is there. A comment above this constant should say why exactly it is that value and not some other value, why it is important for the system, etc.
 

Code:

uint256 hashGenesisBlock("0x000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f");

Lol

As the name of the variable indicates it, it's the genesis block.   I guess it's the public key of the first ever bitcoins.

This is certainly not the best critic of Satoshi's code you can make.

The genesis block part was clear, but note that you have to guess the exact same information. Your guess is also still completely vague. In a computer program there is only one answer and in a good program one should not need guessing.

I don't see why I should even point out any other issue if this is clearly a valid objection.

[grondilu]

The genesis block part was clear, but note that you have to guess the exact same information. Your guess is also still completely vague. In a computer program there is only one answer and in a good program one should not need guessing.

I don't see why I should even point out any other issue if this is clearly a valid objection.

Well, I guess you're right :  at least ther should be a comment.

But anyway we know Satoshi's code is poorly commented.  It is just an example amonst many others.

[wumpus]

I am sure it is possible to reverse-engineer it in a few months to get it into a state in which one can just read through the whole program in an hour or so, but that's not the same as it already being in such a state.

I agree with you on this. It's hard to understand. It lacks comments. But I don't agree it's bad code per-se. I haven't read anything in the code yet that screams 'baaad design'.

If you're going to put in the effort to understand it: At least I'd prefer some good spec / documentation to 'cleaned up, but still hard to read' C++ (or D, a language I don't even know) any day.
That way, people can implement clients in language-agnostic ways.

Monolithic, code in header files, all units very tightly coupled, misuse of templates (when simple inheritance would do).

This is very subjective, my first reaction when looking at the code was also *ouch*. But then I remembered it's C++:

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 agree it's not pretty and hard to get used to, but it is a result of C++'s method of separation between code and interfaces (IE, none at all).

Again, I'd rather have a readable spec.

[ribuck]

I started cleaning up the code. The repo is at http://bitbucket.org/hsoft/bitcoin-clean

A waste of time. If you want to do something really useful for the code, write a test suite for it.

[hsoft]

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").

[wumpus]

I'm only learning C++, but from what I could read, the having "templated" code in headers is a last resort.

That kind of explains it, you're still learning it. You can say one thing about the bitcoin code, namely is not beginner-friendly at all.

The preferred way to go is to instanciate your templates in your cpp files (so that you don't get linking errors)

That doesn't work at all for generic programming. You can instantiate for example vector<int>, vector<float>, but then you could only ever use those two.
If you want vector<X> where X is any type to work you need to put the code for vector in a header. It's weird but C++ is designed like that. Again, look at Boost.

Anyway, IMO: there are more important things to worry about right now than Satoshi's code style. Such as building services that accept bitcoin.

[hsoft]

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).

[hsoft]

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.

[bitcoinex]

Another client is useful, especially since the current Bitcoin client is a big mess. I was *shocked* that cryptography code looked like this.

Can you be more specific? What exactly makes it a 'mess'?

It is need to group related functions with different functional in different files. So you can edit the repository a large group of people without having to manually merge

[grondilu]

Those debates about coding style in C++ are part of the reasons why I've never been much enthusiastic about this language.

What about a plain C implementation of bitcoin ?  Do we really need object paradigm for this anyway ?

[bitcoinex]

Those debates about coding style in C++ are part of the reasons why I've never been much enthusiastic about this language.

What about a plain C implementation of bitcoin ?  Do we really need object paradigm for this anyway ?

C++ is not only an object-oriented programming, but also the best type checking and templates

[wumpus]

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.

But do you really need to re-implement the client to do that? Why not simply use the current client, through JSON-RPC?

hsoft: I do agree that modularizing the current (C++) codebase seems like the best option if you really want to work on the bitcoin client itself. I've seen some good ideas:

• Split into GUI, JSON server, backend library (the libbitcoin idea), which can be built separately.
• Integrate a real build system (based on cmake/autotools/scons...), which autodetects presence/install dir of wxWindows and other deps.
• Add some asynchronous notification mechanism when transactions come in, to prevent polling.
• Build packages for debian/redhat/... and help it getting integrated into the repositories.

IMO: if we alternative bitcoin clients in every possible programming language, just because we like our own language/style better, that doesn't help further the cause of bitcoin itself at all. It's a nerd party only.