From 9de61d08d6c474a6398cf06eb798f9b4db02a274 Mon Sep 17 00:00:00 2001 From: Al Cutter Date: Fri, 18 Sep 2026 13:53:16 +0000 Subject: [PATCH 1/3] Tighten client nodeCache validation. nodeCache was very relaxed in terms of the node IDs it would serve and the tiles it would accept from a fetcher. This PR tightens both, and removes a potential correctness issue. * GetNode explicitly rejects out-of-range nodes. * fetchTileNodes now asserts that a tile is either the precise partial size requested, or a full tile (i.e. the fetcher fell-back to requesting a full tile) * Removed call to GetRootHash. This was unnecessary, but also incorrectly populated the cache with ephemeral nodes. --- client/client.go | 25 ++++++++++++++++++++----- client/client_test.go | 4 +--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/client/client.go b/client/client.go index 4e9b61555..3e6698e27 100644 --- a/client/client.go +++ b/client/client.go @@ -462,6 +462,9 @@ func newNodeCache(f TileFetcherFunc, logSize uint64) *nodeCache { func (n *nodeCache) GetNode(ctx context.Context, id compact.NodeID) ([]byte, error) { return otel.Trace(ctx, "tessera.client.nodecache.GetNode", tracer, func(ctx context.Context, span trace.Span) ([]byte, error) { span.SetAttributes(indexKey.Int64(otel.Clamp64(id.Index)), levelKey.Int64(int64(id.Level))) + if id.Index >= n.logSize>>id.Level { + return nil, fmt.Errorf("node %+v out of range for log of size %d", id, n.logSize) + } // Fast-path: check to see we have this node in the cache and return it directly if so, otherwise we'll need to fetch it. if e, ok := n.nodes.Get(id); ok { return e, nil @@ -483,7 +486,7 @@ func (n *nodeCache) GetNode(ctx context.Context, id compact.NodeID) ([]byte, err p := layout.PartialTileSize(tileLevel, tileIndex, n.logSize) nodes, err := n.fetchTileNodes(ctx, tileLevel, tileIndex, p) if err != nil { - return nil, fmt.Errorf("failed to fetch and populate node cache: %v", err) + return nil, fmt.Errorf("failed to fetch and populate node cache: %w", err) } for k, v := range nodes { n.nodes.Add(k, v) @@ -528,7 +531,22 @@ func (n *nodeCache) fetchTileNodes(ctx context.Context, tileLevel, tileIndex uin return nil, fmt.Errorf("failed to parse tile: %v", err) } - ret := make(map[compact.NodeID][]byte, 256*2-1) + wantSize := layout.TileWidth + if p > 0 { + wantSize = int(p) + } + switch gotLen := len(tile.Nodes); gotLen { + case wantSize: + // We got the exact size tile we asked for, nothing extra to do. + case layout.TileWidth: + // Must have asked for a partial tile and got a full tile back. + // Trim the full tile down to the size of the requested partial tile. + tile.Nodes = tile.Nodes[:wantSize] + default: + return nil, fmt.Errorf("invalid tile: expected %d or %d nodes, got %d", wantSize, layout.TileWidth, gotLen) + } + + ret := make(map[compact.NodeID][]byte, wantSize*2-1) // visitFn is a visitor callback which populates the nodes cache. // Used by the calls to compact range below. visitFn := func(intID compact.NodeID, h []byte) { @@ -546,9 +564,6 @@ func (n *nodeCache) fetchTileNodes(ctx context.Context, tileLevel, tileIndex uin return nil, fmt.Errorf("failed to Append: %v", err) } } - if _, err := r.GetRootHash(visitFn); err != nil { - return nil, fmt.Errorf("failed to visit all nodes: %v", err) - } return ret, nil }) } diff --git a/client/client_test.go b/client/client_test.go index fdfb55426..bc8958ba3 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -208,9 +208,7 @@ func TestNodeCacheHandlesInvalidRequest(t *testing.T) { return h.MarshalText() } - // Large tree, but we're emulating skew since f, above, will return a tile which only knows about 1 - // leaf. - nc := newNodeCache(f, 10) + nc := newNodeCache(f, 1) if got, err := nc.GetNode(ctx, compact.NewNodeID(0, 0)); err != nil { t.Errorf("got %v, want no error", err) From 5c155c58b3babaf5f02ec7afc22448d0315c6fc1 Mon Sep 17 00:00:00 2001 From: Al Cutter Date: Fri, 18 Sep 2026 14:14:13 +0000 Subject: [PATCH 2/3] Add tests --- client/client_test.go | 108 +++++++++++++++++++++++++++++++++++------- 1 file changed, 91 insertions(+), 17 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index bc8958ba3..7377cb4c7 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -198,26 +198,100 @@ func TestCheckLogStateTracker(t *testing.T) { } } -func TestNodeCacheHandlesInvalidRequest(t *testing.T) { +func TestNodeCache(t *testing.T) { ctx := context.Background() - wantBytes := []byte("0123456789ABCDEF0123456789ABCDEF") - f := func(_ context.Context, _, _ uint64, _ uint8) ([]byte, error) { - h := &api.HashTile{ - Nodes: [][]byte{wantBytes}, - } - return h.MarshalText() - } + leafHash := []byte("0123456789ABCDEF0123456789ABCDEF") - nc := newNodeCache(f, 1) - - if got, err := nc.GetNode(ctx, compact.NewNodeID(0, 0)); err != nil { - t.Errorf("got %v, want no error", err) - } else if !bytes.Equal(got, wantBytes) { - t.Errorf("got %v, want %v", got, wantBytes) - } + for _, test := range []struct { + desc string + logSize uint64 + tileNodes int + reqID compact.NodeID + wantErr bool + wantCacheSize int + }{ + { + desc: "valid single leaf", + logSize: 1, + tileNodes: 1, + reqID: compact.NewNodeID(0, 0), + wantCacheSize: 1, + }, + { + desc: "out of range leaf in same tile", + logSize: 1, + tileNodes: 1, + reqID: compact.NewNodeID(0, 1), + wantErr: true, + }, + { + desc: "out of range leaf in next tile", + logSize: layout.TileWidth, + tileNodes: layout.TileWidth, + reqID: compact.NewNodeID(0, layout.TileWidth), + wantErr: true, + }, + { + desc: "out of range high level overflow check", + logSize: layout.TileWidth, + tileNodes: layout.TileWidth, + reqID: compact.NewNodeID(64, 0), + wantErr: true, + }, + { + desc: "truncated partial tile", + logSize: 10, + tileNodes: 1, + reqID: compact.NewNodeID(0, 0), + wantErr: true, + }, + { + desc: "truncated full tile", + logSize: layout.TileWidth, + tileNodes: layout.TileWidth - 1, + reqID: compact.NewNodeID(0, 0), + wantErr: true, + }, + { + desc: "oversized tile", + logSize: layout.TileWidth, + tileNodes: layout.TileWidth + 1, + reqID: compact.NewNodeID(0, 0), + wantErr: true, + }, + { + desc: "partial tile fallback to full tile truncates and omits ephemeral nodes", + logSize: 3, + tileNodes: layout.TileWidth, + reqID: compact.NewNodeID(1, 0), + wantCacheSize: 4, // (0,0), (0,1), (1,0), (0,2) - no leaves >= 3 and no ephemeral (2,0) + }, + } { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + f := func(_ context.Context, _, _ uint64, _ uint8) ([]byte, error) { + nodes := make([][]byte, test.tileNodes) + for i := range nodes { + nodes[i] = leafHash + } + return (&api.HashTile{Nodes: nodes}).MarshalText() + } - if _, err := nc.GetNode(ctx, compact.NewNodeID(0, 1)); err == nil { - t.Error("got no error, want error because ID is out of range") + nc := newNodeCache(f, test.logSize) + got, err := nc.GetNode(ctx, test.reqID) + if gotErr := err != nil; gotErr != test.wantErr { + t.Fatalf("GetNode(%+v) err = %v, wantErr %t", test.reqID, err, test.wantErr) + } + if test.wantErr { + return + } + if len(got) == 0 { + t.Errorf("GetNode(%+v) returned empty hash", test.reqID) + } + if gotSize := nc.nodes.Len(); gotSize != test.wantCacheSize { + t.Errorf("cache size = %d, want %d", gotSize, test.wantCacheSize) + } + }) } } From 8c218555a8c641edc1b2e1a33841540b04d3be30 Mon Sep 17 00:00:00 2001 From: Al Cutter Date: Mon, 21 Sep 2026 10:41:36 +0000 Subject: [PATCH 3/3] Address comments --- client/client.go | 8 ++++++-- client/client_test.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/client.go b/client/client.go index 3e6698e27..af4d5bb52 100644 --- a/client/client.go +++ b/client/client.go @@ -523,7 +523,7 @@ func (n *nodeCache) fetchTileNodes(ctx context.Context, tileLevel, tileIndex uin return otel.Trace(ctx, "tessera.client.nodecache.fetchTileNodes", tracer, func(ctx context.Context, span trace.Span) (map[compact.NodeID][]byte, error) { tileRaw, err := n.getTile(ctx, tileLevel, tileIndex, p) if err != nil { - return nil, fmt.Errorf("failed to fetch tile: %v", err) + return nil, fmt.Errorf("failed to fetch tile: %w", err) } var tile api.HashTile @@ -543,7 +543,11 @@ func (n *nodeCache) fetchTileNodes(ctx context.Context, tileLevel, tileIndex uin // Trim the full tile down to the size of the requested partial tile. tile.Nodes = tile.Nodes[:wantSize] default: - return nil, fmt.Errorf("invalid tile: expected %d or %d nodes, got %d", wantSize, layout.TileWidth, gotLen) + additional := "" + if wantSize < layout.TileWidth { + additional = fmt.Sprintf(" or %d", layout.TileWidth) + } + return nil, fmt.Errorf("invalid tile: expected %d%s nodes, got %d", wantSize, additional, gotLen) } ret := make(map[compact.NodeID][]byte, wantSize*2-1) diff --git a/client/client_test.go b/client/client_test.go index 7377cb4c7..0ab6adac3 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -199,7 +199,7 @@ func TestCheckLogStateTracker(t *testing.T) { } func TestNodeCache(t *testing.T) { - ctx := context.Background() + ctx := t.Context() leafHash := []byte("0123456789ABCDEF0123456789ABCDEF") for _, test := range []struct {