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

Move evidence from data square back to Tendermint block #950

Closed
rootulp opened this issue Nov 3, 2022 · 5 comments
Closed

Move evidence from data square back to Tendermint block #950

rootulp opened this issue Nov 3, 2022 · 5 comments
Assignees

Comments

@rootulp
Copy link
Collaborator

rootulp commented Nov 3, 2022

Context

During the QGB Slashing design finalization meeting, we decided to remove evidence from the data square back to the Tendermint block. This likely involves changes in pkg/shares and celestia-core

Tasks

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 3, 2022

https://github.com/celestiaorg/celestia-core/blob/9002f1bf6be303d2a6f46865ead288812fc9a49a/types/block.go#L1014 makes me think that Evidence was never actually removed from the Tendermint block. @liamsi are you aware of commits or modifications in celestia-core that need to be reverted?

@rootulp rootulp added the epic item groups other items for easier tracking label Nov 3, 2022
@liamsi
Copy link
Member

liamsi commented Nov 3, 2022

@liamsi are you aware of commits or modifications in celestia-core that need to be reverted?

This is the change in question: celestiaorg/celestia-core#19

Particularly, block.go:
https://github.com/celestiaorg/celestia-core/pull/19/files#diff-77409c5f41482f194140901cdad16d5bf6e5e6b3591f275dbf3660f12615078c

This might look like this it does not matter where that field is but in fact only block.Data gets erasure coded and can sampled. Also, the evidence is currently part of the data root.

Before we revert this though, we should capture the rationale of WHY we would want to revert this. As @musalbas said, we should park this for a bit before acting on it.

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 3, 2022

The rationale for reverting is:

  1. to minimize the diff between celestia-core and tendermint
  2. because we don't expect to have state fraud proofs on mainnet in the immediate future and evidence in the data square only becomes desirable after we have state fraud proofs

@evan-forbes evan-forbes changed the title EPIC: move evidence from data square back to Tendermint block Move evidence from data square back to Tendermint block Nov 15, 2022
@evan-forbes evan-forbes removed the epic item groups other items for easier tracking label Nov 15, 2022
@evan-forbes
Copy link
Member

removed the epic tag for now, as this can be accomplished with 1 PR in core, and a PR here to update/delete code in the shares package. pls feel free to change if we think other wise.

@rootulp
Copy link
Collaborator Author

rootulp commented Dec 1, 2022

Done via celestiaorg/celestia-core#894 and #1068

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants