Initial specs migration. by adlerjohn · Pull Request #793 · celestiaorg/celestia-app

adlerjohn · 2022-09-26T22:25:57Z

Fixes #792

An initial migration of the existing specs as of 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.

rootulp

LGTM! Thanks for your hard work on this

specs/src/specs/block_proposer.md

rootulp · 2022-09-26T22:56:30Z

specs/src/specs/consensus.md

+| `TRANSACTION_NAMESPACE_ID`              | `NamespaceID` | `0x0000000000000001` | Transactions: requests that modify the state.                                              |
+| `INTERMEDIATE_STATE_ROOT_NAMESPACE_ID`  | `NamespaceID` | `0x0000000000000002` | Intermediate state roots, committed after every transaction.                               |
+| `EVIDENCE_NAMESPACE_ID`                 | `NamespaceID` | `0x0000000000000003` | Evidence: fraud proofs or other proof of slashable action.                                 |
+| `TAIL_TRANSACTION_PADDING_NAMESPACE_ID` | `NamespaceID` | `0x00000000000000FF` | Tail padding for transactions: padding after all transactions but before messages.         |


[non-blocking][question] is this tail padding after all reserved namespaces? So after evidence but before messages?

Suggested change

| `TAIL_TRANSACTION_PADDING_NAMESPACE_ID` | `NamespaceID` | `0x00000000000000FF` | Tail padding for transactions: padding after all transactions but before messages. |

| `TAIL_RESERVED_NAMESPACE_PADDING_NAMESPACE_ID` | `NamespaceID` | `0x00000000000000FF` | Tail padding for reserved namespaces: padding after all reserved namespaces but before messages. |

same as #793 (comment)

specs/src/specs/consensus.md

specs/src/specs/data_structures.md

specs/theme/highlight.js

specs/README.md

evan-forbes

dope, glad these are finding a home.

liamsi

I would ask to add John, Matt, and me to the specs sub-dir as code-owner.

liamsi · 2022-09-27T09:16:20Z

specs/src/specs/proto/wire.proto

+message WireTxBurn {
+  uint64 amount = 1;
+  TransactionFee fee = 2;
+  uint64 nonce = 3;
+  // 32-byte graffiti
+  bytes graffiti = 4;
+}


This Tx type does not exist. Neither any "graffiti" field.
I am not sure that copying the spec as is without going through the hard work to update it is the best approach but up to you and the core/app team.

liamsi · 2022-09-27T09:17:38Z

specs/src/specs/proto/types.proto

+// ANCHOR: StateFraudProof
+// https://github.com/celestiaorg/celestia-specs/blob/master/specs/networking.md#statefraudproof
+message StateFraudProof {


Can be deleted I assume?

liamsi

Also sets up a CI workflow that will push the built book to a main directory on the gh-pages branch on each merge to main, allowing a rendered form of the book to be displayed on GitHub pages.

I would recommend to remove this until the specs are somewhat up-to-date again. Otherwise, we will be confusing a bunch of folks visiting the published specs again.

musalbas · 2022-09-27T09:23:58Z

Shouldn't the parts about how data is encoded be in Celestia core, not app?

We want to have specs be in the repo that they're relevant for, so different specs for core and app. The specs relevant for core and app should be extracted to the relevant repos, so that if for example someone makes a change to the share formatting specs in core, they edit specs in core, not app. However this PR doesn't seem to achieve that.

musalbas · 2022-09-27T09:42:43Z

Also, it seems a large part of the spec is outdated (sparse Merkle trees, graffiti, etc). IMO there's not much advantage to merging the existing outdated specs into the main branch, as people can already reference the existing specs repo. I think there should be a draft PR to do a basic update and clean up of the specs, as well as decoupling the bits for core and app, so that we have a general structure that can be built on, before merging this.

evan-forbes · 2022-09-27T10:31:46Z

I think there should be a draft PR to do a basic update and clean up of the specs, as well as decoupling the bits for core and app, so that we have a general structure that can be built on, before merging this.

@musalbas I think there was a miscommunication, where those of us on the call agreed to move the specs over first, and then edit them with individual PRs. this is why Rootul and myself approved so quickly and why John made the PR in the first place.

as mentioned, if we don't want to do that, then we can turn this PR into a draft and make the individual PRs on this branch until we're comfortable with merging.

Shouldn't the parts about how data is encoded be in Celestia core, not app?

all encoding code is kept in the application now, so I think it makes sense to keep these specs here.

I would recommend to remove this until the specs are somewhat up-to-date again. Otherwise, we will be confusing a bunch of folks visiting the published specs again.

not publishing the specs until they match the implementation makes a lot of sense. We can split this portion into a different PR.

evan-forbes

we should split the CI publishing mechanism into a different PR #793 (review)

musalbas · 2022-09-27T11:27:46Z

Makes sense, would it make sense to merge this into a different branch like specs in that case, until it's ready for main? Even if there's no CI publishing mechanism, people could still be reading the md files in main.

adlerjohn · 2022-09-27T13:42:55Z

Made a new branch specs-staging that we can squash-merge PRs into, starting with this one, and re-targeted this PR on that branch. This avoids end-users seeing a WIP specs directory.
Removed the deploy step from CI, added a build step to ensure the book actually builds properly.

evan-forbes · 2022-09-27T13:45:55Z

Thanks @adlerjohn! gets my 👍 👍

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.

@rootulp

Ref: #793 (comment) by @rootulp

@rootulp

Ref: celestiaorg/celestia-app#793 (comment) by @rootulp

@rootulp

Ref: celestiaorg/celestia-app#793 (comment) by @rootulp

Initial specs migration.

ae5e6e1

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

adlerjohn self-assigned this Sep 26, 2022

adlerjohn requested review from evan-forbes and rootulp September 26, 2022 22:26

rootulp previously approved these changes Sep 26, 2022

View reviewed changes

evan-forbes approved these changes Sep 27, 2022

View reviewed changes

liamsi reviewed Sep 27, 2022

View reviewed changes

evan-forbes requested changes Sep 27, 2022

View reviewed changes

adlerjohn marked this pull request as draft September 27, 2022 13:13

Build book properly and run on PRs.

8a1c929

adlerjohn dismissed rootulp’s stale review via 8a1c929 September 27, 2022 13:23

Merge branch 'main' into adlerjohn/specs-migration

5278e53

adlerjohn changed the base branch from main to specs-staging September 27, 2022 13:40

Remove deploy step.

ef08996

adlerjohn requested review from evan-forbes and rootulp September 27, 2022 13:43

adlerjohn marked this pull request as ready for review September 27, 2022 13:43

evan-forbes approved these changes Sep 27, 2022

View reviewed changes

adlerjohn merged commit f7f4b96 into specs-staging Sep 27, 2022

adlerjohn deleted the adlerjohn/specs-migration branch September 27, 2022 13:46

adlerjohn mentioned this pull request Sep 27, 2022

Migrate specs over to app #792

Closed

adlerjohn mentioned this pull request Sep 27, 2022

Add install prereq. #800

Merged

adlerjohn added a commit that referenced this pull request Sep 27, 2022

Add install prereq. (#800)

60f341a

Ref: #793 (comment) by @rootulp

rootulp mentioned this pull request Sep 28, 2022

Define CODEOWNERS file #807

Closed

topdev22 added a commit to topdev22/app-celestia that referenced this pull request Sep 26, 2025

Add install prereq. (#800)

b2d3429

Ref: celestiaorg/celestia-app#793 (comment) by @rootulp

clevergoldfox added a commit to clevergoldfox/celestia-app that referenced this pull request Feb 3, 2026

Add install prereq. (#800)

d9da7f7

Ref: celestiaorg/celestia-app#793 (comment) by @rootulp

	\| `TAIL_TRANSACTION_PADDING_NAMESPACE_ID` \| `NamespaceID` \| `0x00000000000000FF` \| Tail padding for transactions: padding after all transactions but before messages. \|
	\| `TAIL_RESERVED_NAMESPACE_PADDING_NAMESPACE_ID` \| `NamespaceID` \| `0x00000000000000FF` \| Tail padding for reserved namespaces: padding after all reserved namespaces but before messages. \|

Conversation

adlerjohn commented Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rootulp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rootulp Sep 26, 2022

Choose a reason for hiding this comment

Uh oh!

evan-forbes Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

evan-forbes left a comment

Choose a reason for hiding this comment

Uh oh!

liamsi left a comment

Choose a reason for hiding this comment

Uh oh!

liamsi Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

liamsi Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

liamsi left a comment

Choose a reason for hiding this comment

Uh oh!

musalbas commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

musalbas commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evan-forbes commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evan-forbes left a comment

Choose a reason for hiding this comment

Uh oh!

musalbas commented Sep 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adlerjohn commented Sep 27, 2022

Uh oh!

evan-forbes commented Sep 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adlerjohn commented Sep 26, 2022 •

edited

Loading

musalbas commented Sep 27, 2022 •

edited

Loading

musalbas commented Sep 27, 2022 •

edited

Loading

evan-forbes commented Sep 27, 2022 •

edited

Loading

musalbas commented Sep 27, 2022 •

edited

Loading