Skip to content

Commit

Permalink
p2p/discover: move bond logic from table to transport (ethereum#17048)
Browse files Browse the repository at this point in the history 
* p2p/discover: move bond logic from table to transport

This commit moves node endpoint verification (bonding) from the table to
the UDP transport implementation. Previously, adding a node to the table
entailed pinging the node if needed. With this change, the ping-back
logic is embedded in the packet handler at a lower level.

It is easy to verify that the basic protocol is unchanged: we still
require a valid pong reply from the node before findnode is accepted.

The node database tracked the time of last ping sent to the node and
time of last valid pong received from the node. Node endpoints are
considered verified when a valid pong is received and the time of last
pong was called 'bond time'. The time of last ping sent was unused. In
this commit, the last ping database entry is repurposed to mean last
ping _received_. This entry is now used to track whether the node needs
to be pinged back.

The other big change is how nodes are added to the table. We used to add
nodes in Table.bond, which ran when a remote node pinged us or when we
encountered the node in a neighbors reply. The transport now adds to the
table directly after the endpoint is verified through ping. To ensure
that the Table can't be filled just by pinging the node repeatedly, we
retain the isInitDone check. During init, only nodes from neighbors
replies are added.

* p2p/discover: reduce findnode failure counter on success

* p2p/discover: remove unused parameter of loadSeedNodes

* p2p/discover: improve ping-back check and comments

* p2p/discover: add neighbors reply nodes always, not just during init
  • Loading branch information
@fjl @firmianavan
fjl authored and firmianavan committed Aug 28, 2018
1 parent 7adb06d commit 82d9ad8
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 245 deletions.
24 changes: 12 additions & 12 deletions p2p/discover/database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ var (
nodeDBNilNodeID = NodeID{} // Special node ID to use as a nil element.
nodeDBNodeExpiration = 24 * time.Hour // Time after which an unseen node should be dropped.
nodeDBCleanupCycle = time.Hour // Time period for running the expiration task.
nodeDBVersion = 5
)

// nodeDB stores all nodes we know about.
Expand Down Expand Up @@ -257,7 +258,7 @@ func (db *nodeDB) expireNodes() error {
}
// Skip the node if not expired yet (and not self)
if !bytes.Equal(id[:], db.self[:]) {
if seen := db.bondTime(id); seen.After(threshold) {
if seen := db.lastPongReceived(id); seen.After(threshold) {
continue
}
}
Expand All @@ -267,29 +268,28 @@ func (db *nodeDB) expireNodes() error {
return nil
}

// lastPing retrieves the time of the last ping packet send to a remote node,
// requesting binding.
func (db *nodeDB) lastPing(id NodeID) time.Time {
// lastPingReceived retrieves the time of the last ping packet sent by the remote node.
func (db *nodeDB) lastPingReceived(id NodeID) time.Time {
return time.Unix(db.fetchInt64(makeKey(id, nodeDBDiscoverPing)), 0)
}

// updateLastPing updates the last time we tried contacting a remote node.
func (db *nodeDB) updateLastPing(id NodeID, instance time.Time) error {
// updateLastPing updates the last time remote node pinged us.
func (db *nodeDB) updateLastPingReceived(id NodeID, instance time.Time) error {
return db.storeInt64(makeKey(id, nodeDBDiscoverPing), instance.Unix())
}

// bondTime retrieves the time of the last successful pong from remote node.
func (db *nodeDB) bondTime(id NodeID) time.Time {
// lastPongReceived retrieves the time of the last successful pong from remote node.
func (db *nodeDB) lastPongReceived(id NodeID) time.Time {
return time.Unix(db.fetchInt64(makeKey(id, nodeDBDiscoverPong)), 0)
}

// hasBond reports whether the given node is considered bonded.
func (db *nodeDB) hasBond(id NodeID) bool {
return time.Since(db.bondTime(id)) < nodeDBNodeExpiration
return time.Since(db.lastPongReceived(id)) < nodeDBNodeExpiration
}

// updateBondTime updates the last pong time of a node.
func (db *nodeDB) updateBondTime(id NodeID, instance time.Time) error {
// updateLastPongReceived updates the last pong time of a node.
func (db *nodeDB) updateLastPongReceived(id NodeID, instance time.Time) error {
return db.storeInt64(makeKey(id, nodeDBDiscoverPong), instance.Unix())
}

Expand Down Expand Up @@ -332,7 +332,7 @@ seek:
if n.ID == db.self {
continue seek
}
if now.Sub(db.bondTime(n.ID)) > maxAge {
if now.Sub(db.lastPongReceived(n.ID)) > maxAge {
continue seek
}
for i := range nodes {
Expand Down
34 changes: 17 additions & 17 deletions p2p/discover/database_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var nodeDBInt64Tests = []struct {
}

func TestNodeDBInt64(t *testing.T) {
db, _ := newNodeDB("", Version, NodeID{})
db, _ := newNodeDB("", nodeDBVersion, NodeID{})
defer db.close()

tests := nodeDBInt64Tests
Expand Down Expand Up @@ -111,27 +111,27 @@ func TestNodeDBFetchStore(t *testing.T) {
inst := time.Now()
num := 314

db, _ := newNodeDB("", Version, NodeID{})
db, _ := newNodeDB("", nodeDBVersion, NodeID{})
defer db.close()

// Check fetch/store operations on a node ping object
if stored := db.lastPing(node.ID); stored.Unix() != 0 {
if stored := db.lastPingReceived(node.ID); stored.Unix() != 0 {
t.Errorf("ping: non-existing object: %v", stored)
}
if err := db.updateLastPing(node.ID, inst); err != nil {
if err := db.updateLastPingReceived(node.ID, inst); err != nil {
t.Errorf("ping: failed to update: %v", err)
}
if stored := db.lastPing(node.ID); stored.Unix() != inst.Unix() {
if stored := db.lastPingReceived(node.ID); stored.Unix() != inst.Unix() {
t.Errorf("ping: value mismatch: have %v, want %v", stored, inst)
}
// Check fetch/store operations on a node pong object
if stored := db.bondTime(node.ID); stored.Unix() != 0 {
if stored := db.lastPongReceived(node.ID); stored.Unix() != 0 {
t.Errorf("pong: non-existing object: %v", stored)
}
if err := db.updateBondTime(node.ID, inst); err != nil {
if err := db.updateLastPongReceived(node.ID, inst); err != nil {
t.Errorf("pong: failed to update: %v", err)
}
if stored := db.bondTime(node.ID); stored.Unix() != inst.Unix() {
if stored := db.lastPongReceived(node.ID); stored.Unix() != inst.Unix() {
t.Errorf("pong: value mismatch: have %v, want %v", stored, inst)
}
// Check fetch/store operations on a node findnode-failure object
Expand Down Expand Up @@ -216,15 +216,15 @@ var nodeDBSeedQueryNodes = []struct {
}

func TestNodeDBSeedQuery(t *testing.T) {
db, _ := newNodeDB("", Version, nodeDBSeedQueryNodes[1].node.ID)
db, _ := newNodeDB("", nodeDBVersion, nodeDBSeedQueryNodes[1].node.ID)
defer db.close()

// Insert a batch of nodes for querying
for i, seed := range nodeDBSeedQueryNodes {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
if err := db.updateLastPongReceived(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to insert bondTime: %v", i, err)
}
}
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestNodeDBPersistency(t *testing.T) {
)

// Create a persistent database and store some values
db, err := newNodeDB(filepath.Join(root, "database"), Version, NodeID{})
db, err := newNodeDB(filepath.Join(root, "database"), nodeDBVersion, NodeID{})
if err != nil {
t.Fatalf("failed to create persistent database: %v", err)
}
Expand All @@ -277,7 +277,7 @@ func TestNodeDBPersistency(t *testing.T) {
db.close()

// Reopen the database and check the value
db, err = newNodeDB(filepath.Join(root, "database"), Version, NodeID{})
db, err = newNodeDB(filepath.Join(root, "database"), nodeDBVersion, NodeID{})
if err != nil {
t.Fatalf("failed to open persistent database: %v", err)
}
Expand All @@ -287,7 +287,7 @@ func TestNodeDBPersistency(t *testing.T) {
db.close()

// Change the database version and check flush
db, err = newNodeDB(filepath.Join(root, "database"), Version+1, NodeID{})
db, err = newNodeDB(filepath.Join(root, "database"), nodeDBVersion+1, NodeID{})
if err != nil {
t.Fatalf("failed to open persistent database: %v", err)
}
Expand Down Expand Up @@ -324,15 +324,15 @@ var nodeDBExpirationNodes = []struct {
}

func TestNodeDBExpiration(t *testing.T) {
db, _ := newNodeDB("", Version, NodeID{})
db, _ := newNodeDB("", nodeDBVersion, NodeID{})
defer db.close()

// Add all the test nodes and set their last pong time
for i, seed := range nodeDBExpirationNodes {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
if err := db.updateLastPongReceived(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update bondTime: %v", i, err)
}
}
Expand All @@ -357,15 +357,15 @@ func TestNodeDBSelfExpiration(t *testing.T) {
break
}
}
db, _ := newNodeDB("", Version, self)
db, _ := newNodeDB("", nodeDBVersion, self)
defer db.close()

// Add all the test nodes and set their last pong time
for i, seed := range nodeDBExpirationNodes {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
if err := db.updateLastPongReceived(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update bondTime: %v", i, err)
}
}
Expand Down
Loading

0 comments on commit 82d9ad8

Please sign in to comment.