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

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

Merged

Conversation

lu-pinto
Copy link
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
@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 hyperledger:main Sep 2, 2024
40 checks passed
@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