Conversation
c4aab79 to
d92f540
Compare
|
Gas diff:
|
test/libsolidity/semanticTests/inlineAssembly/mcopy_as_identifier_pre_cancun.sol
Show resolved
Hide resolved
| // tstoreNotAllowedStaticCall() -> true | ||
| // gas irOptimized: 98419720 | ||
| // gas irOptimized code: 19000 | ||
| // gas legacy: 98409086 | ||
| // gas legacy code: 30000 | ||
| // gas legacyOptimized: 98420962 | ||
| // gas legacyOptimized code: 17800 |
There was a problem hiding this comment.
Hmm... the code cost here seems surprisingly low. I'd expect the deployment cost to be the biggest factor here. Given 200 gas per byte, this means code size of 85 bytes, 150 bytes and 89 bytes, respectively. So I guess it's plausible. Maybe it's just the other cost that seems off. How is this function eating almost 100 million gas? It does not loop or do anything that heavy except for the contract deployment.
There was a problem hiding this comment.
I hadn't noticed that. I will try to check where exactly it is getting that high cost.
There was a problem hiding this comment.
ok. This looks highly suspicious so it would be great if we knew what's happening and if something is broken.
But that's just in case. This is not a blocker for the PR itself. I think it's unlikely that it's caused by it and it's probably that we're just seeing the effects of something else here.
There was a problem hiding this comment.
Note that 100000000 * 63 / 64 = 98437500
There was a problem hiding this comment.
True! And 100000000 is the gas limit in our tests: https://github.com/ethereum/solidity/blob/cc79c91b93d322cd38beec08a1ce575d3622c7e4/test/ExecutionFramework.h#L275
So mystery solved. Looks like a failed staticcall just eats all gas forwarded to it. I did not consider that this might happen in cases other than hitting an invalid opcode. Or is it simply because state-mutating opcodes are considered invalid within a staticcall?
There was a problem hiding this comment.
Yeah, it just hits an invalid opcode and thus eats up all gas forwarded to it. (The same should happen for an sstore)
968d203 to
1396e78
Compare
|
Overall, the PR looks fine and the only blocker are the failing external tests. |
1396e78 to
59b3530
Compare
|
|
8921505 to
c8551da
Compare
c8551da to
a0d7615
Compare
a0d7615 to
a388ccb
Compare
|
Note that this now contains the workaround from #14933 so they'll go in together and CI should pass. |
Closes #14905.
Old PR for Shanghai for reference: #14158.