diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/core/AbstractWorldUpdater.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/core/AbstractWorldUpdater.java index 8ae4b38c4e..c7b8c22f71 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/core/AbstractWorldUpdater.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/core/AbstractWorldUpdater.java @@ -65,6 +65,11 @@ public abstract class AbstractWorldUpdater originalValueSupplier, + final UInt256 currentValue, + final UInt256 newValue) { + + if (currentValue.equals(newValue)) { + return SSTORE_NO_OP_COST; + } else { + final UInt256 originalValue = originalValueSupplier.get(); + if (originalValue.equals(currentValue)) { + return originalValue.isZero() + ? SSTORE_FIRST_DIRTY_NEW_STORAGE_COST + : SSTORE_FIRST_DIRTY_EXISTING_STORAGE_COST; + } else { + return SSTORE_ADDITIONAL_WRITE_COST; + } + } + } + + @Override + // As per https://eips.ethereum.org/EIPS/eip-1283 + public Gas calculateStorageRefundAmount( + final Supplier originalValueSupplier, + final UInt256 currentValue, + final UInt256 newValue) { + + if (currentValue.equals(newValue)) { + return Gas.ZERO; + } else { + final UInt256 originalValue = originalValueSupplier.get(); + if (originalValue.equals(currentValue)) { + if (originalValue.isZero()) { + return Gas.ZERO; + } else if (newValue.isZero()) { + return STORAGE_RESET_REFUND_AMOUNT; + } else { + return Gas.ZERO; + } + } else { + Gas refund = Gas.ZERO; + if (!originalValue.isZero()) { + if (currentValue.isZero()) { + refund = NEGATIVE_STORAGE_RESET_REFUND_AMOUNT; + } else if (newValue.isZero()) { + refund = STORAGE_RESET_REFUND_AMOUNT; + } + } + + if (originalValue.equals(newValue)) { + refund = + refund.plus( + originalValue.isZero() + ? SSTORE_DIRTY_RETURN_TO_UNUSED_REFUND_AMOUNT + : SSTORE_DIRTY_RETURN_TO_ORIGINAL_VALUE_REFUND_AMOUNT); + } + return refund; + } + } + } } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/FrontierGasCalculator.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/FrontierGasCalculator.java index b57fa2c3e1..1c8067dc68 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/FrontierGasCalculator.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/mainnet/FrontierGasCalculator.java @@ -12,6 +12,8 @@ import net.consensys.pantheon.util.bytes.Bytes32; import net.consensys.pantheon.util.bytes.BytesValue; import net.consensys.pantheon.util.uint.UInt256; +import java.util.function.Supplier; + public class FrontierGasCalculator implements GasCalculator { private static final Gas TX_DATA_ZERO_COST = Gas.of(4L); @@ -398,18 +400,17 @@ public class FrontierGasCalculator implements GasCalculator { } @Override - public Gas getStorageResetGasCost() { - return STORAGE_RESET_GAS_COST; - } - - @Override - public Gas getStorageSetGasCost() { - return STORAGE_SET_GAS_COST; + public Gas calculateStorageCost( + final Supplier originalValue, final UInt256 currentValue, final UInt256 newValue) { + return !newValue.isZero() && currentValue.isZero() + ? STORAGE_SET_GAS_COST + : STORAGE_RESET_GAS_COST; } @Override - public Gas getStorageResetRefundAmount() { - return STORAGE_RESET_REFUND_AMOUNT; + public Gas calculateStorageRefundAmount( + final Supplier originalValue, final UInt256 currentValue, final UInt256 newValue) { + return newValue.isZero() && !currentValue.isZero() ? STORAGE_RESET_REFUND_AMOUNT : Gas.ZERO; } @Override diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/GasCalculator.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/GasCalculator.java index e15f231d9e..9d49a1ac68 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/GasCalculator.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/GasCalculator.java @@ -20,12 +20,13 @@ import net.consensys.pantheon.ethereum.vm.operations.MLoadOperation; import net.consensys.pantheon.ethereum.vm.operations.MStore8Operation; import net.consensys.pantheon.ethereum.vm.operations.MStoreOperation; import net.consensys.pantheon.ethereum.vm.operations.SLoadOperation; -import net.consensys.pantheon.ethereum.vm.operations.SStoreOperation; import net.consensys.pantheon.ethereum.vm.operations.SelfDestructOperation; import net.consensys.pantheon.ethereum.vm.operations.Sha3Operation; import net.consensys.pantheon.util.bytes.BytesValue; import net.consensys.pantheon.util.uint.UInt256; +import java.util.function.Supplier; + /** * Provides various gas cost lookups and calculations used during block processing. * @@ -331,25 +332,25 @@ public interface GasCalculator { Gas getSloadOperationGasCost(); /** - * Returns the cost for clearing a value in a {@link SStoreOperation}. - * - * @return the cost for clearing a value in a storage store operation - */ - Gas getStorageResetGasCost(); - - /** - * Returns the cost for setting a value in a {@link SStoreOperation}. + * Returns the cost for an SSTORE operation. * - * @return the cost for setting a value in a storage store operation + * @param originalValue supplies the value from prior to this transaction + * @param currentValue the current value in the affected storage location + * @param newValue the new value to be stored + * @return the gas cost for the SSTORE operation */ - Gas getStorageSetGasCost(); + Gas calculateStorageCost(Supplier originalValue, UInt256 currentValue, UInt256 newValue); /** - * Returns the refund amount for clearing a storage location in a {@link SStoreOperation}. + * Returns the refund amount for an SSTORE operation. * - * @return the refund amount for clearing a storage store operation + * @param originalValue supplies the value from prior to this transaction + * @param currentValue the current value in the affected storage location + * @param newValue the new value to be stored + * @return the gas refund for the SSTORE operation */ - Gas getStorageResetRefundAmount(); + Gas calculateStorageRefundAmount( + Supplier originalValue, UInt256 currentValue, UInt256 newValue); /** * Returns the refund amount for deleting an account in a {@link SelfDestructOperation}. diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/SStoreOperation.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/SStoreOperation.java index a3f23286c8..1425e43e84 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/SStoreOperation.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/vm/operations/SStoreOperation.java @@ -22,18 +22,15 @@ public class SStoreOperation extends AbstractOperation { @Override public Gas cost(final MessageFrame frame) { final UInt256 key = frame.getStackItem(0).asUInt256(); - final UInt256 value = frame.getStackItem(1).asUInt256(); + final UInt256 newValue = frame.getStackItem(1).asUInt256(); final Account account = frame.getWorldState().get(frame.getRecipientAddress()); // Setting storage value to non-zero from zero (i.e. nothing currently at this location) vs. // resetting an existing value. - final UInt256 storedValue = account.getStorageValue(key); + final UInt256 currentValue = account.getStorageValue(key); - if (!value.isZero() && storedValue.isZero()) { - return gasCalculator().getStorageSetGasCost(); - } else { - return gasCalculator().getStorageResetGasCost(); - } + return gasCalculator() + .calculateStorageCost(() -> getOriginalValue(frame, key), currentValue, newValue); } @Override @@ -45,9 +42,11 @@ public class SStoreOperation extends AbstractOperation { assert account != null : "VM account should exists"; // Increment the refund counter. - if (value.isZero() && !account.getStorageValue(key).isZero()) { - frame.incrementGasRefund(gasCalculator().getStorageResetRefundAmount()); - } + final UInt256 originalValue = getOriginalValue(frame, key); + final UInt256 currentValue = account.getStorageValue(key); + frame.incrementGasRefund( + gasCalculator() + .calculateStorageRefundAmount(() -> getOriginalValue(frame, key), currentValue, value)); account.setStorageValue(key.copy(), value.copy()); } @@ -61,4 +60,10 @@ public class SStoreOperation extends AbstractOperation { ? Optional.of(ExceptionalHaltReason.ILLEGAL_STATE_CHANGE) : Optional.empty(); } + + private UInt256 getOriginalValue(final MessageFrame frame, final UInt256 key) { + final Account originalAccount = + frame.getWorldState().getOriginalAccount(frame.getRecipientAddress()); + return originalAccount != null ? originalAccount.getStorageValue(key) : UInt256.ZERO; + } } diff --git a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java index 1d054dca76..d4814ce651 100755 --- a/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java +++ b/ethereum/core/src/main/java/net/consensys/pantheon/ethereum/worldstate/DebuggableMutableWorldState.java @@ -156,5 +156,11 @@ public class DebuggableMutableWorldState extends DefaultMutableWorldState { record(address); return wrapped.get(address); } + + @Override + public Account getOriginalAccount(final Address address) { + record(address); + return wrapped.getOriginalAccount(address); + } } } diff --git a/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/ExecutionContextTestFixture.java b/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/ExecutionContextTestFixture.java index 86c72c1f1e..a99cac7b8d 100755 --- a/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/ExecutionContextTestFixture.java +++ b/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/ExecutionContextTestFixture.java @@ -22,14 +22,18 @@ public class ExecutionContextTestFixture { private final WorldStateArchive stateArchive = new WorldStateArchive(new KeyValueStorageWorldStateStorage(keyValueStorage)); - ProtocolSchedule protocolSchedule = - MainnetProtocolSchedule.create(2, 3, 10, 11, 12, -1, 42); + ProtocolSchedule protocolSchedule; ProtocolContext protocolContext = new ProtocolContext<>(blockchain, stateArchive, null); public ExecutionContextTestFixture() { + this(MainnetProtocolSchedule.create(2, 3, 10, 11, 12, -1, 42)); + } + + public ExecutionContextTestFixture(final ProtocolSchedule protocolSchedule) { GenesisConfig.mainnet() .writeStateTo( new DefaultMutableWorldState(new KeyValueStorageWorldStateStorage(keyValueStorage))); + this.protocolSchedule = protocolSchedule; } public Block getGenesis() { diff --git a/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java b/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java new file mode 100644 index 0000000000..955d4b5bd3 --- /dev/null +++ b/ethereum/core/src/test-support/java/net/consensys/pantheon/ethereum/core/TestCodeExecutor.java @@ -0,0 +1,90 @@ +package net.consensys.pantheon.ethereum.core; + +import net.consensys.pantheon.crypto.SECP256K1.Signature; +import net.consensys.pantheon.ethereum.db.WorldStateArchive; +import net.consensys.pantheon.ethereum.mainnet.MainnetMessageCallProcessor; +import net.consensys.pantheon.ethereum.mainnet.PrecompileContractRegistry; +import net.consensys.pantheon.ethereum.mainnet.ProtocolSchedule; +import net.consensys.pantheon.ethereum.mainnet.ProtocolSpec; +import net.consensys.pantheon.ethereum.vm.Code; +import net.consensys.pantheon.ethereum.vm.MessageFrame; +import net.consensys.pantheon.ethereum.vm.MessageFrame.Type; +import net.consensys.pantheon.ethereum.vm.OperationTracer; +import net.consensys.pantheon.util.bytes.BytesValue; + +import java.math.BigInteger; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.function.Consumer; + +public class TestCodeExecutor { + + private final ExecutionContextTestFixture fixture; + private final BlockHeader blockHeader = new BlockHeaderTestFixture().number(13).buildHeader(); + private static final Address SENDER_ADDRESS = AddressHelpers.ofValue(244259721); + + public TestCodeExecutor(final ProtocolSchedule protocolSchedule) { + fixture = new ExecutionContextTestFixture(protocolSchedule); + } + + public MessageFrame executeCode( + final String code, final long gasLimit, final Consumer accountSetup) { + final ProtocolSpec protocolSpec = fixture.getProtocolSchedule().getByBlockNumber(0); + final WorldUpdater worldState = + createInitialWorldState(accountSetup, fixture.getStateArchive()); + final Deque messageFrameStack = new ArrayDeque<>(); + + final MainnetMessageCallProcessor messageCallProcessor = + new MainnetMessageCallProcessor(protocolSpec.getEvm(), new PrecompileContractRegistry()); + + final Transaction transaction = + Transaction.builder() + .value(Wei.ZERO) + .sender(SENDER_ADDRESS) + .signature(Signature.create(BigInteger.ONE, BigInteger.TEN, (byte) 1)) + .gasLimit(gasLimit) + .to(SENDER_ADDRESS) + .payload(BytesValue.EMPTY) + .gasPrice(Wei.ZERO) + .nonce(0) + .build(); + final MessageFrame initialFrame = + MessageFrame.builder() + .type(Type.MESSAGE_CALL) + .messageFrameStack(messageFrameStack) + .blockchain(fixture.getBlockchain()) + .worldState(worldState) + .initialGas(Gas.of(gasLimit)) + .address(SENDER_ADDRESS) + .originator(SENDER_ADDRESS) + .contract(SENDER_ADDRESS) + .gasPrice(transaction.getGasPrice()) + .inputData(transaction.getPayload()) + .sender(SENDER_ADDRESS) + .value(transaction.getValue()) + .apparentValue(transaction.getValue()) + .code(new Code(BytesValue.fromHexString(code))) + .blockHeader(blockHeader) + .depth(0) + .completer(c -> {}) + .build(); + messageFrameStack.addFirst(initialFrame); + + while (!messageFrameStack.isEmpty()) { + messageCallProcessor.process(messageFrameStack.peekFirst(), OperationTracer.NO_TRACING); + } + return initialFrame; + } + + private WorldUpdater createInitialWorldState( + final Consumer accountSetup, final WorldStateArchive stateArchive) { + final MutableWorldState initialWorldState = stateArchive.getMutable(); + + final WorldUpdater worldState = initialWorldState.updater(); + final MutableAccount senderAccount = worldState.getOrCreate(TestCodeExecutor.SENDER_ADDRESS); + accountSetup.accept(senderAccount); + worldState.commit(); + initialWorldState.persist(); + return stateArchive.getMutable(initialWorldState.rootHash()).updater(); + } +} diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/mainnet/ConstantinopleSstoreGasTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/mainnet/ConstantinopleSstoreGasTest.java new file mode 100644 index 0000000000..90a65bd28c --- /dev/null +++ b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/mainnet/ConstantinopleSstoreGasTest.java @@ -0,0 +1,87 @@ +package net.consensys.pantheon.ethereum.mainnet; + +import static net.consensys.pantheon.util.uint.UInt256.ONE; +import static net.consensys.pantheon.util.uint.UInt256.ZERO; +import static org.assertj.core.api.Assertions.assertThat; + +import net.consensys.pantheon.ethereum.core.Gas; +import net.consensys.pantheon.util.uint.UInt256; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class ConstantinopleSstoreGasTest { + + private static final UInt256 TWO = UInt256.of(2); + + private final ConstantinopleGasCalculator gasCalculator = new ConstantinopleGasCalculator(); + + @Parameters(name = "original: {0}, current: {1}, new: {2}") + public static Object[][] scenarios() { + return new Object[][] { + // Zero no-op + {ZERO, ZERO, ZERO, Gas.of(200), Gas.ZERO}, + + // Zero fresh change + {ZERO, ZERO, ONE, Gas.of(20_000), Gas.ZERO}, + + // Dirty, reset to zero + {ZERO, ONE, ZERO, Gas.of(200), Gas.of(19800)}, + + // Dirty, changed but not reset + {ZERO, ONE, TWO, Gas.of(200), Gas.ZERO}, + + // Dirty no-op + {ZERO, ONE, ONE, Gas.of(200), Gas.ZERO}, + + // Dirty, zero no-op + {ONE, ZERO, ZERO, Gas.of(200), Gas.ZERO}, + + // Dirty, reset to non-zero + {ONE, ZERO, ONE, Gas.of(200), Gas.of(-15000).plus(Gas.of(4800))}, + + // Fresh change to zero + {ONE, ONE, ZERO, Gas.of(5000), Gas.of(15000)}, + + // Fresh change with all non-zero + {ONE, ONE, TWO, Gas.of(5000), Gas.ZERO}, + + // Dirty, clear originally set value + {ONE, TWO, ZERO, Gas.of(200), Gas.of(15000)}, + + // Non-zero no-op + {ONE, ONE, ONE, Gas.of(200), Gas.ZERO}, + }; + } + + @Parameter public UInt256 originalValue; + + @Parameter(value = 1) + public UInt256 currentValue; + + @Parameter(value = 2) + public UInt256 newValue; + + @Parameter(value = 3) + public Gas expectedGasCost; + + @Parameter(value = 4) + public Gas expectedGasRefund; + + @Test + public void shouldChargeCorrectGas() { + assertThat(gasCalculator.calculateStorageCost(() -> originalValue, currentValue, newValue)) + .isEqualTo(expectedGasCost); + } + + @Test + public void shouldRefundCorrectGas() { + assertThat( + gasCalculator.calculateStorageRefundAmount(() -> originalValue, currentValue, newValue)) + .isEqualTo(expectedGasRefund); + } +} diff --git a/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ConstantinopleSStoreOperationGasCostTest.java b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ConstantinopleSStoreOperationGasCostTest.java new file mode 100644 index 0000000000..06ab70c4de --- /dev/null +++ b/ethereum/core/src/test/java/net/consensys/pantheon/ethereum/vm/operations/ConstantinopleSStoreOperationGasCostTest.java @@ -0,0 +1,79 @@ +package net.consensys.pantheon.ethereum.vm.operations; + +import static org.assertj.core.api.Assertions.assertThat; + +import net.consensys.pantheon.ethereum.core.Gas; +import net.consensys.pantheon.ethereum.core.TestCodeExecutor; +import net.consensys.pantheon.ethereum.mainnet.MainnetProtocolSchedule; +import net.consensys.pantheon.ethereum.mainnet.ProtocolSchedule; +import net.consensys.pantheon.ethereum.vm.MessageFrame; +import net.consensys.pantheon.ethereum.vm.MessageFrame.State; +import net.consensys.pantheon.util.uint.UInt256; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class ConstantinopleSStoreOperationGasCostTest { + private static final ProtocolSchedule protocolSchedule = + MainnetProtocolSchedule.create(0, 0, 0, 0, 0, 0, 1); + + @Parameters(name = "Code: {0}, Original: {1}") + public static Object[][] scenarios() { + // Tests specified in EIP-1283. + return new Object[][] { + {"0x60006000556000600055", 0, 412, 0}, + {"0x60006000556001600055", 0, 20212, 0}, + {"0x60016000556000600055", 0, 20212, 19800}, + {"0x60016000556002600055", 0, 20212, 0}, + {"0x60016000556001600055", 0, 20212, 0}, + {"0x60006000556000600055", 1, 5212, 15000}, + {"0x60006000556001600055", 1, 5212, 4800}, + {"0x60006000556002600055", 1, 5212, 0}, + {"0x60026000556003600055", 1, 5212, 0}, + {"0x60026000556001600055", 1, 5212, 4800}, + {"0x60026000556002600055", 1, 5212, 0}, + {"0x60016000556000600055", 1, 5212, 15000}, + {"0x60016000556002600055", 1, 5212, 0}, + {"0x60016000556001600055", 1, 412, 0}, + {"0x600160005560006000556001600055", 0, 40218, 19800}, + {"0x600060005560016000556000600055", 1, 10218, 19800}, + {"0x60026000556000600055", 1, 5212, 15000}, + }; + } + + private TestCodeExecutor codeExecutor; + + @Parameter public String code; + + @Parameter(value = 1) + public int originalValue; + + @Parameter(value = 2) + public int expectedGasUsed; + + @Parameter(value = 3) + public int expectedGasRefund; + + @Before + public void setUp() { + codeExecutor = new TestCodeExecutor(protocolSchedule); + } + + @Test + public void shouldCalculateGasAccordingToEip1283() { + final long gasLimit = 1_000_000; + final MessageFrame frame = + codeExecutor.executeCode( + code, + gasLimit, + account -> account.setStorageValue(UInt256.ZERO, UInt256.of(originalValue))); + assertThat(frame.getState()).isEqualTo(State.COMPLETED_SUCCESS); + assertThat(frame.getRemainingGas()).isEqualTo(Gas.of(gasLimit - expectedGasUsed)); + assertThat(frame.getGasRefund()).isEqualTo(Gas.of(expectedGasRefund)); + } +}