Skip to content

fix(zetaclient): snapshot telemetry peer data #4589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alan747271363-art wants to merge 3 commits into
base: main
Choose a base branch
from
+80 −9
Open

fix(zetaclient): snapshot telemetry peer data #4589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8abeda6
fix(zetaclient): snapshot telemetry peer data
alan747271363-art May 13, 2026
e44a59a
fix(zetaclient): isolate telemetry peer snapshots
alan747271363-art May 13, 2026
2f88a0e
fix(zetaclient): deep copy telemetry peer addresses
alan747271363-art May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions zetaclient/metrics/telemetry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/gorilla/mux"
"github.com/libp2p/go-libp2p/core/peer"
multiaddr "github.com/multiformats/go-multiaddr"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"

Expand Down Expand Up @@ -59,25 +60,49 @@ func NewTelemetryServer() *TelemetryServer {
func (t *TelemetryServer) SetPingRTT(rtt map[peer.ID]int64) {
t.mu.Lock()
defer t.mu.Unlock()
t.rtt = rtt
t.rtt = copyPingRTT(rtt)
}

func (t *TelemetryServer) GetPingRTT() map[peer.ID]int64 {
t.mu.Lock()
defer t.mu.Unlock()
return t.rtt
return copyPingRTT(t.rtt)
}

func (t *TelemetryServer) SetConnectedPeers(peers []peer.AddrInfo) {
t.mu.Lock()
defer t.mu.Unlock()
t.connectedPeers = peers
t.connectedPeers = copyConnectedPeers(peers)
}

func (t *TelemetryServer) GetConnectedPeers() []peer.AddrInfo {
t.mu.Lock()
defer t.mu.Unlock()
return t.connectedPeers
return copyConnectedPeers(t.connectedPeers)
}

func copyPingRTT(rtt map[peer.ID]int64) map[peer.ID]int64 {
rttCopy := make(map[peer.ID]int64, len(rtt))
for peerID, duration := range rtt {
rttCopy[peerID] = duration
}
return rttCopy
}

func copyConnectedPeers(peers []peer.AddrInfo) []peer.AddrInfo {
if peers == nil {
return nil
}

peersCopy := make([]peer.AddrInfo, len(peers))
for i, peerInfo := range peers {
peersCopy[i].ID = peerInfo.ID
if peerInfo.Addrs != nil {
peersCopy[i].Addrs = make([]multiaddr.Multiaddr, len(peerInfo.Addrs))
copy(peersCopy[i].Addrs, peerInfo.Addrs)
}
}
return peersCopy
}
Comment on lines +92 to 106

@greptile-apps greptile-apps Bot May 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 peer.AddrInfo has an Addrs []Multiaddr slice field that is not deep-copied here. append([]peer.AddrInfo(nil), peers...) copies each struct by value, so ID is isolated correctly, but the Addrs slice header in every copied element still shares the same backing array as the original. If any caller replaces an element in snapshot[i].Addrs, that change writes through to the stored slice. For the current HTTP-handler read pattern this is harmless, but it leaves the isolation guarantee incomplete compared to the intent of the rest of this PR.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: zetaclient/metrics/telemetry.go
Line: 91-93

Comment:
`peer.AddrInfo` has an `Addrs []Multiaddr` slice field that is not deep-copied here. `append([]peer.AddrInfo(nil), peers...)` copies each struct by value, so `ID` is isolated correctly, but the `Addrs` slice header in every copied element still shares the same backing array as the original. If any caller replaces an element in `snapshot[i].Addrs`, that change writes through to the stored slice. For the current HTTP-handler read pattern this is harmless, but it leaves the isolation guarantee incomplete compared to the intent of the rest of this PR.

How can I resolve this? If you propose a fix, please make it concise.

@alan747271363-art alan747271363-art May 19, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2f88a0e0: copyConnectedPeers now allocates a new peer.AddrInfo slice and deep-copies each non-nil Addrs slice, so the stored telemetry snapshot no longer shares address slice backing arrays with callers.


// SetP2PID sets p2pid
Expand Down
48 changes: 48 additions & 0 deletions zetaclient/metrics/telemetry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package metrics

import (
"testing"

"github.com/libp2p/go-libp2p/core/peer"
maddr "github.com/multiformats/go-multiaddr"
"github.com/stretchr/testify/require"
)

func TestTelemetryServerCopiesPingRTT(t *testing.T) {
telemetry := NewTelemetryServer()
peerID := peer.ID("peer-a")

rtt := map[peer.ID]int64{peerID: 10}
telemetry.SetPingRTT(rtt)

rtt[peerID] = 20
require.Equal(t, int64(10), telemetry.GetPingRTT()[peerID])

snapshot := telemetry.GetPingRTT()
snapshot[peerID] = 30
require.Equal(t, int64(10), telemetry.GetPingRTT()[peerID])
}

func TestTelemetryServerCopiesConnectedPeers(t *testing.T) {
telemetry := NewTelemetryServer()
peerA := peer.ID("peer-a")
peerB := peer.ID("peer-b")
peerC := peer.ID("peer-c")
addrA := maddr.StringCast("/ip4/127.0.0.1/tcp/1111")
addrB := maddr.StringCast("/ip4/127.0.0.1/tcp/2222")
addrC := maddr.StringCast("/ip4/127.0.0.1/tcp/3333")

peers := []peer.AddrInfo{{ID: peerA, Addrs: []maddr.Multiaddr{addrA}}}
telemetry.SetConnectedPeers(peers)

peers[0].ID = peerB
peers[0].Addrs[0] = addrB
require.Equal(t, peerA, telemetry.GetConnectedPeers()[0].ID)
require.Equal(t, addrA, telemetry.GetConnectedPeers()[0].Addrs[0])

snapshot := telemetry.GetConnectedPeers()
snapshot[0].ID = peerC
snapshot[0].Addrs[0] = addrC
require.Equal(t, peerA, telemetry.GetConnectedPeers()[0].ID)
require.Equal(t, addrA, telemetry.GetConnectedPeers()[0].Addrs[0])
}
8 changes: 3 additions & 5 deletions zetaclient/tss/healthcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,13 @@ func HealthcheckWorker(ctx context.Context, server *tss.Server, p HealthcheckPro
logger = logger.With().Str(logs.FieldModule, logs.ModNameTssHealthCheck).Logger()

// Ping & collect round trip time
var (
host = server.GetP2PHost()
pingRTT = make(map[peer.ID]int64)
mu = sync.Mutex{}
)
host := server.GetP2PHost()

const pingTimeout = 5 * time.Second

pinger := func(ctx context.Context, _ *ticker.Ticker) error {
pingRTT := make(map[peer.ID]int64)
mu := sync.Mutex{}
var wg sync.WaitGroup
for _, peerID := range p.WhitelistPeers {
if peerID == host.ID() {
Expand Down
Loading