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.

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

Eip 684: Prevent overwriting contracts #932

Merged
merged 5 commits into from
Sep 7, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ public byte[] getEncoded() {
return rlpEncoded;
}

public boolean isContractExist() {
return !FastByteComparisons.equal(codeHash, EMPTY_DATA_HASH) || !BigInteger.ZERO.equals(nonce);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will second condition work as expected if initial nonce > 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Nice catch!

}

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 @@ -253,6 +253,13 @@ private void call() {
private void create() {
byte[] newContractAddress = tx.getContractAddress();

AccountState existingAddr = cacheTrack.getAccountState(newContractAddress);
if (existingAddr != null && existingAddr.isContractExist()) {
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
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 @@ -423,6 +424,9 @@ public void createContract(DataWord value, DataWord memStart, DataWord memSize)
byte[] nonce = getStorage().getNonce(senderAddress).toByteArray();
byte[] newAddress = HashUtil.calcNewAddr(getOwnerAddress().getLast20Bytes(), nonce);

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

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
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