Skip to content

refactor SimpleWorld to use Optional for storing accounts and extra null checks#7532

Merged
matkt merged 1 commit intobesu-eth:mainfrom
lu-pinto:simple-world-commit-add-null-check
Sep 2, 2024
Merged

refactor SimpleWorld to use Optional for storing accounts and extra null checks#7532
matkt merged 1 commit intobesu-eth:mainfrom
lu-pinto:simple-world-commit-add-null-check

Conversation

@lu-pinto
Copy link
Copy Markdown
Contributor

@lu-pinto lu-pinto commented Aug 28, 2024

PR description

Fix several NPEs in SimpleWorld and I also decided to use Optional for accounts since the return from get/getAccount was ambiguous between an account that has been deleted or has never existed. Now there's a clear distinction between deleted (Optional.empty()) or has never existed (null). Also getTouchedAccounts returned null values when there were deleted accounts within which can be dangerous to the caller.

Note: SimpleWorld seems to only be used by the test framework, so this is not critical.

Fixed Issue(s)

fixes #7524

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch 2 times, most recently from c95a75b to 483d772 Compare August 28, 2024 16:18
@lu-pinto lu-pinto marked this pull request as ready for review August 28, 2024 16:50
Comment thread evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java Outdated
Comment thread evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java
Comment thread evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java
Comment thread evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java
@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch from 483d772 to 1e26c71 Compare August 29, 2024 01:16
@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch from d16ccc8 to ae29100 Compare August 30, 2024 15:46
@lu-pinto lu-pinto removed the request for review from shemnon August 30, 2024 16:15
@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch from ae29100 to 631ee4b Compare August 30, 2024 16:27
@lu-pinto lu-pinto self-assigned this Aug 30, 2024
@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch 3 times, most recently from de2fced to 15e126d Compare September 2, 2024 09:16
@matkt matkt enabled auto-merge (squash) September 2, 2024 09:26
auto-merge was automatically disabled September 2, 2024 09:43

Head branch was pushed to by a user without write access

@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch from 15e126d to eb186c4 Compare September 2, 2024 09:43
…ull checks

Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@lu-pinto lu-pinto force-pushed the simple-world-commit-add-null-check branch from eb186c4 to 1a39d1f Compare September 2, 2024 10:15
@matkt matkt merged commit 4c4f2f3 into besu-eth:main Sep 2, 2024
@lu-pinto lu-pinto deleted the simple-world-commit-add-null-check branch September 2, 2024 12:16
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.

Bug in SimpleWorld.commit() ?

4 participants