Refactoring validations

Author: nagarev

rskj-core/src/main/java/co/rsk/RskContext.java

@@ -130,6 +130,7 @@
 import org.ethereum.sync.SyncPool;
 import org.ethereum.util.BuildInfo;
 import org.ethereum.util.FileUtil;
+import org.ethereum.vm.OverrideablePrecompiledContracts;
 import org.ethereum.vm.PrecompiledContracts;
 import org.ethereum.vm.program.invoke.ProgramInvokeFactory;
 import org.ethereum.vm.program.invoke.ProgramInvokeFactoryImpl;
@@ -252,6 +253,7 @@ public class RskContext implements NodeContext, NodeBootstrapper {
     private BlockExecutor blockExecutor;
     private BtcBlockStoreWithCache.Factory btcBlockStoreFactory;
     private PrecompiledContracts precompiledContracts;
+    private OverrideablePrecompiledContracts overrideablePrecompiledContracts;
     private BridgeSupportFactory bridgeSupportFactory;
     private PeersInformation peersInformation;
     private StatusResolver statusResolver;
@@ -498,6 +500,16 @@ public synchronized BlockExecutor getBlockExecutor() {
         return blockExecutor;
     }
 
+    public synchronized OverrideablePrecompiledContracts getOverrideablePrecompiledContracts() {
+        checkIfNotClosed();
+
+        if (overrideablePrecompiledContracts == null) {
+            overrideablePrecompiledContracts = new OverrideablePrecompiledContracts(getPrecompiledContracts());
+        }
+
+        return overrideablePrecompiledContracts;
+    }
+
     public synchronized PrecompiledContracts getPrecompiledContracts() {
         checkIfNotClosed();
 
@@ -735,7 +747,8 @@ public synchronized EthModule getEthModule() {
                     getBridgeSupportFactory(),
                     getRskSystemProperties().getGasEstimationCap(),
                     getRskSystemProperties().getCallGasCap(),
-                    getPrecompiledContracts(),
+                    getRskSystemProperties().getActivationConfig(),
+                    getOverrideablePrecompiledContracts(),
                     getRskSystemProperties().getAllowCallStateOverride(),
                     getStateOverrideApplier()
             );
@@ -860,7 +873,7 @@ public synchronized TraceModule getTraceModule() {
                     getBlockExecutor(),
                     getExecutionBlockRetriever(),
                     getBlockTxSignatureCache(),
-                    rskSystemProperties
+                    getRskSystemProperties()
             );
         }
 

rskj-core/src/main/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplier.java

@@ -37,6 +37,13 @@ public DefaultStateOverrideApplier(ActivationConfig activationConfig) {
         this.activationConfig = activationConfig;
     }
 
+    @Override
+    public boolean containsStateChange(AccountOverride accountOverride) {
+        boolean containsStateChange = accountOverride.getBalance() != null || accountOverride.getNonce() != null || accountOverride.getCode() != null;
+        boolean containsStateDiff = accountOverride.getState() != null || accountOverride.getStateDiff() != null;
+        return containsStateChange || containsStateDiff;
+    }
+
     @Override
     public void applyToRepository(Block block, Repository repository, AccountOverride accountOverride, OverrideablePrecompiledContracts overrideablePrecompiledContracts) {
 
@@ -73,7 +80,11 @@ public void applyToRepository(Block block, Repository repository, AccountOverrid
             }
         }
 
-        if (overrideablePrecompiledContracts != null && accountOverride.getMovePrecompileToAddress() != null && accountOverride.getAddress() != null) {
+        if (overrideablePrecompiledContracts != null && accountOverride.getMovePrecompileToAddress() != null) {
+            if (!overrideablePrecompiledContracts.isMovableContract(accountOverride.getAddress())) {
+                throw new IllegalStateException(String.format("Account %s can not be moved", accountOverride.getAddress().toHexString()));
+            }
+
             if (overrideablePrecompiledContracts.isOverridden(accountOverride.getAddress())) {
                 throw new IllegalStateException(String.format("Account %s has already been overridden by a precompile", accountOverride.getAddress().toHexString()));
             }

rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java

@@ -33,6 +33,7 @@
 import co.rsk.util.HexUtils;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang3.tuple.Pair;
+import org.ethereum.config.blockchain.upgrades.ActivationConfig;
 import org.ethereum.core.Block;
 import org.ethereum.core.Blockchain;
 import org.ethereum.core.CallTransaction;
@@ -49,6 +50,7 @@
 import org.ethereum.rpc.parameters.CallArgumentsParam;
 import org.ethereum.rpc.parameters.HexAddressParam;
 import org.ethereum.rpc.parameters.HexDataParam;
+import org.ethereum.vm.DataWord;
 import org.ethereum.vm.GasCost;
 import org.ethereum.vm.PrecompiledContracts;
 import org.ethereum.vm.OverrideablePrecompiledContracts;
@@ -87,7 +89,8 @@ public class EthModule
     private final byte chainId;
     private final long gasEstimationCap;
     private final long gasCallCap;
-    private final PrecompiledContracts precompiledContracts;
+    private final ActivationConfig activationConfig;
+    private final OverrideablePrecompiledContracts overrideablePrecompiledContracts;
     private final boolean allowCallStateOverride;
     private final StateOverrideApplier stateOverrideApplier;
 
@@ -104,7 +107,8 @@ public EthModule(
             BridgeSupportFactory bridgeSupportFactory,
             long gasEstimationCap,
             long gasCallCap,
-            PrecompiledContracts precompiledContracts,
+            ActivationConfig activationConfig,
+            OverrideablePrecompiledContracts overrideablePrecompiledContracts,
             boolean allowCallStateOverride,
             StateOverrideApplier stateOverrideApplier) {
         this.chainId = chainId;
@@ -119,7 +123,8 @@ public EthModule(
         this.bridgeSupportFactory = bridgeSupportFactory;
         this.gasEstimationCap = gasEstimationCap;
         this.gasCallCap = gasCallCap;
-        this.precompiledContracts = precompiledContracts;
+        this.activationConfig = activationConfig;
+        this.overrideablePrecompiledContracts = overrideablePrecompiledContracts;
         this.allowCallStateOverride = allowCallStateOverride;
         this.stateOverrideApplier = stateOverrideApplier;
     }
@@ -155,7 +160,6 @@ public String call(CallArgumentsParam argsParam, BlockIdentifierParam bnOrId, Li
         ExecutionBlockRetriever.Result result = executionBlockRetriever.retrieveExecutionBlock(bnOrId.getIdentifier());
         Block block = result.getBlock();
 
-        OverrideablePrecompiledContracts overrideablePrecompiledContracts = new OverrideablePrecompiledContracts(precompiledContracts);
         MutableRepository mutableRepository = prepareRepository(result, block, shouldPerformStateOverride, accountOverrideList, overrideablePrecompiledContracts);
 
         CallArguments callArgs = argsParam.toCallArguments();
@@ -202,7 +206,13 @@ private MutableRepository prepareRepository(ExecutionBlockRetriever.Result resul
     }
 
     private void applyStateOverride(Block block, MutableRepository mutableRepository, List<AccountOverride> accountOverrideList, OverrideablePrecompiledContracts overrideablePrecompiledContracts) {
+        ActivationConfig.ForBlock blockActivations = activationConfig.forBlock(block.getNumber());
         for (AccountOverride accountOverride : accountOverrideList) {
+            PrecompiledContracts.PrecompiledContract precompiledContract = overrideablePrecompiledContracts
+                    .getContractForAddress(blockActivations, DataWord.valueFromHex(accountOverride.getAddress().toHexString()));
+            if (precompiledContract != null && stateOverrideApplier.containsStateChange(accountOverride)) {
+                throw invalidParamError("Precompiled contracts can not be overridden");
+            }
             stateOverrideApplier.applyToRepository(block, mutableRepository, accountOverride, overrideablePrecompiledContracts);
         }
     }

rskj-core/src/main/java/co/rsk/rpc/modules/eth/StateOverrideApplier.java

@@ -22,5 +22,6 @@
 import org.ethereum.vm.OverrideablePrecompiledContracts;
 
 public interface StateOverrideApplier {
+    boolean containsStateChange(AccountOverride accountOverride);
     void applyToRepository(Block block, Repository repository, AccountOverride accountOverride, OverrideablePrecompiledContracts overrideablePrecompiledContracts);
 }

rskj-core/src/main/java/org/ethereum/vm/OverrideablePrecompiledContracts.java

@@ -21,14 +21,15 @@
 import co.rsk.core.RskAddress;
 import org.ethereum.config.blockchain.upgrades.ActivationConfig;
 
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 public class OverrideablePrecompiledContracts extends PrecompiledContracts {
     private final Map<RskAddress, PrecompiledContract> overriddenPrecompileContracts = new HashMap<>();
-    private final List<RskAddress> overriddenContracts = new ArrayList<>();
+    private final Set<RskAddress> overriddenContracts = new HashSet<>();
 
     public OverrideablePrecompiledContracts(PrecompiledContracts basePrecompiledContracts) {
         super(basePrecompiledContracts.getConfig(), basePrecompiledContracts.getBridgeSupportFactory(), basePrecompiledContracts.getSignatureCache());
@@ -59,4 +60,10 @@ public void addOverride(RskAddress originalAddress, RskAddress targetAddress, Ac
     public boolean isOverridden(RskAddress address) {
         return overriddenContracts.stream().anyMatch(a -> a.equals(address));
     }
+
+    public boolean isMovableContract(RskAddress contractAddress) {
+        byte[] addressBytes = contractAddress.getBytes();
+        return !Arrays.equals(BRIDGE_ADDR.getBytes(), addressBytes) && !Arrays.equals(REMASC_ADDR.getBytes(), addressBytes);
+    }
+
 }

rskj-core/src/test/java/co/rsk/mine/TransactionModuleTest.java

@@ -78,6 +78,7 @@
 import org.ethereum.util.ByteUtil;
 import org.ethereum.util.TransactionFactoryHelper;
 import org.ethereum.vm.GasCost;
+import org.ethereum.vm.OverrideablePrecompiledContracts;
 import org.ethereum.vm.PrecompiledContracts;
 import org.ethereum.vm.program.invoke.ProgramInvokeFactoryImpl;
 import org.junit.jupiter.api.Assertions;
@@ -644,6 +645,11 @@ private Web3Impl internalCreateEnvironment(Blockchain blockchain,
 
         final RepositoryBtcBlockStoreWithCache.Factory btcBlockStoreFactory = new RepositoryBtcBlockStoreWithCache.Factory(
                 config.getNetworkConstants().getBridgeConstants().getBtcParams());
+
+        BridgeSupportFactory bridgeSupportFactory = new BridgeSupportFactory(
+                btcBlockStoreFactory, config.getNetworkConstants().getBridgeConstants(),
+                config.getActivationConfig(), signatureCache);
+
         EthModule ethModule = new EthModule(
                 config.getNetworkConstants().getBridgeConstants(), config.getNetworkConstants().getChainId(), blockchain, transactionPool,
                 reversibleTransactionExecutor1, new ExecutionBlockRetriever(blockchain, null, null),
@@ -653,7 +659,10 @@ repositoryLocator, new EthModuleWalletEnabled(wallet, transactionPool, signature
                         config.getActivationConfig(), signatureCache),
                 config.getGasEstimationCap(),
                 config.getCallGasCap(),
-                null,
+                config.getActivationConfig(),
+                new OverrideablePrecompiledContracts(
+                        new PrecompiledContracts(config, bridgeSupportFactory, signatureCache)
+                ),
                 false,
                 new DefaultStateOverrideApplier(config.getActivationConfig())
         );

rskj-core/src/test/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplierTest.java

@@ -28,18 +28,33 @@
 import co.rsk.trie.TrieStore;
 import co.rsk.trie.TrieStoreImpl;
 import org.ethereum.TestUtils;
-import org.ethereum.core.*;
+import org.ethereum.config.blockchain.upgrades.ActivationConfig;
+import org.ethereum.core.Block;
+import org.ethereum.core.BlockTxSignatureCache;
+import org.ethereum.core.ReceivedTxSignatureCache;
+import org.ethereum.core.Repository;
+import org.ethereum.core.SignatureCache;
 import org.ethereum.datasource.HashMapDB;
 import org.ethereum.db.MutableRepository;
+import org.ethereum.rpc.parameters.AccountOverrideParam;
+import org.ethereum.rpc.parameters.HexAddressParam;
+import org.ethereum.rpc.parameters.HexDataParam;
+import org.ethereum.rpc.parameters.HexNumberParam;
 import org.ethereum.vm.DataWord;
 import org.ethereum.vm.OverrideablePrecompiledContracts;
 import org.ethereum.vm.PrecompiledContracts;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
 import java.math.BigInteger;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Stream;
 
+import static org.ethereum.vm.PrecompiledContracts.BRIDGE_ADDR_STR;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -202,7 +217,39 @@ void applyWithMovePrecompileTo_happyPath() {
         stateOverrideApplier.applyToRepository(blockMock, repository, accountOverride, overrideablePrecompiledContracts);
 
         // Then
-        assertEquals(overrideablePrecompiledContracts.getContractForAddress(testSystemProperties.getActivationConfig().forBlock(blockNumber), precompileAddressInDataWord), overrideablePrecompiledContracts.getContractForAddress(testSystemProperties.getActivationConfig().forBlock(blockNumber), movePrecompileToInDataWord));
+        ActivationConfig.ForBlock blockActivations = testSystemProperties.getActivationConfig().forBlock(blockNumber);
+
+        PrecompiledContracts.PrecompiledContract originalContract = overrideablePrecompiledContracts.getContractForAddress(blockActivations, precompileAddressInDataWord);
+        PrecompiledContracts.PrecompiledContract movedContract = overrideablePrecompiledContracts.getContractForAddress(blockActivations, movePrecompileToInDataWord);
+
+        assertEquals(originalContract, movedContract);
+    }
+
+    @Test
+    void applyWithMovePrecompileTo_notMovableContract_throwsExceptionAsExpected() {
+        // Given
+        RskAddress bridgeAddress = new RskAddress(BRIDGE_ADDR_STR);
+        RskAddress movePrecompileTo = TestUtils.generateAddress("targetAddress");
+
+        AccountOverride accountOverride = new AccountOverride(bridgeAddress);
+        accountOverride.setMovePrecompileToAddress(movePrecompileTo);
+
+        PrecompiledContracts precompiledContracts = getPrecompiledContracts();
+
+        OverrideablePrecompiledContracts overrideablePrecompiledContracts = new OverrideablePrecompiledContracts(precompiledContracts);
+
+        long blockNumber = 1L;
+        Block blockMock = mock(Block.class);
+        when(blockMock.getNumber()).thenReturn(blockNumber);
+
+        // When
+        IllegalStateException exception = assertThrows(IllegalStateException.class, () -> {
+            // This is the second time calling the exact same method so the contract should be already overridden
+            stateOverrideApplier.applyToRepository(blockMock, repository, accountOverride, overrideablePrecompiledContracts);
+        });
+
+        // Then
+        assertEquals("Account " + bridgeAddress.toHexString() + " can not be moved", exception.getMessage());
     }
 
     @Test
@@ -234,6 +281,12 @@ void applyWithMovePrecompileTo_alreadyOverriddenContract_throwsExceptionAsExpect
         assertEquals("Account " + precompileAddress.toHexString() + " has already been overridden by a precompile", exception.getMessage());
     }
 
+    @ParameterizedTest
+    @MethodSource("provideContainsStateChangeTestCases")
+    void containsStateChange_executesAsExpected(AccountOverride accountOverride, boolean expectedResult) {
+        assertEquals(expectedResult, stateOverrideApplier.containsStateChange(accountOverride));
+    }
+
     private PrecompiledContracts getPrecompiledContracts() {
         TestSystemProperties config = new TestSystemProperties();
         SignatureCache signatureCache = new BlockTxSignatureCache(new ReceivedTxSignatureCache());
@@ -242,4 +295,25 @@ private PrecompiledContracts getPrecompiledContracts() {
         return new PrecompiledContracts(config, bridgeSupportFactory, signatureCache);
     }
 
+    private static Stream<Arguments> provideContainsStateChangeTestCases() {
+        return Stream.of(
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, null, null, null, null, null)), false),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, null, null, null, null, new HexAddressParam("0x0000000000000000000000000000000000000001"))), false),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(new HexNumberParam("0x01"), null, null, null, null, new HexAddressParam("0x0000000000000000000000000000000000000001"))), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(new HexNumberParam("0x01"), null, null, null, null, null)), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, new HexNumberParam("0x01"), null, null, null, null)), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, null, new HexDataParam("0x01"), null, null, null)), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, null, null, new HashMap<>(), null, null)), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(null, null, null, null, new HashMap<>(), null)), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(new HexNumberParam("0x01"), new HexNumberParam("0x01"), new HexDataParam("0x01"), new HashMap<>(), new HashMap<>(), new HexAddressParam("0x0000000000000000000000000000000000000001"))), true),
+                Arguments.of(getAccountOverride(new AccountOverrideParam(new HexNumberParam("0x01"), new HexNumberParam("0x01"), new HexDataParam("0x01"), new HashMap<>(), new HashMap<>(), null)), true)
+        );
+    }
+
+    private static AccountOverride getAccountOverride(AccountOverrideParam accountOverrideParam) {
+        RskAddress address = TestUtils.generateAddress("test");
+        AccountOverride accountOverride = new AccountOverride(address);
+        return accountOverride.fromAccountOverrideParam(accountOverrideParam);
+    }
+
 }

rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleGasEstimationDSLTest.java

@@ -16,7 +16,6 @@
  * 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 co.rsk.rpc.modules.eth;
 
 import co.rsk.config.TestSystemProperties;