Waylon/eip4844 #536
Waylon/eip4844 #536
Conversation
|
Some tasks for us:
I can take the first task and I am more than happy to write tests 🙂 |
|
We will test the |
Should be good to go.
Autoparallel
force-pushed
the
waylon/EIP4844
branch
from
e3a4142
to
50ded79
Compare
Should be good to go. Cleaned and commented.
Autoparallel
force-pushed
the
waylon/EIP4844
branch
from
50ded79
to
cc71fc1
Compare
fix: add kzg check conditions
point to curve Bit reversal tests Co-authored-by: Colin Nielsen <[email protected]> Co-authored-by: Ayush Shukla <[email protected]> Signed-off-by: Waylon Jepsen <[email protected]>
| // and otherwise with a zeroed bytes32 value. The opcode has a gas cost of HASH_OPCODE_GAS. | ||
| gas!(interpreter, gas::HASH_OPCODE_GAS); | ||
| pop!(interpreter, index); | ||
| let index = as_usize_or_fail!(interpreter, index, InstructionResult::InvalidOperandOOG); |
There was a problem hiding this comment.
No i don't think it should, can i use as_usize_saturated!() here instead?
| // and replaces it on the stack with tx.blob_versioned_hashes[index] if index < len(tx.blob_versioned_hashes), | ||
| // and otherwise with a zeroed bytes32 value. The opcode has a gas cost of HASH_OPCODE_GAS. | ||
| gas!(interpreter, gas::HASH_OPCODE_GAS); | ||
| pop!(interpreter, index); |
There was a problem hiding this comment.
see pop_top! in other instructions
| } else { | ||
| // else write out a zerod out 32 bytes | ||
| let bytes: [u8; 32] = [0; 32]; | ||
| push!(interpreter, U256::from_be_bytes(bytes)); |
There was a problem hiding this comment.
yes, thats cleaner.
| /// See: https://eips.ethereum.org/EIPS/eip-4844 | ||
| pub fn point_evaluation_run(input: &[u8], gas_limit: u64) -> PrecompileResult { | ||
| // The data is encoded as follows: versioned_hash | z | y | commitment | proof | with z and y being padded 32 byte big endian values | ||
| assert!(input.len() == 192); |
There was a problem hiding this comment.
Is this good assumption? what to do if it is less or more?
There was a problem hiding this comment.
Perhaps not, Can we through an error if they aren't equal. Something like
if input.len() != 192 {
return Err(Error::InvalidInputLength);
}
| let versioned_hash = Bytes48::from_bytes(&input[0..32]).unwrap(); | ||
| let proof = Bytes48::from_bytes(&input[144..192]).unwrap(); | ||
| let kzg_settings = c_kzg::KzgSettings::load_trusted_setup_file( | ||
| "crates/precompile/src/trusted_setup4.txt".into(), |
There was a problem hiding this comment.
Do you need this file in runtime? Where should be placed
There was a problem hiding this comment.
This is a mock trusted setup from that was used for testing with the EF audited bindings from c-kzg, afaik we do need it at runtime and should replace it with the official trusted set up once the ceremony is finished. I am not sure where it should live so i put it here. Do you think it should go in a different directory?
crates/revm/Cargo.toml
Outdated
| @@ -24,6 +24,7 @@ tokio = { version = "1.28", features = ["rt-multi-thread", "macros"], optional = | |||
| ethers-providers = { version = "2.0", optional = true } | |||
| ethers-core = { version = "2.0", optional = true } | |||
| futures = { version = "0.3.27", optional = true } | |||
| rust-kzg = "0.0.0" | |||
There was a problem hiding this comment.
No i was using it at one point when writing the point verification by hand prior to using the EF bindings.
|
Closing in support for #668 |
Closes #526
Should be good for review!