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

Remove square size from data root tuple root generation #1040

Closed
rach-id opened this issue Jul 20, 2023 · 2 comments
Closed

Remove square size from data root tuple root generation #1040

rach-id opened this issue Jul 20, 2023 · 2 comments

Comments

@rach-id
Copy link
Member

rach-id commented Jul 20, 2023

          That's great. Let's do it in a subsequent PR

Originally posted by @cmwaters in #1003 (comment)

@rach-id
Copy link
Member Author

rach-id commented Jul 20, 2023

@cmwaters @evan-forbes I believe this is where we should remove the square size from:

// Data contains all the information needed for a consensus full node to
// reconstruct an extended data square.
message Data {
// Txs that will be applied to state in block.Height + 1 because deferred execution.
// This means that the block.AppHash of this block does not include these txs.
// NOTE: not all txs here are valid. We're just agreeing on the order first.
repeated bytes txs = 1;
reserved 2, 3, 4;
// field number 2 is reserved for intermediate state roots
// field number 3 is reserved for evidence
// field number 4 is reserved for blobs
// SquareSize is the number of rows or columns in the original data square.
uint64 square_size = 5;
// Hash is the root of a binary Merkle tree where the leaves of the tree are
// the row and column roots of an extended data square. Hash is often referred
// to as the "data root".
bytes hash = 6;
}

However, we still need the data root for the QGB. Is there a different way to get it so that we also remove it from here and only leave txs?

@cmwaters
Copy link
Contributor

The data root is actually also part of the header so we can retrieve it from that

rach-id added a commit that referenced this issue Jul 26, 2023
## Description

Closes #1040

After merging this one, I will cherry-pick this change for main. Then, I
will open a separate PR for main to remove the square size and the data
hash from the `Data` struct.

---


#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
rach-id added a commit to rach-id/celestia-core that referenced this issue Jul 26, 2023
## Description

Closes celestiaorg#1040

After merging this one, I will cherry-pick this change for main. Then, I
will open a separate PR for main to remove the square size and the data
hash from the `Data` struct.

---


#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
@rach-id rach-id closed this as completed Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants