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.

Sign up for GitHub

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

Fix gas cost enforcement for constructors and make --enforce-gas-cost-min-value actually work #12282

Merged
merged 5 commits into from
Nov 22, 2021
Merged

Fix gas cost enforcement for constructors and make --enforce-gas-cost-min-value actually work #12282

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cf52827
Fix gas mismatch in constructor not being counted as a failure
cameel Nov 15, 2021
077b3a0
Fix `--enforce-gas-cost-min-value` being applied only when the cost e…
cameel Nov 15, 2021
f42180e
Rename gas constants in ExecutionFramework
cameel Nov 15, 2021
032f2d8
Update gas expectations
cameel Nov 15, 2021
27dc77b
Fix typo in test name: ihneritance -> inheritance
cameel Nov 15, 2021
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: 3 additions & 3 deletions test/ExecutionFramework.cpp
View file
Open in desktop
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>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Constants should be either full caps or just regular variables. UpperCamelCase is reserved for types.

Copy link
Member Author

Choose a reason for hiding this comment

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

So how about full caps? Can we start using that everywhere? The m_ was especially misleading in this case but I often find myself wishing we had some convention for constants to easily distinguish them from normal variables.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the coding style guide says c_ for constants.

Copy link
Member Author

@cameel cameel Nov 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately it does not. Should that be preferred then? You mentioned that convention before but I don't remember seeing it actually used anywhere in the code base.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, we had that once but removed it because we don't really do it for local variables.


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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion test/libsolidity/SemanticTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ bool SemanticTest::checkGasCostExpectation(TestFunctionCall& io_test, bool _comp
if (
!m_enforceGasCost ||
m_gasUsed < m_enforceGasCostMinValue ||
m_gasUsed >= m_gas ||
m_gasUsed >= InitialGas ||
(setting == "ir" && io_test.call().expectations.gasUsed.count(setting) == 0) ||
io_test.call().kind == FunctionCall::Kind::Builtin
)
Expand Down