diff --git a/README.md b/README.md index 356161e..065de5e 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ the `prepare` script. ### Tests -`yarn rust:check` runs `cargo fmt --check`, clippy and the 67 tests in +`yarn rust:check` runs `cargo fmt --check`, clippy and the 73 tests in `rust/src/tests.rs`, which is what CI runs on every PR. They cover the exported functions including the viem parity rules that are easy to get wrong: integers of 48 bits or fewer decode to plain JavaScript numbers and anything wider to a diff --git a/package.json b/package.json index 9252b02..1701b6f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@ambire/react-native-crypto", - "version": "1.0.0", + "version": "1.0.1", "description": "Rust-accelerated keccak256, hex and ABI coding for the Ambire wallet", "homepage": "https://www.ambire.com", "source": "./src/index.ts", @@ -59,7 +59,7 @@ "peerDependencies": { "react": "*", "react-native": "*", - "viem": "2.45.2" + "viem": "2.56.3" }, "codegenConfig": { "name": "RNAmbireCryptoSpec", diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 415c41b..93dc620 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -141,7 +141,7 @@ dependencies = [ [[package]] name = "ambire-crypto" -version = "1.0.0" +version = "1.0.1" dependencies = [ "alloy-dyn-abi", "alloy-json-abi", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index f9dd37f..949ce95 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "ambire-crypto" edition = "2021" -version = "1.0.0" +version = "1.0.1" publish = false [lib] diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 81198b1..889d4a9 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -22,6 +22,12 @@ const BIGINT_TAG: &str = "$bigint"; /// in viem's `utils/abi/decodeAbiParameters.js`. const JS_SAFE_INT_BITS: usize = 48; +/// `Number.MAX_SAFE_INTEGER`. viem's `hexToNumber` throws past it. +const JS_MAX_SAFE_INTEGER: i64 = (1 << 53) - 1; + +/// What viem's `TextDecoder` drops from the start of a decoded string. +const BYTE_ORDER_MARK: char = '\u{feff}'; + /// Returns the 32-byte hash. The JS shim handles hex on both sides. #[uniffi::export] pub fn keccak256(input: Vec) -> Vec { @@ -92,6 +98,20 @@ fn parse_viem_address(value: &str) -> Result { }) } +/// What viem's `encodeAddress` accepts. It calls `isAddress` in strict mode, so +/// an address with any upper-case digit must also carry a valid EIP-55 checksum. +fn parse_viem_strict_address(value: &str) -> Result { + let parsed = parse_viem_address(value)?; + + if value.bytes().any(|b| b.is_ascii_uppercase()) && parsed.to_checksum(None) != value { + return Err(AbiError::InvalidAddress { + msg: format!("{value}: invalid checksum"), + }); + } + + Ok(parsed) +} + /// Lowercase `0x` hex, as ethers' `hexlify` and viem's `bytesToHex` produce it. /// Empty input gives `0x`. /// @@ -274,6 +294,12 @@ fn int_to_json(decimal: String, size: usize) -> Result { msg: format!("{decimal} does not fit the declared {size}-bit width: {e}"), })?; + if !(-JS_MAX_SAFE_INTEGER..=JS_MAX_SAFE_INTEGER).contains(&number) { + return Err(AbiError::DecodeFailed { + msg: format!("{decimal} is outside the JavaScript safe integer range"), + }); + } + Ok(Value::Number(number.into())) } @@ -289,7 +315,9 @@ fn value_to_json(param: &Param, value: &DynSolValue) -> Result DynSolValue::FixedBytes(word, size) => { Ok(Value::String(hex::encode_prefixed(&word[..*size]))) } - DynSolValue::String(s) => Ok(Value::String(s.clone())), + DynSolValue::String(s) => Ok(Value::String( + s.strip_prefix(BYTE_ORDER_MARK).unwrap_or(s).to_string(), + )), // Elements reuse the same `param`; the value says whether each is a tuple. DynSolValue::Array(items) | DynSolValue::FixedArray(items) => Ok(Value::Array( items @@ -545,7 +573,7 @@ fn coerce_scalar(ty: &DynSolType, value: &Value) -> Result { - let parsed = parse_viem_address(arg_to_str(value, "address")?)?; + let parsed = parse_viem_strict_address(arg_to_str(value, "address")?)?; Ok(DynSolValue::Address(parsed)) } DynSolType::Bytes => { diff --git a/rust/src/tests.rs b/rust/src/tests.rs index b644141..c144afe 100644 --- a/rust/src/tests.rs +++ b/rust/src/tests.rs @@ -328,6 +328,74 @@ fn decode_function_result_decodes_a_string_output() { assert_eq!(decode_one("string", &data).unwrap(), r#""abc""#); } +// viem returns a string's bytes as they are, leading NULs included. +#[test] +fn decode_function_result_keeps_leading_nul_bytes_in_a_string_output() { + let data = format!( + "0x{}{}{}", + "0000000000000000000000000000000000000000000000000000000000000020", + "0000000000000000000000000000000000000000000000000000000000000005", + "0000616263000000000000000000000000000000000000000000000000000000" + ); + + assert_eq!(decode_one("string", &data).unwrap(), r#""\u0000\u0000abc""#); +} + +// viem decodes with `TextDecoder`, which drops one byte-order mark, and only at +// the very start. +#[test] +fn decode_function_result_drops_one_leading_byte_order_mark_from_a_string_output() { + let string_data = |body: &str| { + format!( + "0x{}{:064x}{:0<64}", + "0000000000000000000000000000000000000000000000000000000000000020", + body.len() / 2, + body + ) + }; + + for (body, expected) in [ + ("efbbbf61", r#""a""#), + ("efbbbfefbbbf61", "\"\u{feff}a\""), + ("00efbbbf61", "\"\\u0000\u{feff}a\""), + ("61efbbbf", "\"a\u{feff}\""), + ] { + assert_eq!( + decode_one("string", &string_data(body)).unwrap(), + expected, + "string bytes {body}" + ); + } +} + +/// A function returning one empty tuple, which `one_output_abi` cannot express +/// because it gives the output no `components`. +const EMPTY_TUPLE_OUTPUT_ABI: &str = r#"[ + {"type":"function","name":"g","inputs":[],"outputs":[{"name":"","type":"tuple","components":[]}],"stateMutability":"view"} +]"#; + +// alloy's parser cannot read zero-width types, and its decoder collapses them +// to empty values where viem keeps or rejects them, so they go back to viem. +#[test] +fn decode_function_result_hands_over_zero_width_types() { + let fixed_array_error = decode_one("uint256[0]", "0x").unwrap_err(); + assert!( + matches!(fixed_array_error, AbiError::InvalidAbi { .. }), + "expected uint256[0] to be refused as InvalidAbi, got {fixed_array_error}" + ); + + let empty_tuple_error = decode_function_result( + EMPTY_TUPLE_OUTPUT_ABI.to_string(), + "g".to_string(), + "0x".to_string(), + ) + .unwrap_err(); + assert!( + matches!(empty_tuple_error, AbiError::DecodeFailed { .. }), + "expected an empty tuple to be refused as DecodeFailed, got {empty_tuple_error}" + ); +} + // Each of these decodes if the prefix is stripped with `trim_start_matches`, // and viem reads them as different data than alloy does. #[test] @@ -345,8 +413,8 @@ fn decode_function_result_rejects_data_viem_would_read_differently() { } // Nothing range-checks a decoded word against its declared width, so a -// contract can return a `uint48` holding far more than 48 bits. viem renders -// it as a lossy JS number, so refusing sends the call there. +// contract can return a `uint48` holding far more than 48 bits. viem throws +// for it, so refusing sends the call there. #[test] fn decode_function_result_hands_over_a_narrow_int_too_wide_for_a_js_number() { let mut word = [0u8; 32]; @@ -360,6 +428,37 @@ fn decode_function_result_hands_over_a_narrow_int_too_wide_for_a_js_number() { ); } +// viem's `hexToNumber` throws past `Number.MAX_SAFE_INTEGER` on either sign, and +// a JSON number past it would come back rounded rather than refused. +#[test] +fn decode_function_result_hands_over_a_narrow_int_past_the_js_safe_integer_range() { + let max_safe = (1i64 << 53) - 1; + + for (ty, value) in [("uint48", max_safe), ("int48", -max_safe)] { + let word = I256::try_from(value).unwrap().to_be_bytes::<32>(); + + assert_eq!( + decode_one(ty, &hex::encode_prefixed(word)).unwrap(), + value.to_string(), + "{ty} holding {value} is still a safe integer and must decode" + ); + } + + for (ty, value) in [ + ("uint48", max_safe + 1), + ("uint8", max_safe + 2), + ("int48", -max_safe - 1), + ] { + let word = I256::try_from(value).unwrap().to_be_bytes::<32>(); + let result = decode_one(ty, &hex::encode_prefixed(word)); + + assert!( + matches!(result, Err(AbiError::DecodeFailed { .. })), + "expected {ty} holding {value} to be handed over as DecodeFailed, got {result:?}" + ); + } +} + #[test] fn decode_function_result_reports_an_unknown_function() { let error = decode_function_result( @@ -566,6 +665,34 @@ fn encode_function_data_rejects_a_fixed_array_of_the_wrong_length() { ); } +/// A function taking one empty tuple, which `one_arg_abi` cannot express because +/// it gives the input no `components`. +const EMPTY_TUPLE_INPUT_ABI: &str = r#"[ + {"type":"function","name":"f","inputs":[{"name":"x","type":"tuple","components":[]}],"outputs":[],"stateMutability":"nonpayable"} +]"#; + +// Same handover as on the decode side. alloy's parser cannot read these, and its +// encoder would write `string[0]` as static where viem writes an offset. +#[test] +fn encode_function_data_hands_over_zero_width_types() { + let fixed_array_error = encode_one("uint256[0]", "[]").unwrap_err(); + assert!( + matches!(fixed_array_error, AbiError::InvalidAbi { .. }), + "expected uint256[0] to be refused as InvalidAbi, got {fixed_array_error}" + ); + + let empty_tuple_error = encode_function_data( + EMPTY_TUPLE_INPUT_ABI.to_string(), + "f".to_string(), + "[[]]".to_string(), + ) + .unwrap_err(); + assert!( + matches!(empty_tuple_error, AbiError::EncodeFailed { .. }), + "expected an empty tuple to be refused as EncodeFailed, got {empty_tuple_error}" + ); +} + #[test] fn encode_function_data_rejects_a_non_array_for_an_array_parameter() { let error = encode_one("uint256[]", "1").unwrap_err(); @@ -690,6 +817,29 @@ fn encode_function_data_rejects_an_address_viem_would_reject() { } } +// viem's `encodeAddress` calls `isAddress` in strict mode, which takes an +// all-lower-case address as it is and holds any other to its EIP-55 checksum. +#[test] +fn encode_function_data_holds_a_mixed_case_address_to_its_checksum() { + let lower = "0xd8da6bf26964af9d7eed9e03e53415d37aa96045"; + let expected = encode_one("address", &format!(r#""{lower}""#)).unwrap(); + + let valid = encode_one("address", r#""0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045""#); + assert_eq!(valid.unwrap(), expected, "a valid checksum must encode"); + + for address in [ + "0xD8DA6BF26964AF9D7EED9E03E53415D37AA96045", + "0xD8dA6BF26964aF9D7eEd9e03E53415D37aA96045", + ] { + let result = encode_one("address", &format!(r#""{address}""#)); + + assert!( + matches!(result, Err(AbiError::InvalidAddress { .. })), + "expected InvalidAddress for {address}, got {result:?}" + ); + } +} + #[test] fn encode_function_data_rejects_a_non_string_address() { let error = encode_one("address", "1").unwrap_err();