diff --git a/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleAccount.java b/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleAccount.java index f86bba699e3..6ad10d37e4b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleAccount.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleAccount.java @@ -214,21 +214,4 @@ public boolean commit() { return false; } } - - /** - * Push changes into the parent account, if one exists - * - * @return true if a parent account was updated, false if not (this indicates the account should - * be inserted into the parent contact). - */ - public boolean updateParent() { - if (parent instanceof SimpleAccount simpleAccount) { - simpleAccount.balance = balance; - simpleAccount.nonce = nonce; - simpleAccount.storage.putAll(storage); - return true; - } else { - return false; - } - } } diff --git a/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java b/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java index f52e788babf..6d2437a8877 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/fluent/SimpleWorld.java @@ -29,10 +29,10 @@ public class SimpleWorld implements WorldUpdater { /** The Parent. */ - SimpleWorld parent; + private final SimpleWorld parent; /** The Accounts. */ - Map accounts = new HashMap<>(); + private Map> accounts = new HashMap<>(); /** Instantiates a new Simple world. */ public SimpleWorld() { @@ -55,13 +55,15 @@ public WorldUpdater updater() { @Override public Account get(final Address address) { + Optional account = Optional.empty(); if (accounts.containsKey(address)) { - return accounts.get(address); + account = accounts.get(address); } else if (parent != null) { - return parent.get(address); - } else { - return null; + if (parent.get(address) instanceof SimpleAccount accountFromParent) { + account = Optional.of(accountFromParent); + } } + return account.orElse(null); } @Override @@ -70,45 +72,46 @@ public MutableAccount createAccount(final Address address, final long nonce, fin throw new IllegalStateException("Cannot create an account when one already exists"); } SimpleAccount account = new SimpleAccount(address, nonce, balance); - accounts.put(address, account); + accounts.put(address, Optional.of(account)); return account; } @Override public MutableAccount getAccount(final Address address) { - SimpleAccount account = accounts.get(address); + Optional account = accounts.get(address); if (account != null) { - return account; + return account.orElse(null); } Account parentAccount = parent == null ? null : parent.getAccount(address); if (parentAccount != null) { account = - new SimpleAccount( - parentAccount, - parentAccount.getAddress(), - parentAccount.getNonce(), - parentAccount.getBalance(), - parentAccount.getCode()); + Optional.of( + new SimpleAccount( + parentAccount, + parentAccount.getAddress(), + parentAccount.getNonce(), + parentAccount.getBalance(), + parentAccount.getCode())); accounts.put(address, account); - return account; + return account.get(); } return null; } @Override public void deleteAccount(final Address address) { - accounts.put(address, null); + accounts.put(address, Optional.empty()); } @Override public Collection getTouchedAccounts() { - return accounts.values(); + return accounts.values().stream().filter(Optional::isPresent).map(Optional::get).toList(); } @Override public Collection
getDeletedAccountAddresses() { return accounts.entrySet().stream() - .filter(e -> e.getValue() == null) + .filter(e -> e.getValue().isEmpty()) .map(Map.Entry::getKey) .toList(); } @@ -122,8 +125,10 @@ public void revert() { public void commit() { accounts.forEach( (address, account) -> { - if (!account.updateParent()) { - parent.accounts.put(address, account); + if (account.isEmpty() || !account.get().commit()) { + if (parent != null) { + parent.accounts.put(address, account); + } } }); } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/fluent/SimpleWorldTest.java b/evm/src/test/java/org/hyperledger/besu/evm/fluent/SimpleWorldTest.java new file mode 100644 index 00000000000..c7f997bfb9e --- /dev/null +++ b/evm/src/test/java/org/hyperledger/besu/evm/fluent/SimpleWorldTest.java @@ -0,0 +1,331 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.evm.fluent; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.evm.account.Account; +import org.hyperledger.besu.evm.account.MutableAccount; +import org.hyperledger.besu.evm.worldstate.WorldUpdater; + +import org.apache.tuweni.units.bigints.UInt256; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class SimpleWorldTest { + private static final Address ADDRESS1 = Address.fromHexString("0x0"); + private static final Address ADDRESS2 = Address.fromHexString("0x1"); + private static final Address ADDRESS3 = Address.fromHexString("0x2"); + + private SimpleWorld simpleWorld; + + @BeforeEach + void setUp() { + simpleWorld = new SimpleWorld(); + } + + @Test + void get_noAccounts() { + assertThat(simpleWorld.get(ADDRESS1)).isNull(); + } + + @Test + void get_noAccountsWithParent() { + WorldUpdater childUpdater = simpleWorld.updater(); + assertThat(childUpdater.get(ADDRESS1)).isNull(); + } + + @Test + void createAccount_cannotCreateIfExists() { + simpleWorld.createAccount(ADDRESS1); + assertThatThrownBy(() -> simpleWorld.createAccount(ADDRESS1)) + .isInstanceOf(IllegalStateException.class); + } + + @Test + void get_createdAccountExistsInParent() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + WorldUpdater childUpdater = simpleWorld.updater(); + assertThat(childUpdater.get(ADDRESS1)).isEqualTo(account); + } + + @Test + void get_createdAccountExistsInMultiParent() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + WorldUpdater childUpdater = simpleWorld.updater(); + childUpdater = childUpdater.updater(); + childUpdater = childUpdater.updater(); + childUpdater = childUpdater.updater(); + assertThat(childUpdater.get(ADDRESS1)).isEqualTo(account); + } + + @Test + void get_createdAccountExists() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + assertThat(simpleWorld.get(ADDRESS1)).isEqualTo(account); + } + + @Test + void get_createdAccountDeleted() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS1); + assertThat(simpleWorld.get(ADDRESS1)).isNull(); + } + + @Test + void get_revertRemovesAllAccounts() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.revert(); + assertThat(simpleWorld.get(ADDRESS1)).isNull(); + assertThat(simpleWorld.get(ADDRESS2)).isNull(); + assertThat(simpleWorld.get(ADDRESS3)).isNull(); + } + + @Test + void get_commitKeepsAllAccounts() { + MutableAccount acc1 = simpleWorld.createAccount(ADDRESS1); + MutableAccount acc2 = simpleWorld.createAccount(ADDRESS2); + MutableAccount acc3 = simpleWorld.createAccount(ADDRESS3); + simpleWorld.commit(); + assertThat(simpleWorld.get(ADDRESS1)).isEqualTo(acc1); + assertThat(simpleWorld.get(ADDRESS2)).isEqualTo(acc2); + assertThat(simpleWorld.get(ADDRESS3)).isEqualTo(acc3); + } + + @Test + void get_createdAccountDeletedInChild() { + simpleWorld.createAccount(ADDRESS1); + WorldUpdater childUpdater = simpleWorld.updater(); + childUpdater.deleteAccount(ADDRESS1); + assertThat(childUpdater.get(ADDRESS1)).isNull(); + } + + @Test + void getAccount_noAccounts() { + assertThat(simpleWorld.getAccount(ADDRESS1)).isNull(); + } + + @Test + void getAccount_noAccountsWithParent() { + WorldUpdater childUpdater = simpleWorld.updater(); + assertThat(childUpdater.getAccount(ADDRESS1)).isNull(); + } + + @Test + void getAccount_createdAccountExistsInParent() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + account.setStorageValue(UInt256.MAX_VALUE, UInt256.ONE); + WorldUpdater childUpdater = simpleWorld.updater(); + assertThat(childUpdater.getAccount(ADDRESS1).getOriginalStorageValue(UInt256.MAX_VALUE)) + .isEqualTo(UInt256.ONE); + } + + @Test + void getAccount_createdAccountExistsInMultiParent() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + account.setStorageValue(UInt256.MAX_VALUE, UInt256.ONE); + WorldUpdater childUpdater = simpleWorld.updater(); + childUpdater = childUpdater.updater(); + childUpdater = childUpdater.updater(); + childUpdater = childUpdater.updater(); + assertThat(childUpdater.getAccount(ADDRESS1).getOriginalStorageValue(UInt256.MAX_VALUE)) + .isEqualTo(UInt256.ONE); + } + + @Test + void getAccount_createdAccountExists() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + assertThat(simpleWorld.getAccount(ADDRESS1)).isEqualTo(account); + } + + @Test + void getAccount_createdAccountDeleted() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS1); + assertThat(simpleWorld.getAccount(ADDRESS1)).isNull(); + } + + @Test + void getAccount_revertRemovesAllAccounts() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.revert(); + assertThat(simpleWorld.getAccount(ADDRESS1)).isNull(); + assertThat(simpleWorld.getAccount(ADDRESS2)).isNull(); + assertThat(simpleWorld.getAccount(ADDRESS3)).isNull(); + } + + @Test + void getAccount_commitKeepsAllAccounts() { + MutableAccount acc1 = simpleWorld.createAccount(ADDRESS1); + MutableAccount acc2 = simpleWorld.createAccount(ADDRESS2); + MutableAccount acc3 = simpleWorld.createAccount(ADDRESS3); + simpleWorld.commit(); + assertThat(simpleWorld.getAccount(ADDRESS1)).isEqualTo(acc1); + assertThat(simpleWorld.getAccount(ADDRESS2)).isEqualTo(acc2); + assertThat(simpleWorld.getAccount(ADDRESS3)).isEqualTo(acc3); + } + + @Test + void getAccount_createdAccountDeletedInChild() { + simpleWorld.createAccount(ADDRESS1); + WorldUpdater childUpdater = simpleWorld.updater(); + childUpdater.deleteAccount(ADDRESS1); + assertThat(childUpdater.getAccount(ADDRESS1)).isNull(); + } + + @Test + void getTouchedAccounts_createdAccounts() { + Account acc1 = simpleWorld.createAccount(ADDRESS1); + Account acc2 = simpleWorld.createAccount(ADDRESS2); + Account acc3 = simpleWorld.createAccount(ADDRESS3); + assertThat(simpleWorld.getTouchedAccounts().toArray()) + .containsExactlyInAnyOrder(acc1, acc2, acc3); + } + + @Test + void getTouchedAccounts_revertedAccounts() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.revert(); + assertThat(simpleWorld.getTouchedAccounts()).isEmpty(); + } + + @Test + void getTouchedAccounts_createdAndDeletedAccounts() { + Account acc1 = simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + Account acc3 = simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS2); + assertThat(simpleWorld.getTouchedAccounts().toArray()).containsExactlyInAnyOrder(acc1, acc3); + } + + @Test + void getTouchedAccounts_allDeletedAccounts() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS2); + simpleWorld.deleteAccount(ADDRESS3); + assertThat(simpleWorld.getTouchedAccounts()).isEmpty(); + } + + @Test + void getTouchedAccounts_createdAndCommittedAccounts() { + Account acc1 = simpleWorld.createAccount(ADDRESS1); + Account acc2 = simpleWorld.createAccount(ADDRESS2); + Account acc3 = simpleWorld.createAccount(ADDRESS3); + simpleWorld.commit(); + assertThat(simpleWorld.getTouchedAccounts().toArray()) + .containsExactlyInAnyOrder(acc1, acc2, acc3); + } + + @Test + void getDeletedAccountAddresses_singleDeleted() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS2); + assertThat(simpleWorld.getDeletedAccountAddresses().toArray()).containsExactly(ADDRESS2); + } + + @Test + void getDeletedAccountAddresses_allDeleted() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS2); + simpleWorld.deleteAccount(ADDRESS3); + assertThat(simpleWorld.getDeletedAccountAddresses().toArray()) + .containsExactlyInAnyOrder(ADDRESS1, ADDRESS2, ADDRESS3); + } + + @Test + void getDeletedAccountAddresses_allDeletedThenRevert() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS2); + simpleWorld.deleteAccount(ADDRESS3); + simpleWorld.revert(); + assertThat(simpleWorld.getDeletedAccountAddresses()).isEmpty(); + } + + @Test + void getDeletedAccountAddresses_allDeletedThenCommit() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS1); + simpleWorld.deleteAccount(ADDRESS2); + simpleWorld.deleteAccount(ADDRESS3); + simpleWorld.commit(); + assertThat(simpleWorld.getDeletedAccountAddresses().toArray()) + .containsExactlyInAnyOrder(ADDRESS1, ADDRESS2, ADDRESS3); + } + + @Test + void commit_deletedAccountNoNPEs() { + simpleWorld.createAccount(ADDRESS1); + simpleWorld.createAccount(ADDRESS2); + simpleWorld.createAccount(ADDRESS3); + simpleWorld.deleteAccount(ADDRESS1); + simpleWorld.commit(); + } + + @Test + void commit_onlyCommitsNewAccountsToDirectParent() { + WorldUpdater simpleWorldLevel1 = simpleWorld.updater(); + WorldUpdater simpleWorldLevel2 = simpleWorldLevel1.updater(); + MutableAccount createdAccount = simpleWorldLevel2.createAccount(ADDRESS1); + simpleWorldLevel2.commit(); + assertThat(simpleWorldLevel1.getTouchedAccounts().toArray()).containsExactly(createdAccount); + assertThat(simpleWorld.getTouchedAccounts()).isEmpty(); + } + + @Test + void commit_onlyCommitsDeletedAccountsToDirectParent() { + WorldUpdater simpleWorldLevel1 = simpleWorld.updater(); + WorldUpdater simpleWorldLevel2 = simpleWorldLevel1.updater(); + simpleWorldLevel2.createAccount(ADDRESS2); + simpleWorldLevel2.deleteAccount(ADDRESS2); + simpleWorldLevel2.commit(); + assertThat(simpleWorldLevel1.getDeletedAccountAddresses().toArray()).containsExactly(ADDRESS2); + assertThat(simpleWorld.getDeletedAccountAddresses()).isEmpty(); + } + + @Test + void commit_accountsReflectChangesAfterCommit() { + MutableAccount account = simpleWorld.createAccount(ADDRESS1); + account.setStorageValue(UInt256.MAX_VALUE, UInt256.ONE); + WorldUpdater simpleWorldUpdater = simpleWorld.updater(); + + account = simpleWorldUpdater.getAccount(ADDRESS1); + account.setStorageValue(UInt256.MAX_VALUE, UInt256.valueOf(22L)); + simpleWorldUpdater.commit(); + + assertThat(simpleWorldUpdater.get(ADDRESS1).getStorageValue(UInt256.MAX_VALUE)) + .isEqualTo(simpleWorldUpdater.get(ADDRESS1).getOriginalStorageValue(UInt256.MAX_VALUE)); + } +}