Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Eip 684: Prevent overwriting contracts #932

Merged
merged 5 commits into from
Sep 7, 2017
Merged

Eip 684: Prevent overwriting contracts #932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f0f3f0f
Load test JSONs in parallel (greatly improve test running speed)
Nashatyrev Sep 5, 2017
d3ea913
eip-684 impl: throw if new contract address collision
Nashatyrev Sep 5, 2017
1eb2211
Update test commit and remove obsolete excluded test
Nashatyrev Sep 5, 2017
ab3696a
Fix to load test from the same commit as the listed tree
Nashatyrev Sep 6, 2017
5a07de1
When checking whether the contract exists we need to compare the curr…
Nashatyrev Sep 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ethereumj-core/src/main/java/org/ethereum/core/AccountState.java
View file
Open in desktop
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
8 changes: 7 additions & 1 deletion ethereumj-core/src/main/java/org/ethereum/core/TransactionExecutor.java
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ethereumj-core/src/main/java/org/ethereum/vm/program/ProgramResult.java
View file
Open in desktop
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
9 changes: 2 additions & 7 deletions ethereumj-core/src/test/java/org/ethereum/jsontestsuite/GitHubStateTest.java
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions ethereumj-core/src/test/java/org/ethereum/jsontestsuite/GitHubVMTest.java
View file
Open in desktop
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
7 changes: 6 additions & 1 deletion ethereumj-core/src/test/java/org/ethereum/jsontestsuite/suite/BlockchainTestSuite.java
View file
Open in desktop
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
26 changes: 26 additions & 0 deletions ethereumj-core/src/test/java/org/ethereum/jsontestsuite/suite/JSONReader.java
View file
Open in desktop
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