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

docs: hoist ADR status #1407

Merged
merged 6 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
12 changes: 4 additions & 8 deletions docs/architecture/adr-001-abci++-adoption.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 001: ABCI++ Adoption

## Status

Implemented

## Changelog

- 2022-03-03: Initial Commit
Expand Down Expand Up @@ -32,10 +36,6 @@ While the adoption of ABCI++ is inevitable given the already made decision by up
- [Alternatives for Message Inclusion.](https://github.com/celestiaorg/celestia-app/blob/92341dd68ee6e555ec6c0bb780afa3a1c8243a93/adrs/adr008:adopt-ABC%2B%2B-early.md#alternative-approaches)
- [Alternatives for Picking a square size.](https://github.com/celestiaorg/celestia-core/issues/454)

## Decision

Proposed and initial implementation is complete.

## Detailed Design

### [#631](https://github.com/celestiaorg/celestia-core/pull/631) Simplified version of ABCI++ (`celestia-core`)
Expand Down Expand Up @@ -370,10 +370,6 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr
}
```

## Status

The proposed and initial implementation is complete.

## Consequences

### Positive
Expand Down
12 changes: 4 additions & 8 deletions docs/architecture/adr-002-qgb-valset.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 002: QGB ValSet

## Status

Accepted

## Context

To accommodate the requirements of the [Quantum Gravity Bridge](https://github.com/celestiaorg/quantum-gravity-bridge/blob/76efeca0be1a17d32ef633c0fdbd3c8f5e4cc53f/src/QuantumGravityBridge.sol), We will need to add support for `ValSet`s, i.e. Validator Sets, which reflect the current state of the bridge validators.
Expand Down Expand Up @@ -328,11 +332,3 @@ ctx.EventManager().EmitEvent(
),
)
```

## Status

Accepted

## References

- {reference link}
12 changes: 4 additions & 8 deletions docs/architecture/adr-003-qgb-data-commitments.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 003: QGB Data Commitments

## Status

Accepted

## Context

To accommodate the requirements of the [Quantum Gravity Bridge](https://github.com/celestiaorg/quantum-gravity-bridge/blob/76efeca0be1a17d32ef633c0fdbd3c8f5e4cc53f/src/QuantumGravityBridge.sol), We will need to add support for `DataCommitment`s messages, i.e. commitments generated over a set of blocks to attest their existence.
Expand Down Expand Up @@ -129,11 +133,3 @@ ctx.EventManager().EmitEvent(
),
)
```

## Status

Accepted

## References

- {reference link}
8 changes: 4 additions & 4 deletions docs/architecture/adr-004-qgb-relayer-security.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 004: QGB Relayer Security

## Status

Accepted

## Changelog

- 2022-06-05: Synchronous QGB implementation
Expand Down Expand Up @@ -397,10 +401,6 @@ In a nutshell, a new valset will be emitted if any of the following is true:
significantPowerDiff = intCurrMembers.PowerDiff(*intLatestMembers) > 0.05
```

## Status

Accepted

## References

- Tracker issue for the tasks [here](https://github.com/celestiaorg/celestia-app/issues/467).
12 changes: 6 additions & 6 deletions docs/architecture/adr-005-qgb-reduce-state-usage.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 005: QGB Reduce State Usage

## Status

Proposed
rootulp marked this conversation as resolved.
Show resolved Hide resolved

## Context

The first design for the QGB was to use the state extensively to store all the QGB-related data: Attestations, `Valset Confirms` and `DataCommitment Confirms`.
Expand Down Expand Up @@ -40,7 +44,7 @@ However, slashing will be very difficult, especially for liveness, i.e. an orche

Remove the `MsgValsetConfirm` defined in [here](https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L24-L49)
And also, the `MsgDataCommitmentConfirm` defined in [here](
https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L55-L76).
<https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L55-L76>).
rootulp marked this conversation as resolved.
Show resolved Hide resolved
Which were the way orchestrators were able to post confirms to the QGB module.
Then, keep only the state that is created in [EndBlocker](https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/x/qgb/abci.go#L12-L16).
Which are `Attestations`, i.e. `Valset`s and `DataCommitmentRequest`s.
Expand All @@ -63,7 +67,7 @@ We will need to decide on two things:
## Detailed Design

The proposed design consists of keeping the same transaction types we currently have : the `MsgValsetConfirm` defined in [here](https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L24-L49), and the `MsgDataCommitmentConfirm` defined in [here](
https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L55-L76). However, remove all the message server checks defined in the [msg_server.go](https://github.com/celestiaorg/celestia-app/blob/9867b653b2a253ba01cb7889e2dbfa6c9ff67909/x/qgb/keeper/msg_server.go) :
<https://github.com/celestiaorg/celestia-app/blob/a965914b8a467f0384b17d9a8a0bb1ac62f384db/proto/qgb/msgs.proto#L55-L76>). However, remove all the message server checks defined in the [msg_server.go](https://github.com/celestiaorg/celestia-app/blob/9867b653b2a253ba01cb7889e2dbfa6c9ff67909/x/qgb/keeper/msg_server.go) :

```go
// ValsetConfirm handles MsgValsetConfirm.
Expand Down Expand Up @@ -95,10 +99,6 @@ For posting transactions, we will rely on gas fees as a mechanism to limit malic

When it comes to slashing, we can add the `dataRoot` of the blocks to the state during `ProcessProposal`, `FinalizeCommit`, or in some other way to be defined. Then, we will have a way to slash orchestrators after a certain period of time if they didn't post any confirms. The exact details of this will be left for another ADR.

## Status

Proposed

## Consequences

### Positive
Expand Down
8 changes: 4 additions & 4 deletions docs/architecture/adr-006-non-interactive-defaults.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 006: Non-interactive Defaults, Wrapped Transactions, and Subtree Root Message Inclusion Checks

## Status

Accepted

> **Note**
> Unlike normal tendermint/cosmos ADRs, this ADR isn't for deciding whether or not we will implement non-interactive defaults. The goal of this document is to help reviewers and future readers understand what non-interactive defaults are, the considerations that went into the initial implementation, and how it differs from the original specs.

Expand Down Expand Up @@ -678,10 +682,6 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr

The current implementation performs many different estimation and calculation steps. It might be possible to amortize these calculations to each transaction, which would make it a lot easier to confidently arrange an optimal block.

## Status

Accepted

## Consequences

### Positive
Expand Down
8 changes: 4 additions & 4 deletions docs/architecture/adr-007-universal-share-prefix.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 007: Universal Share Prefix

## Status

Implemented
rootulp marked this conversation as resolved.
Show resolved Hide resolved

## Terminology

- **compact share**: a type of share that can accommodate multiple units. Currently, compact shares are used for transactions, and evidence is to efficiently pack this information into as few shares as possible.
Expand Down Expand Up @@ -120,10 +124,6 @@ Logic
1. All shares contain a share version that belongs to a list of supported versions (initially this list contains version `0`)
1. All shares in a reserved namespace belong to one share sequence

## Status

Implemented

## Consequences

### Positive
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 008: square size independent message commitments

## Status

Implemented

## Changelog

- 03.08.2022: Initial Draft
Expand Down Expand Up @@ -63,16 +67,12 @@ func MinSquareSize(shareCount uint64) uint64 {
}
```

## Status

Implemented

## Consequences

### Negative

1. The amount of subtree roots per commitment is O(sqrt(n)), while n is the number of message shares. The worst case for the number of subtree roots is depicted in the diagram below - an entire block missing one share.
![Interactive Commitment 2](./assets/complexity.png)
![Interactive Commitment 2](./assets/complexity.png)
The worst case for the current implementation depends on the square size. If it is the worst square size, as in `msgMinSquareSize`, it is O(sqrt(n)) as well. On the other hand, if the message is only in one row, then it is O(log(n)).
Therefore the height of the tree over the subtree roots is in this implementation O(log(sqrt(n))), while n is the number of message shares. In the current implementation, it varies from O(log(sqrt(n))) to O(log(log(n))) depending on the square size.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ADR 009: Non-Interactive Default Rules for Reduced Padding

## Status

Accepted

## Changelog

- 14.11.2022: Initial Draft
Expand All @@ -18,7 +22,7 @@ Proposed

The upside of this proposal is that it reduces the inter-message padding. The downside is that a message inclusion proof will not be as efficient for large square sizes so the proof will be larger.

> **Note**
> **Note**
> This analysis assumes the implementation of [celestia-app#1004](https://github.com/celestiaorg/celestia-app/issues/1004). If the tree over the subtree roots is not a Namespace Merkle Tree then both methods have the same proof size.

As an example, take the diagram below. Message 1 is 3 shares long and message 2 is 11 shares long.
Expand Down Expand Up @@ -119,17 +123,17 @@ Each row consists of one subtree root, which means if you have log(n) rows you w

![Current ni rules proof size](./assets/current-ni-rules-proof-size.png)

NMT-Node size := 32 bytes + 2\*8 bytes = 48 bytes
NMT-Node size := 32 bytes + 2\*8 bytes = 48 bytes
MT-Node size := 32 bytes

Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (log(n) + log(k) + log(n)) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (log(n) + log(k) + log(n)) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = 48 \* (2\*log(n) + log(k)) + 64 \*log(k)

### Current Non-Interactive Default Rules for k/4

Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (k/4 + log(k) + k/4) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (k/4 + log(k) + k/4) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = 48 \* (k/2 + log(k)) + 64 \*log(k)

### Proposed Non-Interactive Default Rules
Expand All @@ -138,14 +142,14 @@ Each row consists of sqrt(n)/log(n) subtree roots. Which makes in total sqrt(n)

![Proposed ni rules proof size](./assets/proposed-ni-rules-proof-size.png)

Proof size = subtree roots (all rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (sqrt(n) + log(k) + log(n)) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = subtree roots (all rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (sqrt(n) + log(k) + log(n)) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = 48 \* (sqrt(n) + log(k) + log(n)) + 64 \*log(k)

### Proposed Non-Interactive Default Rules for k/4

Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (**k/2** + log(k) + k/4) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = subtree roots (rows) + subtree roots (last row) + blue nodes (parity shares) + 2 \* blue nodes (`DataRoot`)
Proof size = (**k/2** + log(k) + k/4) \* NMT-Node size + 2\*log(k) \* MT-Node size
Proof size = 48 \* (3k/4 + log(k)) + 64 \*log(k)

## 5. What is the worst constructible block with the most amount of padding with old and new non-interactive default rules?
Expand Down Expand Up @@ -192,10 +196,6 @@ The worst-case padding decreases from 1.1 GB to 0.8 GB in 2 GB Blocks. In the cu

You can further optimize the proof size by using the fact the Namespace is known and the same for all the subtree roots. You can do the same trick for parity shares as the namespace is fixed for them too. Both of these optimizations are not included in the analysis and would save the bytes that are used to store the namespace.

## Status

Accepted

## Consequences

### Positive
Expand Down
Loading