Clean up and document gas tiers #14772

cameel · 2024-01-09T15:56:56Z

Related to #14739 (in particular #14737 (comment) and my upcoming MCOPY PR).

I needed to understand how those gas tiers work to add a new one for MCOPY. In the end it turned out that I don't even need to do it (tiers only represent the static cost and for the dynamic part I just need to add MCOPY next to other ...COPY opcodes in GasMeter::estimateMax()). Still, this was not obvious at first and I had to spend some effort to dig up that info, so I decided to add some comments explaining how to use it and how those tiers relate to constants from Execution Specs and opcode groups from Yellow Paper.

The PR also removes some obsolete tiers and an unnecessary exception handler (see review comments).

cameel · 2024-01-09T15:57:48Z

libevmasm/Instruction.h

-	ExtCode,	// 700, Extcode
-	Balance,	// 400, Balance


I'm removing the ExtCode and Balance tiers because they seem obsolete. The instructions using them no longer have a static cost independent of the EVM version so I think they should be using the Special tier.

cameel · 2024-01-09T16:00:48Z

libevmasm/Instruction.h

-	Balance,	// 400, Balance
-	Special,	// multiparam or otherwise special
-	Invalid		// Invalid.
+	// NOTE: Tiers should be ordered by cost, since we sometimes perform comparisons between them.


We do that in

solidity/libyul/optimiser/Metrics.cpp

Lines 185 to 194 in c78f965

void CodeCost::addInstructionCost(evmasm::Instruction _instruction)

{

evmasm::Tier gasPriceTier = evmasm::instructionInfo(_instruction, evmVersionFromDialect(m_dialect)).gasPriceTier;

if (gasPriceTier < evmasm::Tier::VeryLow)

m_cost -= 1;

else if (gasPriceTier < evmasm::Tier::High)

m_cost += 1;

else

m_cost += 49;

}

It's also the only place other than GasMeter::runGas() where we refer to these tiers directly. All other uses are through GasMeter so the removal of the obsolete tiers will have no real effect on compiler behavior.

BTW, I didn't have time to dig into this, but I wonder if that logic in Metrics.cpp is even correct. Is Tier::BlockHash (formerly called Tier::Ext) really supposed add 49 to to cost even though BLOCKHASH costs 20 gas? Should TLOAD/TSTORE also get 49? And isn't this also overpricing some instructions that are in Special not due to dynamic cost but rather becuse their cost is EVM-version dependent?

Maybe, when Tier::Ext was introduced, the logic was not updated accordingly and it went unnoticed...
I have no idea where the value 49 comes from, but it would make sense that Tier::BlockHash should get a value of 20 and then the upcoming Tier::WarmAccess for transient storage opcodes should get 100.
However, I also noticed that SSTORE is in Tier::Special, which would make it underpriced ?

cameel · 2024-01-09T16:02:40Z

libsolidity/interface/CompilerStack.cpp

-	try
-	{
-		// Run optimiser and compile the contract.
-		compiler->compileContract(_contract, _otherCompilers, cborEncodedMetadata);
-	}
-	catch(evmasm::OptimizerException const&)
-	{
-		solAssert(false, "Optimizer exception during compilation");
-	}
+	// Run optimiser and compile the contract.
+	compiler->compileContract(_contract, _otherCompilers, cborEncodedMetadata);


I ran into this when I forgot to update GasMeter::runGas() and got an ICE without the original message and pointing here rather than at the original assert in GasMeter::runGas() that failed. Since OptimizerException inherits from util::Exception and we have top-level handlers for that, I think it's better to just let it through.

cameel · 2024-01-09T16:10:39Z

libevmasm/GasMeter.h

@@ -43,32 +43,34 @@ class KnownState;

 namespace GasCosts
 {
+	/// NOTE: The GAS_... constants referenced by comments are defined for each EVM version in the Execution Specs:
+	/// https://ethereum.github.io/execution-specs/autoapi/ethereum/<evm version>/vm/gas/index.html


Cancun spec is still in a branch: https://github.com/ethereum/execution-specs/blob/forks/cancun/src/ethereum/cancun/vm/gas.py.

Also, the links for earlier versions (e.g. gas docs for shanghai) are broken temporarily. There was a PR merged an hour ago and something must have gone wrong. Fortunately it can also be viewed in the repo. E.g. gas docs for shanghai on master.

cameel · 2024-01-09T16:24:21Z

libevmasm/Instruction.h

-	Low,		// 5, Fast
-	Mid,		// 8, Mid
-	High,		// 10, Slow
-	Ext,		// 20, Ext


I'm not sure where Ext even comes from. It's for BLOCKHASH, which in Execution Specs is simply GAS_BLOCK_HASH, and there's no GAS_EXT there. From the name I at first assumed it could be for EXTCODEHASH and EXTCODESIZE, which in Yellow Paper is in a group called W_extaccount (along with BALANCE), but that's not it. And even in the Yellow Paper BLOCKHASH is in a group of its own.

Since all of this is unnecessarily confusing, I decided to rename this tier to BlockHash to match the Execution Specs.

github-actions · 2024-01-25T12:04:48Z

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

ekpyron

I've only superficially looked through this so far: but just to confirm: there is no actual behavioural change involved in any of this here or did I miss something? If so, I'm fine with just merging.

cameel · 2024-02-12T18:02:04Z

Yeah, this is just a refactor+docs. No non-trivial behavior changes. Like, the removed try/catch may result in a better message showing up on the console and that's all.

- By catching and asserting it we hide the line number and message. - OptimizerException inherits from util::Exception so just letting it through will have the same effect as asserting.

- They're no longer used because the cost depends on the EVM version.

cameel added the refactor label Jan 9, 2024

cameel requested review from r0qs, ekpyron, nikola-matic and matheusaaguiar January 9, 2024 15:56

cameel self-assigned this Jan 9, 2024

cameel commented Jan 9, 2024

View reviewed changes

cameel mentioned this pull request Jan 9, 2024

Introduce transient storage support for inline assembly #14737

Merged

cameel force-pushed the clean-and-document-gas-tiers branch 2 times, most recently from 443fb95 to 0f5d940 Compare January 10, 2024 13:54

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 25, 2024

r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Jan 25, 2024

cameel force-pushed the clean-and-document-gas-tiers branch 2 times, most recently from 06495c3 to 1355f52 Compare January 26, 2024 18:55

cameel force-pushed the clean-and-document-gas-tiers branch from 1355f52 to ade8096 Compare February 8, 2024 11:43

ekpyron reviewed Feb 12, 2024

View reviewed changes

ekpyron approved these changes Feb 13, 2024

View reviewed changes

cameel added 4 commits February 19, 2024 14:35

CompilerStack: Don't swallow details include in OptimizerException

18cabee

- By catching and asserting it we hide the line number and message. - OptimizerException inherits from util::Exception so just letting it through will have the same effect as asserting.

Remove obsolete gas cost tiers

f518815

- They're no longer used because the cost depends on the EVM version.

Document the Tier enum and constants that come from Execution Specs

686f5cd

Rename Ext gas cost tier to BlockHash

a973b4c

cameel force-pushed the clean-and-document-gas-tiers branch from ade8096 to a973b4c Compare February 19, 2024 13:35

cameel enabled auto-merge February 19, 2024 13:35

cameel merged commit 5cf513d into develop Feb 19, 2024
65 of 68 checks passed

cameel deleted the clean-and-document-gas-tiers branch February 19, 2024 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up and document gas tiers #14772

Clean up and document gas tiers #14772

cameel commented Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024

cameel Jan 9, 2024 •

edited

Loading

matheusaaguiar Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

github-actions bot commented Jan 25, 2024

ekpyron left a comment

cameel commented Feb 12, 2024

	void CodeCost::addInstructionCost(evmasm::Instruction _instruction)
	{
	evmasm::Tier gasPriceTier = evmasm::instructionInfo(_instruction, evmVersionFromDialect(m_dialect)).gasPriceTier;
	if (gasPriceTier < evmasm::Tier::VeryLow)
	m_cost -= 1;
	else if (gasPriceTier < evmasm::Tier::High)
	m_cost += 1;
	else
	m_cost += 49;
	}

Clean up and document gas tiers #14772

Clean up and document gas tiers #14772

Conversation

cameel commented Jan 9, 2024 • edited Loading

cameel Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

cameel Jan 9, 2024

Choose a reason for hiding this comment

cameel Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

matheusaaguiar Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

cameel Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

cameel Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

cameel Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 25, 2024

ekpyron left a comment

Choose a reason for hiding this comment

cameel commented Feb 12, 2024

cameel commented Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

matheusaaguiar Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading

cameel Jan 9, 2024 •

edited

Loading