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

Bug in SimpleWorld.commit() ? #7524

Closed
DavePearce opened this issue Aug 26, 2024 · 1 comment · Fixed by #7532
Closed

Bug in SimpleWorld.commit() ? #7524

DavePearce opened this issue Aug 26, 2024 · 1 comment · Fixed by #7532
Assignees
Labels
bug Something isn't working good first issue Good for newcomers P4 Low (ex: Node doesn't start up when the configuration file has unexpected "end-of-line" character) snack Smaller coding task - less than a day for an experienced dev testing

Comments

@DavePearce
Copy link

Description

Currently, SimpleWorld.commit() is defined as follows:

@Override
  public void commit() {
    accounts.forEach(
        (address, account) -> {
          if (!account.updateParent()) {
            parent.accounts.put(address, account);
          }
        });
  }

However, this seems to be in conflict with this:

  @Override
  public Collection<Address> getDeletedAccountAddresses() {
    return accounts.entrySet().stream()
        .filter(e -> e.getValue() == null)
        .map(Map.Entry::getKey)
        .toList();
  }

It seems to me that we want account == null || !account.updateParent() as the criteria in commit() above.

Thoughts?

@non-fungible-nelson non-fungible-nelson added bug Something isn't working testing good first issue Good for newcomers P4 Low (ex: Node doesn't start up when the configuration file has unexpected "end-of-line" character) snack Smaller coding task - less than a day for an experienced dev labels Aug 27, 2024
@lu-pinto
Copy link
Contributor

lu-pinto commented Aug 27, 2024

Yup that needs a null check as deleteAccount puts in nulls in the collection and commit may be called after deleteAccount: AbstractMessageProcessor.java#L115-L116 . So definitely the collection is not null free

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers P4 Low (ex: Node doesn't start up when the configuration file has unexpected "end-of-line" character) snack Smaller coding task - less than a day for an experienced dev testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@DavePearce @non-fungible-nelson @lu-pinto and others