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

Implement eip 3529 #2238

Merged
merged 15 commits into from
May 10, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Added support for the upcoming BAIKAL ephemeral testnet and removed the configuration for the deprecated YOLOv3 ephemeral testnet. [\#2237](https://github.com/hyperledger/besu/pull/2237)
* Added support for the London Network Upgrade, although the block number must be set manually with `--override-genesis-config=londonBlock=<blocknumber>`. This is because the block numbers haven't been determined yet. The next release will include the number in the genesis file so it will support London with no intervention. [\#2237](https://github.com/hyperledger/besu/pull/2237)
* Implemented [EIP-3541](https://eips.ethereum.org/EIPS/eip-3541): Reject new contracts starting with the 0xEF byte [\#2243](https://github.com/hyperledger/besu/pull/2243)
* Implemented [EIP-3529](https://eips.ethereum.org/EIPS/eip-3529): Reduction in refunds [\#2238](https://github.com/hyperledger/besu/pull/2238)


### Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,18 @@ public class BerlinGasCalculator extends IstanbulGasCalculator {
private static final Gas COLD_ACCOUNT_ACCESS_COST = Gas.of(2600);
private static final Gas WARM_STORAGE_READ_COST = Gas.of(100);
private static final Gas ACCESS_LIST_ADDRESS_COST = Gas.of(2400);
private static final Gas ACCESS_LIST_STORAGE_COST = Gas.of(1900);
protected static final Gas ACCESS_LIST_STORAGE_COST = Gas.of(1900);

// redefinitions for EIP-2929
private static final Gas SLOAD_GAS = WARM_STORAGE_READ_COST;
private static final Gas SSTORE_RESET_GAS = Gas.of(5000L).minus(COLD_SLOAD_COST);
protected static final Gas SSTORE_RESET_GAS = Gas.of(5000L).minus(COLD_SLOAD_COST);

// unchanged from Istanbul
private static final Gas SSTORE_SET_GAS = Gas.of(20_000);
private static final Gas SSTORE_CLEARS_SCHEDULE = Gas.of(15_000);

private static final Gas SSTORE_SET_GAS_LESS_SLOAD_GAS = SSTORE_SET_GAS.minus(SLOAD_GAS);
private static final Gas SSTORE_RESET_GAS_LESS_SLOAD_GAS = SSTORE_RESET_GAS.minus(SLOAD_GAS);
protected static final Gas SSTORE_SET_GAS_LESS_SLOAD_GAS = SSTORE_SET_GAS.minus(SLOAD_GAS);
protected static final Gas SSTORE_RESET_GAS_LESS_SLOAD_GAS = SSTORE_RESET_GAS.minus(SLOAD_GAS);
private static final Gas NEGATIVE_SSTORE_CLEARS_SCHEDULE = Gas.ZERO.minus(SSTORE_CLEARS_SCHEDULE);

// unchanged from Frontier
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.mainnet;

import org.hyperledger.besu.ethereum.core.Account;
import org.hyperledger.besu.ethereum.core.Gas;

import org.apache.tuweni.units.bigints.UInt256;

public class LondonGasCalculator extends BerlinGasCalculator {

// redefinitions for EIP-3529
private static final Gas SSTORE_CLEARS_SCHEDULE = SSTORE_RESET_GAS.plus(ACCESS_LIST_STORAGE_COST);

private static final Gas NEGATIVE_SSTORE_CLEARS_SCHEDULE = Gas.ZERO.minus(SSTORE_CLEARS_SCHEDULE);

// redefinitions for EIP-3529
private static final int NEW_MAX_REFUND_QUOTIENT = 5;

protected LondonGasCalculator() {}

// Redefined refund amount from EIP-3529
@Override
public Gas getSelfDestructRefundAmount() {
return Gas.ZERO;
}

// defined in Berlin, but re-implemented with new constants
@Override
// As per https://eips.ethereum.org/EIPS/eip-3529
public Gas calculateStorageRefundAmount(
final Account account, final UInt256 key, final UInt256 newValue) {
Comment on lines +40 to +44
Copy link
Contributor

@garyschulte garyschulte May 7, 2021

Choose a reason for hiding this comment

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

this is identical to the berlin gas calculator, how about just protected accessors for the 2 changed constants? The method is kinda funky to read, that is the only reason I ask

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first I wanted to do this but it seems that this is what was also done for Berlin/Istanbul. Can be to improve readability or explained that this method has been updated. To maintain a consistency I wanted to keep that but we can in a future PR change that if you find it really necessary

final UInt256 currentValue = account.getStorageValue(key);
if (currentValue.equals(newValue)) {
return Gas.ZERO;
} else {
final UInt256 originalValue = account.getOriginalStorageValue(key);
if (originalValue.equals(currentValue)) {
if (originalValue.isZero()) {
return Gas.ZERO;
} else if (newValue.isZero()) {
return SSTORE_CLEARS_SCHEDULE;
} else {
return Gas.ZERO;
}
} else {
Gas refund = Gas.ZERO;
if (!originalValue.isZero()) {
if (currentValue.isZero()) {
refund = NEGATIVE_SSTORE_CLEARS_SCHEDULE;
} else if (newValue.isZero()) {
refund = SSTORE_CLEARS_SCHEDULE;
}
}

if (originalValue.equals(newValue)) {
refund =
refund.plus(
originalValue.isZero()
? SSTORE_SET_GAS_LESS_SLOAD_GAS
: SSTORE_RESET_GAS_LESS_SLOAD_GAS);
}
return refund;
}
}
}

@Override
public long getMaxRefundQuotient() {
return NEW_MAX_REFUND_QUOTIENT;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,7 @@ static ProtocolSpecBuilder londonDefinition(
configStackSizeLimit,
enableRevertReason,
quorumCompatibilityMode)
.gasCalculator(LondonGasCalculator::new)
.transactionValidatorBuilder(
gasCalculator ->
new MainnetTransactionValidator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,13 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ
}
}

protected static Gas refunded(
protected Gas refunded(
final Transaction transaction, final Gas gasRemaining, final Gas gasRefund) {
// Integer truncation takes care of the the floor calculation needed after the divide.
final Gas maxRefundAllowance =
Gas.of(transaction.getGasLimit()).minus(gasRemaining).dividedBy(2);
Gas.of(transaction.getGasLimit())
.minus(gasRemaining)
.dividedBy(gasCalculator.getMaxRefundQuotient());
final Gas refundAllowance = maxRefundAllowance.min(gasRefund);
return gasRemaining.plus(refundAllowance);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,12 @@ private AbstractMessageProcessor getMessageProcessor(final MessageFrame.Type typ
}

@SuppressWarnings("unused")
private static Gas refunded(
final Transaction transaction, final Gas gasRemaining, final Gas gasRefund) {
private Gas refunded(final Transaction transaction, final Gas gasRemaining, final Gas gasRefund) {
// Integer truncation takes care of the the floor calculation needed after the divide.
final Gas maxRefundAllowance =
Gas.of(transaction.getGasLimit()).minus(gasRemaining).dividedBy(2);
Gas.of(transaction.getGasLimit())
.minus(gasRemaining)
.dividedBy(gasCalculator.getMaxRefundQuotient());
final Gas refundAllowance = maxRefundAllowance.min(gasRefund);
return gasRemaining.plus(refundAllowance);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,4 +434,8 @@ default boolean isPrecompile(final Address address) {
default Gas modExpGasCost(final Bytes input) {
return Gas.ZERO;
}

default long getMaxRefundQuotient() {
return 2;
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright ConsenSys AG.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.vm.operations;

import static org.assertj.core.api.Assertions.assertThat;

import org.hyperledger.besu.config.StubGenesisConfigOptions;
import org.hyperledger.besu.config.experimental.ExperimentalEIPs;
import org.hyperledger.besu.ethereum.core.Account;
import org.hyperledger.besu.ethereum.core.Gas;
import org.hyperledger.besu.ethereum.core.TestCodeExecutor;
import org.hyperledger.besu.ethereum.mainnet.MainnetProtocolSchedule;
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.vm.MessageFrame;
import org.hyperledger.besu.ethereum.vm.MessageFrame.State;

import org.apache.tuweni.units.bigints.UInt256;
import org.junit.After;
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 LondonSStoreOperationGasCostTest {

private static ProtocolSchedule protocolSchedule;

@Parameters(name = "Code: {0}, Original: {1}")
public static Object[][] scenarios() {
// Tests specified in EIP-3539.
return new Object[][] {
{"0x60006000556000600055", 0, 212, 0},
{"0x60006000556001600055", 0, 20112, 0},
{"0x60016000556000600055", 0, 20112, 19900},
{"0x60016000556002600055", 0, 20112, 0},
{"0x60016000556001600055", 0, 20112, 0},
{"0x60006000556000600055", 1, 3012, 4800},
Copy link
Contributor

Choose a reason for hiding this comment

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

this would be a good test case for maxRefund <= gasUsed / 5. We don't really cover that case in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no complex logic. Trying to test it I found myself mocking everything. I think it will be more useful to check this part thanks to the reference tests

{"0x60006000556001600055", 1, 3012, 2800},
{"0x60006000556002600055", 1, 3012, 0},
{"0x60026000556000600055", 1, 3012, 4800},
{"0x60026000556003600055", 1, 3012, 0},
{"0x60026000556001600055", 1, 3012, 2800},
{"0x60026000556002600055", 1, 3012, 0},
{"0x60016000556000600055", 1, 3012, 4800},
{"0x60016000556002600055", 1, 3012, 0},
{"0x60016000556001600055", 1, 212, 0},
{"0x600160005560006000556001600055", 0, 40118, 19900},
{"0x600060005560016000556000600055", 1, 5918, 7600},
};
}

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() {
ExperimentalEIPs.eip1559Enabled = true;
protocolSchedule =
MainnetProtocolSchedule.fromConfig(new StubGenesisConfigOptions().londonBlock(0));
codeExecutor = new TestCodeExecutor(protocolSchedule);
}

@Test
public void shouldCalculateGasAccordingToEip3529() {
final long gasLimit = 1_000_000;
final MessageFrame frame =
codeExecutor.executeCode(
code,
Account.DEFAULT_VERSION,
gasLimit,
account -> account.setStorageValue(UInt256.ZERO, UInt256.valueOf(originalValue)));
assertThat(frame.getState()).isEqualTo(State.COMPLETED_SUCCESS);
assertThat(frame.getRemainingGas()).isEqualTo(Gas.of(gasLimit - (expectedGasUsed + 2100)));
assertThat(frame.getGasRefund()).isEqualTo(Gas.of(expectedGasRefund));
}

@After
public void reset() {
ExperimentalEIPs.eip1559Enabled = ExperimentalEIPs.EIP1559_ENABLED_DEFAULT_VALUE;
}
}