Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add built-in seeds for .onion #4582

Merged
merged 1 commit into from
Aug 4, 2014
Merged

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Jul 24, 2014

This makes it possible for a node with -onlynet=tor to bootstrap itself. It also adds the base infrastructure for adding IPv6 seed nodes.

The seeds are taken from reliable nodes in @sipa's crawler as well as https://en.bitcoin.it/wiki/Fallback_Nodes#Tor_nodes.

I've tested this with bitcoind -onlynet=tor -dnsseed=0 -onion=127.0.0.1:9050 after making sure that peers.dat was cleared.

@@ -97,6 +102,64 @@ unsigned int pnSeed[] =
0x13f5094c, 0x7ab32648, 0x542e9fd5, 0x53136bc1, 0x7fdf51c0, 0x802197b2, 0xa2d2cc5b, 0x6b5f4bc0,
};

static SeedSpec6 pnSeed6_main[]={
{{0xfd,0x87,0xd8,0x7e,0xeb,0x43,0x0a,0x26,0x27,0x21,0xae,0x94,0xd5,0xc2,0x72,0x24}, 8333},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: indentation?

@sipa
Copy link
Member

sipa commented Jul 24, 2014

  • We can just store all seed addresses in 16-byte format (even the IPv4 ones)? If code compactness is a worry, we'll likely need to move all of this to a separate file excluded from automatic formatting anyway?
  • Maybe we can add actual IPv6 (non-onion) seed IPs too?
  • Perhaps we should only add seed addresses for networks we're connected to?

None of these are blockers, though.

@laanwj
Copy link
Member Author

laanwj commented Jul 24, 2014

I'm OK with all of those, I cannot help testing the second one though, but if you send a list it's easy to add.

@sipa
Copy link
Member

sipa commented Jul 27, 2014

Untested ACK

@laanwj
Copy link
Member Author

laanwj commented Jul 28, 2014

Added a commit that converts all the IPv4 seeds to the new format as well.

Checked that they're still the same by diffing the output of:

std::vector<CAddress> vAddr = Params().FixedSeeds();
for (std::vector<CAddress>::const_iterator it = vAddr.begin(); it != vAddr.end(); it++)
    printf("%s\n", (*it).ToString().c_str());

@jgarzik
Copy link
Contributor

jgarzik commented Jul 29, 2014

Change LGTM, though I tend to prefer approaches that do not store obfuscated addresses in the git repo.

An alternative is a short C++ program that reads addresses, generates a header file at compile time, and then proceeds with the build. automake has a generated-source pattern to enable this.

@sipa
Copy link
Member

sipa commented Jul 29, 2014

We already have python scripts for automatic source generation (inside the qt/ directory, afaik), and C++ tools at compile time are harder (you need to compile them for the host system first...).

A text file per network with all seeds in it, and a python script to generate a cpp source file for that sounds nice, though.

@jgarzik
Copy link
Contributor

jgarzik commented Jul 30, 2014

To make an analogy, we ship the compiler (python script) + compiler output, but not the source code.

@laanwj
Copy link
Member Author

laanwj commented Jul 30, 2014

This change already adds a python script to generate the .h file. Adding the source address lists and calling it as part of the build system is fine to me, and a small step.

Strong disagree with porting it to C++. I've seen how much trouble native-compiled tools create in other projects (Mesa, miniupnpc AFAIK) and it's useless work if we already have a Python script.

@laanwj
Copy link
Member Author

laanwj commented Jul 30, 2014

Ok - I moved the seed generation script and input files to share/seeds, and included the source files.

I stopped short of actually integrating it into the build system. Generating the file every time is not necessary, and it may complicate building for those using MSVC and other build systems. Note that the other autogenerated file, src/qt/bitcoinstrings.cpp, is also checked in.

@@ -80,4 +80,3 @@ It will do the following automatically:
- add missing translations to the build system (TODO)

See doc/translation-process.md for more information.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this, we don't want white-space changes ^^ (evil-grin)... no I'm sorry, everything is fine for me :).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK we haven't got an auto formatter for text files :p Anyhow, this is not on purpose, something was added and removed again and supposedly I removed a newline too many.

@Diapolo
Copy link

Diapolo commented Jul 30, 2014

What makes a good seed node? I'm just asking because my 2 Tor hs are not in this list, but are also not running 24/7 but only when I use my PC.

@laanwj
Copy link
Member Author

laanwj commented Jul 30, 2014

The current onion nodes come from two place:

If you have a node that's not online 24/7 it makes no sense to add it, IMO.

@laanwj laanwj added the P2P label Jul 31, 2014
@sipa
Copy link
Member

sipa commented Aug 1, 2014

In light of @jgarzik's comment to not have obfuscated addresses in the source code, it would be nice to convert the hex IP addresses into dotted quad notation in the text file.

@laanwj
Copy link
Member Author

laanwj commented Aug 1, 2014

I didn't obfuscate those, I literally copied those from the previous source!

@sipa
Copy link
Member

sipa commented Aug 1, 2014

@laanwj Didn't mean to say you were, and not a blocker either. Just saying it would be nice if the 'primary' source was in the most human readable format possible.

@sipa
Copy link
Member

sipa commented Aug 1, 2014

Untested ACK

This makes it possible for a node with `-onlynet=tor` to bootstrap
itself.

It also adds the base infrastructure for adding IPv6 seed nodes.

Also represent IPv4 fixed seed addresses in 16-byte format.
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4582_a60120e951ae94d0553c5515229432e016fb0657/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@Diapolo
Copy link

Diapolo commented Aug 4, 2014

It's really good to have some seeds for hidden-services integrated!

@laanwj
Copy link
Member Author

laanwj commented Aug 4, 2014

Squashed and added the deobfuscated IPv4s as comment.

@laanwj laanwj merged commit a60120e into bitcoin:master Aug 4, 2014
laanwj added a commit that referenced this pull request Aug 4, 2014
a60120e Add built-in seeds for .onion (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants