bluealloy / revm

Ethereum Virtual Machine written in rust that is fast and simple to use
https://bluealloy.github.io/revm/
MIT License
1.57k stars 513 forks source link

Unexpected Coinbase Account Balance Changes During Ethereum Merge Fork Tests #489

Closed itsyaasir closed 1 year ago

itsyaasir commented 1 year ago

Hello,

I've been running the Ethereum test suite for the Merge fork against my node, with the REVM virtual machine as my executor.

During testing, I've encountered a number of failures that appear to stem from changes in the coinbase account balance. The test suite expects these balances to remain constant shown in both the pre-state and post-state, however, this is not the case in my testing environment.

Upon setting the Block Environment and defining the coinbase within the REVM, I've observed that the coinbase account balances are being modified. This seems to be odds with the expected behavior defined in the test suite.

A specific test illustrating this issue can be found here. The coinbase account in the Merge section reflects a constant balance between the pre-state and post-state.

Could you provide clarification on whether this is the correct behavior for the REVM executor? If it's not, I would appreciate any guidance or direction on how to resolve this discrepancy.

Thank you in advance for your assistance

rakita commented 1 year ago

Hi, the same test suite is run inside revm (to be precise revme binary) and we are checking the receipt/state root.

Quick search shows that coinbase balance did get changed from "balance" : "0x0898ee", to ` "balance" : "0x1bc16d674ed098ee".

And it is natural after the merge block rewards are removed but fee transfer after every tx is still present, so if tx has executed fee will be taken and added to coinbase account.

itsyaasir commented 1 year ago

I apologize for the confusion in my previous communication. I had attached the wrong test file. The test I am referring to is here (Merge test).

Upon further investigation, I discovered that the transaction execution fails(not sure if it is the expected behavior) with Halt { error: OutOfGas, gas_used: U256(43360) }.

The test attached expects the balances of the coinbase account to remain unchanged. However, I've observed that the balance does get altered when running the test against my node using the revm executor.

I am unsure if this is the intended behavior. Could you please clarify this? Thanks for getting back to me.

rakita commented 1 year ago

I apologize for the confusion in my previous communication. I had attached the wrong test file. The test I am referring to is here (Merge test).

Upon further investigation, I discovered that the transaction execution fails(not sure if it is the expected behavior) with Halt { error: OutOfGas, gas_used: U256(43360) }.

This is a runtime error (as in halt), the fee should still be deducted.

The test attached expects the balances of the coinbase account to remain unchanged. However, I've observed that the balance does get altered when running the test against my node using the revm executor. I am unsure if this is the intended behavior. Could you please clarify this? Thanks for getting back to me.

It should be changed as fee is collected, I now see that in pre and post state coinbase balance remains the same, this seems like a bug in the test.

itsyaasir commented 1 year ago

Hey @rakita , thanks for getting back to me, I managed to fix this issue, this issue was caused by misconfiguration of block environment values. This test is passing and I don't think there's a bug with the test, as the coinbase balances are to remain unchanged.

Thanks again.