CP-6: Validate the destination address for deployment transactions

This commit adds logic that prevents a contract from being deployed if the destination address has an associated code, storage, or positive nonce.

org.aion.avm.core/src/org/aion/avm/core/AvmImpl.java

@@ -424,19 +424,23 @@ private AvmWrappedTransactionResult commonInvoke(IExternalState parentKernel
 
         // do nothing for balance transfers of which the recipient is not a DApp address.
         if (isCreate) {
-            if ((null != this.histogramDataCollector) || (null != this.contractCaptureTool)) {
-                CodeAndArguments codeAndArguments = CodeAndArguments.decodeFromBytes(transactionData);
-                // If this data is invalid, we will get null.  We don't bother tracking this.
-                if (null != codeAndArguments) {
-                    if (null != this.histogramDataCollector) {
-                        this.histogramDataCollector.collectDataFromJarBytes(codeAndArguments.code);
-                    }
-                    if (null != this.contractCaptureTool) {
-                        this.contractCaptureTool.captureDeployment(parentKernel.getBlockNumber(), senderAddress, recipient, nonce, codeAndArguments.code, codeAndArguments.arguments);
+            if (DestinationAddressIsValidForCreate(recipient, thisTransactionKernel)) {
+                if ((null != this.histogramDataCollector) || (null != this.contractCaptureTool)) {
+                    CodeAndArguments codeAndArguments = CodeAndArguments.decodeFromBytes(transactionData);
+                    // If this data is invalid, we will get null.  We don't bother tracking this.
+                    if (null != codeAndArguments) {
+                        if (null != this.histogramDataCollector) {
+                            this.histogramDataCollector.collectDataFromJarBytes(codeAndArguments.code);
+                        }
+                        if (null != this.contractCaptureTool) {
+                            this.contractCaptureTool.captureDeployment(parentKernel.getBlockNumber(), senderAddress, recipient, nonce, codeAndArguments.code, codeAndArguments.arguments);
+                        }
                     }
                 }
+                result = DAppCreator.create(this.capabilities, thisTransactionKernel, this, task, senderAddress, recipient, effectiveTransactionOrigin, transactionData, transactionHash, energyLimit, energyPrice, transactionValue, result, this.preserveDebuggability, this.enableVerboseContractErrors, this.enableBlockchainPrintln);
+            } else {
+                result = TransactionResultUtil.setNonRevertedFailureAndEnergyUsed(result, AvmInternalError.FAILED_NON_DEFAULT_ACCOUNT, energyLimit);
             }
-            result = DAppCreator.create(this.capabilities, thisTransactionKernel, this, task, senderAddress, recipient, effectiveTransactionOrigin, transactionData, transactionHash, energyLimit, energyPrice, transactionValue, result, this.preserveDebuggability, this.enableVerboseContractErrors, this.enableBlockchainPrintln);
         } else { // call
             // See if this call is trying to reenter one already on this call-stack.  If so, we will need to partially resume its state.
             ReentrantDAppStack.ReentrantState stateToResume = task.getReentrantDAppStack().tryShareState(recipient);
@@ -650,4 +654,17 @@ private void cleanupTransformedCodeCache() {
         };
         this.transformedCodeCache.removeValueIf(condition);
     }
+
+    /**
+     * Checks to see if the destination address is valid for deployment.
+     * An address is valid if it has no code or storage associated with it, and its nonce is zero
+     *
+     * @param destinationAddress destination address for deployment
+     * @return {@code true}  if the address is valid,  {@code false}  otherwise
+     */
+    private boolean DestinationAddressIsValidForCreate(AionAddress destinationAddress, IExternalState externalState) {
+        return externalState.getNonce(destinationAddress).signum() == 0 &&
+                externalState.getCode(destinationAddress) == null &&
+                !externalState.hasStorage(destinationAddress);
+    }
 }

org.aion.avm.core/src/org/aion/kernel/AvmWrappedTransactionResult.java

@@ -34,6 +34,7 @@ public enum AvmInternalError {
         FAILED_REVERTED                 ("Failed: reverted", true, false),
         FAILED                          ("Failed", true, false),
         FAILED_RETRANSFORMATION         ("Failed: re-transformation failure", true, false),
+        FAILED_NON_DEFAULT_ACCOUNT      ("Failed: destination address has a non-default state", true, false),
         REJECTED_INVALID_VALUE          ("Rejected: invalid value", false, true),
         REJECTED_INVALID_ENERGY_PRICE   ("Rejected: invalid energy price", false, true),
         REJECTED_INVALID_ENERGY_LIMIT   ("Rejected: invalid energy limit", false, true),

org.aion.avm.core/test/org/aion/avm/core/DeploymentNonDefaultConditionTest.java

@@ -5,32 +5,37 @@
 import org.aion.avm.core.util.Helpers;
 import org.aion.avm.userlib.CodeAndArguments;
 import org.aion.avm.userlib.abi.ABIStreamingEncoder;
+import org.aion.avm.utilities.JarBuilder;
 import org.aion.kernel.TestingBlock;
 import org.aion.kernel.TestingState;
 import org.aion.types.AionAddress;
 import org.aion.types.Transaction;
 import org.aion.types.TransactionResult;
+import org.junit.*;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.math.BigInteger;
+import java.util.Collections;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 public class DeploymentNonDefaultConditionTest {
 
     private TestingState kernel;
     private AvmImpl avm;
     private AionAddress deployer = TestingState.PREMINED_ADDRESS;
-
+    private IExternalCapabilities capabilities;
 
     @Before
     public void setup() {
         TestingBlock block = new TestingBlock(new byte[32], 1, Helpers.randomAddress(), System.currentTimeMillis(), new byte[0]);
         this.kernel = new TestingState(block);
-        this.avm = CommonAvmFactory.buildAvmInstanceForConfiguration(new EmptyCapabilities(), new AvmConfiguration());
+        capabilities = new EmptyCapabilities();
+        this.avm = CommonAvmFactory.buildAvmInstanceForConfiguration(capabilities, new AvmConfiguration());
     }
 
     @Test
@@ -81,6 +86,87 @@ public void testHasStorage() {
         Assert.assertFalse(kernel.hasStorage(dappAddress));
     }
 
+    @Test
+    public void testNonceNotZeroFail() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+
+        AionAddress dappAddress = capabilities.generateContractAddress(deployer, kernel.getNonce(deployer));
+        kernel.incrementNonce(dappAddress);
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.ZERO);
+        assertTrue(result.transactionStatus.isFailed());
+        assertEquals(5_000_000, result.energyUsed);
+    }
+
+    @Test
+    public void testCodeNotEmpty() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+
+        AionAddress dappAddress = capabilities.generateContractAddress(deployer, kernel.getNonce(deployer));
+        kernel.putCode(dappAddress, new byte[1]);
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.ZERO);
+        assertTrue(result.transactionStatus.isFailed());
+        assertEquals(5_000_000, result.energyUsed);
+    }
+
+    @Test
+    public void testStorageNotEmpty() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+
+        AionAddress dappAddress = capabilities.generateContractAddress(deployer, kernel.getNonce(deployer));
+        byte[] key = Helpers.randomBytes(32);
+        byte[] value = Helpers.randomBytes(50);
+        kernel.putStorage(dappAddress, key, value);
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.ZERO);
+        assertTrue(result.transactionStatus.isFailed());
+        assertEquals(5_000_000, result.energyUsed);
+    }
+
+    @Test
+    public void testInitialBalanceSuccess() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+
+        AionAddress dappAddress = capabilities.generateContractAddress(deployer, kernel.getNonce(deployer));
+        kernel.adjustBalance(dappAddress, BigInteger.TEN);
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.ZERO);
+        assertTrue(result.transactionStatus.isSuccess());
+    }
+
+    @Test
+    public void testInternalTransactions() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.ZERO);
+        assertTrue(result.transactionStatus.isSuccess());
+        AionAddress dappAddress = new AionAddress(result.copyOfTransactionOutput().orElseThrow());
+
+        AionAddress newAddress = capabilities.generateContractAddress(dappAddress, kernel.getNonce(dappAddress));
+        kernel.putCode(newAddress, new byte[1]);
+
+        // deploy to an address with code using internal transactions
+        byte[] internalCreateJar = JarBuilder.buildJarForMainClassAndExplicitClassNamesAndBytecode(ConstantBillingTarget.class, Collections.emptyMap());
+        txData = new CodeAndArguments(internalCreateJar, new byte[0]).encodeToBytes();
+
+        byte[] data = new ABIStreamingEncoder().encodeOneString("call").encodeOneByteArray(txData).encodeOneBoolean(true).toBytes();
+        result = callDapp(kernel, deployer, dappAddress, data);
+        Assert.assertFalse(result.transactionStatus.isSuccess());
+    }
+
+    @Test
+    public void testTransferFailed() {
+        byte[] jar = UserlibJarBuilder.buildJarForMainAndClassesAndUserlib(NonDefaultConditionTarget.class);
+        byte[] txData = new CodeAndArguments(jar, new byte[0]).encodeToBytes();
+
+        AionAddress dappAddress = capabilities.generateContractAddress(deployer, kernel.getNonce(deployer));
+        kernel.putCode(dappAddress, new byte[1]);
+        TransactionResult result = deploy(deployer, kernel, txData, BigInteger.TEN);
+        assertTrue(result.transactionStatus.isFailed());
+        assertEquals(0, kernel.getBalance(dappAddress).signum());
+    }
+
     @After
     public void tearDown() {
         avm.shutdown();

org.aion.avm.core/test/org/aion/avm/core/NonDefaultConditionTarget.java

@@ -1,8 +1,11 @@
 package org.aion.avm.core;
 
 import avm.Blockchain;
+import avm.Result;
 import org.aion.avm.userlib.abi.ABIDecoder;
 
+import java.math.BigInteger;
+
 public class NonDefaultConditionTarget {
 
     public static byte[] getStorage(byte[] key) {
@@ -17,6 +20,11 @@ public static void selfDestruct() {
         Blockchain.selfDestruct(Blockchain.getCaller());
     }
 
+    public static void call(byte[] dappBytes, boolean expectedResult) {
+        Result createResult = Blockchain.create(BigInteger.ZERO, dappBytes, Blockchain.getRemainingEnergy());
+        Blockchain.require(expectedResult == createResult.isSuccess());
+    }
+
     public static byte[] main() {
         ABIDecoder decoder = new ABIDecoder(Blockchain.getData());
         String methodName = decoder.decodeMethodName();
@@ -38,6 +46,9 @@ public static byte[] main() {
                 case "selfDestruct":
                     selfDestruct();
                     break;
+                case "call":
+                    call(decoder.decodeOneByteArray(), decoder.decodeOneBoolean());
+                    break;
             }
             return new byte[0];
         }