flesh out reification method to hamt

[willscott]

Design questions at this point

• Should we always try to fallback to filecoin mode / do we think the 'feature detection' type of switching makes sense?
• Can we late-load filling in the LinkPrototype until the first time we encounter a link in the reified dag, or are there cases we may want to update before we do anything that would cause us to see one? Is there a way to get it out of the linksystem defaults in this case?

[mvdan]

Are we going with the fancier Reify signature after all? I still tend to agree with Eric here: #30 (comment) - but maybe you two have had offline discussions I'm not aware of.

You seem to use the parameters mostly to prefill "linking" fields, but I guess one could always call WithLinking afterwards if that's needed.

Should we always try to fallback to filecoin mode / do we think the 'feature detection' type of switching makes sense?

It makes sense to me, at least.

Can we late-load filling in the LinkPrototype until the first time we encounter a link in the reified dag

I think my answer here is the same as the above - to allow the user to call WithLinking as necessary.

[BigLep]

@willscott : what are the next steps here?

[rvagg]

btw I'm happy to engage here if you still have the energy to pursue this further, it'd be good to get more ADL work over the line

[willscott]

From dan's final comment I wasn't clear on if there was a preference for keeping the current top level reification signature or changing to the (i think older) other one.

I tend to prefer the current one, because that's what we can hook up to signal in selectors.

Once we're agreed on that and the two design questions i posed in the top post of this PR, we can do one more pass through the code, although I think the logic is already mostly there. Probably worth having some basic tests/fixtures added though.

[willscott]

want to see if you have a preference on reification method signature, @rvagg ?

[rvagg]

I think I have a mild leaning toward the simpler one, mainly because WithLinking() already exists and does the thing that's needed, but I'm not so familiar with this codebase yet and maybe the redundancy makes sense? On the other hand, this signature matches go-unixfsnode so maybe we just roll with that. Your call I think.

The fallback to filecoin mode is a more interesting question to me, it's not obvious that this is something you would want to do as it basically makes the root node optional for every hamt data, and it's not in the spec (which is of course flexible and we could change that). It seems to me that it's something you'd want to opt in to—the filecoin hamt is like it is because all of that config data is carried by the system itself, you know it's a filecoin hamt and therefore it has those parameters. The spec is intended for a more generic data structure where you don't necessarily need to carry that contextual information over. But arguments over the nature of "context" always get kind of bogged down on these points.

Any ideas of what could be done to make this an opt-in thing? Or is that going to require a new method: ReifyFilecoin()?

[willscott]

Reify(ipld.LinkContext, ipld.Node, *ipld.LinkSystem) (ipld.Node, error)
is the signature that is used in ipld-prime https://github.com/ipld/go-ipld-prime/blob/master/linking/types.go#L158 so implementation of that is what allows this to be used as an ADL

[rvagg]

@willscott I'm not entirely clear on what the sticking point with the signature here is .. it seems to me that it's about Eric's point that reifying an ADL is something you should be doing intentionally after you've decided that something should be interpreted as an ADL, and not in the way that we mostly use go-unixfsnode where we basically try and smush everything through it and silently fail. Your implementation here isn't quiet about failure, like go-unixfsnode, so it's not ideal to use in the same way.

So is the contention here that the signature should be different because we want to signal, with the signature, that this should be called intentionally and directly and not wired up in a NodeReifier pipeline?

[willscott]

i don't understand the purpose of the other one:

• we can already use this code manually, e.g. in filecoin cases where we manually construct hamts.
• This is the signature needed for NodeReifier - if the interpretAs selector is used, that's an intentional signal to attempt ADL reification at that point.
• I don't know if the choice of 'should it fail back to substrate, or fail the operation' when the node passed in to reifiy isn't appropriate is a semantic that we need to worry about. That seems in some sense like it's up to the lens to define that semantic for itself.
• i'm pretty convinced at this point that the ipld prime signature as proposed in this PR currently is the useful one to add, so that selectors can interact with HAMTs

[rvagg]

Yeah ok, well I guess interpretas was added after most of the discussion has happened here, so I guess the signature is locked in unless we wanted to go and change it there.

So, we should just move forward with this then I suppose and get it done.