add trustless IPFS gateway, use patched rclone, remove bitswap

[parkan]

remove the dead bitswap protocol and add an /ipfs/ endpoint implementing the trustless gateway spec via boxo/gateway

StorageBlockStore pools rclone handlers per storage and holds open a streaming file reader to serve sequential block reads efficiently -- DAG nodes return from the DB without storage I/O, and leaf blocks from the same file share a single held-open rclone stream

supports dag-scope=all/entity/block, path resolution, entity-bytes, format=car and format=raw. stale source files detected at read time are mapped to HTTP 409 via boxo's error status mechanism

[anjor]

I found 3 issues worth addressing in this PR:

1. handler/wallet/export_keys.go
migrateWalletAssignments skips unresolved legacy wallet_assignments rows, but still drops the table afterward. That makes the first export-keys run destructive for malformed keys, failed exports, or any actor that never gets a corresponding wallets row, because the preparation-wallet link is lost permanently. I think this should either fail when unresolved rows remain, or preserve the legacy table until every row has been migrated successfully.

2. store/storage_blockstore.go
   The cached s.active reader is shared across requests, but it was opened with the ctx from whichever request populated it. readerWithRetry stores that context and checks it on every read and reopen, so a later request can inherit a canceled or timed-out context from an earlier one and fail spuriously. Reusing the stream only within a single request, or decoupling the cached reader from request-scoped context, would avoid that.

3. store/storage_blockstore.go
   The mutex in readFileBlock is held across handler.Read, object validation, and the actual block read. That means one slow backend read serializes every /ipfs/ request through this process-wide blockstore. Under concurrent gateway traffic this looks like a single-reader bottleneck. I think the critical section should be narrowed to cache bookkeeping only, or moved to per-request state instead of a global lock.

[parkan]

I think (1) landed on the wrong PR (concerning export_keys.go), maybe some stale state on your end? should have been addressed in #650

concerns (2) and (3) are valid in specific concurrent traffic cases, mostly materializing as a throughput bottleneck across serialized reads (3) and (much less likely to happen) same-file reads with different expectations in (2); acknowledge both but don't think they strictly block the baseline functionality here

[parkan]

ok actually yes it's possible to inherit a cancelled context which is quite bad, working on a fix

[parkan]

ok, context ownership has been shifted to the blockstore from the request; this should decouple cross-request concerns

[parkan]

no idea why the CI got stuck on this, trying to manually run

[anjor]

Code review

Found 3 issues worth flagging (none are blockers, but all affect the gateway path):

1. Potential nil panic on carBlock.File in readFileBlock — CarBlock.File is a nullable pointer (commit c23b61c made it nullable for DAG-node blocks that have no source file). If a block has RawBlock == nil AND FileID == NULL (e.g., source file deleted, FK set to NULL), readFileBlock dereferences carBlock.File unconditionally and panics.

singularity/store/storage_blockstore.go

Lines 73 to 76 in 3198937

	storage := *carBlock.File.Attachment.Storage
	file := *carBlock.File
	blockLen := int64(carBlock.BlockLength())

2. Mutex held across slow I/O in readFileBlock — s.mu.Lock() is held from line 71 through handler.Read() (line 99) and io.ReadFull (via readFromActive, line 119). Under concurrent gateway traffic, all /ipfs/ requests serialize behind the slowest backend read. Acknowledged in prior discussion as non-blocking for baseline, but worth tracking.

singularity/store/storage_blockstore.go

Lines 70 to 125 in 3198937

	func (s *StorageBlockStore) readFileBlock(ctx context.Context, carBlock model.CarBlock, c cid.Cid) (blocks.Block, error) {
	s.mu.Lock()
	defer s.mu.Unlock()
	storage := *carBlock.File.Attachment.Storage
	file := *carBlock.File
	blockLen := int64(carBlock.BlockLength())
	handler, err := s.getHandler(ctx, storage)
	if err != nil {
	return nil, errors.WithStack(err)
	}
	// try to read from held-open stream
	if s.active != nil && s.active.fileID == file.ID && s.active.offset == carBlock.FileOffset {
	data, err := s.readFromActive(blockLen)
	if err != nil {
	s.closeActive()
	return nil, err
	}
	return blocks.NewBlockWithCid(data, c)
	}
	// different file or non-sequential offset -- close old, open new
	s.closeActive()
	// use a detached context for the cached reader so it outlives the
	// request that created it. closeActive() cancels it on cleanup.
	readerCtx, readerCancel := context.WithCancel(context.Background())
	reader, obj, err := handler.Read(readerCtx, file.Path, carBlock.FileOffset, file.Size-carBlock.FileOffset)
	if err != nil {
	readerCancel()
	return nil, errors.WithStack(err)
	}
	same, explanation := storagesystem.IsSameEntry(ctx, file, obj)
	if !same {
	reader.Close()
	readerCancel()
	return nil, errors.Wrap(ErrFileHasChanged, explanation)
	}
	s.active = &fileReader{
	fileID: file.ID,
	reader: reader,
	offset: carBlock.FileOffset,
	cancel: readerCancel,
	}
	data, err := s.readFromActive(blockLen)
	if err != nil {
	s.closeActive()
	return nil, err
	}
	return blocks.NewBlockWithCid(data, c)
	}

3. Pooled RCloneHandler instances never closed on shutdown — StorageBlockStore.Close() only cleans up the active file reader. The handlers map holding RClone filesystem instances (which can hold persistent S3/HTTP connections) has no shutdown path. The HTTP server also doesn't call Close() on the blockstore during Shutdown().

singularity/store/storage_blockstore.go

Lines 191 to 195 in 3198937

	func (s *StorageBlockStore) Close() {
	s.mu.Lock()
	defer s.mu.Unlock()
	s.closeActive()
	}

None of these block the baseline gateway functionality, but (1) is a crash risk worth guarding against.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[parkan]

nil guard and blockstore close added

mutex discussed above, accept for POC

re: closing rclone Fs, Shutdown() is an implementation detail and not consistently exposed by all backends, no Close() is exposed on any of them and they should be considered stateless even if in reality there's connection multiplexing etc -- can consider special casing stateful backends like SFTP but that's out of scope here

[anjor]

All three review items addressed — nil guard, blockstore close, and mutex accepted for POC. LGTM.