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

ADR-002: InfoReservedByte #651

Closed
wants to merge 1 commit into from

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Aug 23, 2022

Description

Moved from celestiaorg/celestia-core#843
ADR with implementation details for celestiaorg/celestia-core#839

@rootulp rootulp added documentation Improvements or additions to documentation C: Celestia app labels Aug 23, 2022
@rootulp rootulp self-assigned this Aug 23, 2022
@rootulp
Copy link
Collaborator Author

rootulp commented Aug 24, 2022

We had a meeting on info reserved byte and aligned on using it universally rather than just for message shares. See #659

Closing this PR but will use the contents of this ADR in a new one for the universal share encoding format.

@rootulp rootulp closed this Aug 24, 2022
@evan-forbes
Copy link
Member

Closing this PR but will use the contents of this ADR in a new one for the universal share encoding format.

initially these seem like separate proposals/implementation efforts, but I can see how they might be blocking. Is that why they were merged in #660?

@rootulp
Copy link
Collaborator Author

rootulp commented Aug 24, 2022

I merged them because I had the wrong interpretation of the previous proposal. I thought the info byte was only applicable to message shares. Given info share byte applies to both types of shares, I renamed and updated the ADR. This PR (for info bye just on message shares) seems no longer necessary.

With regards to implementation: we can split PRs into smaller chunks (i.e. one for updating the message share format and a separate one for updating the reserved namespace share format). I don't think one is strictly blocking the other but I do think it would be easier to tackle this work post merging in-flight share refactor PRs.

@rootulp rootulp deleted the rp/adr-info-reserved-byte branch August 24, 2022 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants