diff --git a/ethjson/src/uint.rs b/ethjson/src/uint.rs index ea55d07..b97ad46 100644 --- a/ethjson/src/uint.rs +++ b/ethjson/src/uint.rs @@ -82,19 +82,25 @@ impl<'a> Visitor<'a> for UintVisitor { where E: Error, { + let parse = |value: &str| { + if value.len() > 64 { + return Err(Error::custom( + format!( + "Invalid hex value 0x{value}: value too big (length={})", + value.len() + ) + .as_str(), + )); + } + U256::from_str(value) + .map_err(|e| Error::custom(format!("Invalid hex value 0x{value}: {e}").as_str())) + }; + let value = match value.len() { 0 => U256::from(0), 2 if value.starts_with("0x") => U256::from(0), - _ if value.starts_with("0x") => { - if value.len() > 66 { - return Err(Error::custom( - format!("Invalid hex value {}: value too big", value).as_str(), - )); - } - U256::from_str(&value[2..]).map_err(|e| { - Error::custom(format!("Invalid hex value {}: {}", value, e).as_str()) - })? - } + _ if value.starts_with("0x:bigint 0x") => parse(&value[12..])?, + _ if value.starts_with("0x") => parse(&value[2..])?, _ => U256::from_dec_str(value).map_err(|e| { Error::custom(format!("Invalid decimal value {}: {:?}", value, e).as_str()) })?, @@ -177,7 +183,7 @@ mod test { assert_eq!( err.to_string(), format!( - "Invalid hex value {}: value too big at line 1 column 69", + "Invalid hex value {}: value too big (length=65) at line 1 column 69", hex ) ); diff --git a/jsontests/src/state.rs b/jsontests/src/state.rs index 9c9ed59..cf4ea39 100644 --- a/jsontests/src/state.rs +++ b/jsontests/src/state.rs @@ -261,19 +261,18 @@ fn test_run(name: &str, test: Test) { // Test case may be expected to fail with an unsupported tx type if the current fork is // older than Berlin (see EIP-2718). However, this is not implemented in sputnik itself and rather // in the code hosting sputnik. https://github.com/rust-blockchain/evm/pull/40 - let tx_type = TxType::from_txbytes(&state.txbytes); - if matches!( - spec, - ForkSpec::EIP150 - | ForkSpec::EIP158 | ForkSpec::Frontier - | ForkSpec::Homestead - | ForkSpec::Byzantium - | ForkSpec::Constantinople - | ForkSpec::ConstantinopleFix - | ForkSpec::Istanbul - ) && tx_type != TxType::Legacy - && state.expect_exception == Some("TR_TypeNotSupported".to_string()) - { + let expect_tx_type_not_supported = + matches!( + spec, + ForkSpec::EIP150 + | ForkSpec::EIP158 | ForkSpec::Frontier + | ForkSpec::Homestead | ForkSpec::Byzantium + | ForkSpec::Constantinople + | ForkSpec::ConstantinopleFix + | ForkSpec::Istanbul + ) && TxType::from_txbytes(&state.txbytes) != TxType::Legacy + && state.expect_exception.as_deref() == Some("TR_TypeNotSupported"); + if expect_tx_type_not_supported { continue; } diff --git a/jsontests/tests/state.rs b/jsontests/tests/state.rs index ebefc3b..49034f0 100644 --- a/jsontests/tests/state.rs +++ b/jsontests/tests/state.rs @@ -1,8 +1,8 @@ use evm_jsontests::state as statetests; -use std::collections::HashMap; use std::fs::{self, File}; use std::io::BufReader; use std::path::PathBuf; +use std::{collections::HashMap, path::Path}; pub fn run(dir: &str) { let _ = env_logger::try_init(); @@ -20,11 +20,18 @@ pub fn run(dir: &str) { let path = entry.path(); - let file = File::open(path).expect("Open file failed"); + if should_skip(&path) { + println!("Skipping test case {path:?}"); + continue; + } + + let file = File::open(&path).expect("Open file failed"); let reader = BufReader::new(file); - let coll: HashMap = - serde_json::from_reader(reader).expect("Parse test cases failed"); + let coll: HashMap = serde_json::from_reader(reader) + .unwrap_or_else(|e| { + panic!("Parsing test case {:?} failed: {:?}", path, e); + }); for (name, test) in coll { statetests::test(&name, test); @@ -32,6 +39,38 @@ pub fn run(dir: &str) { } } +// NOTE: Add a comment here explaining why you're skipping a test case. +const SKIPPED_CASES: &[&str] = &[ + // This is an expected failure case for testing that the VM rejects + // transactions with values that are too large, but it's geth + // specific because geth parses the hex string later in the test + // run, whereas this test runner parses everything up-front before + // running the test. + "stTransactionTest/ValueOverflow", + // The below test cases are failing in geth too and as such are + // skipped here until they are fixed there (otherwise we don't know + // what the expected value should be for each test output). + "stTransactionTest/HighGasPrice", + "stCreateTest/CreateTransactionHighNonce", +]; + +fn should_skip(path: &Path) -> bool { + let matches = |case: &str| { + let file_stem = path.file_stem().unwrap(); + let dir_path = path.parent().unwrap(); + let dir_name = dir_path.file_name().unwrap(); + Path::new(dir_name).join(file_stem) == Path::new(case) + }; + + for case in SKIPPED_CASES { + if matches(case) { + return true; + } + } + + false +} + #[test] fn st_args_zero_one_balance() { run("res/ethtests/GeneralStateTests/stArgsZeroOneBalance")