EVM: dont panic on invalid precompile address #1042
Conversation
actors/evm/src/interpreter/uints.rs
Outdated
| @@ -66,6 +66,12 @@ impl U256 { | |||
| buf | |||
| } | |||
|
|
|||
| /// Returns bottom 20 bytes | |||
| pub fn to_address_bytes(&self) -> [u8; 20] { | |||
There was a problem hiding this comment.
builtin-actors/actors/evm/src/interpreter/address.rs
Lines 19 to 25 in 54417ac
We don't need a second way to do this (the existing way also avoids allocating).
| if precompiles::Precompiles::<RT>::is_precompile(&dst) { | ||
| match precompiles::Precompiles::call_precompile(system, dst, input_data, context) { |
There was a problem hiding this comment.
Just call and have call_precompile return an error if the precompile doesn't exist.
There was a problem hiding this comment.
that would give us with Result<Result<Vec<u8>, PrecompileError>, ()> how about Option<Result<Vec<u8>, PrecompileError>> ?
| @@ -61,8 +61,8 @@ const fn gen_native_precompiles<RT: Runtime>() -> [PrecompileFn<RT>; 4] { | |||
| } | |||
| } | |||
|
|
|||
| pub fn is_reserved_precompile_address(addr: [u8; 20]) -> bool { | |||
| let [prefix, middle @ .., index] = addr; | |||
| pub fn is_reserved_precompile_address(addr: &U256) -> bool { | |||
There was a problem hiding this comment.
Rather than change this to a U256, I'd change it to an EthAddress. It'll be faster and will simplify a lot of code.
…nstead of whatever insanity i did last night
| } | ||
| } | ||
| None => { | ||
| log::warn!(target: "evm", "Non-existing precompile address: {:?}", EthAddress::from(dst)); | ||
| (0, vec![]) |
There was a problem hiding this comment.
So, this should actually return success.
There was a problem hiding this comment.
Er, well... You know what? I don't care. Technically, we should return success.... but this is just so stupid.
Let's just return an error.
This: