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

feat: Add FaultProofFixture definition #70

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

BrianBland
Copy link
Collaborator

Description

Adds a new FaultProofFixture type definition, which is intended to provide all of the data required to execute an offline fault proof test

Tests

Added basic serialization/deserialization roundtrip tests

Additional context

Add any other context about the problem you're solving.

Metadata

  • Fixes #[Link to Issue]

Copy link
Member

@clabby clabby left a comment

Choose a reason for hiding this comment

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

Looks good at a high level. Changes in the FPP status; I didn't enumerate the full list for you (missed unfinished) on discord.

Should also probably add a TryFrom<u8> for FaultProofStatus.

Comment on lines 38 to 75
/// The fault proof status is the result of executing the fault proof program.
#[derive(Serialize, Deserialize, Debug, Default, PartialEq, Eq)]
pub enum FaultProofStatus {
/// The claim is valid.
#[default]
Valid,
/// The claim is invalid.
Invalid,
/// Executing the program resulted in a panic.
Panic,
/// The status is unknown.
Unknown
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// The fault proof status is the result of executing the fault proof program.
#[derive(Serialize, Deserialize, Debug, Default, PartialEq, Eq)]
pub enum FaultProofStatus {
/// The claim is valid.
#[default]
Valid,
/// The claim is invalid.
Invalid,
/// Executing the program resulted in a panic.
Panic,
/// The status is unknown.
Unknown
}
/// The fault proof status is the result of executing the fault proof program.
#[derive(Serialize, Deserialize, Debug, Default, PartialEq, Eq)]
#[repr(u8)]
pub enum FaultProofStatus {
/// The claim is valid.
#[default]
Valid = 0,
/// The claim is invalid.
Invalid = 1,
/// Executing the program resulted in a panic.
Panic = 2,
/// The program has not exited.
Unfinished = 3,
/// The status is unknown.
Unknown
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks 👍, will iterate on this a bit more as I wire this up to the op-program.

@refcell
Copy link
Collaborator

refcell commented Aug 21, 2024

Added you as a collaborator btw @BrianBland so you don't need if a fork if you don't want :)

@BrianBland BrianBland force-pushed the add-fault-proof-def branch 2 times, most recently from 360c1ec to 25c51e5 Compare August 26, 2024 18:26
@BrianBland BrianBland marked this pull request as ready for review August 26, 2024 19:19
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.

3 participants