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.

Sign up for GitHub

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

Jump to bottom

docs: replace evidence with PFBs in square diagram #1436

Merged
merged 9 commits into from
Mar 3, 2023
Merged

docs: replace evidence with PFBs in square diagram #1436

merged 9 commits into from
Mar 3, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Mar 1, 2023 •
edited
Loading

Closes #1424 and #1437
Opens #1435 and #1440

The data square arrangement matches the implementation here. Note ISRs are specified but not yet implemented.

@rootulp rootulp added the specs directly relevant to the specs label Mar 1, 2023
@rootulp rootulp self-assigned this Mar 1, 2023
@rootulp rootulp requested a review from MSevey as a code owner March 1, 2023 19:55
evan-forbes
evan-forbes previously approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me! we might want to reference other tendermint documentation about their evidence wherever we talk about all of our fraud proofs (yet to be written iirc)

staheri14
staheri14 reviewed Mar 2, 2023
View reviewed changes
Copy link
Contributor

@staheri14 staheri14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing specs up to date! everything looks good to me! I did have a few questions though which I commented on the PR.

specs/src/specs/data_structures.md Show resolved Hide resolved
specs/src/specs/consensus.md Outdated Show resolved Hide resolved
specs/src/specs/consensus.md Show resolved Hide resolved
specs/src/specs/data_structures.md Show resolved Hide resolved
specs/src/specs/data_structures.md Outdated Show resolved Hide resolved
@rootulp rootulp requested a review from evan-forbes March 2, 2023 19:22
@rootulp rootulp requested a review from staheri14 March 2, 2023 19:23
evan-forbes
evan-forbes previously approved these changes Mar 3, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from my end, fixing the link as per other reviewer, and potentially opening an issue as a reminder to mention this in the light client spec over yonder in celestia-node

staheri14
staheri14 previously approved these changes Mar 3, 2023
View reviewed changes
Copy link
Contributor

@staheri14 staheri14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks for addressing the comments.

@rootulp rootulp dismissed stale reviews from staheri14 and evan-forbes via 9c1fb3f March 3, 2023 20:06
@rootulp @staheri14
Update specs/src/specs/consensus.md
9c1fb3f 
Co-authored-by: Sanaz Taheri <35961250+staheri14@users.noreply.github.com>
evan-forbes
evan-forbes previously approved these changes Mar 3, 2023
View reviewed changes
@rootulp rootulp mentioned this pull request Mar 3, 2023
Open
@rootulp rootulp requested a review from evan-forbes March 3, 2023 20:29
@rootulp rootulp requested a review from staheri14 March 3, 2023 20:29
@rootulp rootulp merged commit 1547891 into celestiaorg:specs-staging Mar 3, 2023
@rootulp rootulp deleted the rp/square-diagrams branch March 3, 2023 20:43
This was referenced Mar 6, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-forbes evan-forbes evan-forbes approved these changes

@mojtaba-esk mojtaba-esk Awaiting requested review from mojtaba-esk

@liamsi liamsi Awaiting requested review from liamsi

@adlerjohn adlerjohn Awaiting requested review from adlerjohn

@MSevey MSevey Awaiting requested review from MSevey

@staheri14 staheri14 Awaiting requested review from staheri14

Assignees

@rootulp rootulp

Labels
specs directly relevant to the specs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rootulp @staheri14 @evan-forbes