Several patches #100

fsb4000 · 2015-02-18T05:05:48Z

This introduces the concept of the 'sync node', which is the one we
asked for missing blocks. In case the sync node goes away, a new one
will be selected.

For now, the heuristic is very simple, but it can easily be extended
later to add better policies.

Same as novacoin-project/novacoin@28f9882
Similar to bitcoin/bitcoin@6ed71b5
(I don't delete old sync code because it does conflict with nothing and that's OK "Ask the first connected node for block updates" before this node can be selected for block updates in net.cpp)
(Although I can remove old sync code if necessary.)
Issue description: bitcoin/bitcoin#1234

This introduces the concept of the 'sync node', which is the one we asked for missing blocks. In case the sync node goes away, a new one will be selected. For now, the heuristic is very simple, but it can easily be extended later to add better policies.

fsb4000 · 2015-02-19T05:45:08Z

I added "using pthread_create instead CreateThread on Windows". Not sure that it is somehow connected with the problem, but it's the only thing that I did Windows only.(for compatibility with mingw 4.9.2)
Check whether it does break WinBuilder or not...

fsb4000 · 2015-02-20T19:57:45Z

Maybe #6 fixed. Need additional tests...

fsb4000 · 2015-02-21T12:16:43Z

OpenSSL Fix = bitcoin/bitcoin#5634 and bitcoin/bitcoin#5640

…ast blockchain checkpoint

fsb4000 · 2015-02-23T08:20:21Z

Make lsn_reset ("detach databases") optional and off by default = bitcoin/bitcoin#1119

This reverts commit 25580b1.

domob1812 · 2015-02-24T11:39:22Z

Thanks for the pull request! In general, please don't submit "several patches" but split them into logical units (for the future).

Also, without having reviewed the code in full, I have at least some suggestions right now:

The lsn_reset patch seems fine, but I would prefer to have lsn_reset on by default. Otherwise people may get problems when they try to backup / restore / transfer their data folder and have no idea why. If they are aware of the possible implications, then they can turn it off for themselves.
Please do not include the hardcoded nodes. I don't think that's a good idea (do we have more info about the nodes, like their capacity, who runs them, how they are secured). We have IRC discovery for that, and may add DNS seeds in the future.
Please do not introduce checkpoints. It was a deliberate decision not to have them (not by me, but it was made). Also, you don't even mention that in your forum post. That's a very good demonstration of why I suggest to have multiple pull requests for multiple things, and not just a generic "several patches".

domob1812 · 2015-02-24T11:40:13Z

src/huntercoin.cpp

+Fourth Octet: 	185
+(67 * 16777216) + (225 * 65536) + (171 * 256) + (185) = 1138863033
+*/
+unsigned int pnSeed[] = {2990412266, 1138863033 };


Why add the comment and not just #define a macro that does the calculation for you?

(Besides, I don't think we should hardcode the nodes.)

fsb4000 · 2015-02-24T12:36:10Z

@domob1812:

Nit pick: Please remove the blank line.

Sure. Added to the last commit(Automatic creation huntercoin.conf) I do not know how to edit an arbitrary commit, only the last. Let me know if you know how to edit an any commit.

@domob1812:

Why add the comment and not just #define a macro that does the calculation for you?
(Besides, I don't think we should hardcode the nodes.)

Without adding nodes, huntercoin does not start synchronization from scratch. Although it works, if user downloads the database from your site( http://chain.huntercoin.org/ )(because your database has addr.dat with node IPs)
Of course I can add a macro. Could you prompt me a name for this macro?

domob1812 · 2015-02-24T12:44:46Z

Just add another commit which cleans up everything. (This is not perfect, but I think it is good enough for now.)

Actually, Huntercoin should discover nodes via IRC (as Bitcoin did back in the day). Is this broken? If so, then we should indeed try to fix it. If not, I don't think we should be adding the hardcoded nodes.

fsb4000 · 2015-02-24T12:50:49Z

IRC code is good, but IRC servers often is offline(or maybe they blocked some IPs, not sure)
Sometimes everything is OK, sometimes I get
IRC connect failed (at debug.log)
IRC waiting 71 seconds to reconnect
..................................
IRC connect failed
IRC waiting 138 seconds to reconnect
and so. I think better use all ways to connect: IRC + harcode seeds + hardcode dns seeds...

For example, classic namecoin has 3 hardcoded nodes: https://github.com/namecoin/namecoin/blob/b043fba28018721b68c90edfc81b9eacb070b47d/src/namecoin.cpp#L2702

fsb4000 · 2015-02-25T13:19:47Z

@domob1812:

Do we have more info about the nodes, like their capacity, who runs them, how they are secured?

Yes, we have. User can't synchronize. The official advices were:
BGB:

Yes this has been a problem. You can try adding my node
addnode=67.225.171.185

Snailbrain:

you can try this node:- 178.62.17.234
but have limited to 40 connections max and all used atm..
but if you give me your ip and port is open, i can connect=you - i believe this should work even though i'm maxed.

So I added 67.225.171.185 and 178.62.17.234 in the code.

…re the last blockchain checkpoint" This reverts commit 79490f4.

domob1812 · 2015-02-25T13:27:31Z

Ok, I see. In that case, if IRC is down sometimes (wasn't aware of that), I think adding the nodes makes sense. Please just use a macro like

#define CONVERT_IP4(a, b, c, d)
(((a) << 24) + ((b) << 16) + ((c) << 8) + (d))

instead of the manual computation.

fsb4000 · 2015-02-25T13:28:20Z

@domob1812:

Please do not introduce checkpoints. It was a deliberate decision not to have them (not by me, but it was made). Also, you don't even mention that in your forum post. That's a very good demonstration of why I suggest to have multiple pull requests for multiple things, and not just a generic "several patches".

Done, https://github.com/fsb4000/huntercoin/commit/6b7eed1d458ea9d17fbbcc2823cd1f71cb777834

@domob1812:

Please just use a macro like
#define CONVERT_IP4(a, b, c, d) \
(((a) << 24) + ((b) << 16) + ((c) << 8) + (d))
instead of the manual computation.

Done, https://github.com/fsb4000/huntercoin/commit/057a64ed87a1a0e3261bf7ef60784375ca041a7b
(third IP I found here: https://bitcointalk.org/index.php?topic=435170.msg8583919#msg8583919 )

fsb4000 · 2015-02-25T15:16:28Z

@domob1812 :

The lsn_reset patch seems fine, but I would prefer to have lsn_reset on by default. Otherwise people may get problems when they try to backup / restore / transfer their data folder and have no idea why. If they are aware of the possible implications, then they can turn it off for themselves.

done, https://github.com/fsb4000/huntercoin/commit/1ea795dd5055c331793686a6f155c6a5c2ebb2b5

Snailbrain, please,in the next release which will include the pull request, provide details about command line command -detachdb=0 which really speeds up the client shutdown
(-detachdb=0 for huntercoind and uncheck checkbox "Detach databases at shutdown" for Qt version)

fsb4000 · 2015-02-25T15:39:27Z

"Automatic creation huntercoin.conf" is similar to jl777/btcd@be2f732 (the first time where I saw the auto creation .conf file)

Several patches

fsb4000 added 2 commits February 18, 2015 14:30

boost 1.57 compatibility

f15ead7

fsb4000 added 3 commits February 20, 2015 13:09

using boost::thread instead pthread

ee57e7d

Miniupnp 1.9 compatibility

0a32d4f

Qt performance is increased

25580b1

OpenSSL Fix

6061115

fsb4000 added 2 commits February 22, 2015 19:15

Skip ECDSA signature verification when connecting blocks before the l…

79490f4

…ast blockchain checkpoint

Make lsn_reset ("detach databases") optional and off by default.

99e3bba

fsb4000 added 3 commits February 23, 2015 14:25

added 2 new nodes

32ec28a

Revert "Qt performance is increased"

b1ab058

This reverts commit 25580b1.

another attempt to increase the Qt performance.

183ca0d

fsb4000 changed the title ~~Make sure we always have a node to do IBD from~~ Several patches Feb 23, 2015

domob1812 reviewed Feb 24, 2015
View reviewed changes

fsb4000 added 2 commits February 25, 2015 19:25

Automatic creation huntercoin.conf

0e757e3

Revert "Skip ECDSA signature verification when connecting blocks befo…

6b7eed1

…re the last blockchain checkpoint" This reverts commit 79490f4.

fsb4000 added 2 commits February 25, 2015 20:28

use CONVERT_IP4 macro instead of the manual computation.

057a64e

lsn_reset is "on" by default

1ea795d

chronokings added a commit that referenced this pull request Mar 12, 2015

Merge pull request #100 from fsb4000/sync

5d71c05

Several patches

chronokings merged commit 5d71c05 into chronokings:master Mar 12, 2015

fsb4000 deleted the sync branch June 17, 2015 07:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Several patches #100

Several patches #100

fsb4000 commented Feb 18, 2015

fsb4000 commented Feb 19, 2015

fsb4000 commented Feb 20, 2015

fsb4000 commented Feb 21, 2015

fsb4000 commented Feb 23, 2015

domob1812 commented Feb 24, 2015

domob1812 Feb 24, 2015

fsb4000 commented Feb 24, 2015

domob1812 commented Feb 24, 2015

fsb4000 commented Feb 24, 2015

fsb4000 commented Feb 25, 2015

domob1812 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015

Several patches #100

Several patches #100

Conversation

fsb4000 commented Feb 18, 2015

fsb4000 commented Feb 19, 2015

fsb4000 commented Feb 20, 2015

fsb4000 commented Feb 21, 2015

fsb4000 commented Feb 23, 2015

domob1812 commented Feb 24, 2015

domob1812 Feb 24, 2015

Choose a reason for hiding this comment

fsb4000 commented Feb 24, 2015

domob1812 commented Feb 24, 2015

fsb4000 commented Feb 24, 2015

fsb4000 commented Feb 25, 2015

domob1812 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015

fsb4000 commented Feb 25, 2015