1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
|
Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194]
helo=mx.sourceforge.net)
by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76)
(envelope-from <michael.offel@web.de>) id 1QgknY-0003VW-Tc
for bitcoin-development@lists.sourceforge.net;
Tue, 12 Jul 2011 21:47:20 +0000
Received: from fmmailgate03.web.de ([217.72.192.234])
by sog-mx-4.v43.ch3.sourceforge.com with esmtp (Exim 4.76)
id 1QgknX-0004A9-LH for bitcoin-development@lists.sourceforge.net;
Tue, 12 Jul 2011 21:47:20 +0000
Received: from smtp01.web.de ( [172.20.0.243])
by fmmailgate03.web.de (Postfix) with ESMTP id D0C30195488EE
for <bitcoin-development@lists.sourceforge.net>;
Tue, 12 Jul 2011 23:47:13 +0200 (CEST)
Received: from [87.194.33.247] (helo=[192.168.1.64])
by smtp01.web.de with esmtp (WEB.DE 4.110 #2) id 1QgknR-0002om-00
for bitcoin-development@lists.sourceforge.net;
Tue, 12 Jul 2011 23:47:13 +0200
Date: Tue, 12 Jul 2011 22:47:12 +0100
From: Michael Offel <michael.offel@web.de>
X-Priority: 3 (Normal)
Message-ID: <602127524.20110712224712@web.de>
To: bitcoin-development@lists.sourceforge.net
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: michael.offel@web.de
X-Sender: michael.offel@web.de
X-Provags-ID: V01U2FsdGVkX188VnVuBLkNHeQMdNVYVhOYLzXGzkYowF0rpgzV
D+IYIxhd1VVmiUsP83ZXO3W+X4JIVtjAOuRNOoM3yBVWiJ/5wO
3t9aGdH3Wwi6cjYDoVAg==
X-Spam-Score: 0.0 (/)
X-Spam-Report: Spam Filtering performed by mx.sourceforge.net.
See http://spamassassin.org/tag/ for more details.
-0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/,
no trust [217.72.192.234 listed in list.dnswl.org]
0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider
(michael.offel[at]web.de)
-0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay
domain
0.0 T_TO_NO_BRKTS_FREEMAIL To: misformatted and free email service
X-Headers-End: 1QgknX-0004A9-LH
Subject: Re: [Bitcoin-development] overall bitcoin client code quality
X-BeenThere: bitcoin-development@lists.sourceforge.net
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: <bitcoin-development.lists.sourceforge.net>
List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
<mailto:bitcoin-development-request@lists.sourceforge.net?subject=unsubscribe>
List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=bitcoin-development>
List-Post: <mailto:bitcoin-development@lists.sourceforge.net>
List-Help: <mailto:bitcoin-development-request@lists.sourceforge.net?subject=help>
List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/bitcoin-development>,
<mailto:bitcoin-development-request@lists.sourceforge.net?subject=subscribe>
X-List-Received-Date: Tue, 12 Jul 2011 21:47:21 -0000
Monday, July 11, 2011, 12:31:20 AM, Jeff Garzik wrote:
>> 2. isolation of modules
> It is a long term goal to move towards 'libbitcoin"
What about creating a branch and start libbtc by implementing a small
module like irc or p2p connection handling and use the new lib in the
client. I think this would be a proper start for a new clean code base
without having a non functional client for some time and it also
provides some kind of red line between libbtc (cleaned up code) and
the old code base, making it easy to maintain order.
Would this approach be accepted for a merge?
Monday, July 11, 2011, 12:36:53 AM, Matt Corallo wrote:
>> While I can guess what the CScript class does I would more like to understand the thoughts behind the decision to implement some crypto macros in a compile time interpreter and
>> why Berkeley db 4 is used at all.
> At the time Bitcoin began being built, Ubuntu 9.04 (or was it 9.10?) was
> used, as it offered the oldest libc on the newest OS. Ubuntu 9.04 just
> happened to only have db4.7. For backward compatibility, db4.7 has been
> used ever since (except, for some reason, the osx builds). In 0.4,
> db4.8 will be used. If you are asking why bdb was used to begin with,
> why not? its an excellent db and why reinvent the wheel?
It was more meant as an rhetorical question. A documented decision
would give anyone the chance of arguing against the usage of a library
instead of asking stupid questions. A mailing list archive suits well
for this type of information, so let me try to get some information
here. Db4 is an excellent choice if you need indexed database
functionality without SQL interface. But compared to an stl map lookup
and fopen, fwrite and fclose it is much harder to understand and
brings a lot performance overhead. This is true as long as your
information are small enough to stay in main memory. A stl map storing
file offsets is also not that hard to write and understand. On the
other side using an SQL interface would bring the advantage of
swapping database providers. An enterprise website could use oracle
while the average user could use sqlite. Also is db4 used for any type
of disk storage, this makes files like wallet.dat some kind of hard to
read. It is in no way more secure than storing private key's in an xml
file. But it is much harder to maintain and understand by the user and
the average programer.
> If you are on Windows, why are you on Windows? ;)
I'm forced to to use windows by the type of clients I'm working for.
And during leisure I like to use a System that does not need much
effort to simply do what it is made for. ;)
>>
>> 6. hardcoded values
>>
>> There are tons of hardcoded values for the official and the testnet block chain. It would be a great step to move all chain related settings to a chain description file. This would allow custom chains and clean up the code.
> This one is an interesting debate. There is no real reason to do this
> aside from some questionable code cleanup. Also, there is no reason to
> encourage improperly-implemented alternate chains. Alternate chains
> should be designed in such a way as to share the main chain's difficulty
> as described by Mike on the forum, not just make a new chain and hope it
> sticks.
It is not that interesting as it looks first. There is no good in
running multiple chains for production use. To share the difficulty is
indeed a good start to solve the problem. That's also one of the
things I don't like off the QBitcoin client. What I meant is just
to have the possibility to have all adjustable parameters in one place
and to be able to quickly build an internal testnet without crazy
firewalling to prevent it from dying. The first would allow to detect
problematic ddos protection settings early and giving the average user
the possibility to adjust all important settings if he knows what he
does. That includes not only alternate chains. One could choose to
include transactions only at a higher fee or at no fee at all.
Everyone could do such things by changing the code anyway. But not all
brilliant administrators or users are programmers.
>> The official Bitcoin client should be some kind of an reference project for other clients and must therefore be extra clean and well documented.
> True, but it is much higher priority to clean up the code than comment
> it better, plus there are various other features/more user-facing issues
> that need fixed as well, so...
Good code is the best documentation anyway.
Monday, July 11, 2011, 3:01:51 AM, Luke-Jr wrote:
>> My overall suggestion is to begin a complete rewrite, inspired by the old
>> code rather than moving a lot of "known to be somehow functional" around.
> There are many rewrites in progress, often with much better designs.
There is no other client that uses C and is meant to be a full
implementation and platform independent except QBitcoin. QBitcoin seems
to have no public repository to work on or I have overlooked it ?!?
Starting a new client on my own is just like starting an other never
ending and never used open source project.
>> The official Bitcoin client should be some kind of an reference project
>> for other clients and must therefore be extra clean and well documented.
> Bitcoin is supposed to be an authorityless project. There is no official.
While there is no authority, there is just one fully working client to
look at. This may lead to an working but instable network if other
clients are trying to interpret net.cpp and fail on it in details.
>> *everything else*
> Fix it yourself and submit the changes. If they don't get merged, fork.
As I said, there is no need for an other never ending story. I would
like to know if my affords have a chance to get merged or accepted
before I start to work on it.
Tuesday, July 12, 2011, 4:31:07 AM, Gavin Andresen wrote:
> We'll just tell everybody to stop using bitcoin so much for six months
> or so while we implement a much better client. It will be exactly
> like the bitcoin we have now, except with a much nicer internal
> architecture and much cleaner code-base, and we're pretty sure we can
> get it done in six months if everything goes exactly as planned.
It is some kind of arrogant to believe that anyone would stop using
bitcoin if some programers decide to stop working for some month. And
it is also fond to not fix bugs in the old code base if they appear.
Also there are lots of people out there using old clients anyway. The
important improvement is more about quick extendibility and therefore
more feature rich secure code. This would not only help the official
code base, it would also improve trust and result in better external
bitcoin related projects.
> Then again, it might be easier to modify the
> CRITICAL_SECTION code to detect and report deadlocks (anybody have
> experience doing that?).
That would be true if possible, but I'm pretty sure that the only way
to detect deadlocks is by either analyzing the code or single step
simulating it, what is really tricky on network applications.
Tuesday, July 12, 2011, 6:19:28 AM, Jeff Garzik wrote:
>> While spying on the old code, I noticed one major problem that could be
>> fixed quite easily. That is, the 1 class-per .h/.cpp rule is completely
>> ignored in main.h/cpp and net.h/cpp If all of the classes in the project
>> were re-factored to their own files,
> This is about the last thing we should do, and it's one of the worst
> coding practices of many C++ projects (and unfortunately carried over
> to Java by force). See Knuth and his work on literate programming.
> Putting logically similar code -together- is often more impactful and
> meaningful than splitting up files into dozens (hundreds or thousands,
> in large projects) of tiny, 20-line files.
We seem to have very different opinions on that, but let's try to be
objective. I belive that every class should be able to stand on its
own. That way it can be reused in other projects or situations in the
same project. And it is much more easy to catch and extend class
behavior if it is isolated to one file instead of multiple files or
mixed between other class methods in one file. On the other hand, what
is bad on having 50-80 code files in bitcoin? In terms of source
control it even gives some kind of easier to read history and fewer
merge conflicts. When you start writing a class for exactly one
propose in one specific situation used by one other class you should
think about writing a nested class, which can and should be
implemented in the same cpp file. That way you can achieve you similar
code in one location while accepting the rule others like.
Another nice side effect is the ability to see a class dependency list
be looking at the include listing.
Tuesday, July 12, 2011, 8:21:12 AM, John Smith wrote:
> re:4) I also don't see why boost would be a 'nonstandard
> dependency'. From what I understand, a large part of the new C++0x
> standard is derived from boost. Also C++ compilers are getting
> faster and more smart all the time, so I absolutely don't see "build speed" as a goal.
Don't get me wrong. If boost would be used for something meaningful
there would be no point in removing it. Everything non questionable
about boost does already find its way into most stl implementations.
And everything that find it's way into C++ 0x does it for the reason
that it is better handled by an language extension than by an boost
construct. Otherwise there would be no point in extending the language.
Michael
|
