Skip to content

Commit

Permalink
Merge pull request #12282 from ethereum/fix-gas-test-enforcement
Browse files Browse the repository at this point in the history
Fix gas cost enforcement for constructors and make `--enforce-gas-cost-min-value` actually work
  • Loading branch information
chriseth authored Nov 22, 2021
2 parents 4361478 + 27dc77b commit defc74c
Show file tree
Hide file tree
Showing 42 changed files with 107 additions and 28 deletions.
6 changes: 3 additions & 3 deletions test/ExecutionFramework.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,15 @@ void ExecutionFramework::sendMessage(bytes const& _data, bool _isCreation, u256
message.kind = EVMC_CALL;
message.destination = EVMHost::convertToEVMC(m_contractAddress);
}
message.gas = m_gas.convert_to<int64_t>();
message.gas = InitialGas.convert_to<int64_t>();

evmc::result result = m_evmcHost->call(message);

m_output = bytes(result.output_data, result.output_data + result.output_size);
if (_isCreation)
m_contractAddress = EVMHost::convertFromEVMC(result.create_address);

m_gasUsed = m_gas - result.gas_left;
m_gasUsed = InitialGas - result.gas_left;
m_transactionSuccessful = (result.status_code == EVMC_SUCCESS);

if (m_showMessages)
Expand All @@ -216,7 +216,7 @@ void ExecutionFramework::sendEther(h160 const& _addr, u256 const& _amount)
message.value = EVMHost::convertToEVMC(_amount);
message.kind = EVMC_CALL;
message.destination = EVMHost::convertToEVMC(_addr);
message.gas = m_gas.convert_to<int64_t>();
message.gas = InitialGas.convert_to<int64_t>();

m_evmcHost->call(message);
}
Expand Down
5 changes: 3 additions & 2 deletions test/ExecutionFramework.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ class ExecutionFramework
}

protected:
u256 const GasPrice = 10 * gwei;
u256 const InitialGas = 100000000;

void selectVM(evmc_capabilities _cap = evmc_capabilities::EVMC_CAPABILITY_EVM1);
void reset();

Expand Down Expand Up @@ -302,8 +305,6 @@ class ExecutionFramework
bool m_transactionSuccessful = true;
util::h160 m_sender = account(0);
util::h160 m_contractAddress;
u256 const m_gasPrice = 10 * gwei;
u256 const m_gas = 100000000;
bytes m_output;
u256 m_gasUsed;
};
Expand Down
11 changes: 7 additions & 4 deletions test/libsolidity/SemanticTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,10 @@ TestCase::TestResult SemanticTest::runTest(
if (m_transactionSuccessful == test.call().expectations.failure)
success = false;
if (success && !checkGasCostExpectation(test, _isYulRun))
{
success = false;
m_gasCostFailure = true;
}

test.setFailure(!m_transactionSuccessful);
test.setRawBytes(bytes());
Expand Down Expand Up @@ -562,14 +565,14 @@ bool SemanticTest::checkGasCostExpectation(TestFunctionCall& io_test, bool _comp
// We don't check gas if enforce gas cost is not active
// or test is run with abi encoder v1 only
// or gas used less than threshold for enforcing feature
// or the test has used up all available gas (test will fail anyway)
// or setting is "ir" and it's not included in expectations
// or if the called function is an isoltest builtin e.g. `smokeTest` or `storageEmpty`
if (
!m_enforceGasCost ||
(
(setting == "ir" || m_gasUsed < m_enforceGasCostMinValue || m_gasUsed >= m_gas) &&
io_test.call().expectations.gasUsed.count(setting) == 0
) ||
m_gasUsed < m_enforceGasCostMinValue ||
m_gasUsed >= InitialGas ||
(setting == "ir" && io_test.call().expectations.gasUsed.count(setting) == 0) ||
io_test.call().kind == FunctionCall::Kind::Builtin
)
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ contract C {
// compileViaYul: also
// ----
// constructor(): 1, 2, 3 ->
// gas irOptimized: 143598
// gas legacy: 183490
// gas legacyOptimized: 151938
// a(uint256): 0 -> 1
// a(uint256): 1 -> 2
// a(uint256): 2 -> 3
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,8 @@ contract Creator {
// compileViaYul: also
// ----
// constructor(): 1, 2, 3, 4 ->
// gas irOptimized: 132278
// gas legacy: 176789
// gas legacyOptimized: 129585
// r() -> 4
// ch() -> 3
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,8 @@ contract Test {
// compileViaYul: also
// ----
// constructor(): 7, 0x40, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz" ->
// gas irOptimized: 291443
// gas legacy: 309842
// gas legacyOptimized: 260801
// m_x() -> 7
// m_s() -> 0x20, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz"
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ contract Main {
// compileViaYul: also
// ----
// constructor(): "abc", true
// gas irOptimized: 112563
// gas legacy: 145838
// gas legacyOptimized: 104017
// getFlag() -> true
// getName() -> "abc"
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ contract C {
// compileViaYul: also
// ----
// constructor(): 1, 2, 3, 4 ->
// gas irOptimized: 180731
// gas legacy: 221377
// gas legacyOptimized: 177671
// a() -> 1
// b(uint256): 0 -> 2
// b(uint256): 1 -> 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ contract B is A {
// compileViaYul: true
// ----
// constructor() ->
// gas irOptimized: 122233
// y() -> 42
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ contract B is A {
// compileViaYul: also
// ----
// constructor() ->
// gas irOptimized: 122233
// gas legacy: 135046
// gas legacyOptimized: 116176
// y() -> 42
2 changes: 2 additions & 0 deletions test/libsolidity/semanticTests/constructor_with_params.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ contract C {
// compileViaYul: also
// ----
// constructor(): 2, 0 ->
// gas irOptimized: 104227
// gas legacy: 117158
// i() -> 2
// k() -> 0
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ contract D is B, C {
// compileViaYul: also
// ----
// constructor(): 2, 0 ->
// gas irOptimized: 159542
// gas legacy: 170665
// gas legacyOptimized: 145396
// i() -> 2
// j() -> 2
// k() -> 1
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,8 @@ contract D is C {
// compileViaYul: also
// ----
// constructor(): 2, 0 ->
// gas irOptimized: 124844
// gas legacy: 139250
// gas legacyOptimized: 119367
// i() -> 2
// k() -> 1
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ contract C {
// compileViaYul: also
// ----
// constructor() ->
// gas legacy: 249112
// gas irOptimized: 177344
// gas legacy: 250376
// gas legacyOptimized: 174522
// deposit(bytes32), 18 wei: 0x1234 ->
// ~ emit Deposit(address,bytes32,uint256) from 0xf01f7809444bd9a93a854361c6fae3f23d9e23db: #0x0fdd67305928fcac8d213d1e47bfa6165cd0b87b, #0x1234, 0x00
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ contract C {
// ----
// constructor()
// ~ emit E((uint8,int16),(uint8,int16)): #0xad3228b676f7d3cd4284a5443f17f1962b36e491b30a40b2405849e597ba5fb5, 0x00, 0x00
// gas legacy: 150662
// gas legacy: 150602
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ contract FixedFeeRegistrar is Registrar {
// compileViaYul: also
// ----
// constructor()
// gas irOptimized: 425623
// gas irOptimized: 426283
// gas legacy: 936897
// gas legacyOptimized: 490983
// reserve(string), 69 ether: 0x20, 3, "abc" ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ contract DepositContract is IDepositContract, ERC165 {
// compileViaYul: also
// ----
// constructor()
// gas irOptimized: 1558013
// gas legacy: 2580394
// gas legacyOptimized: 1775403
// gas irOptimized: 1558001
// gas legacy: 2436584
// gas legacyOptimized: 1776483
// supportsInterface(bytes4): 0x0 -> 0
// supportsInterface(bytes4): 0xffffffff00000000000000000000000000000000000000000000000000000000 -> false # defined to be false by ERC-165 #
// supportsInterface(bytes4): 0x01ffc9a700000000000000000000000000000000000000000000000000000000 -> true # ERC-165 id #
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ contract test {
// compileViaYul: also
// ----
// constructor()
// gas irOptimized: 1924584
// gas legacy: 2602700
// gas irOptimized: 1924392
// gas legacy: 2480887
// gas legacyOptimized: 1874490
// div(int256,int256): 3141592653589793238, 88714123 -> 35412542528203691288251815328
// gas irOptimized: 22137
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ contract test {
// ----
// constructor()
// gas irOptimized: 1778342
// gas legacy: 2356230
// gas legacy: 2250130
// gas legacyOptimized: 1746528
// div(uint256,uint256): 3141592653589793238, 88714123 -> 35412542528203691288251815328
// gas irOptimized: 22004
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract test {
// ----
// constructor()
// gas irOptimized: 465357
// gas legacy: 733634
// gas legacy: 672749
// gas legacyOptimized: 479606
// prb_pi() -> 3141592656369545286
// gas irOptimized: 57478
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ contract test {
// ----
// constructor()
// gas irOptimized: 702619
// gas legacy: 1188228
// gas legacy: 1130761
// gas legacyOptimized: 750416
// toSlice(string): 0x20, 11, "hello world" -> 11, 0xa0
// gas irOptimized: 22660
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ contract D {
// compileViaYul: also
// ----
// constructor(): 2 ->
// gas irOptimized: 200295
// gas legacy: 245842
// gas legacyOptimized: 195676
// f() -> 2
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,7 @@ contract D {
// compileViaYul: also
// ----
// constructor(): 2 ->
// gas irOptimized: 200458
// gas legacy: 246202
// gas legacyOptimized: 195914
// f() -> 2
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ contract C {
// compileViaYul: also
// ----
// constructor(), 1 ether ->
// gas irOptimized: 308423
// gas legacy: 465314
// gas legacyOptimized: 510004
// gas legacyOptimized: 304481
// f(uint256): 0 -> FAILURE
// f(uint256): 1 -> FAILURE
// f(uint256): 2 -> FAILURE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ contract C {
// revertStrings: debug
// ----
// constructor(), 1 ether ->
// gas irOptimized: 448383
// gas legacy: 834272
// gas legacyOptimized: 510004
// f(uint256): 0 -> FAILURE, hex"08c379a0", 0x20, 37, "Target contract does not contain", " code"
// f(uint256): 1 -> FAILURE, hex"08c379a0", 0x20, 37, "Target contract does not contain", " code"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ contract test {
// compileViaYul: also
// ----
// constructor(), 20 wei ->
// gas irOptimized: 285350
// gas legacy: 402654
// gas legacyOptimized: 274470
// sendAmount(uint256): 5 -> 5
// outOfGas() -> FAILURE # call to helper should not succeed but amount should be transferred anyway #
// checkState() -> false, 15
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ contract test {
// compileViaYul: also
// ----
// constructor(), 20 wei ->
// gas irOptimized: 285350
// gas legacy: 402654
// gas legacyOptimized: 274470
// sendAmount(uint256): 5 -> 5
// outOfGas() -> FAILURE # call to helper should not succeed but amount should be transferred anyway #
// checkState() -> false, 15
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ contract Main {
// compileViaYul: also
// ----
// constructor(), 20 wei ->
// gas irOptimized: 102862
// gas legacy: 116691
// gas legacyOptimized: 100361
// s() -> true
3 changes: 3 additions & 0 deletions test/libsolidity/semanticTests/immutable/use_scratch.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ contract C {
// compileViaYul: also
// ----
// constructor(): 3 ->
// gas irOptimized: 137184
// gas legacy: 209361
// gas legacyOptimized: 139324
// f() -> 84, 23
// m(uint256): 3 -> 7
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ contract Main {
// compileViaYul: also
// ----
// constructor(), 22 wei ->
// gas irOptimized: 288778
// gas legacy: 402045
// gas legacyOptimized: 266772
// getFlag() -> true
// getName() -> "abc"
// getBalances() -> 12, 10
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ contract ClientReceipt {
// compileViaYul: also
// ----
// constructor(), 2000 wei ->
// gas irOptimized: 191881
// gas legacy: 235167
// gas legacyOptimized: 180756
// gas irOptimized: 184076
// gas legacy: 235195
// gas legacyOptimized: 176766
// balance -> 1500
// gas irOptimized: 191881
// gas legacy: 235167
Expand Down
4 changes: 3 additions & 1 deletion test/libsolidity/semanticTests/smoke/constructor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ contract C {
// compileViaYul: also
// ----
// constructor(), 2 wei: 3 ->
// gas legacy: 148000
// gas irOptimized: 111723
// gas legacy: 151416
// gas legacyOptimized: 108388
// state() -> 3
// balance() -> 2
// balance -> 2
Expand Down
3 changes: 3 additions & 0 deletions test/libsolidity/semanticTests/state/blockhash_basic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ contract C {
// compileViaYul: also
// ----
// constructor()
// gas irOptimized: 119839
// gas legacy: 155081
// gas legacyOptimized: 107997
// genesisHash() -> 0x3737373737373737373737373737373737373737373737373737373737373737
// currentHash() -> 0
// f(uint256): 0 -> 0x3737373737373737373737373737373737373737373737373737373737373737
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ contract ERC20 {
// constructor()
// ~ emit Transfer(address,address,uint256): #0x00, #0x1212121212121212121212121212120000000012, 0x14
// gas irOptimized: 442239
// gas legacy: 861547
// gas legacy: 861559
// gas legacyOptimized: 420959
// totalSupply() -> 20
// gas irOptimized: 23415
Expand Down
5 changes: 4 additions & 1 deletion test/libsolidity/semanticTests/various/address_code.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ contract C {
function h() public view returns (uint) { return address(1).code.length; }
}
// ====
// compileViaYul: also
// compileToEwasm: also
// compileViaYul: also
// ----
// constructor() ->
// gas irOptimized: 199687
// gas legacy: 241124
// gas legacyOptimized: 155549
// initCode() -> 0x20, 0
// f() -> true
// g() -> 0
Expand Down
3 changes: 2 additions & 1 deletion test/libsolidity/semanticTests/various/code_length.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ contract C {
}
}
// ====
// compileViaYul: also
// compileToEwasm: also
// compileViaYul: also
// ----
// constructor()
// gas legacy: 126455
// f(): true, true -> true, true
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ contract C {
// compileViaYul: also
// ----
// constructor(), 23 wei ->
// gas legacy: 100517
// f() -> 0
// g() -> 1
// h() -> 23
Loading

0 comments on commit defc74c

Please sign in to comment.