Last block info for selectors #418
Conversation
austinabell
requested review from
timvermeulen,
StaticallyTypedAnxiety,
flodesi and
RajarupanSampanthan
| self.last_block = Some(LastBlockInfo { | ||
| path: self.path.clone(), | ||
| link: cid.clone(), | ||
| }); | ||
| let mut node = resolver.load_link(cid).await.map_err(Error::Link)?; | ||
| while let Some(Ipld::Link(c)) = node { | ||
| node = resolver.load_link(&c).await.map_err(Error::Link)?; |
There was a problem hiding this comment.
Would self.last_block's CID not need to be updated here as well?
There was a problem hiding this comment.
no, so the intention is that the first link traversed is recorded, if you provide the most recent when there are multiple traversals, you could lose the connection from the initial node. (This way you do have a theoretical way to regenerate the connection by following the last block link)
This case is never really hit in practice, because we never link to an ipld node that is a link, but this just handles that edge case so traversal doesn't stop if this is ever found.
There was a problem hiding this comment.
the intention is that the first link traversed is recorded
It seems like it's still possible here to traverse several links with maybe some other layers in between, and it will only save the last one. Or do you in that case actually want to save the last one only?
There was a problem hiding this comment.
But what I mean, is that if you save the last link traversed instead of the first, you can never know how many links were traversed and the last block would just be an Ipld Link, which isn't useful because you'd have to do some custom logic to check the previously visited block to get the original block, and that may not be available if you are only walking matched nodes.
This is all very in depth unnecessarily because this case is never hit in practice for our uses, it's just a benefit to allow this over the go implementations stopping traversal.
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #414
Other information and links