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

chore: upgrade to markdownlint v0.39.0 #3200

Merged
merged 1 commit into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/markdown-linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
node-version: 18

- name: Install markdownlint-cli
run: npm install -g markdownlint-cli@0.32.1
run: npm install -g markdownlint-cli@0.39.0
shell: bash

- name: Run markdownlint
Expand Down
2 changes: 2 additions & 0 deletions .markdownlint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
"code_blocks": false # Disable rule for hard tabs in code blocks
"MD013": false # Disable rule for line length
"MD033": false # Disable rule banning inline HTML
"MD055":
"style": "leading_and_trailing" # Instead of "consistent" (default) which can vary per file, use one style across the entire project
13 changes: 7 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ celestia-appd --help
### Environment variables

| Variable | Explanation | Default value | Required |
| --------------- | ---------------------------------- | -------------------------------------------------------- | -------- |
|-----------------|------------------------------------|----------------------------------------------------------|----------|
| `CELESTIA_HOME` | Home directory for the application | User home dir. [Ref](https://pkg.go.dev/os#UserHomeDir). | Optional |

### Create your own single node devnet
Expand Down Expand Up @@ -119,7 +119,7 @@ This repo attempts to conform to [conventional commits](https://www.conventional
### Tools

1. Install [golangci-lint](https://golangci-lint.run/welcome/install) 1.57.0
1. Install [markdownlint](https://github.com/DavidAnson/markdownlint)
1. Install [markdownlint](https://github.com/DavidAnson/markdownlint) 0.39.0
1. Install [hadolint](https://github.com/hadolint/hadolint)
1. Install [yamllint](https://yamllint.readthedocs.io/en/stable/quickstart.html)
1. Install [markdown-link-check](https://github.com/tcort/markdown-link-check)
Expand Down Expand Up @@ -154,10 +154,11 @@ Package-specific READMEs aim to explain implementation details for developers th

## Audits

Date | Auditor | Version | Report
-----------|-----------------------------------------------|-------------------------------------------------------------------------------------|--------------------------------------------------------
2023/9/15 | [Informal Systems](https://informal.systems/) | [v1.0.0-rc6](https://github.com/celestiaorg/celestia-app/releases/tag/v1.0.0-rc6) | [informal-systems.pdf](docs/audit/informal-systems.pdf)
2023/10/17 | [Binary Builders](https://binary.builders/) | [v1.0.0-rc10](https://github.com/celestiaorg/celestia-app/releases/tag/v1.0.0-rc10) | [binary-builders.pdf](docs/audit/binary-builders.pdf)
| Date | Auditor | Version | Report |
|-------------|-------------------------------------------------|---------------------------------------------------------------------------------------|----------------------------------------------------------|
| ----------- | ----------------------------------------------- | ------------------------------------------------------------------------------------- | -------------------------------------------------------- |
| 2023/9/15 | [Informal Systems](https://informal.systems/) | [v1.0.0-rc6](https://github.com/celestiaorg/celestia-app/releases/tag/v1.0.0-rc6) | [informal-systems.pdf](docs/audit/informal-systems.pdf) |
| 2023/10/17 | [Binary Builders](https://binary.builders/) | [v1.0.0-rc10](https://github.com/celestiaorg/celestia-app/releases/tag/v1.0.0-rc10) | [binary-builders.pdf](docs/audit/binary-builders.pdf) |

## Careers

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ To recap, the `SubtreeRootThreshold` determines the index of where a blob must s

For example, assume `SubtreeRootThreshold = 64`. This would mean that the blobs smaller than the `64` can start at an index that is a multiple of one and therefore introduce zero padding. Blobs that are larger than `64` but smaller than `64 * 2 = 128` can use an index that is a multiple of 2 to get a maximum of 1 padding share. Blobs that are larger than `64 * 2 = 128` but smaller than `64 * 4 = 256` can use an index that is a multiple of 4 to get a maximum of 3 padding shares and so on.

Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding
--------------------------------|------------------------|-----------------|-------------------
blob <= 64 | 1 | multiple of 1 | 0
64 < blob <= 128 | 2 | multiple of 2 | 1
128 < blob <= 256 | 4 | multiple of 4 | 3
256 < blob <= 512 | 8 | multiple of 8 | 7
| Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |
Comment on lines +52 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

The table formatting changes improve readability and consistency. However, consider adding a header row to enhance clarity further.

+ | Blob size range | Subtree root width | Index multiple | Worst-case padding |
  |---------------------------------|------------------------|-----------------|--------------------|
  | blob <= 64                      | 1                      | multiple of 1   | 0                  |
  | 64 < blob <= 128                | 2                      | multiple of 2   | 1                  |
  | 128 < blob <= 256               | 4                      | multiple of 4   | 3                  |
  | 256 < blob <= 512               | 8                      | multiple of 8   | 7                  |

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
| Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |
| Blob size range | Subtree root width | Index multiple | Worst-case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |


Note: the threshold doesn't need to be `64` and the implementation versions this constant so that it is possible to modify over time.

Comment on lines 49 to 60
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-15]

Consider adding a comma before "but" when it connects two independent clauses, unless they are closely connected and short.

- This is a good improvement but we can reduce padding further.
+ This is a good improvement, but we can reduce padding further.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- analyze the worst case padding for different blob sizes
+ analyze the worst-case padding for different blob sizes

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [23-23]

Add a comma after introductory phrases for clarity.

- In the naive approach if the block proposer aligned blobs one after another
+ In the naive approach, if the block proposer aligned blobs one after another

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

Consider adding a comma before "but" when it connects two independent clauses.

- have the lowest ratio of data to padding but also have small blob inclusion proofs.
+ have the lowest ratio of data to padding, but also have small blob inclusion proofs.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

Add a comma after introductory phrases for clarity.

- From now on this threshold is called the `SubtreeRootThreshold`
+ From now on, this threshold is called the `SubtreeRootThreshold`

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [35-35]

This sentence should start with an uppercase letter.

- where `blob` is the length of the blob in shares
+ Where `blob` is the length of the blob in shares

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [65-65]

Add a comma after introductory phrases for clarity.

- If light nodes can process this proof size without a problem then we can use this bound.
+ If light nodes can process this proof size without a problem, then we can use this bound.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [79-79]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- In the current worst case a Celestia block is full of blobs of size 1
+ In the current worst-case a Celestia block is full of blobs of size 1

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [81-81]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- ### Worst Case Padding
+ ### Worst-Case Padding

Expand Down
72 changes: 36 additions & 36 deletions docs/architecture/adr-015-namespace-id-size.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,21 @@ The namespace ID must provide at least 72 bits of randomness [^2] to satisfy cri

Another way to analyze this criteria is to determine the probability of duplicates if there exist N randomly generated namespaces. Columns in the table below represent the approximate probability that a collision would occur if N (e.g. 1 billion) random namespaces are generated.[^3]

Namespace ID size (bytes) | 1 billion (10^9) | 1 trillion (10^12) | 1 quadrillion (10^15) | 1 quintillion (10^18)
--------------------------|------------------|--------------------|-----------------------|----------------------
8 | ~0.02674 | 1 | 1 | 1
16 | 0 | ~1.4432e-15 | ~1.4693e-9 | ~0.00147
20 | 0 | 0 | 0 | ~3.4205e-13
32 | 0 | 0 | 0 | 0
| Namespace ID size (bytes) | 1 billion (10^9) | 1 trillion (10^12) | 1 quadrillion (10^15) | 1 quintillion (10^18) |
|---------------------------|------------------|--------------------|-----------------------|-----------------------|
| 8 | ~0.02674 | 1 | 1 | 1 |
| 16 | 0 | ~1.4432e-15 | ~1.4693e-9 | ~0.00147 |
| 20 | 0 | 0 | 0 | ~3.4205e-13 |
| 32 | 0 | 0 | 0 | 0 |

> As a rule of thumb, a hash function with range of size N can hash on the order of sqrt(N) values before running into collisions.[^4]

Namespace ID size (bytes) | Hash function range | Can hash this many items before running into collision
--------------------------|---------------------|-------------------------------------------------------
8 | 2^64 | 2^32 = ~4 billion items
16 | 2^128 | 2^64 = ~1 quintillion items
20 | 2^160 | 2^80 = ~1 septillion items
32 | 2^256 | 2^128 = ~3.4 quintillion items
| Namespace ID size (bytes) | Hash function range | Can hash this many items before running into collision |
|---------------------------|---------------------|--------------------------------------------------------|
| 8 | 2^64 | 2^32 = ~4 billion items |
| 16 | 2^128 | 2^64 = ~1 quintillion items |
| 20 | 2^160 | 2^80 = ~1 septillion items |
| 32 | 2^256 | 2^128 = ~3.4 quintillion items |

### Criteria 2

Expand Down Expand Up @@ -104,23 +104,23 @@ There are some tradeoffs to consider when choosing a namespace ID size.

The namespace is prefixed to each NMT data leaf. Two namespaces are prefixed to each NMT non-leaf hash. Therefore, the nodes of an NMT will be larger based on the namespace size. Assuming shares are 512 bytes:

Namespace size (bytes) | NMT data leaf size (bytes) | NMT inner node size (bytes)
-----------------------|----------------------------|----------------------------
8 | 8 + 512 = 520 | 2*8 + 32 = 48
16 | 16 + 512 = 528 | 2*16 + 32 = 64
20 | 20 + 512 = 532 | 2*20 + 32 = 72
32 | 32 + 512 = 544 | 2*32 + 32 = 96
| Namespace size (bytes) | NMT data leaf size (bytes) | NMT inner node size (bytes) |
|------------------------|----------------------------|-----------------------------|
| 8 | 8 + 512 = 520 | 2*8 + 32 = 48 |
| 16 | 16 + 512 = 528 | 2*16 + 32 = 64 |
| 20 | 20 + 512 = 532 | 2*20 + 32 = 72 |
| 32 | 32 + 512 = 544 | 2*32 + 32 = 96 |

### NMT proof size

Increasing the size of NMT nodes will increase the size of the NMT proof. Assuming shares are 512 bytes, square size is 128, the NMT for a row will contain 2 * 128 leaves. If the NMT proof is for a single leaf:

Namespace size (bytes) | Unencoded NMT proof size (bytes) | Protobuf encoded NMT proof size (bytes) | Protobuf encoded NMT proof with [gzip](https://pkg.go.dev/compress/gzip) (bytes)
-----------------------|----------------------------------|-----------------------------------------|---------------------------------------------------------------------------------
8 | 336 | 354 | 382
16 | 448 | 466 | 408
20 | 504 | 522 | 466
32 | 672 | 690 | 630
| Namespace size (bytes) | Unencoded NMT proof size (bytes) | Protobuf encoded NMT proof size (bytes) | Protobuf encoded NMT proof with [gzip](https://pkg.go.dev/compress/gzip) (bytes) |
|------------------------|----------------------------------|-----------------------------------------|----------------------------------------------------------------------------------|
| 8 | 336 | 354 | 382 |
| 16 | 448 | 466 | 408 |
| 20 | 504 | 522 | 466 |
| 32 | 672 | 690 | 630 |

Note: if the NMT proof is an absence proof, an additional leaf node is included in the proof.

Expand All @@ -147,12 +147,12 @@ If the namespace size is increased, the maximum possible blob will decrease. Giv

If the namespace size is increased, whenever a SHA256 invocation takes place (e.g. NMT's [HashNode](https://github.com/celestiaorg/nmt/blob/fd00c52175c48bad64d03444689162fb9c6bee41/hasher.go#L265)), more data needs to be SHA256'ed. For example:

Namespace size (bytes) | [HashNode](https://github.com/celestiaorg/nmt/blob/fd00c52175c48bad64d03444689162fb9c6bee41/hasher.go#L302)'s SHA256 max data (bytes)
-----------------------|--------------------------------------------------------------------------------------------------------------------------------------
8 | 97
16 | 129
32 | 193
33 | 197
| Namespace size (bytes) | [HashNode](https://github.com/celestiaorg/nmt/blob/fd00c52175c48bad64d03444689162fb9c6bee41/hasher.go#L302)'s SHA256 max data (bytes) |
|------------------------|---------------------------------------------------------------------------------------------------------------------------------------|
| 8 | 97 |
| 16 | 129 |
| 32 | 193 |
| 33 | 197 |

The data size is calculated as follows:
`SHA256(domain_sep || left_min || left_max || left_data_hash || right_min || right_max || right_data_hash)` = 1 + namespaceSize + namespaceSize + 32 + namespaceSize + namespaceSize + 32 which simplifies to (4 \* namespaceSIze) + (2 \* 32) + 1.
Expand All @@ -161,12 +161,12 @@ Hashing has a high performance impact in ZK contexts. Since a larger namespace s

Based on this StackOverflow [post](https://crypto.stackexchange.com/questions/54852/what-happens-if-a-sha-256-input-is-too-long-longer-than-512-bits), when the data provided to SHA256 exceeds 56 bytes, the data must be chunked into [64 byte blocks](https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/crypto/sha256/sha256.go;l=29;drc=995c0f310c087c9cbc49112ecc48459a96310451) with a trailing 56 byte block + 8 bytes to store the original data length as a `uint64`.

Namespace size (bytes) | [HashNode](https://github.com/celestiaorg/nmt/blob/fd00c52175c48bad64d03444689162fb9c6bee41/hasher.go#L302)'s SHA256 max data (bytes) | SHA256 compression invocations
-----------------------|---------------------------------------------------------------------------------------------------------------------------------------|-------------------------------
N/A | 56 | 1
0 to 13 | 64 + 56 = 120 | 2
14 to 29 | 64 + 64 + 56 = 184 | 3
30 to 45 | 64 + 64 + 64 + 56 = 248 | 4
| Namespace size (bytes) | [HashNode](https://github.com/celestiaorg/nmt/blob/fd00c52175c48bad64d03444689162fb9c6bee41/hasher.go#L302)'s SHA256 max data (bytes) | SHA256 compression invocations |
|------------------------|---------------------------------------------------------------------------------------------------------------------------------------|--------------------------------|
| N/A | 56 | 1 |
| 0 to 13 | 64 + 56 = 120 | 2 |
| 14 to 29 | 64 + 64 + 56 = 184 | 3 |
| 30 to 45 | 64 + 64 + 64 + 56 = 248 | 4 |

Note: to verify the number of SHA256 compression invocations, we analyzed the number of loop executions inside the Golang SHA256 implementation [here](https://github.com/golang/go/blob/96add980ad27faed627f26ef1ab09e8fe45d6bd1/src/crypto/sha256/sha256block.go#L83) and it matches the expected number of invocations in the table above. See raw data [here](https://gist.github.com/rootulp/4cfc10c1c80a15cc57f0b35f330ac542).

Expand Down
6 changes: 3 additions & 3 deletions docs/architecture/adr-019-strict-inflation-schedule.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ Q: What is the skew between `BlocksPerYear` on popular Cosmos SDK chains and the

Cosmos Hub (a.k.a Gaia) sets `BlocksPerYear` to 4,360,000 [here](https://github.com/cosmos/gaia/blob/8a522e98a2863205cf02fb97f8ad27d091670b9d/docs/governance/current-parameters.json#L86). Cosmos Hub has a block time 6.353 ms. Numia data doesn't have complete data for 2021 so we'll examine 2022:

Year | BlocksPerYear | Actual # of Blocks | Skew
-----|---------------|--------------------|------
2022 | 4,360,000 | 4,580,463 | 5.05%
| Year | BlocksPerYear | Actual # of Blocks | Skew |
|------|---------------|--------------------|-------|
| 2022 | 4,360,000 | 4,580,463 | 5.05% |

Source: [Numia query](https://console.cloud.google.com/bigquery?sq=611612269782:f0c42f9584c448c78a4ec5f118c2091c)
Comment on lines 119 to 126
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [100-100]

The verb "trigger" should agree with its subject, suggesting a revision to "triggers". For example: "...when a validator or delegator triggers a withdrawal..."

- At some point in the future when a validator or delegator trigger a withdrawal, tokens are withdrawn from...
+ At some point in the future when a validator or delegator triggers a withdrawal, tokens are withdrawn from...

Loading