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

Acceptance Proofs #75

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Apr 5, 2024

ACP to introduce consensus proofs into the ProposerVM block header

Rendered.

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim
Copy link
Contributor Author

joshua-kim commented Apr 5, 2024

Images seem to look strange in the diff but I believe they will work once merged (preview): https://github.com/joshua-kim/ACPs/blob/consensus-proofs/ACPs/75_consensus_proofs/75-consensus-proofs.md

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@aaronbuchwald
Copy link
Contributor

Repeating @richardpringle 's suggestion on ACP-73: #73 (comment), it would be great to add a link to the branch/relevant file to the PR description, so you can quickly jump to it.

PChainHeight uint64
Timestamp time.Time
Proposer ids.NodeID
+ Proof []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to do this in a separate struct or put this on some sort of delay (i.e. proof for 256 blocks ago)? Otherwise, we can't support multiple pending blocks AFAICT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pseudocode just intended to be used as a reference so if it's in a separate struct/not I don't have a strong opinion on. Regarding a delay - I believe this does need to be on a delay as I think it needs to be the proof for the block at PChainHeight which is currently already on a delay via window-ing.

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim changed the title Consensus Proofs Acceptance Proofs Apr 9, 2024
@richardpringle
Copy link

@joshua-kim, you Rendered link broke with the file rename. The new link should be here:
https://github.com/joshua-kim/ACPs/blob/consensus-proofs/ACPs/75_consensus_proofs/75-acceptance-proofs.md

Copy link
Contributor

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

Great work, added some style comments

ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
ACPs/75_consensus_proofs/75-acceptance-proofs.md Outdated Show resolved Hide resolved
joshua-kim and others added 5 commits April 10, 2024 13:24
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
ACP: 75
Title: Acceptance Proofs
Author(s): Joshua Kim
Discussions-To: https://github.com/avalanche-foundation/ACPs/discussions/76
Copy link
Contributor

Choose a reason for hiding this comment

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

I will update this to the official discussion after merging.

@avalanche-foundation-admin avalanche-foundation-admin merged commit b0e9d24 into avalanche-foundation:main May 3, 2024
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

Successfully merging this pull request may close these issues.

None yet

6 participants