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 the data hash and square size from Data #1045

Closed
rach-id opened this issue Jul 25, 2023 · 2 comments · Fixed by #1051
Closed

Remove the data hash and square size from Data #1045

rach-id opened this issue Jul 25, 2023 · 2 comments · Fixed by #1051
Assignees
Labels
Milestone

Comments

@rach-id
Copy link
Member

rach-id commented Jul 25, 2023

After implementing #1040, we should remove the unused data hash and 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;
}

The PR doing this should be targetting the main branch.

@evan-forbes
Copy link
Member

are we still wanting to do this after our conversations in #1051?

@rach-id
Copy link
Member Author

rach-id commented Aug 2, 2023

I am still looking at how it can be done, but I am not prioritizing it at the moment because it's post mainnet stuff.
I am fine if you prefer to close it for now

@rootulp rootulp added this to the Post-mainnet milestone Aug 8, 2023
cmwaters added a commit that referenced this issue Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants