fix(levm): resize memory in generic_call before any validation

[lima-limon-inc]

Motivation

The CALLCODE operand was not resizing the memory with its new size; it was only using the new size to calculate the gas cost. This turned out to be a problem common to all the CALL* family of operands.

Description

Right at the beginning of generic_call, we now resize the memory of the current call frame. This is because, the memory should be expanded even if the caller does not have enough funds to proceed with the transaction.

This comes from the test stRandom/randomStatetest248.json. In this test, the caller is trying to transfer U256::MAX to a different address. In our code, we only expand the memory IF the user has enough funds; but we should expand the memory regardless of funds.

[maximopalopoli]

LGTM, now I have a doubt if this should also be done in the other call ops

[JereSalo]

I'm kinda confused about this. It may be accurate behavior but we already do a resizing when we call load_range in generic_call. Are we doing something wrong? Can you elaborate more on the explanation in this? Thanks!
Do we need to do this in other call opcodes?

[lima-limon-inc]

LGTM, now I have a doubt if this should also be done in the other call ops

I was thinking the same thing! Let me elaborate

[lima-limon-inc]

This comes from the test stRandom/randomStatetest248.json. In this test, the caller is trying to transfer U256::MAX to a different address. In our code, we only expand the memory IF the user has enough funds; but we should expand the memory regardless of funds (or so it seems).

Seen here:
https://github.com/lambdaclass/ethrex/blob/main/crates/vm/levm/src/opcode_handlers/system.rs#L646-L651

We check to see if the user has enough funds, and if not we return. Therefore, load_range is never executed.

Plus, I think this raises some question regarding load_range and calculate_memory_size. They seem to be trying to do similar things, but differently.

Something seems fishy about it.

[lima-limon-inc]

I'm kinda confused about this. It may be accurate behavior but we already do a resizing when we call load_range in generic_call. Are we doing something wrong? Can you elaborate more on the explanation in this? Thanks! Do we need to do this in other call opcodes?

Maybe a slight refactor of generic_call instead? Especially if all the call* opcodes have this same behavior

[JereSalo]

So if the appropriate behavior would be to expand memory before making the necessary validations what would happen if the only change that we made to the code would be to just move the load_range to the beginning of generic_call?
Did you find anything in the execution specs regarding this?
I think that by just changing the order in generic_call it should work properly, right? We have expand memory before the validations that push zero to the stack. It sounds quite reasonable.
Let me know the results if you try it.

[lima-limon-inc]

So if the appropriate behavior would be to expand memory before making the necessary validations what would happen if the only change that we made to the code would be to just move the load_range to the beginning of generic_call? Did you find anything in the execution specs regarding this? I think that by just changing the order in generic_call it should work properly, right? We have expand memory before the validations that push zero to the stack. It sounds quite reasonable. Let me know the results if you try it.

That was the "refactor" I had in mind! (You know what they say about great minds ;) )
I'm gonna try it and see what happens.

[lima-limon-inc]

So if the appropriate behavior would be to expand memory before making the necessary validations what would happen if the only change that we made to the code would be to just move the load_range to the beginning of generic_call? Did you find anything in the execution specs regarding this? I think that by just changing the order in generic_call it should work properly, right? We have expand memory before the validations that push zero to the stack. It sounds quite reasonable. Let me know the results if you try it.

Hi Jere, quick update. I pushed (ce3bf5a) said slight refactor.
The same tests pass.

At least according to the python implementation, all CALL* type operands expand the memory; so it would appear to be a "valid change" to do it in generic_call.

They do so before calling generic_call.; however, since we are doing said expansion right at the beginning of generic_call the results are "equivalent". (Similar to what we did with the PC)