Spec migration #796

adlerjohn · 2022-09-27T13:47:20Z

Staging branch for specs migration. Draft PR so it can be tracked.

Fixes #792 An initial migration of the existing specs as of [`e59efd63a2165866584833e91e1cb8a6ed8c8203`](https://github.com/celestiaorg/celestia-specs/tree/e59efd63a2165866584833e91e1cb8a6ed8c8203). It's expected that there are a few broken links and extra content that has to be pruned, along with incorrect content. The important thing is to not miss anything. Also sets up a CI workflow that will build the book.

See https://github.com/celestiaorg/celestia-app/actions/runs/3136139613/jobs/5092673489

Run [Markdown Table Prettifier](https://github.com/darkriszty/MarkdownTablePrettify-VSCodeExt).

@rootulp

Ref: #793 (comment) by @rootulp

rootulp · 2022-09-28T18:56:17Z

hmm I can't merge main into this branch

I wanted to merge main to pick up the new CODEOWNERS file

Closes #807

musalbas · 2022-10-19T14:45:20Z

What's the status of this?

Closes #728

Closes #729 [Rendered](https://github.com/celestiaorg/celestia-app/blob/e6f00791df41a4257b677df52747e6453856770e/specs/src/specs/data_structures.md#share) Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

rootulp · 2022-11-01T20:49:34Z

@musalbas apologies for delayed response. @adlerjohn and I triaged issues in #650 today.

I think that the specs-staging branch is already a more accurate representation of the specs than https://celestiaorg.github.io/celestia-specs/latest/index.html so I'd like to merge specs-staging to main ASAP. But if we want to block this merge on updating specs to match the existing implementation, we can block on all the issues in the "Pure specs" section.

Closes #883 Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>

Part of #1153 Should only be merged if #1181 is merged

Closes #1120 Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

Closes #1151 Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

Closes #1247

Closes #1175 See the implementation [here](https://github.com/celestiaorg/celestia-app/blob/d7ac0b84765db3f649cd82326e3ead066d164637/pkg/shares/tail_padding.go#L9-L21). ## Screenshot <img width="891" alt="Screenshot 2023-01-17 at 11 04 14 AM" src="https://user-images.githubusercontent.com/3699047/212949826-4b213925-91f0-4653-b375-545c89c17acc.png">

musalbas · 2023-01-20T14:28:35Z

What's the latest with this and generally adding specs to celestia-app/core now?

rootulp · 2023-01-20T14:56:00Z

This branch is the most up to date version of the specs but it's not publicly visible yet. IMO we should merge this branch to main, make it publicly visible, and take down the existing specs site at https://celestiaorg.github.io/celestia-specs/latest/index.html

The remaining specs issues are tracked under #650 . We intend on prioritizing specs and documentation work after the core/app feature freeze for incentivized testnet.

Context: #1353 Closes #1344 ## Screenshot <img width="746" alt="Screenshot 2023-02-08 at 11 55 07 AM" src="https://user-images.githubusercontent.com/3699047/217598202-15737680-d81b-4771-9940-75aa1830eaa2.png">

…#1420) This aims to clarify that the minimum square size in non-interactive default rules should be a power of 2. For example, for a message of length 23, the minimum square size is `8*8` (but not `5*5`). (based on a discussion in Slack) - [x] NA ~~New and updated code has appropriate documentation~~ - [x] NA ~~New and updated code has new and/or updated testing~~ - [x] Required CI checks are passing - [x] NA ~~Visual proof for any user facing features like CLI or documentation updates~~ - [x] NA ~~Linked issues closed with keywords~~

## Motivation The table of contents at the top of each file can be a maintenance burden. We can auto-generate it with [mdbook-toc](https://github.com/badboy/mdbook-toc). ## Screenshot Before | After --- | --- <img width="500" alt="before" src="https://user-images.githubusercontent.com/3699047/222535783-8ad53ec3-43db-4e4b-ad8f-ea974061190b.png"> | <img width="479" alt="after" src="https://user-images.githubusercontent.com/3699047/222535796-39c4172d-79b9-4e64-a492-7a68c37f5381.png">

cmwaters · 2023-03-14T10:57:32Z

I have some general comments that can possible be followed up at a later point:

Can we break up data structures into smaller more self-explanatory files so that parts can be easily accessed in the table of contents. I also think it's less intuitive to explain a spec by just dumping all the data structures that are included in it. Would prefer more of a hierarchical function-orientated tree.
Can we add a section explaining how the "celestia app" fits in with the rest of the system and include links so people can be easily redirected if they're interested with something in "celestia node" for example. Furthermore, I think calling it "celestia app" can be better worded. As this isn't the specification of the entire system, rather a component of it, we should look to distinguish it functionally. Call it "celestia data publication specification" (to complement "celestia data availability specfication") if we want to be deliberate about having two networks.
I don't think Rationale should be a separate section but should be embedded right next to the part in the spec it refers to. I should read what a certain protocol is and directly after why it is so.
It's evident that the specification includes not just novel work specific to Celestia but the components that it is built on top of. There is a lot of work on Tendermint's consensus algorithm (such as block validity, leader selection) and how the validator set is controlled. What about other components like governance, IBC, vesting, upgrades that still seem to be missing. It seems that not all state transition messages have been documented.
Is QGB spec somewhere else?
Can we have some preamble/context in the top level pages to each of the subpages.
Are we sure we want a copy of many of the proto files in the spec as well? These are not the canonical ones that get used in generation and will constantly become outdated.

rootulp · 2023-03-14T15:57:24Z

Overall I'm on-board with basically all of the suggestions you proposed. I think the feedback could be spun out into individual issues that we add to the epic rather than blocking feedback on this PR.

Can we break up data structures into smaller more self-explanatory files

I'm on-board. I agree that the data structures page is long and difficult to read top to bottom so I think multiple logically contained data structures pages would be helpful for readability.

I don't think Rationale should be a separate section but should be embedded right next to the part in the spec it refers to.

Agreed. I think we should move the rationale/message_block_layout section to block_proposer#laying-out-transactions-and-messages

Is QGB spec somewhere else?

Perhaps https://github.com/celestiaorg/celestia-app/blob/main/x/qgb/README.md

Are we sure we want a copy of many of the proto files in the spec as well?

Good question. I think we should not duplicate the proto definitions in the specs unless we have an automated way to update them if the canonical ones change.

evan-forbes

are the markdown linter complaints valid?

  ERROR: 2 dead links found!
  [✖] .consensus.md#constants → Status: 400
  [✖] ../rationale/distributing_rewards.md → Status: 400

other than that I think we're okay to merge (and actually merge, not squash!)

we should also add @cmwaters feedback into the spec epic as @rootulp mentioned because I think its really good

Running make lint locally doesn't catch these errors

rootulp · 2023-03-15T14:32:24Z

are the markdown linter complaints valid?

They are valid but they don't appear when make lint is run locally. They appear resolved after the most recent commit.

Docker / hadolint fails but that occurs on other branches too (potentially b/c Github outage).

rootulp · 2023-03-15T21:22:12Z

Will merge this by EOD tomorrow if no more feedback.

.github/workflows/gh-pages.yml

specs/src/SUMMARY.md

MSevey

overall looks good to me, didn't review the specs themselves, waiting until they are deployed and we can submit clean up PRs.

evan-forbes · 2023-03-16T14:27:43Z

its weird that the docker builds are still failing here despite passing in other PRs, we didn't change anything related here afaict

edit: Rootul fixed it

Co-authored-by: Matthew Sevey <mjsevey@gmail.com>

adlerjohn added the specs directly relevant to the specs label Sep 27, 2022

adlerjohn self-assigned this Sep 27, 2022

liamsi force-pushed the specs-staging branch from f7f4b96 to b2a850a Compare September 27, 2022 14:17

adlerjohn added 4 commits September 27, 2022 10:26

Fix markdown lint errors. (#798)

ddb9d6a

See https://github.com/celestiaorg/celestia-app/actions/runs/3136139613/jobs/5092673489

Add deploy step to mdbook workflow. (#797)

dcf6d4a

Run table formatter. (#799)

dac49d1

Run [Markdown Table Prettifier](https://github.com/darkriszty/MarkdownTablePrettify-VSCodeExt).

Add install prereq. (#800)

60f341a

Ref: #793 (comment) by @rootulp

chore: specs-staging CODEOWNERS (#812)

2cc65da

Closes #807

rootulp and others added 2 commits October 19, 2022 12:09

docs: describe message length varint (#806)

eb3791e

Closes #728

specs: universal share prefix (#856)

26f11cb

Closes #729 [Rendered](https://github.com/celestiaorg/celestia-app/blob/e6f00791df41a4257b677df52747e6453856770e/specs/src/specs/data_structures.md#share) Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

rootulp and others added 6 commits November 24, 2022 15:40

specs: two reserved bytes (#939)

3206843

Closes #883 Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>

chore: uint32 for message size (#1182)

b610a0b

Part of #1153 Should only be merged if #1181 is merged

specs: fixed sequence length (#1124)

351e2d3

Closes #1120 Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

specs: non-interactive default rules for reduced padding (#1156)

41c0859

Closes #1151 Co-authored-by: John Adler <adlerjohn@users.noreply.github.com>

docs: spec namespaced padding share (#1248)

3c66c2d

Closes #1247

rootulp and others added 5 commits February 8, 2023 15:28

docs: revise specs for padding shares (#1359)

0dc7d17

Context: #1353 Closes #1344 ## Screenshot <img width="746" alt="Screenshot 2023-02-08 at 11 55 07 AM" src="https://user-images.githubusercontent.com/3699047/217598202-15737680-d81b-4771-9940-75aa1830eaa2.png">

docs: replace evidence with PFBs in square diagram (#1436)

1547891

docs: note about Q3 (#1467)

cb2ccdf

Merge branch 'main' into specs-staging

309e4e2

rootulp self-assigned this Mar 14, 2023

rootulp added this to the Mainnet milestone Mar 14, 2023

rootulp marked this pull request as ready for review March 14, 2023 17:05

rootulp requested a review from evan-forbes as a code owner March 14, 2023 17:05

rootulp requested review from MSevey and liamsi March 14, 2023 17:05

evan-forbes reviewed Mar 14, 2023

View reviewed changes

lint: fix markdownlint

3ff81e8

MSevey requested a review from a team March 14, 2023 22:11

fix: attempt to resolve markdownlint errors

b439e04

Running make lint locally doesn't catch these errors

rootulp requested a review from evan-forbes March 15, 2023 21:22

MSevey reviewed Mar 16, 2023

View reviewed changes

.github/workflows/gh-pages.yml Outdated Show resolved Hide resolved

MSevey reviewed Mar 16, 2023

View reviewed changes

.github/workflows/gh-pages.yml Show resolved Hide resolved

MSevey reviewed Mar 16, 2023

View reviewed changes

specs/src/SUMMARY.md Show resolved Hide resolved

MSevey previously approved these changes Mar 16, 2023

View reviewed changes

Update .github/workflows/gh-pages.yml

f2e2e18

Co-authored-by: Matthew Sevey <mjsevey@gmail.com>

rootulp dismissed MSevey’s stale review via f2e2e18 March 16, 2023 14:37

MSevey requested a review from a team March 16, 2023 14:38

rootulp mentioned this pull request Mar 16, 2023

ci: add preview github action for specs #1495

Closed

rootulp requested a review from MSevey March 16, 2023 14:46

MSevey approved these changes Mar 16, 2023

View reviewed changes

rootulp merged commit b3f0b02 into main Mar 16, 2023

evan-forbes mentioned this pull request Mar 16, 2023

EPIC: Add an official spec for the app #650

Closed

36 tasks

rootulp deleted the specs-staging branch October 5, 2023 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spec migration #796

Spec migration #796

adlerjohn commented Sep 27, 2022 •

edited

rootulp commented Sep 28, 2022

musalbas commented Oct 19, 2022

rootulp commented Nov 1, 2022

musalbas commented Jan 20, 2023

rootulp commented Jan 20, 2023

cmwaters commented Mar 14, 2023

rootulp commented Mar 14, 2023 •

edited

evan-forbes left a comment

rootulp commented Mar 15, 2023

rootulp commented Mar 15, 2023

MSevey left a comment

evan-forbes commented Mar 16, 2023 •

edited

Spec migration #796

Spec migration #796

Conversation

adlerjohn commented Sep 27, 2022 • edited

rootulp commented Sep 28, 2022

musalbas commented Oct 19, 2022

rootulp commented Nov 1, 2022

musalbas commented Jan 20, 2023

rootulp commented Jan 20, 2023

cmwaters commented Mar 14, 2023

rootulp commented Mar 14, 2023 • edited

evan-forbes left a comment

Choose a reason for hiding this comment

rootulp commented Mar 15, 2023

rootulp commented Mar 15, 2023

MSevey left a comment

Choose a reason for hiding this comment

evan-forbes commented Mar 16, 2023 • edited

adlerjohn commented Sep 27, 2022 •

edited

rootulp commented Mar 14, 2023 •

edited

evan-forbes commented Mar 16, 2023 •

edited