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

chore(op-test-vectors): Update ExecutionFixture to use op-alloy-consensus types #73

Merged
merged 10 commits into from
Aug 24, 2024
Merged

chore(op-test-vectors): Update ExecutionFixture to use op-alloy-consensus types #73

merged 10 commits into from
Aug 24, 2024

Conversation

0xKitsune
Copy link
Contributor

This PR addresses #40, replacing the all anvil types in the ExecutionFixture struct with types from op-alloy-consensus . Additonally opt8n was updated to reflect these changes.

pub struct ExecutionFixture {
    /// The execution environment sets up the current block context.
    pub env: ExecutionEnvironment,
    /// The initial state of the accounts before running the transactions, also called the
    /// "pre-state".
    pub alloc: HashMap<Address, AccountState>,
    /// The expected state of the accounts after running the transactions, also called the
    /// "post-state".
    pub out_alloc: HashMap<Address, AccountState>,
    /// Transactions to execute.
    #[serde(rename = "txs")]
    pub transactions: Vec<OpTypedTransaction>,
    /// The expected result after executing transactions.
    pub result: ExecutionResult,
}

@0xKitsune 0xKitsune added A-op-test-vectors Area: op-test-vectors crate M-dependencies Meta: Dependency related labels Aug 22, 2024
refcell
refcell reviewed Aug 23, 2024
View reviewed changes
bin/opt8n/src/opt8n.rs
@@ -183,6 +201,94 @@ impl Opt8n {
}
}

// TODO: Consider adding `From` implementation for
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree - I will add issues in op-alloy for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I can open a PR for these issues on op-alloy later today/tomorrow.

refcell
refcell reviewed Aug 23, 2024
View reviewed changes
crates/op-test-vectors/src/execution.rs
@@ -159,53 +98,4 @@ mod tests {
.expect("failed to parse expected result");
assert_eq!(serialized_value, expected_value);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a test/tests here for deserializing the op types?

refcell
refcell approved these changes Aug 23, 2024
View reviewed changes
Copy link
Collaborator

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Approved, let's just add a few tests for deserialization

@refcell refcell mentioned this pull request Aug 23, 2024
Closed
@clabby
Copy link
Member

clabby commented Aug 24, 2024

Merging to unblock.

@clabby clabby merged commit 5e8700f into main Aug 24, 2024
5 checks passed
@clabby clabby deleted the 0xkitsune/alloy branch August 24, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refcell refcell refcell approved these changes

Assignees
No one assigned
Labels
A-op-test-vectors Area: op-test-vectors crate M-dependencies Meta: Dependency related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0xKitsune @clabby @refcell