-
Notifications
You must be signed in to change notification settings - Fork 264
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 DataAvailabilityHeader from the Block #865
Comments
It could be not part of the block in the same way the last commit doesn't need to be part of the block, if you define "block" as just the transactions and messages https://github.com/celestiaorg/celestia-specs/blob/ec98170398dfc6394423ee79b00b71038879e211/src/specs/data_structures.md#block. There's no downside to decoupling other than engineering time. |
The last commit can't be re-computed by info that is in that same block. The DAHeader can though.
You'd need to store both decoupled anyways. Otherwise, when serving a DAHeader, you would always have the full cost of loading the whole Block structure from disk. The actual engineering cost was incurred when including it in the Block struct in the first place. |
this was completed a long time ago when switched to treating tendermint like a black box |
…ackport #865) (#970) * fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865) * avoid recursive call after rename to (*PeerState).MarshalJSON * add test * add change doc * explain for nolint * fix lint * fix golangci-lint to v1.52.2 * fix golangci-lint to v1.52.2 * Revert "fix golangci-lint to v1.52.2" This reverts commit 598a9ef4c86fc29cf038251676c33a222217826c. * Revert "fix golangci-lint to v1.52.2" This reverts commit a8aad121e27382813e95b1911b1b560c62e1c7c3. * Reintroduced `cmtjson` * Avoid copying Mutex * Avoid copying Mutex -- 2nd try, more succint * Update .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md * Update consensus/reactor_test.go --------- Co-authored-by: Sergio Mena <sergio@informal.systems> (cherry picked from commit f6ea09171a2bf9f695f59b65f5c51e4a8c168015) # Conflicts: # consensus/reactor_test.go * Revert "fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865)" * fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865) * avoid recursive call after rename to (*PeerState).MarshalJSON * add test * add change doc * explain for nolint * fix lint * fix golangci-lint to v1.52.2 * fix golangci-lint to v1.52.2 * Revert "fix golangci-lint to v1.52.2" This reverts commit 598a9ef4c86fc29cf038251676c33a222217826c. * Revert "fix golangci-lint to v1.52.2" This reverts commit a8aad121e27382813e95b1911b1b560c62e1c7c3. * Reintroduced `cmtjson` * Avoid copying Mutex * Avoid copying Mutex -- 2nd try, more succint * Update .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md * Update consensus/reactor_test.go --------- Co-authored-by: Sergio Mena <sergio@informal.systems> --------- Co-authored-by: mmsqe <mavis@crypto.com> Co-authored-by: Sergio Mena <sergio@informal.systems>
It was discussed synchronously that we don't actually need to have the data availability header in the block. I think this makes sense, as the data root is already in the header, and tying the DAH to the block forces the implementation to include it in all of the storage/handling of the Block struct.
I could definitely be missing something, could we actually remove the data availability header from the block?
The text was updated successfully, but these errors were encountered: