feat(tests): callcode gas test when call stipend is applied to gas limit

[acolytec3]

🗒️ Description

Correctly evaluate CALLCODE gas usage when a call stipend is applied to gas limit

Adds a new test to cover a bug that was discovered in the EthereumJS EVM implementation of the CALLCODE opcode where we were incorrectly applying the call stipend to the gas limit applied to the new call frame before evaluating the gas consumed by the call frame so would incorrectly continue execution of the CALLCODE call frame instead of going OOG.

🔗 Related Issues

Covers the scenario described in this issue.

✅ Checklist

• All: Set appropriate labels for the changes.
• All: Considered squashing commits to improve commit history.
• All: Added an entry to CHANGELOG.md.
• All: Considered updating the online docs in the ./docs/ directory.
• Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
• Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

[acolytec3]

This new test covers a bug that was identified in the EthereumJS EVM implementation that was recently fixed by @jochem-brouwer in our code. I'm not 100% sure where to place this test in the broader test suite so have opened this PR to get conversation started and will work through the checklist over the next day or two.

[spencer-tb]

Thanks for the contribution :)

I've added a refactor in a PR to your branch here: acolytec3#1
This uses some pytest parameterization, alongside the Opcodes class to replace the bytecode with.

Currently test placement is allocated based on when the feature/EIP was introduced.
As CALLCODE was introduced in frontier, I moved it there!

[acolytec3]

Thanks for the contribution :)

I've added a refactor in a PR to your branch here: acolytec3#1 This uses some pytest parameterization, alongside the Opcodes class to replace the bytecode with.

Currently test placement is allocated based on when the feature/EIP was introduced. As CALLCODE was introduced in frontier, I moved it there!

Thanks! I wasn't sure where to put it but have merged your PR. Anything else you need from me on it?

[spencer-tb]

Thanks! I wasn't sure where to put it but have merged your PR. Anything else you need from me on it?

Great! A commit squash would be ideal so everything is in 1 commit

[acolytec3]

Done. Looks like I mucked up something in the changelog when I squashed things 🤔 Any ideas here?

[spencer-tb]

All good! 💯

[danceratopz]

This is looking very good! I just proposed a small change in acolytec3#2 for readability, but it's not essential.

One thing that would be helpful would be a comment on the gas values used, see comment below.

[spencer-tb]

I added a PR re. the recent comments: acolytec3#3, but after looking over this again, I get a mismatch for the CALL opcode when filling with geth. That is when using the values specified in:

['f1', 36600, '0x'], // 36600 is CALL fee
['f2', 11600, '0x'], // 11600 is CALLCODE fee
['f1', 36600 + 7 * 3, '0x01'], // 36600 is CALL fee + 7 * 3 gas for 7 PUSH opcodes
['f2', 11600 + 7 * 3, '0x01'], // 11600 is CALLCODE fee + 7 * 3 gas for 7 PUSH opcodes

What values do you guys get in the dynamic gas formulas?

CALL gas breakdowns: (https://www.evm.codes/#f1)
memory_exp_cost + code_exec_cost + address_access_cost + positive_value_cost + empty_account_cost

CALLCODE gas breakdowns: (https://www.evm.codes/#f2)
memory_exp_cost + code_exec_cost + address_access_cost + positive_value_cost

[danceratopz]

The renaming made in acolytec3#3 got me thinking (as gets trickier to give a good name to tests targeting multiple opcodes!).

Does it make sense to expand this test to all execution opcodes, i.e., DELEGATECALL and STATICCALL while we're at it?

[jochem-brouwer]

This test is specific for the call stipend which we erroneously added to the current gas limit. This only applies to CALL opcodes which allow endowment (sending value) so the other candidate besides CALLCODE is CALL. 😊

[jochem-brouwer]

I wanted to resolve the merge conflict but for some reason the PR does not show up right. Could you cherry-pick acolytec3@76ed643 in here?

EDIT: that diff looks also weird 🤔 Not sure whats going on here

[danceratopz]

Looks great! Thanks a lot for the gas calculation explanations / sufficient/insufficient gas improvements!

[danceratopz]

LGTM!