-
Notifications
You must be signed in to change notification settings - Fork 131
Description
Background
- There are currently two packages of the
merkledag
library. There is the one here in boxo and go-merkledag. - 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
- 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
- There exist places in the code that do type checks against specific types such as
boxo/ipld/unixfs/file/unixfile.go
Line 155 in 8059f18
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:
- rename dagpb protobuf to merkledag.v1 #323
- rename unixfs protobuf package to unixfs.v1 #318
- feat: remove ipld legacy and format globals in ipld/merkledag #322
- Feat: reduce globals #324
- feat: remove block decoding global registry go-ipld-format#80
Validate the changes with consumers::
- Kubo: chore: update boxo to version with fewer globals kubo#9907
- Lotus: chore: migrate to boxo filecoin-project/lotus#10921
- Boost: update boxo filecoin-project/boost#1492
- go-fil-markets: chore: migrate to boxo filecoin-project/go-fil-markets#792
- ipfs-cluster: feat: update go-ipld-format and boxo to remove globals ipfs-cluster/ipfs-cluster#1936 (along with related dependencies feat: update go-ipld-format and boxo to remove globals go-ds-crdt#201 and feat: update go-ipld-format and boxo to remove globals hsanjuan/ipfs-lite#296)
Disruption phase:
- (go-ipld-format) Merge feat: remove block decoding global registry go-ipld-format#80
- (boxo) Merge Feat: reduce globals #324
- (Kubo) Merge chore: update boxo to version with fewer globals kubo#9907
- Remove boxo/car (will be tracked in Handle relationship between boxo and go-car and go-merkledag #218)