feat(levm): implement precompile sha2-256 #1520
Conversation
…ass/ethrex into precompiles_support_scaffolding
|
Was it necessary to link precompiles branches to one another? Also, it would be good to have a basic unit test for sha-256 |
crates/vm/levm/src/precompiles.rs
Outdated
| ) -> Result<Bytes, VMError> { | ||
| Ok(Bytes::new()) | ||
| let data_size: u64 = calldata |
There was a problem hiding this comment.
Why not use a usize instead?
There was a problem hiding this comment.
You'll end up doing the conversion anyways but doing it inside of the gas cost function its cleaner in my opinion
There was a problem hiding this comment.
crates/vm/levm/src/precompiles.rs
Outdated
|
|
||
| *consumed_gas = consumed_gas | ||
| .checked_add(cost) | ||
| .ok_or(PrecompileError::GasConsumedOverflow)?; |
There was a problem hiding this comment.
Shouldn't this be an internal error in this scenario? It shouldn't ever happen, right?
There was a problem hiding this comment.
In my opinion it can happen because we pass a pointer to the consumed gas, although it is as possible as the other uses of GasConsumedOverflow throughout the vm (that should not happen since they should be lower than gas limit)
crates/vm/levm/src/precompiles.rs
Outdated
| if gas_for_call < cost { | ||
| return Err(VMError::PrecompileError(PrecompileError::NotEnoughGas)); | ||
| } | ||
|
|
||
| *consumed_gas = consumed_gas | ||
| .checked_add(cost) | ||
| .ok_or(PrecompileError::GasConsumedOverflow)?; |
There was a problem hiding this comment.
Maybe we can abstract this behavior into a function that receives gas_for_call, consumed_gas and the cost, sending the consumed_gas as mutable.
There was a problem hiding this comment.
Agree we have to do this in every precompile so a dedicated function would be nice
There was a problem hiding this comment.
crates/vm/levm/src/precompiles.rs
Outdated
| if gas_for_call < cost { | ||
| return Err(VMError::PrecompileError(PrecompileError::NotEnoughGas)); | ||
| } | ||
|
|
||
| *consumed_gas = consumed_gas | ||
| .checked_add(cost) | ||
| .ok_or(PrecompileError::GasConsumedOverflow)?; |
There was a problem hiding this comment.
Agree we have to do this in every precompile so a dedicated function would be nice
crates/vm/levm/src/precompiles.rs
Outdated
| ) -> Result<Bytes, VMError> { | ||
| Ok(Bytes::new()) | ||
| let data_size: u64 = calldata |
There was a problem hiding this comment.
You'll end up doing the conversion anyways but doing it inside of the gas cost function its cleaner in my opinion
…l-precompile-sha2-256
…l-precompile-sha2-256
Motivation
The goal is to implement the sha2 256 precompile.
Description
For this precompile I struggled to add unit tests, but the EF tests related to sha2 256 precompile are passing.