Adding the structure for transient storage opcodes

- Adding scratch from the DSL tests and contracts for tests - The tests are still failing, I am investigating to find the root cause - Adding more logic to the map structures - Finishing test validation for basic scenarios of TLOAD/TSTORE - Now we need to add more scenarios to validate that the memory isn't shared and erased in the end of the transaction.
rsksmart · Oct 14, 2024 · 7149f88 · 7149f88
1 parent d6dc804
commit 7149f88
Show file tree

Hide file tree

Showing 7 changed files with 338 additions and 77 deletions.
diff --git a/rskj-core/src/main/java/org/ethereum/db/MutableRepository.java b/rskj-core/src/main/java/org/ethereum/db/MutableRepository.java
@@ -31,6 +31,7 @@
 import org.ethereum.core.Repository;
 import org.ethereum.crypto.HashUtil;
 import org.ethereum.crypto.Keccak256Helper;
+import org.ethereum.datasource.HashMapDB;
 import org.ethereum.vm.DataWord;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -39,7 +40,7 @@
 import java.math.BigInteger;
 import java.util.*;
 
-public class MutableRepository implements Repository {
+public class MutableRepository implements Repository, TransientStorageRepository {
     private static final Logger logger = LoggerFactory.getLogger("repository");
     private static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
     public static final Keccak256 KECCAK_256_OF_EMPTY_ARRAY = new Keccak256(Keccak256Helper.keccak256(EMPTY_BYTE_ARRAY));
@@ -48,6 +49,7 @@ public class MutableRepository implements Repository {
     private final TrieKeyMapper trieKeyMapper;
     private final MutableTrie mutableTrie;
     private final IReadWrittenKeysTracker tracker;
+    private Repository transientRepository;
 
     public MutableRepository(TrieStore trieStore, Trie trie) {
         this(new MutableTrieImpl(trieStore, trie));
@@ -406,4 +408,22 @@ private Optional<Keccak256> internalGetValueHash(byte[] key) {
         tracker.addNewReadKey(new ByteArrayWrapper(key));
         return mutableTrie.getValueHash(key);
     }
+
+    @Override
+    public Repository getTransientRepository() {
+        if(transientRepository == null) {
+            transientRepository = getMutableRepository();
+        }
+
+        return transientRepository;
+    }
+
+    @Override
+    public synchronized void clearTransientRepository() {
+        transientRepository =  getMutableRepository();
+    }
+
+    private static MutableRepository getMutableRepository() {
+        return new MutableRepository(new MutableTrieImpl(new TrieStoreImpl(new HashMapDB()), new Trie()));
+    }
 }
diff --git a/rskj-core/src/main/java/org/ethereum/db/TransientStorageRepository.java b/rskj-core/src/main/java/org/ethereum/db/TransientStorageRepository.java
@@ -0,0 +1,34 @@
+/*
+ * This file is part of RskJ
+ * Copyright (C) 2024 RSK Labs Ltd.
+ * (derived from ethereumJ library, Copyright (c) 2016 <ether.camp>)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package org.ethereum.db;
+
+import org.ethereum.core.Repository;
+
+public interface TransientStorageRepository {
+    /**
+     * Returns the transientRepository initializing a new one if the current one is null
+     */
+    Repository getTransientRepository();
+
+    /**
+     *  Delete all the data for this repository
+     */
+    void clearTransientRepository();
+}
diff --git a/rskj-core/src/main/java/org/ethereum/vm/VM.java b/rskj-core/src/main/java/org/ethereum/vm/VM.java
@@ -1327,30 +1327,30 @@ else if (oldValue != null && newValue.isZero()) {
     }
 
     protected void doTLOAD(){
-
         DataWord key = program.stackPop();
-        DataWord address = program.getOwnerAddress();
-
         if (isLogEnabled) {
-            logger.info("Executing TLOAD with parameters: address={} | key = {}", address, key);
+            logger.info("Executing TLOAD with parameters: key = {}",  key);
         }
+        DataWord val = program.transientStorageLoad(key);
 
-        program.transientStorageSave(key, address);
-        // key could be returned to the pool, but storageLoad semantics should be checked
+        if (val == null) {
+            val = DataWord.ZERO;
+        }
+
+        program.stackPush(val);
+        // key could be returned to the pool, but transientStorageLoad semantics should be checked
         // to make sure storageLoad always gets a copy, not a reference.
         program.step();
     }
 
     protected void doTSTORE(){
-
+        DataWord address = program.stackPop();
         DataWord value = program.stackPop();
-        DataWord address = program.getOwnerAddress();
-        DataWord key = DataWord.ZERO;
 
         if (isLogEnabled) {
-            logger.info("Executing TSTORE with parameters: address={} | key = {} | value = {}",  address, key, value);
+            logger.info("Executing TSTORE with parameters: address={} | value = {}",  address, value);
         }
-        program.transientStorageLoad(address, key, value);
+        program.transientStorageSave(address, value);
         program.step();
     }
 
@@ -1967,9 +1967,16 @@ protected void executeOpcode() {
             break;
             case OpCodes.OP_SSTORE: doSSTORE();
                 break;
-            case OpCodes.OP_TLOAD: doTLOAD();
+            case OpCodes.OP_TLOAD:
+                if (!activations.isActive(RSKIP446)) {
+                    throw Program.ExceptionHelper.invalidOpCode(program);
+                }
+                doTLOAD();
                 break;
             case OpCodes.OP_TSTORE: doTSTORE();
+                if (!activations.isActive(RSKIP446)) {
+                    throw Program.ExceptionHelper.invalidOpCode(program);
+                }
                 break;
             case OpCodes.OP_JUMP: doJUMP();
             break;

diff --git a/rskj-core/src/main/java/org/ethereum/vm/program/Program.java b/rskj-core/src/main/java/org/ethereum/vm/program/Program.java
@@ -29,6 +29,8 @@
 import co.rsk.rpc.modules.trace.CallType;
 import co.rsk.rpc.modules.trace.CreationData;
 import co.rsk.rpc.modules.trace.ProgramSubtrace;
+import co.rsk.trie.Trie;
+import co.rsk.trie.TrieStoreImpl;
 import co.rsk.vm.BitSet;
 import com.google.common.annotations.VisibleForTesting;
 import org.ethereum.config.Constants;
@@ -40,6 +42,8 @@
 import org.ethereum.core.SignatureCache;
 import org.ethereum.core.Transaction;
 import org.ethereum.crypto.HashUtil;
+import org.ethereum.datasource.HashMapDB;
+import org.ethereum.db.MutableRepository;
 import org.ethereum.util.ByteUtil;
 import org.ethereum.util.FastByteComparisons;
 import org.ethereum.vm.DataWord;
@@ -72,8 +76,10 @@
 import java.math.BigInteger;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 
@@ -95,7 +101,6 @@ public class Program {
     private static final Logger logger = LoggerFactory.getLogger("VM");
     private static final Logger gasLogger = LoggerFactory.getLogger("gas");
 
-
     public static final long MAX_MEMORY = (1<<30);
 
     //Max size for stack checks
@@ -115,6 +120,7 @@ public class Program {
     private final Stack stack;
     private final Memory memory;
     private final Storage storage;
+    private final Map<RskAddress, MutableRepository> transientStorages;
     private byte[] returnDataBuffer;
 
     private final ProgramResult result = new ProgramResult();
@@ -178,6 +184,7 @@ public Program(
         this.stack = setupProgramListener(new Stack());
         this.stack.ensureCapacity(1024); // faster?
         this.storage = setupProgramListener(new Storage(programInvoke));
+        this.transientStorages = new HashMap<>();
         this.deletedAccountsInBlock = new HashSet<>(deletedAccounts);
         this.signatureCache = signatureCache;
         precompile();
@@ -453,6 +460,16 @@ public Repository getStorage() {
         return this.storage;
     }
 
+    public MutableRepository getTransientStorage(RskAddress addr) {
+        MutableRepository current = transientStorages.get(addr);
+        if(current == null) {
+            current = new MutableRepository(new TrieStoreImpl(new HashMapDB()), new Trie());
+            transientStorages.put(addr, current);
+        }
+        return current;
+    }
+
+
     @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
     public void createContract(DataWord value, DataWord memStart, DataWord memSize) {
         RskAddress senderAddress = new RskAddress(getOwnerAddress());
@@ -985,8 +1002,10 @@ private void storageSave(byte[] key, byte[] val) {
         getStorage().addStorageRow(getOwnerRskAddress(), keyWord, valWord);
     }
 
-    public void transientStorageSave(DataWord key, DataWord address) {
-
+    public void transientStorageSave(DataWord key, DataWord value) {
+        RskAddress addr = getOwnerRskAddress();
+        MutableRepository storage = getTransientStorage(addr);
+        storage.addStorageRow(addr, key, value);
     }
 
     private RskAddress getOwnerRskAddress() {
@@ -1099,8 +1118,11 @@ public DataWord storageLoad(DataWord key) {
         return getStorage().getStorageValue(getOwnerRskAddress(), key);
     }
 
-    public void transientStorageLoad(DataWord address, DataWord key, DataWord value) {
+    public DataWord transientStorageLoad(DataWord key) {
+        RskAddress addr = getOwnerRskAddress();
+        MutableRepository currentTransientStorage = getTransientStorage(addr);
 
+        return currentTransientStorage.getStorageValue(addr, key);
     }
 
     public DataWord getPrevHash() {

diff --git a/rskj-core/src/test/java/co/rsk/vm/opcode/TransientStorageDslTest.java b/rskj-core/src/test/java/co/rsk/vm/opcode/TransientStorageDslTest.java
@@ -28,13 +28,15 @@
 import org.ethereum.core.Block;
 import org.ethereum.core.Transaction;
 import org.ethereum.core.TransactionReceipt;
+import org.ethereum.core.util.TransactionReceiptUtil;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.io.FileNotFoundException;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 public class TransientStorageDslTest {
 
@@ -45,23 +47,59 @@ void testTransientStorageOpcodesExecutionsWithRSKIPActivated() throws FileNotFou
         WorldDslProcessor processor = new WorldDslProcessor(world);
         processor.processCommands(parser);
 
-        String contractCreationTx = "txTestTransientStorageContract";
-        Transaction contractTransaction = world.getTransactionByName("txTestTransientStorageContract");
-        assertNotNull(contractTransaction);
+        String mainContractTransientStorageCreationTxName = "txTestTransientStorageContract";
+        assertTransactionReceiptWithStatus(world, mainContractTransientStorageCreationTxName, "b01", true);
 
-        Block bestBlock = world.getBlockByName("b03");
-        Assertions.assertEquals(1, bestBlock.getTransactionsList().size());
-        TransactionReceipt contractTransactionReceipt = world.getTransactionReceiptByName(contractCreationTx);
+        String secondaryContractTransientStorageCreationTxName = "txTestTransientStorageOtherContract";
+        assertTransactionReceiptWithStatus(world, secondaryContractTransientStorageCreationTxName, "b02", true);
 
-        assertNotNull(contractTransactionReceipt);
-        byte[] status = contractTransactionReceipt.getStatus();
-        assertNotNull(status);
-        assertEquals(1, status.length);
-        assertEquals(1, status[0]);
+        String checkingOpcodesTxName = "txTestTransientStorageOpCodes";
+        TransactionReceipt txReceipt = assertTransactionReceiptWithStatus(world, checkingOpcodesTxName, "b03", true);
+        Assertions.assertEquals(1, TransactionReceiptUtil.getEventCount(txReceipt, "OK", null));
+
+        String checkingOpcodesTxName2 = "txTestTransientStorageOpCodesOtherValue";
+        TransactionReceipt txReceipt2 = assertTransactionReceiptWithStatus(world, checkingOpcodesTxName2, "b04", true);
+        Assertions.assertEquals(1, TransactionReceiptUtil.getEventCount(txReceipt2, "OK", null));
+    }
+
+    @Test
+    void testTransientStorageOpcodesShareMemorySameTransaction() throws FileNotFoundException, DslProcessorException {
+        DslParser parser = DslParser.fromResource("dsl/transaction_storage_rskip446/tload_tstore_basic_contract.txt");
+        World world = new World();
+        WorldDslProcessor processor = new WorldDslProcessor(world);
+        processor.processCommands(parser);
+
+        String mainContractTransientStorageCreationTxName = "txTestTransientStorageContract";
+        assertTransactionReceiptWithStatus(world, mainContractTransientStorageCreationTxName, "b01", true);
+
+        String secondaryContractTransientStorageCreationTxName = "txTestTransientStorageOtherContract";
+        assertTransactionReceiptWithStatus(world, secondaryContractTransientStorageCreationTxName, "b02", true);
+
+        String checkingOpcodesTxName = "txTestTransientStorageNestedTransactionShareMemory";
+        TransactionReceipt txReceipt = assertTransactionReceiptWithStatus(world, checkingOpcodesTxName, "b05", true);
+        Assertions.assertEquals(1, TransactionReceiptUtil.getEventCount(txReceipt, "OK", null));
+    }
+
+    @Test
+    void testTransientStorageOpcodesDoesntShareMemoryFromOtherContract() throws FileNotFoundException, DslProcessorException {
+        DslParser parser = DslParser.fromResource("dsl/transaction_storage_rskip446/tload_tstore_basic_contract.txt");
+        World world = new World();
+        WorldDslProcessor processor = new WorldDslProcessor(world);
+        processor.processCommands(parser);
+
+        String mainContractTransientStorageCreationTxName = "txTestTransientStorageContract";
+        assertTransactionReceiptWithStatus(world, mainContractTransientStorageCreationTxName, "b01", true);
+
+        String secondaryContractTransientStorageCreationTxName = "txTestTransientStorageOtherContract";
+        assertTransactionReceiptWithStatus(world, secondaryContractTransientStorageCreationTxName, "b02", true);
+
+        String checkingOpcodesTxName = "txTestTransientStorageNestedTransactionOtherContractDoesntShareMemory";
+        TransactionReceipt txReceipt = assertTransactionReceiptWithStatus(world, checkingOpcodesTxName, "b06", true);
+        Assertions.assertEquals(1, TransactionReceiptUtil.getEventCount(txReceipt, "ERROR", new String[]{"bytes32"}));
     }
 
     @Test
-    void testTransientStorageOpcodesExecutionsWithRSKIPDeactivated() throws FileNotFoundException, DslProcessorException {
+    void testTransientStorageOpcodesExecutionFailsWithRSKIPDeactivated() throws FileNotFoundException, DslProcessorException {
         TestSystemProperties rskip446Disabled = new TestSystemProperties(rawConfig ->
                 rawConfig.withValue("blockchain.config.hardforkActivationHeights.lovell700", ConfigValueFactory.fromAnyRef(-1))
         );
@@ -71,19 +109,36 @@ void testTransientStorageOpcodesExecutionsWithRSKIPDeactivated() throws FileNotF
         WorldDslProcessor processor = new WorldDslProcessor(world);
         processor.processCommands(parser);
 
-        String contractCreationTx = "txTestTransientStorageContract";
-        Transaction contractTransaction = world.getTransactionByName("txTestTransientStorageContract");
-        assertNotNull(contractTransaction);
+        String mainContractTransientStorageCreationTxName = "txTestTransientStorageContract";
+        assertTransactionReceiptWithStatus(world, mainContractTransientStorageCreationTxName, "b01", true);
+
+        String secondaryContractTransientStorageCreationTxName = "txTestTransientStorageOtherContract";
+        assertTransactionReceiptWithStatus(world, secondaryContractTransientStorageCreationTxName, "b02", true);
+
+        String checkingOpcodesTxName = "txTestTransientStorageOpCodes";
+       assertTransactionReceiptWithStatus(world, checkingOpcodesTxName, "b03", false);
+    }
 
-        Block bestBlock = world.getBlockByName("b03");
-        Assertions.assertEquals(1, bestBlock.getTransactionsList().size());
-        TransactionReceipt contractTransactionReceipt = world.getTransactionReceiptByName(contractCreationTx);
+    private static TransactionReceipt assertTransactionReceiptWithStatus(World world, String txName, String blockName, boolean withSuccess) {
+        Transaction txCreation = world.getTransactionByName(txName);
+        assertNotNull(txCreation);
 
-        assertNotNull(contractTransactionReceipt);
-        byte[] status = contractTransactionReceipt.getStatus();
+        Block block = world.getBlockByName(blockName);
+        assertEquals(1, block.getTransactionsList().size());
+
+        TransactionReceipt txReceipt = world.getTransactionReceiptByName(txName);
+        assertNotNull(txReceipt);
+
+        byte[] status = txReceipt.getStatus();
         assertNotNull(status);
-        assertEquals(1, status.length);
-        assertEquals(1, status[0]);
+
+        if(withSuccess) {
+            assertEquals(1, status.length);
+            assertEquals(1, status[0]);
+        } else {
+            assertEquals(0, status.length);
+        }
+        return txReceipt;
     }
 
 }
diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java
@@ -126,6 +126,7 @@ class ActivationConfigTest {
             "    rskip434: arrowhead631",
             "    rskip428: lovell700",
             "    rskip438: lovell700",
+            "    rskip446: lovell700",
             "}"
     ));