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

Validate tests #26

Merged
merged 9 commits into from
Jan 13, 2023
Merged

Validate tests #26

merged 9 commits into from
Jan 13, 2023

Conversation

zsluedem
Copy link
Collaborator

@zsluedem zsluedem commented Dec 19, 2022

** You have to install Yarn and foundry to make it work**

Install foundry

This is the rust codes of validating tests in https://github.com/eth-infinitism/bundler/blob/main/packages/bundler/test/ValidateManager.test.ts .

  1. The pr introduce foundry as a testing evm node which would provide enough function for bundler to interact with.
  2. Include bundler repo
  3. Introduces the validating test cases.
  4. Enable clippy lint in tests.

tests/integration_test.rs Outdated Show resolved Hide resolved
tests/integration_test.rs Outdated Show resolved Hide resolved
tests/integration_test.rs Outdated Show resolved Hide resolved
tests/integration_test.rs Outdated Show resolved Hide resolved
tests/integration_test.rs Outdated Show resolved Hide resolved
@zsluedem zsluedem marked this pull request as draft December 20, 2022 01:05
@Vid201
Copy link
Member

Vid201 commented Dec 20, 2022

This is good, will be very helpful 🚀

Cargo.toml Outdated Show resolved Hide resolved
@zsluedem zsluedem force-pushed the integrations branch 7 times, most recently from 3a2b182 to ee44d6c Compare January 7, 2023 04:38
@zsluedem zsluedem changed the title Integration tests example Validate tests Jan 8, 2023
}
}

fn validate(_user_op: UserOperation) -> anyhow::Result<()> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Vid201 You have to implement this method for validations you wrote in your pr.

}

#[tokio::test]
#[should_panic]
Copy link
Collaborator Author

@zsluedem zsluedem Jan 8, 2023

Choose a reason for hiding this comment

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

@Vid201 The #[should_panic] flag should be removed when the validating function is implemented. The case panic right now is because validate always return Ok

@zsluedem zsluedem marked this pull request as ready for review January 8, 2023 04:37
@zsluedem zsluedem requested a review from Vid201 January 8, 2023 04:37
Makefile Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
tests/validate_tests.rs Show resolved Hide resolved
tests/validate_tests.rs Show resolved Hide resolved
tests/validate_tests.rs Outdated Show resolved Hide resolved
@Vid201
Copy link
Member

Vid201 commented Jan 9, 2023

Looks good @zsluedem. I wonder if these tests will stay relevant in the future or if we should focus on https://github.com/eth-infinitism/bundler-spec-tests. This was released two weeks ago and will be the official test suite for bundlers (maybe integrate running these tests instead in the repo).

What do you think?

@zsluedem
Copy link
Collaborator Author

Looks good @zsluedem. I wonder if these tests will stay relevant in the future or if we should focus on https://github.com/eth-infinitism/bundler-spec-tests. This was released two weeks ago and will be the official test suite for bundlers (maybe integrate running these tests instead in the repo).

What do you think?

Hm...I didn't know the test suite. I read the test codes there today and compare. They got very similar test contracts for opcode-banning parts.

The test suite would be run with a bundler client and an execution client from higher-layer integration perspective. I think we could have both because this could help you develop the simulation checking function (I think). You don't need to have a fully functional bundler to run the test suite right now.

But I am Ok if you want to just use the test suite.

@Vid201
Copy link
Member

Vid201 commented Jan 11, 2023

Looks good @zsluedem. I wonder if these tests will stay relevant in the future or if we should focus on https://github.com/eth-infinitism/bundler-spec-tests. This was released two weeks ago and will be the official test suite for bundlers (maybe integrate running these tests instead in the repo).
What do you think?

Hm...I didn't know the test suite. I read the test codes there today and compare. They got very similar test contracts for opcode-banning parts.

The test suite would be run with a bundler client and an execution client from higher-layer integration perspective. I think we could have both because this could help you develop the simulation checking function (I think). You don't need to have a fully functional bundler to run the test suite right now.

But I am Ok if you want to just use the test suite.

Ok, great. I didn't go into many details yet. Let's have both tests then, one for easier and faster development, and the other for validating everything and getting official results.

README.md Outdated Show resolved Hide resolved
@Vid201
Copy link
Member

Vid201 commented Jan 13, 2023

LGTM 🚀

@zsluedem zsluedem merged commit 4d7fb60 into silius-rs:main Jan 13, 2023
@zsluedem zsluedem deleted the integrations branch January 13, 2023 13:47
@zsluedem zsluedem mentioned this pull request Jan 14, 2023
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.

2 participants