Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add fr32-sha2-256-trunc254-padded-binary-tree codec #61

Merged

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw requested a review from hacdias September 25, 2023 14:58
Copy link
Collaborator

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

⏩ in b 4 the upstream PR!

@olizilla
Copy link
Collaborator

@alanshaw plz run node fetch-tables.js in this project, so we get the latest multicodecs and then add in fr32-sha2-256-trunc254-padded-binary-tree codec so it's less likely to get overwitten while we wait for the multicodec PR to land.

@alanshaw
Copy link
Member Author

Oh lol ok!

@alanshaw alanshaw merged commit 8022e51 into master Sep 25, 2023
@alanshaw alanshaw deleted the feat/add-fr32-sha2-256-trunc254-padded-binary-tree-codec branch September 25, 2023 17:46
@aschmahmann
Copy link

aschmahmann commented Sep 26, 2023

I suspect you're trying to show this off in Iceland which is the reason for the quick merge, which I get. Although it makes me a little uncomfortable that:

  1. This is in before the multicodec PR is in
  2. This is in before there is consensus on what the format actually is. There are two drafts based on competing requests (FRC: Piece Multihash CID filecoin-project/FIPs#758 and frc(0069): change v2 piece multihashes to enable arbitrarily sized data filecoin-project/FIPs#808).
    • The first got merged as a pro-forma thing by the managers of the Filecoin FIPs repo with minimal input (basically I proposed and Riba +1'd while Steven, Irakli and Mikeal asked for an alternative which became 808)
    • The second has had some engagement, but no consensus (and no input from the people who originally requested it 😅)

I'm hoping to get people in a room at FilDev Iceland on Wendesday 9/27 @1:30 PM UTC and get them to decide if the result is 758, 808 or something else which means this in theory is subject to change.

FYI for anyone looking for that conversation's zoom link you can ask in #fil-dev-summit in Filecoin Slack and tag me.

Note: Not asking for a revert, we'll revisit in a few days and see if anything needs changing 😄. Just an FYI that things are potentially subject to change here.

@olizilla
Copy link
Collaborator

understood. I'm thinking more generally that it would be nice for the cid inspector tool to ahead of "what is confirmed" so it can unpack any cid you might find. The more the merrier... but perhaps with a suitable warning where it determines that a given input is from a PR rather than accepted.

I'm imagining tweaking this repo to build it's list of codecs from both the table.csv and PRs to it, and labelling PR entries as such, and have a github cron job keep it up to date. Otherwise, I just get pinged when this website fails to decode a hot new cid.

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.

3 participants