Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

Feature/eip 1283 #1173

Merged
merged 7 commits into from
Sep 10, 2018
Merged

Feature/eip 1283 #1173

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b826ad9
Merge branch 'develop' into feature/eip-1283
zilm13 Sep 5, 2018
a6a5f61
EIP-1283 SSTORE Net gas metering without dirty maps
zilm13 Sep 6, 2018
923d6d0
Protect RefundCounter which could go negative since EIP-1283
zilm13 Sep 6, 2018
b035bb0
Keep storageLoad for SLOAD as clear helper
zilm13 Sep 9, 2018
81ccc2b
Add clone repo method for usage in EIP-1283
zilm13 Sep 9, 2018
8ab754a
Update originalValue javadoc for eip-1283 reference
zilm13 Sep 9, 2018
53098a6
Fine polishing according to eip-1283 pr review
zilm13 Sep 10, 2018
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/config/BlockchainConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,10 @@ String validateTransactionChanges(BlockStore blockStore, Block curBlock, Transac
* EXTCODEHASH opcode
*/
boolean eip1052();

/**
* EIP 1283: https://eips.ethereum.org/EIPS/eip-1283
* Net gas metering for SSTORE without dirty maps
*/
boolean eip1283();
}
5 changes: 5 additions & 0 deletions ethereumj-core/src/main/java/org/ethereum/config/blockchain/AbstractConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ public boolean eip145() {
return false;
}

@Override
public boolean eip1283() {
return false;
}

@Override
public String toString() {
return getClass().getSimpleName();
Expand Down
5 changes: 5 additions & 0 deletions ethereumj-core/src/main/java/org/ethereum/config/blockchain/ConstantinopleConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,9 @@ public boolean eip1052() {
public boolean eip145() {
return true;
}

@Override
public boolean eip1283() {
return true;
}
}
5 changes: 5 additions & 0 deletions ethereumj-core/src/main/java/org/ethereum/config/blockchain/Eip150HFConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,9 @@ public boolean eip145() {
public boolean eip1052() {
return false;
}

@Override
public boolean eip1283() {
return false;
}
}
2 changes: 1 addition & 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 @@ -396,7 +396,7 @@ public TransactionExecutionSummary finalization() {
// Accumulate refunds for suicides
result.addFutureRefund(result.getDeleteAccounts().size() * config.getBlockchainConfig().
getConfigForBlock(currentBlock.getNumber()).getGasCost().getSUICIDE_REFUND());
long gasRefund = Math.min(result.getFutureRefund(), getGasUsed() / 2);
long gasRefund = Math.min(Math.max(0, result.getFutureRefund()), getGasUsed() / 2);
byte[] addr = tx.isContractCreation() ? tx.getContractAddress() : tx.getReceiveAddress();
m_endGas = m_endGas.add(BigInteger.valueOf(gasRefund));

Expand Down
5 changes: 5 additions & 0 deletions ethereumj-core/src/main/java/org/ethereum/vm/GasCost.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class GasCost {
private final int SET_SSTORE = 20000;
private final int RESET_SSTORE = 5000;
private final int REFUND_SSTORE = 15000;
private final int REUSE_SSTORE = 200;
private final int CREATE = 32000;

private final int JUMPDEST = 1;
Expand Down Expand Up @@ -169,6 +170,10 @@ public int getREFUND_SSTORE() {
return REFUND_SSTORE;
}

public int getREUSE_SSTORE() {
return REUSE_SSTORE;
}

public int getCREATE() {
return CREATE;
}
Expand Down
60 changes: 48 additions & 12 deletions ethereumj-core/src/main/java/org/ethereum/vm/VM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,18 +241,54 @@ public void step(Program program) {
}
break;
case SSTORE:
DataWord currentValue = program.getCurrentValue(stack.peek());
if (currentValue == null) currentValue = DataWord.ZERO;
DataWord newValue = stack.get(stack.size() - 2);
DataWord oldValue = program.storageLoad(stack.peek());
if (oldValue == null && !newValue.isZero())
gasCost = gasCosts.getSET_SSTORE();
else if (oldValue != null && newValue.isZero()) {
// todo: GASREFUND counter policy

// refund step cost policy.
program.futureRefundGas(gasCosts.getREFUND_SSTORE());
gasCost = gasCosts.getCLEAR_SSTORE();
} else
gasCost = gasCosts.getRESET_SSTORE();

if (blockchainConfig.eip1283()) { // Net gas metering for SSTORE
if (newValue.equals(currentValue)) {
gasCost = gasCosts.getREUSE_SSTORE();
} else {
DataWord origValue = program.getOriginalValue(stack.peek());
if (origValue == null) origValue = DataWord.ZERO;
if (currentValue.equals(origValue)) {
if (origValue.isZero()) {
gasCost = gasCosts.getSET_SSTORE();
} else {
gasCost = gasCosts.getCLEAR_SSTORE();
if (newValue.isZero()) {
program.futureRefundGas(gasCosts.getREFUND_SSTORE());
}
}
} else {
gasCost = gasCosts.getREUSE_SSTORE();
if (!origValue.isZero()) {
if (currentValue.isZero()) {
program.futureRefundGas(-gasCosts.getREFUND_SSTORE());
} else if (newValue.isZero()) {
program.futureRefundGas(gasCosts.getREFUND_SSTORE());
}
}
if (origValue.equals(newValue)) {
if (origValue.isZero()) {
program.futureRefundGas(gasCosts.getSET_SSTORE() - gasCosts.getREUSE_SSTORE());
} else {
program.futureRefundGas(gasCosts.getCLEAR_SSTORE() - gasCosts.getREUSE_SSTORE());
}
}
}
}
} else { // Before EIP-1283 cost calculation
if (currentValue.isZero() && !newValue.isZero())
gasCost = gasCosts.getSET_SSTORE();
else if (!currentValue.isZero() && newValue.isZero()) {
// refund step cost policy.
program.futureRefundGas(gasCosts.getREFUND_SSTORE());
gasCost = gasCosts.getCLEAR_SSTORE();
} else {
gasCost = gasCosts.getRESET_SSTORE();
}
}
break;
case SLOAD:
gasCost = gasCosts.getSLOAD();
Expand Down Expand Up @@ -1092,7 +1128,7 @@ else if (oldValue != null && newValue.isZero()) {
break;
case SLOAD: {
DataWord key = program.stackPop();
DataWord val = program.storageLoad(key);
DataWord val = program.getCurrentValue(key);

if (logger.isInfoEnabled())
hint = "key: " + key + " value: " + val;
Expand Down
21 changes: 21 additions & 0 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 @@ -88,6 +88,7 @@ public class Program {
private Stack stack;
private Memory memory;
private Storage storage;
private Repository originalRepo;
private byte[] returnDataBuffer;

private ProgramResult result = new ProgramResult();
Expand Down Expand Up @@ -136,6 +137,7 @@ public Program(byte[] codeHash, byte[] ops, ProgramInvoke programInvoke, Transac
traceListener = new ProgramTraceListener(config.vmTrace());
this.memory = setupProgramListener(new Memory());
this.stack = setupProgramListener(new Stack());
this.originalRepo = programInvoke.getRepository().getSnapshotTo(programInvoke.getRepository().getRoot());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooops, we could have not only RepositoryImpl here at least in tests.
Any ideas how can we get copy of Repo here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you, please, be more specific with the problem?

this.storage = setupProgramListener(new Storage(programInvoke));
this.trace = new ProgramTrace(config, programInvoke);
this.blockchainConfig = config.getBlockchainConfig().getConfigForBlock(programInvoke.getNumber().longValue());
Expand Down Expand Up @@ -789,10 +791,29 @@ public byte[] getReturnDataBufferData(DataWord off, DataWord size) {
Arrays.copyOfRange(returnDataBuffer, off.intValueSafe(), off.intValueSafe() + size.intValueSafe());
}

@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd not deprecate storageLoad cause SLOAD op should keep using it. getCurrentValue and storageLoad are different operations in terms of semantics, I'd decouple them from each other.

/*
* @deprecated
* Use {@link #getCurrentValue(DataWord)} instead
*/
public DataWord storageLoad(DataWord key) {
return getStorage().getStorageValue(getOwnerAddress().getLast20Bytes(), key);
}

/**
* @return current Storage data for key
*/
public DataWord getCurrentValue(DataWord key) {
return getStorage().getStorageValue(getOwnerAddress().getLast20Bytes(), key);
}

/*
* @return Storage data at the beginning of Program execution
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's extend description with a reference to the EIP-1283

*/
public DataWord getOriginalValue(DataWord key) {
return originalRepo.getStorageValue(getOwnerAddress().getLast20Bytes(), key);
}

public DataWord getPrevHash() {
return invoke.getPrevHash();
}
Expand Down
Loading