Skip to content

go-merkledag duplicates and global variables #291

@aschmahmann

Description

@aschmahmann

Background

  1. There are currently two packages of the merkledag library. There is the one here in boxo and go-merkledag.
  2. There is a library https://github.com/ipfs/go-ipld-legacy that is used to create nodes that satisfy the go-ipld-format and go-ipld-prime interfaces
  3. That library uses a global variable to manage specific types of conversions, such as getting a DagPB specific type rather than a generic untyped node. https://github.com/ipfs/go-ipld-legacy/blob/219032a33b3037ac7754ea9f04c4b1ead533f364/decode.go#L32
  4. There exist places in the code that do type checks against specific types such as
    case *dag.ProtoNode:

Bug

These interactions collectively mean that depending on whether go-merkledag gets initialized before or after boxo/ipld/merkledag you may/may not get an error if you call boxo/ipld/merkledag/DagService.Get and pass the result into boxo/ipld/unixfs/file/NewUnixFSFile.

Proposed fixes

There are obviously lots of problems here that could resolve this particular issue given the many unfortunate practices that have led us here (two versions of similar code, using global variables to manage state, doing explicit type checks, etc.).

Probably the easiest is to copy go-ipld-legacy into boxo. At that point the entry paths into go-ipld-legacy will be a little more obvious/contained and we can figure out the best way to go from there.

Another option that might not be too bad would be to remove the global variables from go-ipld-legacy, and force an instance to be instantiated with a converter registry, which would likely be a breaking change. That repo doesn't appear to be highly utilized explicitly (as opposed to via merkledag) so it probably wouldn't be too rough on consumers. That being said it's certainly more disruptive than a copy.

Open to better suggestions if folks have them.

Tracking steps

Stage the changes:

Validate the changes with consumers::

Disruption phase:

Metadata

Metadata

Assignees

Labels

need/triageNeeds initial labeling and prioritization

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions