Skip to content

Empty identity CID should be indexed when options are set#316

Merged
masih merged 1 commit intomasterfrom
masih/v2-idx-id-empty-digest
Jul 6, 2022
Merged

Empty identity CID should be indexed when options are set#316
masih merged 1 commit intomasterfrom
masih/v2-idx-id-empty-digest

Conversation

@masih
Copy link
Copy Markdown
Member

@masih masih commented Jul 6, 2022

There is an edge-case where if the storing identity CIDs are enabled in
a CARv2, one could technically store an empty identity CID which ends up
with a singlewidthindex width of 8.

Such CAR files indeed exist out there and the validation changes
introduced in 2.4.0 means such CAR file indices are no longer readable
, even if regenerated.

The question is should this be considered a valid index/readable index?

@masih
Copy link
Copy Markdown
Member Author

masih commented Jul 6, 2022

Cc @nonsense @dirkmc

@masih masih requested a review from willscott July 6, 2022 15:35
There is an edge-case where if the storing identity CIDs are enabled in
a CARv2, one could technically store an empty identity CID which ends up
with a `singlewidthindex` width of 8.

Such CAR files indeed exist out there and the validation changes
introduced in `2.4.0` means such CAR file indices are no longer readable
, even if regenerated.

The question is should this be considered a valid index/readable index?
@masih masih force-pushed the masih/v2-idx-id-empty-digest branch from 44f424a to 853cb36 Compare July 6, 2022 15:37
Copy link
Copy Markdown
Member

@willscott willscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems fine. not the index's problem to enforce constraints to not do silly things at other layers of the stack

@masih masih merged commit b2e14f0 into master Jul 6, 2022
@masih masih deleted the masih/v2-idx-id-empty-digest branch July 6, 2022 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants