diff --git a/pstoreds/addr_book.go b/pstoreds/addr_book.go index bba648e..2f75265 100644 --- a/pstoreds/addr_book.go +++ b/pstoreds/addr_book.go @@ -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 diff --git a/pstoremem/addr_book.go b/pstoremem/addr_book.go index 19b60f9..90ffb17 100644 --- a/pstoremem/addr_book.go +++ b/pstoremem/addr_book.go @@ -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 + } } } } diff --git a/test/addr_book_suite.go b/test/addr_book_suite.go index 72a899c..ea6078c 100644 --- a/test/addr_book_suite.go +++ b/test/addr_book_suite.go @@ -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)) + }) } }