diff --git a/CHANGELOG.md b/CHANGELOG.md index aae74ac26..aec181dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ The following emojis are used to highlight certain changes: - `bitswap/server`: incoming identity CIDs in wantlist messages are now silently ignored instead of killing the connection to the remote peer. Some IPFS implementations naively send identity CIDs, and disconnecting them for it caused unnecessary churn. [#1117](https://github.com/ipfs/boxo/pull/1117) - `bitswap/network`: `ExtractHTTPAddress` now infers default ports for portless HTTP multiaddrs (e.g. `/dns/host/https` without `/tcp/443`). [#1123](https://github.com/ipfs/boxo/pull/1123) +- `mfs`: preserve CidBuilder object in `setNodeData()`, `Mkdir()` and `NewRoot()`. [#1125](https://github.com/ipfs/boxo/pull/1125) ### Security diff --git a/mfs/dir.go b/mfs/dir.go index 1d9bf4379..dbe52cf37 100644 --- a/mfs/dir.go +++ b/mfs/dir.go @@ -370,6 +370,7 @@ func (d *Directory) ForEachEntry(ctx context.Context, f func(NodeListing) error) func (d *Directory) Mkdir(name string) (*Directory, error) { return d.MkdirWithOpts(name, MkdirOpts{ + CidBuilder: d.unixfsDir.GetCidBuilder(), MaxLinks: d.unixfsDir.GetMaxLinks(), MaxHAMTFanout: d.unixfsDir.GetMaxHAMTFanout(), HAMTShardingSize: d.unixfsDir.GetHAMTShardingSize(), @@ -392,10 +393,6 @@ func (d *Directory) MkdirWithOpts(name string, opts MkdirOpts) (*Directory, erro } } - // hector: no idea why this option is overridden, but it must be to - // keep backwards compatibility. CidBuilder from the options is - // manually set in `Mkdir` (ops.go) though. - opts.CidBuilder = d.GetCidBuilder() dirobj, err := NewEmptyDirectory(d.ctx, name, d, d.dagService, d.prov, opts) if err != nil { return nil, err diff --git a/mfs/file.go b/mfs/file.go index 547d196b2..78409c47d 100644 --- a/mfs/file.go +++ b/mfs/file.go @@ -272,6 +272,14 @@ func (fi *File) SetModTime(ts time.Time) error { func (fi *File) setNodeData(data []byte) error { nd := dag.NodeWithData(data) + + // Preserve previous node's CidBuilder + if oldNode, ok := fi.node.(*dag.ProtoNode); ok { + if builder := oldNode.CidBuilder(); builder != nil { + nd.SetCidBuilder(builder) + } + } + err := fi.dagService.Add(context.TODO(), nd) if err != nil { return err diff --git a/mfs/ops.go b/mfs/ops.go index 8a4401f8b..c4dcf5afe 100644 --- a/mfs/ops.go +++ b/mfs/ops.go @@ -175,6 +175,7 @@ func Mkdir(r *Root, pth string, opts MkdirOpts) error { // opts to make the parents leave MkParents and Flush as false. parentsOpts := MkdirOpts{ + CidBuilder: opts.CidBuilder, MaxLinks: opts.MaxLinks, MaxHAMTFanout: opts.MaxHAMTFanout, HAMTShardingSize: opts.HAMTShardingSize, @@ -190,11 +191,6 @@ func Mkdir(r *Root, pth string, opts MkdirOpts) error { if err != nil { return err } - // MkdirWithOps uses cur.GetCidBuilder regardless of - // the option. So we must set it manually. - if opts.CidBuilder != nil { - mkd.SetCidBuilder(opts.CidBuilder) - } fsn = mkd } else if err != nil { return err @@ -214,12 +210,6 @@ func Mkdir(r *Root, pth string, opts MkdirOpts) error { } } - // Again, MkdirWithOpts ignores opts.CidBuilder so must be applied - // here. - if opts.CidBuilder != nil { - final.SetCidBuilder(opts.CidBuilder) - } - if opts.Flush { err := final.Flush() if err != nil { diff --git a/mfs/root.go b/mfs/root.go index ef243cde6..7627bf924 100644 --- a/mfs/root.go +++ b/mfs/root.go @@ -10,7 +10,6 @@ import ( chunker "github.com/ipfs/boxo/chunker" dag "github.com/ipfs/boxo/ipld/merkledag" ft "github.com/ipfs/boxo/ipld/unixfs" - uio "github.com/ipfs/boxo/ipld/unixfs/io" "github.com/ipfs/boxo/provider" ipld "github.com/ipfs/go-ipld-format" logging "github.com/ipfs/go-log/v2" @@ -103,61 +102,10 @@ type Root struct { repub *Republisher prov provider.MultihashProvider chunker chunker.SplitterGen // chunker factory for files, nil means default - - // Directory settings to propagate to child directories when loaded from disk. - maxLinks int - maxHAMTFanout int - hamtShardingSize int - sizeEstimationMode *uio.SizeEstimationMode -} - -// RootOption is a functional option for configuring a Root. -type RootOption func(*Root) - -// WithChunker sets the chunker factory for files created in this MFS. -// If not set (or nil), chunker.DefaultSplitter is used. -func WithChunker(c chunker.SplitterGen) RootOption { - return func(r *Root) { - r.chunker = c - } -} - -// WithMaxLinks sets the max links for directories created in this MFS. -// This is used to determine when to switch between BasicDirectory and HAMTDirectory. -func WithMaxLinks(n int) RootOption { - return func(r *Root) { - r.maxLinks = n - } -} - -// WithSizeEstimationMode sets the size estimation mode for directories in this MFS. -// This controls how directory size is estimated for HAMT sharding decisions. -func WithSizeEstimationMode(mode uio.SizeEstimationMode) RootOption { - return func(r *Root) { - r.sizeEstimationMode = &mode - } -} - -// WithMaxHAMTFanout sets the max fanout (width) for HAMT directories in this MFS. -// This controls the maximum number of children per HAMT bucket. -// Must be a power of 2 and multiple of 8. -func WithMaxHAMTFanout(n int) RootOption { - return func(r *Root) { - r.maxHAMTFanout = n - } -} - -// WithHAMTShardingSize sets the per-directory size threshold for switching to HAMT. -// When a directory's estimated size exceeds this threshold, it converts to HAMT. -// If not set (0), the global uio.HAMTShardingSize is used. -func WithHAMTShardingSize(size int) RootOption { - return func(r *Root) { - r.hamtShardingSize = size - } } // NewRoot creates a new Root and starts up a republisher routine for it. -func NewRoot(ctx context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf PubFunc, prov provider.MultihashProvider, opts ...RootOption) (*Root, error) { +func NewRoot(ctx context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf PubFunc, prov provider.MultihashProvider, mkdirOpts MkdirOpts) (*Root, error) { var repub *Republisher if pf != nil { repub = NewRepublisher(pf, repubQuick, repubLong, node.Cid()) @@ -167,10 +115,6 @@ func NewRoot(ctx context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf Pu repub: repub, } - for _, opt := range opts { - opt(root) - } - fsn, err := ft.FSNodeFromBytes(node.Data()) if err != nil { log.Error("IPNS pointer was not unixfs node") @@ -186,17 +130,20 @@ func NewRoot(ctx context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf Pu } // Apply root-level directory settings - if root.maxLinks > 0 { - newDir.unixfsDir.SetMaxLinks(root.maxLinks) + if mkdirOpts.CidBuilder != nil { + newDir.unixfsDir.SetCidBuilder(mkdirOpts.CidBuilder) + } + if mkdirOpts.MaxLinks > 0 { + newDir.unixfsDir.SetMaxLinks(mkdirOpts.MaxLinks) } - if root.maxHAMTFanout > 0 { - newDir.unixfsDir.SetMaxHAMTFanout(root.maxHAMTFanout) + if mkdirOpts.MaxHAMTFanout > 0 { + newDir.unixfsDir.SetMaxHAMTFanout(mkdirOpts.MaxHAMTFanout) } - if root.hamtShardingSize > 0 { - newDir.unixfsDir.SetHAMTShardingSize(root.hamtShardingSize) + if mkdirOpts.HAMTShardingSize > 0 { + newDir.unixfsDir.SetHAMTShardingSize(mkdirOpts.HAMTShardingSize) } - if root.sizeEstimationMode != nil { - newDir.unixfsDir.SetSizeEstimationMode(*root.sizeEstimationMode) + if mkdirOpts.SizeEstimationMode != nil { + newDir.unixfsDir.SetSizeEstimationMode(*mkdirOpts.SizeEstimationMode) } root.dir = newDir @@ -212,24 +159,9 @@ func NewRoot(ctx context.Context, ds ipld.DAGService, node *dag.ProtoNode, pf Pu // NewEmptyRoot creates an empty Root directory with the given directory // options. A republisher is created if PubFunc is not nil. -func NewEmptyRoot(ctx context.Context, ds ipld.DAGService, pf PubFunc, prov provider.MultihashProvider, mkdirOpts MkdirOpts, rootOpts ...RootOption) (*Root, error) { +func NewEmptyRoot(ctx context.Context, ds ipld.DAGService, pf PubFunc, prov provider.MultihashProvider, mkdirOpts MkdirOpts) (*Root, error) { root := new(Root) - for _, opt := range rootOpts { - opt(root) - } - - // Pass settings from root opts to mkdir opts if not already set - if mkdirOpts.Chunker == nil { - mkdirOpts.Chunker = root.chunker - } - if mkdirOpts.MaxHAMTFanout == 0 && root.maxHAMTFanout > 0 { - mkdirOpts.MaxHAMTFanout = root.maxHAMTFanout - } - if mkdirOpts.HAMTShardingSize == 0 && root.hamtShardingSize > 0 { - mkdirOpts.HAMTShardingSize = root.hamtShardingSize - } - dir, err := NewEmptyDirectory(ctx, "", root, ds, prov, mkdirOpts) if err != nil { return nil, err