p2p/discover: fix race involving the seed node iterator

[fjl]

nodeDB.querySeeds was not safe for concurrent use but could be called
concurrenty on multiple goroutines in the following case:

• the table was empty
• a timed refresh started
• a lookup was started and initiated refresh

These conditions are unlikely to coincide during normal use, but are
much more likely to occur all at once when the user's machine just woke
from sleep. The root cause of the issue is that querySeeds reused the
same leveldb iterator until it was exhausted.

This commit moves the refresh scheduling logic into its own goroutine
(so only one refresh is ever active) and changes querySeeds to not use
a persistent iterator. The seed node selection is now more random and
ignores nodes that have not been contacted in the last 5 days.

Fixes #1660.

@karalabe wrote the previous implementation of the refresh logic and might be the
best person to judge these changes.

---

Note: The last commit removes the strict version matching on discovery packets
because it makes protocol upgrades much harder. I'm trying to sneak this in before
1.2.0.

[robotally]

Vote	Count	Reviewers
👍	2	@karalabe @obscuren
👎	0

Updated: Wed Sep 30 15:07:14 UTC 2015

[karalabe]

Do we want to delete the self query test? At a certain point we had issues with geth trying to connect to itself because self id ended up in the seed database. Wouldn't it be worth to keep this test (and the self test check in the lookup mechanism) still in?

[fjl]

In hindsight, I think that removing the self entries is misguided. We should rather make sure that
self never ends up in the database.

[fjl]

I added a check to not include self in the seed result slice.

[alexvandesande]

👏🏻 Have no meaningful comment but would like to say that I enjoyed looking at the code and thanks for finding the bug.

[karalabe]

LGTM 👍

[Gustav-Simonsson]

Just out of curiosity, what is the advantage of this label compared to labelling the innermost for loop?

[fjl]

The loop is structured like this:

for {
     node = load random node
     check
     check
     check
     add node as result
}

If any of the checks fail, the node is not added to the result. continue seek is supposed to make that clear.

[obscuren]

👍 squash commits please