Skip to content

Commit

Permalink
p2p/discover: s/lastPong/bondTime/, update TestUDP_findnode
Browse files Browse the repository at this point in the history
I forgot to change the check in udp.go when I changed Table.bond to be
based on lastPong instead of node presence in db. Rename lastPong to
bondTime and add hasBond so it's clearer what this DB key is used for
now.
  • Loading branch information
fjl committed Feb 16, 2018
1 parent 32301a4 commit aeedec4
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 25 deletions.
17 changes: 11 additions & 6 deletions p2p/discover/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,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.lastPong(id); seen.After(threshold) {
if seen := db.bondTime(id); seen.After(threshold) {
continue
}
}
Expand All @@ -278,13 +278,18 @@ func (db *nodeDB) updateLastPing(id NodeID, instance time.Time) error {
return db.storeInt64(makeKey(id, nodeDBDiscoverPing), instance.Unix())
}

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

// updateLastPong updates the last time a remote node successfully contacted.
func (db *nodeDB) updateLastPong(id NodeID, instance time.Time) error {
// hasBond reports whether the given node is considered bonded.
func (db *nodeDB) hasBond(id NodeID) bool {
return time.Since(db.bondTime(id)) < nodeDBNodeExpiration
}

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

Expand Down Expand Up @@ -327,7 +332,7 @@ seek:
if n.ID == db.self {
continue seek
}
if now.Sub(db.lastPong(n.ID)) > maxAge {
if now.Sub(db.bondTime(n.ID)) > maxAge {
continue seek
}
for i := range nodes {
Expand Down
18 changes: 9 additions & 9 deletions p2p/discover/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ func TestNodeDBFetchStore(t *testing.T) {
t.Errorf("ping: value mismatch: have %v, want %v", stored, inst)
}
// Check fetch/store operations on a node pong object
if stored := db.lastPong(node.ID); stored.Unix() != 0 {
if stored := db.bondTime(node.ID); stored.Unix() != 0 {
t.Errorf("pong: non-existing object: %v", stored)
}
if err := db.updateLastPong(node.ID, inst); err != nil {
if err := db.updateBondTime(node.ID, inst); err != nil {
t.Errorf("pong: failed to update: %v", err)
}
if stored := db.lastPong(node.ID); stored.Unix() != inst.Unix() {
if stored := db.bondTime(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 @@ -224,8 +224,8 @@ func TestNodeDBSeedQuery(t *testing.T) {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to insert lastPong: %v", i, err)
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to insert bondTime: %v", i, err)
}
}

Expand Down Expand Up @@ -332,8 +332,8 @@ func TestNodeDBExpiration(t *testing.T) {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update pong: %v", i, err)
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update bondTime: %v", i, err)
}
}
// Expire some of them, and check the rest
Expand Down Expand Up @@ -365,8 +365,8 @@ func TestNodeDBSelfExpiration(t *testing.T) {
if err := db.updateNode(seed.node); err != nil {
t.Fatalf("node %d: failed to insert: %v", i, err)
}
if err := db.updateLastPong(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update pong: %v", i, err)
if err := db.updateBondTime(seed.node.ID, seed.pong); err != nil {
t.Fatalf("node %d: failed to update bondTime: %v", i, err)
}
}
// Expire the nodes and make sure self has been evacuated too
Expand Down
6 changes: 3 additions & 3 deletions p2p/discover/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func (tab *Table) loadSeedNodes(bond bool) {
}
for i := range seeds {
seed := seeds[i]
age := log.Lazy{Fn: func() interface{} { return time.Since(tab.db.lastPong(seed.ID)) }}
age := log.Lazy{Fn: func() interface{} { return time.Since(tab.db.bondTime(seed.ID)) }}
log.Debug("Found seed node in database", "id", seed.ID, "addr", seed.addr(), "age", age)
tab.add(seed)
}
Expand Down Expand Up @@ -596,7 +596,7 @@ func (tab *Table) bond(pinged bool, id NodeID, addr *net.UDPAddr, tcpPort uint16
}
// Start bonding if we haven't seen this node for a while or if it failed findnode too often.
node, fails := tab.db.node(id), tab.db.findFails(id)
age := time.Since(tab.db.lastPong(id))
age := time.Since(tab.db.bondTime(id))
var result error
if fails > 0 || age > nodeDBNodeExpiration {
log.Trace("Starting bonding ping/pong", "id", id, "known", node != nil, "failcount", fails, "age", age)
Expand Down Expand Up @@ -663,7 +663,7 @@ func (tab *Table) ping(id NodeID, addr *net.UDPAddr) error {
if err := tab.net.ping(id, addr); err != nil {
return err
}
tab.db.updateLastPong(id, time.Now())
tab.db.updateBondTime(id, time.Now())
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion p2p/discover/udp.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ func (req *findnode) handle(t *udp, from *net.UDPAddr, fromID NodeID, mac []byte
if expired(req.Expiration) {
return errExpired
}
if age := time.Since(t.db.lastPong(fromID)); age > nodeDBNodeExpiration {
if !t.db.hasBond(fromID) {
// No bond exists, we don't process the packet. This prevents
// an attack vector where the discovery protocol could be used
// to amplify traffic in a DDOS attack. A malicious actor
Expand Down
8 changes: 2 additions & 6 deletions p2p/discover/udp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,8 @@ func TestUDP_findnode(t *testing.T) {

// ensure there's a bond with the test node,
// findnode won't be accepted otherwise.
test.table.db.updateNode(NewNode(
PubkeyID(&test.remotekey.PublicKey),
test.remoteaddr.IP,
uint16(test.remoteaddr.Port),
99,
))
test.table.db.updateBondTime(PubkeyID(&test.remotekey.PublicKey), time.Now())

// check that closest neighbors are returned.
test.packetIn(nil, findnodePacket, &findnode{Target: testTarget, Expiration: futureExp})
expected := test.table.closest(targetHash, bucketSize)
Expand Down

0 comments on commit aeedec4

Please sign in to comment.