Skip to content
This repository has been archived by the owner on Feb 12, 2025. It is now read-only.

fix: storage address #15

Merged
merged 5 commits into from
Aug 23, 2023
Merged

fix: storage address #15

merged 5 commits into from
Aug 23, 2023

Conversation

greged93
Copy link
Contributor

@greged93 greged93 commented Aug 23, 2023

This PR prevents the post state verification from accessing the storage value at offset = 1, if the post state expected value is smaller than U128::MAX. Also added some light cleaning (removing unnecessary Arc, added test case for mul file, ...).

Time spent on this PR: 0.2 day

Resolves: #14

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Testing

What is the new behavior?

Fixes the storage access issue du to expected value lower than U128::MAX.

Does this introduce a breaking change?

  • Yes
  • No

Copy link
Contributor

@jobez jobez left a comment

Choose a reason for hiding this comment

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

other than adjusting the error message, lgtm

@@ -58,11 +57,19 @@ async fn handle_pre_state(
//// 'handle' methods attempt to abstract the data coming from BlockChainTestCase
//// from more general logic that can be used across tests
impl BlockchainTestCase {
fn test(&self, test_name: &str) -> Result<BlockchainTest, ef_tests::Error> {
let test = self.tests.get(test_name).ok_or_else(|| {
ef_tests::Error::Assertion(format!("failed test {}: missing pre state", test_name))
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the right error message for this case?

@greged93 greged93 force-pushed the fix/storage-address branch from 288237d to 396b7a1 Compare August 23, 2023 16:33
Copy link
Contributor

@jobez jobez left a comment

Choose a reason for hiding this comment

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

lgtm

@jobez jobez merged commit 69606a3 into kkrt-labs:main Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: missing storage for key
2 participants