From de6babb67a06a0b33ef098285d92d5a7786196eb Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 18 Sep 2025 05:40:24 +0200 Subject: [PATCH 1/3] feat: add CheckIfPinnedWithType for efficient pin checks adds CheckIfPinnedWithType method to Pinner interface that allows checking specific pin types with optional name loading. this enables efficient pin operations like 'ipfs pin ls --names' without loading all pins. - CheckIfPinned now delegates to CheckIfPinnedWithType for consistency necessary for https://github.com/ipfs/js-kubo-rpc-client/pull/343 --- pinning/pinner/dspinner/pin.go | 222 +++++++++++++++++++++++++++------ pinning/pinner/pin.go | 23 +++- 2 files changed, 202 insertions(+), 43 deletions(-) diff --git a/pinning/pinner/dspinner/pin.go b/pinning/pinner/dspinner/pin.go index 3ba10b297..b1b19cf8e 100644 --- a/pinning/pinner/dspinner/pin.go +++ b/pinning/pinner/dspinner/pin.go @@ -585,64 +585,204 @@ func (p *pinner) isPinnedWithType(ctx context.Context, c cid.Cid, mode ipfspinne // // TODO: If a CID is pinned by multiple pins, should they all be reported? func (p *pinner) CheckIfPinned(ctx context.Context, cids ...cid.Cid) ([]ipfspinner.Pinned, error) { - pinned := make([]ipfspinner.Pinned, 0, len(cids)) - toCheck := cid.NewSet() + // Simply delegate to CheckIfPinnedWithType with Any mode and no names + return p.CheckIfPinnedWithType(ctx, ipfspinner.Any, false, cids...) +} +// loadPinName attempts to load the pin name if includeNames is true. +// It logs errors but doesn't fail the operation if name loading fails. +func (p *pinner) loadPinName(ctx context.Context, pin *ipfspinner.Pinned, pinID string, includeNames bool) { + if !includeNames { + return + } + pinData, err := p.loadPin(ctx, pinID) + if err != nil { + log.Errorf("failed to load pin %s: %v", pinID, err) + return + } + pin.Name = pinData.Name +} + +// CheckIfPinnedWithType implements the Pinner interface, checking specific pin types. +// This method is optimized to only check the requested pin type(s). +func (p *pinner) CheckIfPinnedWithType(ctx context.Context, mode ipfspinner.Mode, includeNames bool, cids ...cid.Cid) ([]ipfspinner.Pinned, error) { p.lock.RLock() defer p.lock.RUnlock() - // First check for non-Indirect pins directly - for _, c := range cids { - cidKey := c.KeyString() - has, err := p.cidRIndex.HasAny(ctx, cidKey) - if err != nil { - return nil, err - } - if has { - pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.Recursive}) - } else { - has, err = p.cidDIndex.HasAny(ctx, cidKey) + switch mode { + case ipfspinner.Any: + // Check all pin types + pinned := make([]ipfspinner.Pinned, 0, len(cids)) + toCheck := cid.NewSet() + + // First check for non-Indirect pins directly + for _, c := range cids { + cidKey := c.KeyString() + + // Check recursive pins + ids, err := p.cidRIndex.Search(ctx, cidKey) if err != nil { return nil, err } - if has { - pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.Direct}) + if len(ids) > 0 { + pin := ipfspinner.Pinned{Key: c, Mode: ipfspinner.Recursive} + p.loadPinName(ctx, &pin, ids[0], includeNames) + pinned = append(pinned, pin) } else { - toCheck.Add(c) + // Check direct pins + ids, err = p.cidDIndex.Search(ctx, cidKey) + if err != nil { + return nil, err + } + if len(ids) > 0 { + pin := ipfspinner.Pinned{Key: c, Mode: ipfspinner.Direct} + p.loadPinName(ctx, &pin, ids[0], includeNames) + pinned = append(pinned, pin) + } else { + toCheck.Add(c) + } + } + } + + // Check for indirect pins + if toCheck.Len() > 0 { + var walkErr error + visited := cid.NewSet() + err := p.cidRIndex.ForEach(ctx, "", func(key, value string) bool { + var rk cid.Cid + rk, walkErr = cid.Cast([]byte(key)) + if walkErr != nil { + return false + } + walkErr = merkledag.Walk(ctx, merkledag.GetLinksWithDAG(p.dserv), rk, func(c cid.Cid) bool { + if toCheck.Len() == 0 || !visited.Visit(c) { + return false + } + if toCheck.Has(c) { + pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.Indirect, Via: rk}) + toCheck.Remove(c) + } + return true + }, merkledag.Concurrent()) + if walkErr != nil { + return false + } + return toCheck.Len() > 0 + }) + if err != nil { + return nil, err } + if walkErr != nil { + return nil, walkErr + } + } + + // Anything left in toCheck is not pinned + for _, k := range toCheck.Keys() { + pinned = append(pinned, ipfspinner.Pinned{Key: k, Mode: ipfspinner.NotPinned}) + } + return pinned, nil + + case ipfspinner.Recursive, ipfspinner.Direct: + // Check only the specific index + return p.checkPinsInIndex(ctx, mode, includeNames, cids...) + + case ipfspinner.Indirect: + // Only check for indirect pins (expensive - requires traversal of all recursive pins' graphs) + return p.checkIndirectPins(ctx, cids...) + + case ipfspinner.Internal: + // Internal pins are not exposed to users, return NotPinned + // Note: this is legacy behavior kept for backward-compatibility + pinned := make([]ipfspinner.Pinned, 0, len(cids)) + for _, c := range cids { + pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.NotPinned}) } + return pinned, nil + + default: + // For unknown modes, return an error to maintain backward compatibility + return nil, fmt.Errorf( + "invalid Pin Mode '%d', must be one of {%d, %d, %d, %d, %d}", + mode, ipfspinner.Direct, ipfspinner.Indirect, ipfspinner.Recursive, + ipfspinner.Internal, ipfspinner.Any) } +} - var e error - visited := cid.NewSet() - err := p.cidRIndex.ForEach(ctx, "", func(key, value string) bool { - var rk cid.Cid - rk, e = cid.Cast([]byte(key)) - if e != nil { - return false +// checkPinsInIndex is a helper that checks for pins in a specific index based on mode (pin type). +// It selects either the recursive or direct index depending on the mode parameter. +func (p *pinner) checkPinsInIndex(ctx context.Context, mode ipfspinner.Mode, includeNames bool, cids ...cid.Cid) ([]ipfspinner.Pinned, error) { + pinned := make([]ipfspinner.Pinned, 0, len(cids)) + + // Select the appropriate index based on mode (pin type) + var index dsindex.Indexer + if mode == ipfspinner.Recursive { + index = p.cidRIndex + } else { + index = p.cidDIndex + } + + for _, c := range cids { + cidKey := c.KeyString() + ids, err := index.Search(ctx, cidKey) + if err != nil { + return nil, err + } + + if len(ids) > 0 { + pin := ipfspinner.Pinned{Key: c, Mode: mode} + p.loadPinName(ctx, &pin, ids[0], includeNames) + pinned = append(pinned, pin) + } else { + pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.NotPinned}) } - e = merkledag.Walk(ctx, merkledag.GetLinksWithDAG(p.dserv), rk, func(c cid.Cid) bool { - if toCheck.Len() == 0 || !visited.Visit(c) { + } + + return pinned, nil +} + +// checkIndirectPins checks if the given cids are pinned indirectly +func (p *pinner) checkIndirectPins(ctx context.Context, cids ...cid.Cid) ([]ipfspinner.Pinned, error) { + pinned := make([]ipfspinner.Pinned, 0, len(cids)) + toCheck := cid.NewSet() + + // Check all CIDs for indirect pins, regardless of their direct pin status + // A CID can be both directly pinned AND indirectly pinned through a parent + for _, c := range cids { + toCheck.Add(c) + } + + // Now check for indirect pins by traversing recursive pins + if toCheck.Len() > 0 { + var walkErr error + visited := cid.NewSet() + err := p.cidRIndex.ForEach(ctx, "", func(key, value string) bool { + var rk cid.Cid + rk, walkErr = cid.Cast([]byte(key)) + if walkErr != nil { return false } - - if toCheck.Has(c) { - pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.Indirect, Via: rk}) - toCheck.Remove(c) + walkErr = merkledag.Walk(ctx, merkledag.GetLinksWithDAG(p.dserv), rk, func(c cid.Cid) bool { + if toCheck.Len() == 0 || !visited.Visit(c) { + return false + } + if toCheck.Has(c) { + pinned = append(pinned, ipfspinner.Pinned{Key: c, Mode: ipfspinner.Indirect, Via: rk}) + toCheck.Remove(c) + } + return true + }, merkledag.Concurrent()) + if walkErr != nil { + return false } - - return true - }, merkledag.Concurrent()) - if e != nil { - return false + return toCheck.Len() > 0 + }) + if err != nil { + return nil, err + } + if walkErr != nil { + return nil, walkErr } - return toCheck.Len() > 0 - }) - if err != nil { - return nil, err - } - if e != nil { - return nil, e } // Anything left in toCheck is not pinned diff --git a/pinning/pinner/pin.go b/pinning/pinner/pin.go index b5b2fd187..efcb1c771 100644 --- a/pinning/pinner/pin.go +++ b/pinning/pinner/pin.go @@ -108,10 +108,29 @@ type Pinner interface { // old one Update(ctx context.Context, from, to cid.Cid, unpin bool) error - // Check if a set of keys are pinned, more efficient than - // calling IsPinned for each key + // CheckIfPinned checks if the given cids are pinned, returning their pin types. + // This is more efficient than calling IsPinned for each key. + // Note: The returned Pinned structs do not include pin names, which makes this + // method fast as it avoids loading pin metadata from the datastore. CheckIfPinned(ctx context.Context, cids ...cid.Cid) ([]Pinned, error) + // CheckIfPinnedWithType checks if the given cids are pinned with a specific pin type. + // This is more efficient than CheckIfPinned when you only need to check specific pin types. + // + // The mode parameter specifies which pin type(s) to check: + // - Recursive: Only checks the recursive pin index + // - Direct: Only checks the direct pin index + // - Indirect: Only checks for indirect pins (requires graph traversal) + // - Any: Checks all pin types (equivalent to CheckIfPinned) + // + // The includeNames parameter controls whether pin names are loaded: + // - false: Returns pin types only (fast, uses only indexes) + // - true: Also loads pin names from datastore (slower, requires additional reads) + // + // This method is particularly useful for filtered pin listings (e.g., ipfs pin ls --type=recursive) + // as it avoids checking unnecessary indexes and graph traversals. + CheckIfPinnedWithType(ctx context.Context, mode Mode, includeNames bool, cids ...cid.Cid) ([]Pinned, error) + // PinWithMode is for manually editing the pin structure. Use with // care! If used improperly, garbage collection may not be // successful. From eb966a7d7fa93e858492fce62b9156a7f2ec5395 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 18 Sep 2025 05:43:19 +0200 Subject: [PATCH 2/3] docs: add changelog entry for CheckIfPinnedWithType --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 805bbe884..e98cd152d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The following emojis are used to highlight certain changes: ### Added +- `pinning/pinner`: Added `CheckIfPinnedWithType` method to `Pinner` interface for efficient type-specific pin checks with optional name loading ([#1035](https://github.com/ipfs/boxo/pull/1035)) + - Enables checking specific pin types (recursive, direct, indirect) without loading all pins + - Optional `includeNames` parameter controls whether pin names are loaded from datastore + - `CheckIfPinned` now delegates to `CheckIfPinnedWithType` for consistency - `gateway`: Enhanced error handling and UX for timeouts: - Added retrieval state tracking for timeout diagnostics. When retrieval timeouts occur, the error messages now include detailed information about which phase failed (path resolution, provider discovery, connecting, or data retrieval) and provider statistics including failed peer IDs [#1015](https://github.com/ipfs/boxo/pull/1015) [#1023](https://github.com/ipfs/boxo/pull/1023) - Added `Config.DiagnosticServiceURL` to configure a CID retrievability diagnostic service. When set, 504 Gateway Timeout errors show a "Check CID retrievability" button linking to the service with `?cid=` [#1023](https://github.com/ipfs/boxo/pull/1023) From 634cc189f022284fd6f150d4d01e3e2039b56b97 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 18 Sep 2025 05:54:04 +0200 Subject: [PATCH 3/3] test: add tests for CheckIfPinnedWithType tests cover: - type-specific pin checking (direct, recursive, indirect) - pin name loading with includeNames flag - multiple CIDs in single call - edge cases (not pinned, internal mode, invalid mode) - backward compatibility with CheckIfPinned --- pinning/pinner/dspinner/pin_withtype_test.go | 158 +++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 pinning/pinner/dspinner/pin_withtype_test.go diff --git a/pinning/pinner/dspinner/pin_withtype_test.go b/pinning/pinner/dspinner/pin_withtype_test.go new file mode 100644 index 000000000..a53809260 --- /dev/null +++ b/pinning/pinner/dspinner/pin_withtype_test.go @@ -0,0 +1,158 @@ +package dspinner + +import ( + "context" + "testing" + + bs "github.com/ipfs/boxo/blockservice" + blockstore "github.com/ipfs/boxo/blockstore" + offline "github.com/ipfs/boxo/exchange/offline" + mdag "github.com/ipfs/boxo/ipld/merkledag" + ipfspin "github.com/ipfs/boxo/pinning/pinner" + cid "github.com/ipfs/go-cid" + ds "github.com/ipfs/go-datastore" + dssync "github.com/ipfs/go-datastore/sync" + "github.com/stretchr/testify/require" +) + +func TestCheckIfPinnedWithType(t *testing.T) { + const ( + withNames = true + withoutNames = false + ) + + ctx := context.Background() + dstore := dssync.MutexWrap(ds.NewMapDatastore()) + bstore := blockstore.NewBlockstore(dstore) + bserv := bs.New(bstore, offline.Exchange(bstore)) + dserv := mdag.NewDAGService(bserv) + + p, err := New(ctx, dstore, dserv) + require.NoError(t, err) + + // Create test DAG structure: + // aNode (recursive pin) -> bNode (also direct pin) + // cNode (not pinned) + + // Create child node b - will be both directly and indirectly pinned + bNode := mdag.NodeWithData([]byte("b data")) + err = dserv.Add(ctx, bNode) + require.NoError(t, err) + bk := bNode.Cid() + + // Create unrelated node c - won't be pinned + cNode := mdag.NodeWithData([]byte("c data")) + err = dserv.Add(ctx, cNode) + require.NoError(t, err) + ck := cNode.Cid() + + // Create parent node a that links to b + aNode := mdag.NodeWithData([]byte("a data")) + err = aNode.AddNodeLink("child", bNode) + require.NoError(t, err) + err = dserv.Add(ctx, aNode) + require.NoError(t, err) + ak := aNode.Cid() + + // Pin bNode directly with a name + p.PinWithMode(ctx, bk, ipfspin.Direct, "direct-pin-b") + err = p.Flush(ctx) + require.NoError(t, err) + + // Pin aNode recursively with a name (this makes bNode also indirectly pinned) + p.PinWithMode(ctx, ak, ipfspin.Recursive, "recursive-pin-a") + err = p.Flush(ctx) + require.NoError(t, err) + + t.Run("Direct mode with names", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Direct, withNames, bk) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.Direct, pinned[0].Mode) + require.Equal(t, "direct-pin-b", pinned[0].Name) + }) + + t.Run("Direct mode without names", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Direct, withoutNames, bk) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.Direct, pinned[0].Mode) + require.Empty(t, pinned[0].Name) + }) + + t.Run("Recursive mode with names", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Recursive, withNames, ak) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.Recursive, pinned[0].Mode) + require.Equal(t, "recursive-pin-a", pinned[0].Name) + }) + + t.Run("Indirect mode - bk should be indirect via ak", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Indirect, withoutNames, bk) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.Indirect, pinned[0].Mode) + require.Equal(t, ak, pinned[0].Via) + }) + + t.Run("Any mode - direct takes precedence", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Any, withNames, bk) + require.NoError(t, err) + require.Len(t, pinned, 1) + // bk is pinned both directly and indirectly, but direct takes precedence + require.Equal(t, ipfspin.Direct, pinned[0].Mode) + require.Equal(t, "direct-pin-b", pinned[0].Name) + }) + + t.Run("Not pinned", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Direct, withoutNames, ck) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.NotPinned, pinned[0].Mode) + }) + + t.Run("Multiple CIDs", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Any, withNames, ak, bk, ck) + require.NoError(t, err) + require.Len(t, pinned, 3) + + // Check results for each CID + results := make(map[cid.Cid]ipfspin.Pinned) + for _, p := range pinned { + results[p.Key] = p + } + + require.Equal(t, ipfspin.Recursive, results[ak].Mode) + require.Equal(t, "recursive-pin-a", results[ak].Name) + + require.Equal(t, ipfspin.Direct, results[bk].Mode) + require.Equal(t, "direct-pin-b", results[bk].Name) + + require.Equal(t, ipfspin.NotPinned, results[ck].Mode) + }) + + t.Run("Internal mode returns NotPinned", func(t *testing.T) { + pinned, err := p.CheckIfPinnedWithType(ctx, ipfspin.Internal, withoutNames, bk) + require.NoError(t, err) + require.Len(t, pinned, 1) + require.Equal(t, ipfspin.NotPinned, pinned[0].Mode) + }) + + t.Run("Invalid mode returns error", func(t *testing.T) { + _, err := p.CheckIfPinnedWithType(ctx, ipfspin.Mode(99), withoutNames, bk) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid Pin Mode") + }) + + t.Run("CheckIfPinned delegates to CheckIfPinnedWithType", func(t *testing.T) { + // CheckIfPinned should behave like CheckIfPinnedWithType with Any mode and no names + pinned1, err := p.CheckIfPinned(ctx, ak, bk, ck) + require.NoError(t, err) + + pinned2, err := p.CheckIfPinnedWithType(ctx, ipfspin.Any, withoutNames, ak, bk, ck) + require.NoError(t, err) + + require.Equal(t, pinned1, pinned2) + }) +}