Skip to content

Commit

Permalink
Merge pull request #932 from ethereum/eip-684
Browse files Browse the repository at this point in the history
Eip 684: Prevent overwriting contracts
  • Loading branch information
mkalinin committed Sep 7, 2017
2 parents a152764 + 5a07de1 commit 5193432
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.ethereum.core;

import org.ethereum.config.BlockchainConfig;
import org.ethereum.config.SystemProperties;
import org.ethereum.crypto.HashUtil;
import org.ethereum.util.FastByteComparisons;
Expand Down Expand Up @@ -136,6 +137,11 @@ public byte[] getEncoded() {
return rlpEncoded;
}

public boolean isContractExist(BlockchainConfig blockchainConfig) {
return !FastByteComparisons.equal(codeHash, EMPTY_DATA_HASH) ||
!blockchainConfig.getConstants().getInitialNonce().equals(nonce);
}

public boolean isEmpty() {
return FastByteComparisons.equal(codeHash, EMPTY_DATA_HASH) &&
BigInteger.ZERO.equals(balance) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spongycastle.util.encoders.Hex;
import org.springframework.beans.factory.annotation.Autowired;

import java.math.BigInteger;
import java.util.List;
Expand Down Expand Up @@ -253,6 +252,13 @@ private void call() {
private void create() {
byte[] newContractAddress = tx.getContractAddress();

AccountState existingAddr = cacheTrack.getAccountState(newContractAddress);
if (existingAddr != null && existingAddr.isContractExist(blockchainConfig)) {
execError("Trying to create a contract with existing contract address: 0x" + Hex.toHexString(newContractAddress));
m_endGas = BigInteger.ZERO;
return;
}

//In case of hashing collisions (for TCK tests only), check for any balance before createAccount()
BigInteger oldBalance = track.getBalance(newContractAddress);
cacheTrack.createAccount(tx.getContractAddress());
Expand Down
16 changes: 11 additions & 5 deletions ethereumj-core/src/main/java/org/ethereum/vm/program/Program.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.ethereum.config.BlockchainConfig;
import org.ethereum.config.CommonConfig;
import org.ethereum.config.SystemProperties;
import org.ethereum.core.AccountState;
import org.ethereum.core.Repository;
import org.ethereum.core.Transaction;
import org.ethereum.crypto.HashUtil;
Expand Down Expand Up @@ -414,15 +415,18 @@ public void createContract(DataWord value, DataWord memStart, DataWord memSize)
if (logger.isInfoEnabled())
logger.info("creating a new contract inside contract run: [{}]", Hex.toHexString(senderAddress));

BlockchainConfig blockchainConfig = config.getBlockchainConfig().getConfigForBlock(getNumber().longValue());
// actual gas subtract
DataWord gasLimit = config.getBlockchainConfig().getConfigForBlock(getNumber().longValue()).
getCreateGas(getGas());
DataWord gasLimit = blockchainConfig.getCreateGas(getGas());
spendGas(gasLimit.longValue(), "internal call");

// [2] CREATE THE CONTRACT ADDRESS
byte[] nonce = getStorage().getNonce(senderAddress).toByteArray();
byte[] newAddress = HashUtil.calcNewAddr(getOwnerAddress().getLast20Bytes(), nonce);

AccountState existingAddr = getStorage().getAccountState(newAddress);
boolean contractAlreadyExists = existingAddr != null && existingAddr.isContractExist(blockchainConfig);

if (byTestingSuite()) {
// This keeps track of the contracts created for a test
getResult().addCallCreate(programCode, EMPTY_BYTE_ARRAY,
Expand Down Expand Up @@ -460,9 +464,11 @@ public void createContract(DataWord value, DataWord memStart, DataWord memSize)
this, new DataWord(newAddress), getOwnerAddress(), value, gasLimit,
newBalance, null, track, this.invoke.getBlockStore(), false, byTestingSuite());

ProgramResult result = ProgramResult.empty();
ProgramResult result = ProgramResult.createEmpty();

if (isNotEmpty(programCode)) {
if (contractAlreadyExists) {
result.setException(new BytecodeExecutionException("Trying to create a contract with existing contract address: 0x" + Hex.toHexString(newAddress)));
} else if (isNotEmpty(programCode)) {
VM vm = new VM(config);
Program program = new Program(programCode, programInvoke, internalTx, config).withCommonConfig(commonConfig);
vm.play(program);
Expand Down Expand Up @@ -494,7 +500,7 @@ this, new DataWord(newAddress), getOwnerAddress(), value, gasLimit,
long storageCost = getLength(code) * getBlockchainConfig().getGasCost().getCREATE_DATA();
long afterSpend = programInvoke.getGas().longValue() - storageCost - result.getGasUsed();
if (afterSpend < 0) {
if (!config.getBlockchainConfig().getConfigForBlock(getNumber().longValue()).getConstants().createEmptyContractOnOOG()) {
if (!blockchainConfig.getConstants().createEmptyContractOnOOG()) {
result.setException(Program.Exception.notEnoughSpendingGas("No gas to return just created contract",
storageCost, this));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public void merge(ProgramResult another) {
}
}

public static ProgramResult empty() {
public static ProgramResult createEmpty() {
ProgramResult result = new ProgramResult();
result.setHReturn(EMPTY_BYTE_ARRAY);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class GitHubStateTest {

static String commitSHA = "f17609b3c2fc7404addf3d228955b16ae8e0cfb4";
static String commitSHA = "d098a00a4e7108f7965d1ca81e2fd51fc5b1e11b";
static String treeSHA = "fdcc301d8fdd65e6f1b56ffca9a786aa337c7bb8"; // https://github.com/ethereum/tests/tree/develop/GeneralStateTests/
static GitHubJSONTestSuite.Network[] targetNets = {
GitHubJSONTestSuite.Network.Frontier,
Expand Down Expand Up @@ -57,7 +57,7 @@ public void clean() {
// it reduces impact on GitHub API
public void stSingleTest() throws IOException {
GeneralStateTestSuite.runSingle(
"stZeroKnowledge/pointAdd.json", commitSHA, GitHubJSONTestSuite.Network.Byzantium);
"stRevertTest/RevertDepthCreateAddressCollision.json", commitSHA, GitHubJSONTestSuite.Network.Byzantium);
}

@Test
Expand Down Expand Up @@ -96,11 +96,6 @@ public void stCallCreateCallCodeTest() throws IOException {
Set<String> excluded = new HashSet<>();
excluded.add("CallRecursiveBombPreCall"); // Max Gas value is pending to be < 2^63

// the test creates a contract with the same address as existing contract (which is not possible in
// live). In this case we need to clear the storage in TransactionExecutor.create
// return back to this case when the contract deleting will be implemented
excluded.add("createJS_ExampleContract");

suite.runAll("stCallCreateCallCodeTest", excluded);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ public void testVMGitHub() throws ParseException {
@Test // testing full suite
public void testRandomVMGitHub() throws ParseException {

String shacommit = "c5eafb85390eee59b838a93ae31bc16a5fd4f7b1";
List<String> fileNames = getFileNamesForTreeSha(shacommit);
String treeSha = "c5eafb85390eee59b838a93ae31bc16a5fd4f7b1";
List<String> fileNames = getFileNamesForTreeSha(treeSha);
List<String> excludedFiles =
Collections.singletonList(
""
Expand All @@ -171,7 +171,7 @@ public void testRandomVMGitHub() throws ParseException {

if (excludedFiles.contains(fileName)) continue;
System.out.println("Running: " + fileName);
String json = JSONReader.loadJSON("VMTests//RandomTests/" + fileName);
String json = JSONReader.loadJSONFromCommit("VMTests//RandomTests/" + fileName, shacommit);
GitHubJSONTestSuite.runGitHubJsonVMTest(json);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import static org.ethereum.jsontestsuite.suite.JSONReader.listJsonBlobsForTreeSha;
import static org.ethereum.jsontestsuite.suite.JSONReader.loadJSONFromCommit;
import static org.ethereum.jsontestsuite.suite.JSONReader.loadJSONsFromCommit;

/**
* @author Mikhail Kalinin
Expand Down Expand Up @@ -49,8 +50,12 @@ private static void run(List<String> checkFiles,
if (checkFiles.isEmpty()) return;

List<BlockTestSuite> suites = new ArrayList<>();
List<String> filenames = new ArrayList<>();
for (String file : checkFiles) {
String json = loadJSONFromCommit(TEST_ROOT + file, commitSHA);
filenames.add(TEST_ROOT + file);
}
List<String> jsons = loadJSONsFromCommit(filenames, commitSHA, 64);
for (String json : jsons) {
suites.add(new BlockTestSuite(json));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

import org.apache.commons.codec.binary.Base64;
import org.slf4j.Logger;
Expand All @@ -40,6 +41,31 @@ public class JSONReader {

private static Logger logger = LoggerFactory.getLogger("TCK-Test");

public static List<String> loadJSONsFromCommit(List<String> filenames, final String shacommit, int threads) {
ExecutorService threadPool = Executors.newFixedThreadPool(threads);
List<Future<String>> retF = new ArrayList<>();
for (final String filename : filenames) {
Future<String> f = threadPool.submit(new Callable<String>() {
@Override
public String call() throws Exception {
return loadJSONFromCommit(filename, shacommit);
}
});
retF.add(f);
}

List<String> ret = new ArrayList<>();
for (Future<String> f : retF) {
try {
ret.add(f.get());
} catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}
}

return ret;
}

public static String loadJSON(String filename) {
String json = "";
if (!SystemProperties.getDefault().vmTestLoadLocal())
Expand Down

0 comments on commit 5193432

Please sign in to comment.