diff --git a/aion_fastvm b/aion_fastvm index 071e2b9900..a4e9f2672e 160000 --- a/aion_fastvm +++ b/aion_fastvm @@ -1 +1 @@ -Subproject commit 071e2b9900cf808017adf0f9a4eeb254ae095254 +Subproject commit a4e9f2672e3903307d51ec4fb8b23b6e791ea72a diff --git a/modAion/src/org/aion/zero/db/AionContractDetailsImpl.java b/modAion/src/org/aion/zero/db/AionContractDetailsImpl.java index 1bfcdee825..5c05c9fa76 100644 --- a/modAion/src/org/aion/zero/db/AionContractDetailsImpl.java +++ b/modAion/src/org/aion/zero/db/AionContractDetailsImpl.java @@ -6,9 +6,7 @@ import static org.aion.crypto.HashUtil.h256; import java.util.Arrays; -import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -19,7 +17,6 @@ import org.aion.mcf.db.AbstractContractDetails; import org.aion.mcf.ds.XorDataSource; import org.aion.mcf.trie.SecureTrie; -import org.aion.mcf.vm.types.DataWord; import org.aion.rlp.RLP; import org.aion.rlp.RLPElement; import org.aion.rlp.RLPItem; @@ -70,15 +67,25 @@ public AionContractDetailsImpl(byte[] code) throws Exception { */ @Override public void put(ByteArrayWrapper key, ByteArrayWrapper value) { + Objects.requireNonNull(key); + Objects.requireNonNull(value); + + // The following must be done before making this call: // We strip leading zeros of a DataWord but not a DoubleDataWord so that when we call get // we can differentiate between the two. - if (value.isZero()) { - storageTrie.delete(key.getData()); - } else { - byte[] data = RLP.encodeElement(value.getData()); - storageTrie.update(key.getData(), data); - } + byte[] data = RLP.encodeElement(value.getData()); + storageTrie.update(key.getData(), data); + + this.setDirty(true); + this.rlpEncoded = null; + } + + @Override + public void delete(ByteArrayWrapper key) { + Objects.requireNonNull(key); + + storageTrie.delete(key.getData()); this.setDirty(true); this.rlpEncoded = null; @@ -95,7 +102,7 @@ public void put(ByteArrayWrapper key, ByteArrayWrapper value) { public ByteArrayWrapper get(ByteArrayWrapper key) { byte[] data = storageTrie.get(key.getData()); return (data == null || data.length == 0) - ? DataWord.ZERO.toWrapper() + ? null : new ByteArrayWrapper(RLP.decode2(data).get(0).getRLPData()); } @@ -207,60 +214,6 @@ public byte[] getEncoded() { return rlpEncoded; } - /** - * Returns a mapping of all the key-value pairs who have keys in the collection keys. - * - * @param keys The keys to query for. - * @return The associated mappings. - */ - @Override - public Map getStorage(Collection keys) { - Map storage = new HashMap<>(); - if (keys == null) { - throw new IllegalArgumentException("Input keys can't be null"); - } else { - for (ByteArrayWrapper key : keys) { - ByteArrayWrapper value = get(key); - - // we check if the value is not null, - // cause we keep all historical keys - if ((value != null) && (!value.isZero())) { - storage.put(key, value); - } - } - } - - return storage; - } - - /** - * Sets the storage to contain the specified keys and values. This method creates pairings of - * the keys and values by mapping the i'th key in storageKeys to the i'th value in - * storageValues. - * - * @param storageKeys The keys. - * @param storageValues The values. - */ - @Override - public void setStorage( - List storageKeys, List storageValues) { - for (int i = 0; i < storageKeys.size(); ++i) { - put(storageKeys.get(i), storageValues.get(i)); - } - } - - /** - * Sets the storage to contain the specified key-value mappings. - * - * @param storage The specified mappings. - */ - @Override - public void setStorage(Map storage) { - for (ByteArrayWrapper key : storage.keySet()) { - put(key, storage.get(key)); - } - } - /** * Get the address associated with this AionContractDetailsImpl. * diff --git a/modAionBase/src/org/aion/base/db/IContractDetails.java b/modAionBase/src/org/aion/base/db/IContractDetails.java index 4d22722729..fe21e2ffa4 100644 --- a/modAionBase/src/org/aion/base/db/IContractDetails.java +++ b/modAionBase/src/org/aion/base/db/IContractDetails.java @@ -1,7 +1,6 @@ package org.aion.base.db; import java.util.Collection; -import java.util.List; import java.util.Map; import org.aion.base.util.ByteArrayWrapper; import org.aion.vm.api.interfaces.Address; @@ -9,15 +8,20 @@ public interface IContractDetails { /** - * Inserts key and value as a key-value pair. If the underlying byte array of value consists - * only of zero bytes then ay existing key-value pair that has the same key as key will be - * deleted. + * Inserts a key-value pair containing the given key and the given value. * - * @param key The key. - * @param value The value. + * @param key the key to be inserted + * @param value the value to be inserted */ void put(ByteArrayWrapper key, ByteArrayWrapper value); + /** + * Deletes any key-value pair that matches the given key. + * + * @param key the key to be deleted + */ + void delete(ByteArrayWrapper key); + /** * Returns the value associated with key. * @@ -113,27 +117,19 @@ public interface IContractDetails { byte[] getEncoded(); /** - * Returns a mapping of all the key-value pairs who have keys in the collection keys. + * Returns a mapping of all the key-value pairs that have keys in the given collection keys. * - * @param keys The keys to query for. - * @return The associated mappings. + * @param keys the keys to query for + * @return the associated mappings */ Map getStorage(Collection keys); - /** - * Sets the storage to contain the specified keys and values. This method creates pairings of - * the keys and values by mapping the i'th key in storageKeys to the i'th value in - * storageValues. - * - * @param storageKeys The keys. - * @param storageValues The values. - */ - void setStorage(List storageKeys, List storageValues); - /** * Sets the storage to contain the specified key-value mappings. * - * @param storage The specified mappings. + * @param storage the specified mappings + * @apiNote Used for testing. + * @implNote A {@code null} value is interpreted as deletion. */ void setStorage(Map storage); diff --git a/modAionBase/src/org/aion/base/db/IRepositoryCache.java b/modAionBase/src/org/aion/base/db/IRepositoryCache.java index 442c32a5b1..ce8cc4f0c8 100644 --- a/modAionBase/src/org/aion/base/db/IRepositoryCache.java +++ b/modAionBase/src/org/aion/base/db/IRepositoryCache.java @@ -76,10 +76,18 @@ public interface IRepositoryCache extends IRepository { * Store the given data at the given key in the account associated with the given address. * * @param address the address of the account of interest - * @param key the key at which the date will be stored + * @param key the key at which the data will be stored * @param value the data to be stored */ void addStorageRow(Address address, ByteArrayWrapper key, ByteArrayWrapper value); + /** + * Remove the given storage key from the account associated with the given address. + * + * @param address the address of the account of interest + * @param key the key for which the data will be removed + */ + void removeStorageRow(Address address, ByteArrayWrapper key); + void flushTo(IRepository repo, boolean clearStateAfterFlush); } diff --git a/modAionBase/src/org/aion/base/util/ByteArrayWrapper.java b/modAionBase/src/org/aion/base/util/ByteArrayWrapper.java index befcf75464..3bd946e1d7 100644 --- a/modAionBase/src/org/aion/base/util/ByteArrayWrapper.java +++ b/modAionBase/src/org/aion/base/util/ByteArrayWrapper.java @@ -75,6 +75,15 @@ public boolean isZero() { return true; } + /** + * Checks if the stored byte array is empty. + * + * @return {@code true} if empty, {@code false} otherwise + */ + public boolean isEmpty() { + return data.length == 0; + } + public ByteArrayWrapper copy() { int length = data.length; byte[] bs = new byte[length]; diff --git a/modAionImpl/src/org/aion/zero/impl/AionGenesis.java b/modAionImpl/src/org/aion/zero/impl/AionGenesis.java index d57e262715..2748220be6 100644 --- a/modAionImpl/src/org/aion/zero/impl/AionGenesis.java +++ b/modAionImpl/src/org/aion/zero/impl/AionGenesis.java @@ -349,6 +349,7 @@ protected byte[] generateRootHash() { AionContractDetailsImpl networkBalanceStorage = new AionContractDetailsImpl(); for (Map.Entry entry : this.networkBalance.entrySet()) { + // we assume there are no deletions in the genesis networkBalanceStorage.put( new DataWord(entry.getKey()).toWrapper(), wrapValueForPut(new DataWord(entry.getValue()))); @@ -370,7 +371,9 @@ protected byte[] generateRootHash() { } private static ByteArrayWrapper wrapValueForPut(DataWord value) { - return (value.isZero()) ? DataWord.ZERO.toWrapper() : new ByteArrayWrapper(value.getNoLeadZeroesData()); + return (value.isZero()) + ? DataWord.ZERO.toWrapper() + : new ByteArrayWrapper(value.getNoLeadZeroesData()); } private static byte[] generateExtraData(int chainId) { diff --git a/modAionImpl/src/org/aion/zero/impl/AionHubUtils.java b/modAionImpl/src/org/aion/zero/impl/AionHubUtils.java index 3a20bbae49..09bc4661dd 100644 --- a/modAionImpl/src/org/aion/zero/impl/AionHubUtils.java +++ b/modAionImpl/src/org/aion/zero/impl/AionHubUtils.java @@ -20,6 +20,7 @@ public static void buildGenesis(AionGenesis genesis, AionRepositoryImpl reposito track.createAccount(networkBalanceAddress); for (Map.Entry addr : genesis.getNetworkBalances().entrySet()) { + // assumes only additions are performed in the genesis track.addStorageRow( networkBalanceAddress, new DataWord(addr.getKey()).toWrapper(), @@ -37,6 +38,8 @@ public static void buildGenesis(AionGenesis genesis, AionRepositoryImpl reposito } private static ByteArrayWrapper wrapValueForPut(DataWord value) { - return (value.isZero()) ? value.toWrapper() : new ByteArrayWrapper(value.getNoLeadZeroesData()); + return (value.isZero()) + ? value.toWrapper() + : new ByteArrayWrapper(value.getNoLeadZeroesData()); } } diff --git a/modAionImpl/src/org/aion/zero/impl/StandaloneBlockchain.java b/modAionImpl/src/org/aion/zero/impl/StandaloneBlockchain.java index c01f3abe64..57b93393e9 100644 --- a/modAionImpl/src/org/aion/zero/impl/StandaloneBlockchain.java +++ b/modAionImpl/src/org/aion/zero/impl/StandaloneBlockchain.java @@ -324,6 +324,7 @@ public boolean isAvmEnabled() { track.createAccount(ContractFactory.getTotalCurrencyContractAddress()); for (Map.Entry key : genesis.getNetworkBalances().entrySet()) { + // assumes only additions can be made in the genesis track.addStorageRow( ContractFactory.getTotalCurrencyContractAddress(), new DataWord(key.getKey()).toWrapper(), diff --git a/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryDummy.java b/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryDummy.java index e99e69abda..df44bb878a 100644 --- a/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryDummy.java +++ b/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryDummy.java @@ -139,24 +139,33 @@ public BigInteger getBalance(Address addr) { public ByteArrayWrapper getStorageValue(Address addr, ByteArrayWrapper key) { IContractDetails details = getContractDetails(addr); ByteArrayWrapper value = (details == null) ? null : details.get(key); - if (value == null) { - return null; - } - return (value.isZero()) ? null : value; - } - - public void addStorageRow(Address addr, ByteArrayWrapper key, ByteArrayWrapper value) { - IContractDetails details = getContractDetails(addr); - if (details == null) { - createAccount(addr); - details = getContractDetails(addr); + if (value != null && value.isZero()) { + // TODO: remove when integrating the AVM + // used to ensure FVM correctness + throw new IllegalStateException( + "The contract address " + + addr.toString() + + " returned a zero value for the key " + + key.toString() + + " which is not a valid stored value for the FVM. "); } - details.put(key, value); - detailsDB.put(ByteArrayWrapper.wrap(addr.toBytes()), details); + return value; } + // never used + // public void addStorageRow(Address addr, ByteArrayWrapper key, ByteArrayWrapper value) { + // IContractDetails details = getContractDetails(addr); + // + // if (details == null) { + // createAccount(addr); + // details = getContractDetails(addr); + // } + // details.put(key, value); + // detailsDB.put(ByteArrayWrapper.wrap(addr.toBytes()), details); + // } + public byte[] getCode(Address addr) { IContractDetails details = getContractDetails(addr); diff --git a/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryImpl.java b/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryImpl.java index 9830359931..58742e6efb 100644 --- a/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryImpl.java +++ b/modAionImpl/src/org/aion/zero/impl/db/AionRepositoryImpl.java @@ -314,11 +314,7 @@ public BigInteger getBalance(Address address) { @Override public ByteArrayWrapper getStorageValue(Address address, ByteArrayWrapper key) { IContractDetails details = getContractDetails(address); - ByteArrayWrapper value = (details == null) ? null : details.get(key); - if (value == null) { - return null; - } - return (value.isZero()) ? null : value; + return (details == null) ? null : details.get(key); } @Override diff --git a/modAionImpl/test/org/aion/db/AionContractDetailsTest.java b/modAionImpl/test/org/aion/db/AionContractDetailsTest.java index 91dc20dc68..2a7fb9957d 100644 --- a/modAionImpl/test/org/aion/db/AionContractDetailsTest.java +++ b/modAionImpl/test/org/aion/db/AionContractDetailsTest.java @@ -1,6 +1,7 @@ package org.aion.db; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.HashMap; @@ -120,13 +121,11 @@ public void test_2() throws Exception { byte[] val_1 = ByteUtil.hexStringToBytes("0000000000000000000000000000000c"); byte[] key_2 = ByteUtil.hexStringToBytes("5a448d1967513482947d1d3f6104316f"); - byte[] val_2 = ByteUtil.hexStringToBytes("00000000000000000000000000000000"); byte[] key_3 = ByteUtil.hexStringToBytes("5a448d1967513482947d1d3f61043171"); byte[] val_3 = ByteUtil.hexStringToBytes("00000000000000000000000000000014"); byte[] key_4 = ByteUtil.hexStringToBytes("18d63b70aa690ad37cb50908746c9a54"); - byte[] val_4 = ByteUtil.hexStringToBytes("00000000000000000000000000000000"); byte[] key_5 = ByteUtil.hexStringToBytes("5a448d1967513482947d1d3f61043170"); byte[] val_5 = ByteUtil.hexStringToBytes("00000000000000000000000000000078"); @@ -160,9 +159,9 @@ public void test_2() throws Exception { contractDetails.setAddress(address); contractDetails.put(new DataWord(key_0).toWrapper(), new DataWord(val_0).toWrapper()); contractDetails.put(new DataWord(key_1).toWrapper(), new DataWord(val_1).toWrapper()); - contractDetails.put(new DataWord(key_2).toWrapper(), new DataWord(val_2).toWrapper()); + contractDetails.delete(new DataWord(key_2).toWrapper()); contractDetails.put(new DataWord(key_3).toWrapper(), new DataWord(val_3).toWrapper()); - contractDetails.put(new DataWord(key_4).toWrapper(), new DataWord(val_4).toWrapper()); + contractDetails.delete(new DataWord(key_4).toWrapper()); contractDetails.put(new DataWord(key_5).toWrapper(), new DataWord(val_5).toWrapper()); contractDetails.put(new DataWord(key_6).toWrapper(), new DataWord(val_6).toWrapper()); contractDetails.put(new DataWord(key_7).toWrapper(), new DataWord(val_7).toWrapper()); @@ -186,20 +185,14 @@ public void test_2() throws Exception { ByteUtil.toHexString( contractDetails_.get(new DataWord(key_1).toWrapper()).getData())); - assertEquals( - ByteUtil.toHexString(val_2), - ByteUtil.toHexString( - contractDetails_.get(new DataWord(key_2).toWrapper()).getData())); + assertNull(contractDetails_.get(new DataWord(key_2).toWrapper())); assertEquals( ByteUtil.toHexString(val_3), ByteUtil.toHexString( contractDetails_.get(new DataWord(key_3).toWrapper()).getData())); - assertEquals( - ByteUtil.toHexString(val_4), - ByteUtil.toHexString( - contractDetails_.get(new DataWord(key_4).toWrapper()).getData())); + assertNull(contractDetails_.get(new DataWord(key_4).toWrapper())); assertEquals( ByteUtil.toHexString(val_5), @@ -284,14 +277,15 @@ public void testExternalStorageSerialization() { assertEquals(ByteUtil.toHexString(code), ByteUtil.toHexString(deserialized.getCode())); for (DataWord key : elements.keySet()) { - assertEquals(elements.get(key).toWrapper(), wrapValueFromGet(deserialized.get(key.toWrapper()))); + assertEquals( + elements.get(key).toWrapper(), + wrapValueFromGet(deserialized.get(key.toWrapper()))); } DataWord deletedKey = elements.keySet().iterator().next(); - deserialized.put(deletedKey.toWrapper(), DataWord.ZERO.toWrapper()); - deserialized.put( - new DataWord(RandomUtils.nextBytes(16)).toWrapper(), DataWord.ZERO.toWrapper()); + deserialized.delete(deletedKey.toWrapper()); + deserialized.delete(new DataWord(RandomUtils.nextBytes(16)).toWrapper()); } @Test @@ -337,12 +331,16 @@ public void testExternalStorageTransition() { deserialized = deserialize(deserialized.getEncoded(), externalStorage); for (DataWord key : elements.keySet()) { - assertEquals(elements.get(key).toWrapper(), wrapValueFromGet(deserialized.get(key.toWrapper()))); + assertEquals( + elements.get(key).toWrapper(), + wrapValueFromGet(deserialized.get(key.toWrapper()))); } } private static ByteArrayWrapper wrapValueForPut(DataWord value) { - return (value.isZero()) ? new ByteArrayWrapper(value.getData()) : new ByteArrayWrapper(value.getNoLeadZeroesData()); + return (value.isZero()) + ? new ByteArrayWrapper(value.getData()) + : new ByteArrayWrapper(value.getNoLeadZeroesData()); } private static ByteArrayWrapper wrapValueFromGet(ByteArrayWrapper value) { diff --git a/modMcf/src/org/aion/mcf/db/AbstractContractDetails.java b/modMcf/src/org/aion/mcf/db/AbstractContractDetails.java index 957e4cc226..5cfe0e56d2 100644 --- a/modMcf/src/org/aion/mcf/db/AbstractContractDetails.java +++ b/modMcf/src/org/aion/mcf/db/AbstractContractDetails.java @@ -4,6 +4,8 @@ import static org.aion.crypto.HashUtil.h256; import static org.aion.util.bytes.ByteUtil.EMPTY_BYTE_ARRAY; +import com.google.common.annotations.VisibleForTesting; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import org.aion.base.db.IContractDetails; @@ -92,13 +94,62 @@ public boolean isDeleted() { @Override public String toString() { - String ret = - " Code: " - + (codes.size() < 2 - ? Hex.toHexString(getCode()) - : codes.size() + " versions") - + "\n"; - ret += " Storage: " + getStorageHash(); + String ret; + + if (codes != null) { + ret = + " Code: " + + (codes.size() < 2 + ? Hex.toHexString(getCode()) + : codes.size() + " versions") + + "\n"; + } else { + ret = " Code: null\n"; + } + + byte[] storage = getStorageHash(); + if (storage != null) { + ret += " Storage: " + Hex.toHexString(storage); + } else { + ret += " Storage: null"; + } + return ret; } + + @VisibleForTesting + @Override + public void setStorage(Map storage) { + for (Map.Entry entry : storage.entrySet()) { + ByteArrayWrapper key = entry.getKey(); + ByteArrayWrapper value = entry.getValue(); + + if (value != null) { + put(key, value); + } else { + delete(key); + } + } + } + + @Override + public Map getStorage(Collection keys) { + Map storage = new HashMap<>(); + + if (keys == null) { + throw new IllegalArgumentException("Input keys cannot be null"); + } else { + for (ByteArrayWrapper key : keys) { + ByteArrayWrapper value = get(key); + + // we check if the value is not null, + // cause we keep all historical keys + if (value != null) { + storage.put(key, value); + } + } + } + + return storage; + } } diff --git a/modMcf/src/org/aion/mcf/db/AbstractRepositoryCache.java b/modMcf/src/org/aion/mcf/db/AbstractRepositoryCache.java index e1e970e42c..5cc0202e84 100644 --- a/modMcf/src/org/aion/mcf/db/AbstractRepositoryCache.java +++ b/modMcf/src/org/aion/mcf/db/AbstractRepositoryCache.java @@ -310,12 +310,18 @@ public void addStorageRow(Address address, ByteArrayWrapper key, ByteArrayWrappe } @Override - public ByteArrayWrapper getStorageValue(Address address, ByteArrayWrapper key) { - ByteArrayWrapper value = getContractDetails(address).get(key); - if (value == null) { - return null; + public void removeStorageRow(Address address, ByteArrayWrapper key) { + lockDetails.writeLock().lock(); + try { + getContractDetails(address).delete(key); + } finally { + lockDetails.writeLock().unlock(); } - return (value.isZero()) ? null : value; + } + + @Override + public ByteArrayWrapper getStorageValue(Address address, ByteArrayWrapper key) { + return getContractDetails(address).get(key); } @Override diff --git a/modMcf/src/org/aion/mcf/db/ContractDetailsCacheImpl.java b/modMcf/src/org/aion/mcf/db/ContractDetailsCacheImpl.java index ccd2835d3c..4532ad03c7 100644 --- a/modMcf/src/org/aion/mcf/db/ContractDetailsCacheImpl.java +++ b/modMcf/src/org/aion/mcf/db/ContractDetailsCacheImpl.java @@ -1,9 +1,7 @@ package org.aion.mcf.db; -import java.util.Collection; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -50,10 +48,21 @@ public static ContractDetailsCacheImpl copy(ContractDetailsCacheImpl cache) { */ @Override public void put(ByteArrayWrapper key, ByteArrayWrapper value) { + Objects.requireNonNull(key); + Objects.requireNonNull(value); + storage.put(key, value); setDirty(true); } + @Override + public void delete(ByteArrayWrapper key) { + Objects.requireNonNull(key); + + storage.put(key, null); + setDirty(true); + } + /** * Returns the value associated with key if it exists, otherwise returns null. * @@ -62,24 +71,29 @@ public void put(ByteArrayWrapper key, ByteArrayWrapper value) { */ @Override public ByteArrayWrapper get(ByteArrayWrapper key) { - ByteArrayWrapper value = storage.get(key); - if (value != null) { - value = value.copy(); - } else { + ByteArrayWrapper value; + + // go to parent if not locally stored + if (!storage.containsKey(key)) { if (origContract == null) { return null; } value = origContract.get(key); - // TODO: the VM must pad the given ZERO value if expecting a fixed size byte array - value = (value == null) ? ByteArrayWrapper.ZERO : value; - storage.put(key.copy(), value.isZero() ? ByteArrayWrapper.ZERO.copy() : value.copy()); - } - if (value == null || value.isZero()) { - return null; - } else { - return value; + // save a copy to local storage + if (value != null) { + storage.put(key.copy(), value.copy()); + } else { + storage.put(key.copy(), null); + } + } else { // check local storage + value = storage.get(key); + + if (value != null) { + value = value.copy(); + } } + return value; } /** @@ -110,65 +124,6 @@ public byte[] getEncoded() { throw new RuntimeException("Not supported by this implementation."); } - /** - * Returns a mapping of all the key-value pairs who have keys in the collection keys. - * - * @param keys The keys to query for. - * @return The associated mappings. - */ - @Override - public Map getStorage(Collection keys) { - Map storage = new HashMap<>(); - if (keys == null) { - throw new IllegalArgumentException("Input keys can't be null"); - } else { - for (ByteArrayWrapper key : keys) { - ByteArrayWrapper value = get(key); - - // we check if the value is not null, - // cause we keep all historical keys - if ((value != null) && (!value.isZero())) { - storage.put(key, value); - } - } - } - - return storage; - } - - /** - * Sets the storage to contain the specified keys and values. This method creates pairings of - * the keys and values by mapping the i'th key in storageKeys to the i'th value in - * storageValues. - * - * @param storageKeys The keys. - * @param storageValues The values. - */ - @Override - public void setStorage( - List storageKeys, List storageValues) { - - for (int i = 0; i < storageKeys.size(); ++i) { - - ByteArrayWrapper key = storageKeys.get(i); - ByteArrayWrapper value = storageValues.get(i); - - put(key, value); - } - } - - /** - * Sets the storage to contain the specified key-value mappings. - * - * @param storage The specified mappings. - */ - @Override - public void setStorage(Map storage) { - for (Map.Entry entry : storage.entrySet()) { - put(entry.getKey(), entry.getValue()); - } - } - /** * Get the address associated with this ContractDetailsCacheImpl. * @@ -211,7 +166,12 @@ public void commit() { } for (ByteArrayWrapper key : storage.keySet()) { - origContract.put(key, storage.get(key)); + ByteArrayWrapper value = storage.get(key); + if (value != null) { + origContract.put(key, storage.get(key)); + } else { + origContract.delete(key); + } } if (origContract instanceof AbstractContractDetails) { diff --git a/modMcf/src/org/aion/mcf/vm/types/KernelInterfaceForFastVM.java b/modMcf/src/org/aion/mcf/vm/types/KernelInterfaceForFastVM.java index 1919f88104..b7c575ab6f 100644 --- a/modMcf/src/org/aion/mcf/vm/types/KernelInterfaceForFastVM.java +++ b/modMcf/src/org/aion/mcf/vm/types/KernelInterfaceForFastVM.java @@ -76,18 +76,30 @@ public byte[] getCode(Address address) { public void putStorage(Address address, byte[] key, byte[] value) { ByteArrayWrapper storageKey = alignDataToWordSize(key); ByteArrayWrapper storageValue = alignValueToWordSizeForPut(value); + if (value == null || value.length == 0 || storageValue.isZero()) { + // used to ensure FVM correctness + throw new IllegalArgumentException( + "Put with null, empty or zero byte array values is not allowed for the FVM. For deletions, make explicit calls to the delete method."); + } + this.repositoryCache.addStorageRow(address, storageKey, storageValue); } @Override public void removeStorage(Address address, byte[] key) { - // TODO: remove this placeholder for changes made in a reorganized commit + ByteArrayWrapper storageKey = alignDataToWordSize(key); + this.repositoryCache.removeStorageRow(address, storageKey); } @Override public byte[] getStorage(Address address, byte[] key) { ByteArrayWrapper storageKey = alignDataToWordSize(key); ByteArrayWrapper value = this.repositoryCache.getStorageValue(address, storageKey); + if (value != null && (value.isZero() || value.isEmpty())) { + // used to ensure FVM correctness + throw new IllegalStateException( + "A zero or empty value was retrieved from storage. Storing zeros is not allowed by the FVM. An incorrect put was previously performed instead of an explicit call to the delete method."); + } return (value == null) ? DataWord.ZERO.getData() : alignValueToWordSizeForGet(value); } diff --git a/modMcf/test/org/aion/mcf/db/AionRepositoryCacheTest.java b/modMcf/test/org/aion/mcf/db/AionRepositoryCacheTest.java index 77b792fc6a..9b7870c98b 100644 --- a/modMcf/test/org/aion/mcf/db/AionRepositoryCacheTest.java +++ b/modMcf/test/org/aion/mcf/db/AionRepositoryCacheTest.java @@ -77,11 +77,11 @@ public void testGetStorageValueNoSuchAddress() { public void testGetStorageValueIsSingleZero() { Address address = getNewAddress(); IDataWord key = new DataWord(RandomUtils.nextBytes(DataWord.BYTES)); - cache.addStorageRow(address, key.toWrapper(), DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key.toWrapper()); assertNull(cache.getStorageValue(address, key.toWrapper())); key = new DoubleDataWord(RandomUtils.nextBytes(DoubleDataWord.BYTES)); - cache.addStorageRow(address, key.toWrapper(), DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key.toWrapper()); assertNull(cache.getStorageValue(address, key.toWrapper())); } @@ -89,11 +89,11 @@ public void testGetStorageValueIsSingleZero() { public void testGetStorageValueIsDoubleZero() { Address address = getNewAddress(); IDataWord key = new DataWord(RandomUtils.nextBytes(DataWord.BYTES)); - cache.addStorageRow(address, key.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key.toWrapper()); assertNull(cache.getStorageValue(address, key.toWrapper())); key = new DoubleDataWord(RandomUtils.nextBytes(DoubleDataWord.BYTES)); - cache.addStorageRow(address, key.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key.toWrapper()); assertNull(cache.getStorageValue(address, key.toWrapper())); } @@ -126,20 +126,19 @@ public void testGetStorageValueWithZeroKeyAndValue() { Address address = getNewAddress(); // single-single - cache.addStorageRow(address, DataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, DataWord.ZERO.toWrapper()); assertNull(cache.getStorageValue(address, DataWord.ZERO.toWrapper())); // single-double - cache.addStorageRow(address, DataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, DataWord.ZERO.toWrapper()); assertNull(cache.getStorageValue(address, DataWord.ZERO.toWrapper())); // double-single - cache.addStorageRow(address, DoubleDataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, DoubleDataWord.ZERO.toWrapper()); assertNull(cache.getStorageValue(address, DoubleDataWord.ZERO.toWrapper())); // double-double - cache.addStorageRow( - address, DoubleDataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, DoubleDataWord.ZERO.toWrapper()); assertNull(cache.getStorageValue(address, DoubleDataWord.ZERO.toWrapper())); } @@ -151,7 +150,7 @@ public void testOverwriteValueWithSingleZero() { new DoubleDataWord(RandomUtils.nextBytes(DoubleDataWord.BYTES)).toWrapper(); cache.addStorageRow(address, key, value); assertEquals(value, cache.getStorageValue(address, key)); - cache.addStorageRow(address, key, DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key); assertNull(cache.getStorageValue(address, key)); } @@ -163,7 +162,7 @@ public void testOverwriteValueWithDoubleZero() { ByteArrayWrapper value = new DataWord(RandomUtils.nextBytes(DataWord.BYTES)).toWrapper(); cache.addStorageRow(address, key, value); assertEquals(value, cache.getStorageValue(address, key)); - cache.addStorageRow(address, key, DoubleDataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key); assertNull(cache.getStorageValue(address, key)); } @@ -227,7 +226,7 @@ private void deleteEveryNthEntry(Address address, List keys, i int count = 1; for (ByteArrayWrapper key : keys) { if (count % n == 0) { - cache.addStorageRow(address, key, DataWord.ZERO.toWrapper()); + cache.removeStorageRow(address, key); } count++; } diff --git a/modMcf/test/org/aion/mcf/db/IContractDetailsTest.java b/modMcf/test/org/aion/mcf/db/IContractDetailsTest.java index 596dbc7cce..3cbc653f59 100644 --- a/modMcf/test/org/aion/mcf/db/IContractDetailsTest.java +++ b/modMcf/test/org/aion/mcf/db/IContractDetailsTest.java @@ -1,10 +1,11 @@ package org.aion.mcf.db; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -95,35 +96,31 @@ public void testPutDoubleZeroKey() { @Test public void testPutZeroKeyAndValue() { // Try single-single - cache1.put(DataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + cache1.delete(DataWord.ZERO.toWrapper()); ByteArrayWrapper result = cache1.get(DataWord.ZERO.toWrapper()); - assertTrue(result.isZero()); - assertTrue(result instanceof ByteArrayWrapper); - cache2.put(DataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + assertNull(result); + cache2.delete(DataWord.ZERO.toWrapper()); assertNull(cache2.get(DataWord.ZERO.toWrapper())); // Try single-double - cache1.put(DataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache1.delete(DataWord.ZERO.toWrapper()); result = cache1.get(DataWord.ZERO.toWrapper()); - assertTrue(result.isZero()); - assertTrue(result instanceof ByteArrayWrapper); - cache2.put(DataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + assertNull(result); + cache2.delete(DataWord.ZERO.toWrapper()); assertNull(cache2.get(DataWord.ZERO.toWrapper())); // Try double-single - cache1.put(DoubleDataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + cache1.delete(DoubleDataWord.ZERO.toWrapper()); result = cache1.get(DoubleDataWord.ZERO.toWrapper()); - assertTrue(result.isZero()); - assertTrue(result instanceof ByteArrayWrapper); - cache2.put(DoubleDataWord.ZERO.toWrapper(), DataWord.ZERO.toWrapper()); + assertNull(result); + cache2.delete(DoubleDataWord.ZERO.toWrapper()); assertNull(cache2.get(DoubleDataWord.ZERO.toWrapper())); // Try double-double - cache1.put(DoubleDataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + cache1.delete(DoubleDataWord.ZERO.toWrapper()); result = cache1.get(DoubleDataWord.ZERO.toWrapper()); - assertTrue(result.isZero()); - assertTrue(result instanceof ByteArrayWrapper); - cache2.put(DoubleDataWord.ZERO.toWrapper(), DoubleDataWord.ZERO.toWrapper()); + assertNull(result); + cache2.delete(DoubleDataWord.ZERO.toWrapper()); assertNull(cache2.get(DoubleDataWord.ZERO.toWrapper())); } @@ -228,8 +225,7 @@ public void testCommitEnMassOriginalIsAionContract() { try { if (count % deleteOdds == 0) { assertNull(impl.get(key)); - assertTrue(cache1.get(key).isZero()); - assertTrue(cache1.get(key) instanceof ByteArrayWrapper); + assertNull(cache1.get(key)); } else { assertEquals(impl.get(key), cache1.get(key)); } @@ -279,6 +275,44 @@ public void testCommitEnMassOriginalIsContractDetails() { } } + /** + * This test is specific to the ContractDetailsCacheImpl class, which at times holds a different + * storage value for contracts. This test checks that after an update to the cache object, the + * original value from the contract details is not returned for use. + */ + @Test + public void testCacheUpdatedAndGetWithOriginalAionContract() { + + ByteArrayWrapper key = getRandomWord(true).toWrapper(); + ByteArrayWrapper value1 = getRandomWord(true).toWrapper(); + ByteArrayWrapper value2 = getRandomWord(true).toWrapper(); + + // ensure the second value is different + // unlikely to be necessary + while (Arrays.equals(value1.getData(), value2.getData())) { + value2 = getRandomWord(true).toWrapper(); + } + + // ensure the initial cache has the value + cache1.put(key, value1); + + ContractDetailsCacheImpl impl = new ContractDetailsCacheImpl(cache1); + + // check that original value is retrieved + assertThat(impl.get(key)).isEqualTo(value1); + + // delete and check that value is missing + impl.delete(key); + assertThat(impl.get(key)).isEqualTo(null); + + // add new value and check correctness + impl.put(key, value2); + assertThat(impl.get(key)).isEqualTo(value2); + + // clean-up + cache1.delete(key); + } + // <------------------------------------------HELPERS-------------------------------------------> /** @@ -323,13 +357,13 @@ private void doPutKeyValueThenOverwriteValueWithZero( // Test DataWord. cache.put(key, value); assertEquals(value, cache.get(key)); - cache.put(key, DataWord.ZERO.toWrapper()); + cache.delete(key); checkGetNonExistentPairing(cache, key); // Test DoubleDataWord. cache.put(key, value); assertEquals(value, cache.get(key)); - cache.put(key, DoubleDataWord.ZERO.toWrapper()); + cache.delete(key); checkGetNonExistentPairing(cache, key); } @@ -392,7 +426,7 @@ private void deleteEveryNthEntry(IContractDetails cache, List int count = 1; for (ByteArrayWrapper key : keys) { if (count % n == 0) { - cache.put(key, DataWord.ZERO.toWrapper()); + cache.delete(key); } count++; } @@ -405,7 +439,12 @@ private void massPutIntoCache( int size = keys.size(); assertEquals(size, values.size()); for (int i = 0; i < size; i++) { - cache.put(keys.get(i), values.get(i)); + ByteArrayWrapper value = values.get(i); + if (value == null || value.isZero()) { + cache.delete(keys.get(i)); + } else { + cache.put(keys.get(i), values.get(i)); + } } } @@ -445,7 +484,7 @@ private IDataWord getRandomWord(boolean isSingleWord) { private void doSetZeroValueViaStorageTest(IContractDetails cache) { Map storage = new HashMap<>(); ByteArrayWrapper key = new DataWord(RandomUtils.nextBytes(DataWord.BYTES)).toWrapper(); - storage.put(key, DataWord.ZERO.toWrapper()); + storage.put(key, null); cache.setStorage(storage); checkGetNonExistentPairing(cache, key); } @@ -456,7 +495,7 @@ private void checkKeyValueMapping( for (ByteArrayWrapper key : storage.keySet()) { ByteArrayWrapper value = storage.get(key); - if (value.isZero()) { + if (value == null) { checkGetNonExistentPairing(cache, key); } else { assertEquals(value, cache.get(key)); @@ -477,7 +516,7 @@ private Map getKeyValueMappingInBulk( assertEquals(size, values.size()); for (int i = 0; i < size; i++) { if ((i + 1) % n == 0) { - storage.put(keys.get(i), DoubleDataWord.ZERO.toWrapper()); + storage.put(keys.get(i), null); } else { storage.put(keys.get(i), values.get(i)); } @@ -491,13 +530,7 @@ private Map getKeyValueMappingInBulk( */ private void checkGetNonExistentPairing(IContractDetails cache, ByteArrayWrapper key) { try { - if (cache instanceof AionContractDetailsImpl) { - ByteArrayWrapper result = cache.get(key); - assertTrue(result.isZero()); - assertTrue(result instanceof ByteArrayWrapper); - } else { - assertNull(cache.get(key)); - } + assertNull(cache.get(key)); } catch (AssertionError e) { System.err.println("\nAssertion failed on key: " + Hex.toHexString(key.getData())); e.printStackTrace(); diff --git a/modPrecompiled/src/org/aion/precompiled/contracts/ATB/BridgeStorageConnector.java b/modPrecompiled/src/org/aion/precompiled/contracts/ATB/BridgeStorageConnector.java index 9dad74707a..f200c6776b 100644 --- a/modPrecompiled/src/org/aion/precompiled/contracts/ATB/BridgeStorageConnector.java +++ b/modPrecompiled/src/org/aion/precompiled/contracts/ATB/BridgeStorageConnector.java @@ -219,18 +219,24 @@ private byte[] getWORD(@Nonnull final DataWord key) { } private void setWORD(@Nonnull final DataWord key, @Nonnull final DataWord word) { - this.track.addStorageRow( - contractAddress, - key.toWrapper(), - (word.isZero()) - ? word.toWrapper() - : new ByteArrayWrapper(word.getNoLeadZeroesData())); + if (word.isZero()) { + this.track.removeStorageRow(contractAddress, key.toWrapper()); + } else { + this.track.addStorageRow( + contractAddress, + key.toWrapper(), + new ByteArrayWrapper(word.getNoLeadZeroesData())); + } } private void setDWORD(@Nonnull final DataWord key, @Nonnull final byte[] dword) { assert dword.length > 16; - this.track.addStorageRow( - contractAddress, key.toWrapper(), new DoubleDataWord(dword).toWrapper()); + DoubleDataWord ddw = new DoubleDataWord(dword); + if (ddw.isZero()) { + this.track.removeStorageRow(contractAddress, key.toWrapper()); + } else { + this.track.addStorageRow(contractAddress, key.toWrapper(), ddw.toWrapper()); + } } private byte[] getDWORD(@Nonnull final DataWord key) { diff --git a/modPrecompiled/src/org/aion/precompiled/contracts/AionAuctionContract.java b/modPrecompiled/src/org/aion/precompiled/contracts/AionAuctionContract.java index 4e148fea5d..9932e426f3 100644 --- a/modPrecompiled/src/org/aion/precompiled/contracts/AionAuctionContract.java +++ b/modPrecompiled/src/org/aion/precompiled/contracts/AionAuctionContract.java @@ -488,10 +488,8 @@ private void processAuction(Address domainAddress) { domainName = getNameFromStorage(auctionDomainsAddressName, domainAddress); addBigIntegerToStorage(domainAddress, BID_KEY_COUNTER, new BigInteger("0")); // remove from auction domains - this.track.addStorageRow( - auctionDomainsAddress, - new DataWord(blake128(domainAddress.toBytes())).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); + this.track.removeStorageRow( + auctionDomainsAddress, new DataWord(blake128(domainAddress.toBytes())).toWrapper()); addToActiveDomains(domainAddress, winnerAddress, domainName, secondHighestBid); printWinner(domainAddress, winnerAddress, secondHighestBid, domainName); } @@ -606,30 +604,23 @@ private void removeActiveDomain(Address domainAddress, long expireTime) { printRemoveActiveDomain(domainAddress); // erase - this.track.addStorageRow( - activeDomainsAddress, - new DataWord(blake128(domainAddress.toBytes())).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); - this.track.addStorageRow( + this.track.removeStorageRow( + activeDomainsAddress, new DataWord(blake128(domainAddress.toBytes())).toWrapper()); + this.track.removeStorageRow( activeDomainsAddress, - new DataWord(blake128(blake128(domainAddress.toBytes()))).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); - this.track.addStorageRow( + new DataWord(blake128(blake128(domainAddress.toBytes()))).toWrapper()); + this.track.removeStorageRow( activeDomainsAddressName, - new DataWord(blake128(domainAddress.toBytes())).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); - this.track.addStorageRow( + new DataWord(blake128(domainAddress.toBytes())).toWrapper()); + this.track.removeStorageRow( activeDomainsAddressName, - new DataWord(blake128(blake128(domainAddress.toBytes()))).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); - this.track.addStorageRow( + new DataWord(blake128(blake128(domainAddress.toBytes()))).toWrapper()); + this.track.removeStorageRow( activeDomainsAddressValue, - new DataWord(blake128(domainAddress.toBytes())).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); - this.track.addStorageRow( + new DataWord(blake128(domainAddress.toBytes())).toWrapper()); + this.track.removeStorageRow( activeDomainsAddressTime, - new DataWord(blake128(domainAddress.toBytes())).toWrapper(), - DoubleDataWord.ZERO.toWrapper()); + new DataWord(blake128(domainAddress.toBytes())).toWrapper()); } /** diff --git a/modPrecompiled/test/org/aion/precompiled/contracts/DummyRepo.java b/modPrecompiled/test/org/aion/precompiled/contracts/DummyRepo.java index 4317229f04..a2e20fec50 100644 --- a/modPrecompiled/test/org/aion/precompiled/contracts/DummyRepo.java +++ b/modPrecompiled/test/org/aion/precompiled/contracts/DummyRepo.java @@ -117,11 +117,20 @@ public void addStorageRow(Address addr, ByteArrayWrapper key, ByteArrayWrapper v map.put(key.toString(), value.getData()); } + @Override + public void removeStorageRow(Address addr, ByteArrayWrapper key) { + Map map = storage.computeIfAbsent(addr, k -> new HashMap<>()); + map.put(key.toString(), null); + } + @Override public ByteArrayWrapper getStorageValue(Address addr, ByteArrayWrapper key) { Map map = storage.get(addr); if (map != null && map.containsKey(key.toString())) { byte[] res = map.get(key.toString()); + if (res == null) { + return null; + } if (res.length <= DataWord.BYTES) { return new DataWord(res).toWrapper(); } else if (res.length == DoubleDataWord.BYTES) {