fix multiple TTL bugs

The first fix independently extends the address expiration time and the address
TTL:

By example:

* We have an address with a TTL of 4s that will expire in 1s.
* We update it with a TTL of 3s.

Before this change:

* We end up with an address with a TTL of 3s that will expire in 3s.

After this change:

* We end up with an address with a TTL of 4s that will expire in 3s.

---

The second fix prevents the in-memory addressbook from announcing existing
addresses every time their TTLs get updated.

---

The third fix correctly updates TTLs for existing addresses in the on-disk
addressbook.

This fixes https://github.com/libp2p/go-libp2p-identify/issues/2
This commit is contained in:
Steven Allen 2019-07-24 16:29:51 -07:00
parent ad0faef7c7
commit 2fb6d7a48f
3 changed files with 52 additions and 8 deletions

View File

@ -347,11 +347,21 @@ Outer:
for _, have := range pr.Addrs {
if incoming.Equal(have.Addr) {
existed[i] = true
if mode == ttlExtend && have.Expiry > newExp {
// if we're only extending TTLs but the addr already has a longer one, we skip it.
continue Outer
switch mode {
case ttlOverride:
have.Ttl = int64(ttl)
have.Expiry = newExp
case ttlExtend:
if int64(ttl) > have.Ttl {
have.Ttl = int64(ttl)
}
if newExp > have.Expiry {
have.Expiry = newExp
}
default:
panic("BUG: unimplemented ttl mode")
}
have.Expiry = newExp
// we found the address, and addresses cannot be duplicate,
// so let's move on to the next.
continue Outer

View File

@ -132,7 +132,7 @@ func (mab *memoryAddrBook) AddAddr(p peer.ID, addr ma.Multiaddr, ttl time.Durati
// AddAddrs gives memoryAddrBook addresses to use, with a given ttl
// (time-to-live), after which the address is no longer valid.
// If the manager has a longer TTL, the operation is a no-op for that address
// This function never reduces the TTL or expiration of an address.
func (mab *memoryAddrBook) AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Duration) {
// if ttl is zero, exit. nothing to do.
if ttl <= 0 {
@ -156,10 +156,19 @@ func (mab *memoryAddrBook) AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Du
}
asBytes := addr.Bytes()
a, found := amap[string(asBytes)] // won't allocate.
if !found || exp.After(a.Expires) {
if !found {
// not found, save and announce it.
amap[string(asBytes)] = &expiringAddr{Addr: addr, Expires: exp, TTL: ttl}
mab.subManager.BroadcastAddr(p, addr)
} else {
// Update expiration/TTL independently.
// We never want to reduce either.
if ttl > a.TTL {
a.TTL = ttl
}
if exp.After(a.Expires) {
a.Expires = exp
}
}
}
}

View File

@ -86,9 +86,13 @@ func testAddAddress(ab pstore.AddrBook) func(*testing.T) {
// after the initial TTL has expired, check that only the third address is present.
time.Sleep(1200 * time.Millisecond)
AssertAddressesEqual(t, addrs[2:], ab.Addrs(id))
// make sure we actually set the TTL
ab.UpdateAddrs(id, time.Hour, 0)
AssertAddressesEqual(t, nil, ab.Addrs(id))
})
t.Run("adding an existing address with an earlier expiration is noop", func(t *testing.T) {
t.Run("adding an existing address with an earlier expiration never reduces the expiration", func(t *testing.T) {
id := GeneratePeerIDs(1)[0]
addrs := GenerateAddrs(3)
@ -102,6 +106,27 @@ func testAddAddress(ab pstore.AddrBook) func(*testing.T) {
time.Sleep(2100 * time.Millisecond)
AssertAddressesEqual(t, addrs, ab.Addrs(id))
})
t.Run("adding an existing address with an earlier expiration never reduces the TTL", func(t *testing.T) {
id := GeneratePeerIDs(1)[0]
addrs := GenerateAddrs(1)
ab.AddAddrs(id, addrs, 4*time.Second)
// 4 seconds left
time.Sleep(3 * time.Second)
// 1 second left
ab.AddAddrs(id, addrs, 3*time.Second)
// 3 seconds left
time.Sleep(2)
// 1 seconds left.
// We still have the address.
AssertAddressesEqual(t, addrs, ab.Addrs(id))
// The TTL wasn't reduced
ab.UpdateAddrs(id, 4*time.Second, 0)
AssertAddressesEqual(t, nil, ab.Addrs(id))
})
}
}