Send all gas on byzantium by chriseth · Pull Request #3599 · argotorg/solidity

chriseth · 2018-02-26T17:26:19Z

Depends on #3569

axic · 2018-02-26T18:28:00Z

libsolidity/codegen/ExpressionCompiler.cpp

 		m_context << dupInstruction(m_context.baseToCurrentStackOffset(gasStackPos));
-	else if (m_context.experimentalFeatureActive(ExperimentalFeature::V050))
+	else if (m_context.evmVersion().canOverchargeGasForCall())
 		// Send all gas (requires tangerine whistle EVM)


How about actually adding tanginewhistle as a level? Should be pretty straightfoward, one place to change it.

We can do that. It was between homestead and byzantium, right?

Then I'll also add spurious dragon.

That could be used for the code size limit and the exp repricing check if we implement it.

axic · 2018-02-26T18:28:34Z

The send-all-gas change looks good.

axic · 2018-03-04T23:32:56Z

libsolidity/codegen/ExpressionCompiler.cpp

-		m_context << u256(retSize) << Instruction::ADD << Instruction::MSTORE;
+		// Touch the end of the output area so that we do not pay for memory resize during the call
+		// (which we would have to subtract from the gas left)
+		// We could also just use MLOAD; POP right before the gas calculation, but the optimizer


Even this comment is outdated at this point, since mload was changed in the optimiser :)

axic · 2018-03-04T23:35:58Z

libsolidity/codegen/ExpressionCompiler.cpp

-	// We could also just use MLOAD; POP right before the gas calculation, but the optimizer
-	// would remove that, so we use MSTORE here.
-	if (!_functionType.gasSet() && retSize > 0)
+	if (!m_context.evmVersion().canOverchargeGasForCall())


Actually this code piece may still be useful if there is out of gas condition, it stops at memory expansion and not at the time of the return of the call (it is an edge case though).

Also not sure this doesn't depend on hasReturndata the option?

I'm not 100% sure I understand. The problem is that the CALL opcode performs memory expansion for the return area. If I remember correctly, the CALL opcode first performs memory expansion to accomodate both the argument and the return data memory area. After that, pre-tangerine-whistle VMs would check if there is still enough gas left to send the specified amount with the call. Post-tangerine-whistle VMs would not fail if this check is negative, but just send as much gas as is still left.

"it stops .. not at the time of the return of the call" - as far as I know, this can never happen, since the memory is expanded before the call is performed. Moving this to after the call was one of the early suggestions as part of EIP5.

This should be unrelated to hasReturndata.

Had to check this again also discussed with @chfast. Apparently memory expansion happens before the call and the remainder gas is taken as you've said. My comment is invalid then.

Also not including this code should make no difference in behaviour but should save gas by not doing it manually.

chriseth · 2018-03-05T11:25:08Z

Changed the backticks.

axic · 2018-03-05T18:11:54Z

Tests fail on gas disagreement:

/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "function_calls": check gas.value == m_gasUsed failed [41801 != 41991]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "multiple_external_functions": check gas.value == m_gasUsed failed [41801 != 41991]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "multiple_external_functions": check gas.value == m_gasUsed failed [21769 != 21919]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "exponent_size": check gas.value == m_gasUsed failed [21752 != 21832]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "exponent_size": check gas.value == m_gasUsed failed [21740 != 21860]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "balance_gas": check gas.value == m_gasUsed failed [21729 != 22109]
/home/travis/build/ethereum/solidity/test/libsolidity/GasMeter.cpp(99): error in "extcodesize_gas": check gas.value == m_gasUsed failed [21486 != 22166]

chriseth · 2018-03-05T18:31:00Z

@axic I think this is an error in travis, because it is not present in circle

… to homestead).

chriseth · 2018-03-05T18:58:47Z

The error is already in develop, not in this branch. I think it came with "evmVersion".

chriseth requested a review from axic February 26, 2018 17:26

chriseth mentioned this pull request Feb 26, 2018

v050 should be an analysis-only experimental feature (and thus not create a warning) #3576

Closed

axic reviewed Feb 26, 2018

View reviewed changes

chriseth mentioned this pull request Feb 26, 2018

Do not warn about 0.5.0 experimental pragma. #3600

Merged

chriseth force-pushed the sendAllGasOnByzantium branch 2 times, most recently from d8fe874 to 9e4a911 Compare March 2, 2018 11:26

axic reviewed Mar 4, 2018

View reviewed changes

chriseth force-pushed the sendAllGasOnByzantium branch from 9e4a911 to e472893 Compare March 5, 2018 11:16

axic approved these changes Mar 5, 2018

View reviewed changes

Do not retain any gas in external calls (except if EVM version is set…

83fcf00

… to homestead).

chriseth force-pushed the sendAllGasOnByzantium branch from e472893 to 83fcf00 Compare March 5, 2018 18:32

chriseth merged commit be797cb into develop Mar 5, 2018

axic deleted the sendAllGasOnByzantium branch March 5, 2018 20:22

Conversation

chriseth commented Feb 26, 2018

Uh oh!

axic Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

axic commented Feb 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriseth commented Mar 5, 2018

Uh oh!

axic commented Mar 5, 2018

Uh oh!

chriseth commented Mar 5, 2018

Uh oh!

chriseth commented Mar 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

axic Feb 26, 2018 •

edited

Loading