search

ipld / js-block

IPLD Block Interface
6 stars 3 forks source link

feat: check for circular references in input #14

Closed mikeal closed 4 years ago

mikeal commented 4 years ago

After updating a few of the codecs I realized that something all the codecs have to do is check for circular references.

I’m wondering if this should be moved to either the Block interface or to js-multiformats. It’s more error prone than you’d expect and it doesn’t make a lot of sense to me to have every codec handle it themselves.

Thoughts? @rvagg @carsonfarmer

rvagg commented 4 years ago

For the flexible formats, sure, having higher-level utilities would be useful, for the fixed formats, like dag-pb (I think) and certainly bitcoin, I wouldn't want to have some automatic check do something I can assure is not circular. So perhaps an interface that is opt-in or opt-out by the codec? A checkCircular boolean on the thing that gets add()ed, or a function that overrides the default that could be made a noop?

carsonfarmer commented 4 years ago

Hmmm, I haven't thought about this too much. Is the concern that there will be 'hidden' circular references because the codecs may be developed in 'isolation' without considering how they'll be consumed? I like @rvagg's suggestion of an opt in or opt out flag such as checkCircular. Makes sense to make it opt out, as I think it should be an explicit action to skip such checks if they are needed.

mikeal commented 4 years ago

Hrm...

I hadn’t thought about the codecs other than dag-*. Of those, dag-json already does a traversal in order to modify the input into a new object, so I could factor out is-circular and instead check for circular references in the traversal, which would save a traversal. And we could perhaps do the same thing in @rvagg’s new dag-cbor codec.

Closing for now.