Repository: stellar/rs-soroban-env
Pull request #729 (2023-03-15 20:17:27)

Fix an i128 conversion bug
Fix a missing-sign-extension bug in i128 conversion to u64 payloads (which is, of course, exactly the type of bug I was worried I'd write if we used _signed_ payloads; sigh, there is no avoiding this bug class)


Patch:
diff --git a/soroban-env-common/src/env_val.rs b/soroban-env-common/src/env_val.rs
index 569f33e4..68e465fb 100644
--- a/soroban-env-common/src/env_val.rs
+++ b/soroban-env-common/src/env_val.rs
@@ -236,10 +236,13 @@ where
                 hi: 0,
                 lo: val.get_body(),
             })),
-            Tag::I128Small => Ok(ScVal::I128(Int128Parts {
-                hi: 0,
-                lo: val.get_signed_body() as u64,
-            })),
+            Tag::I128Small => {
+                let body = val.get_signed_body() as i128;
+                Ok(ScVal::I128(Int128Parts {
+                    hi: (body >> 64) as u64,
+                    lo: body as u64,
+                }))
+            }
             Tag::U256Small => {
                 let val = U256::new(val.get_body() as u128);
                 Ok(ScVal::U256(Uint256(val.to_be_bytes())))
diff --git a/soroban-env-common/src/num.rs b/soroban-env-common/src/num.rs
index d9fbf67d..8ec809d1 100644
--- a/soroban-env-common/src/num.rs
+++ b/soroban-env-common/src/num.rs
@@ -270,13 +270,68 @@ pub fn is_small_i256(i: &I256) -> bool {
     is_small_i64(word) && *i == I256::from(word)
 }
 
+pub const MIN_SMALL_U64: u64 = 0;
+pub const MAX_SMALL_U64: u64 = 0x00ff_ffff_ffff_ffff_u64;
+
+pub const MIN_SMALL_I64: i64 = 0xff80_0000_0000_0000_u64 as i64;
+pub const MAX_SMALL_I64: i64 = 0x007f_ffff_ffff_ffff_u64 as i64;
+
+static_assertions::const_assert!(MIN_SMALL_I64 == -36_028_797_018_963_968_i64);
+static_assertions::const_assert!(MAX_SMALL_I64 == 36_028_797_018_963_967_i64);
+
+pub const MIN_SMALL_U128: u128 = MIN_SMALL_U64 as u128;
+pub const MAX_SMALL_U128: u128 = MAX_SMALL_U64 as u128;
+
+pub const MIN_SMALL_I128: i128 = MIN_SMALL_I64 as i128;
+pub const MAX_SMALL_I128: i128 = MAX_SMALL_I64 as i128;
+
+pub const MIN_SMALL_U256: U256 = U256::new(MIN_SMALL_U128);
+pub const MAX_SMALL_U256: U256 = U256::new(MAX_SMALL_U128);
+
+pub const MIN_SMALL_I256: I256 = I256::new(MIN_SMALL_I128);
+pub const MAX_SMALL_I256: I256 = I256::new(MAX_SMALL_I128);
+
 #[test]
 fn test_small_ints() {
-    assert!(!is_small_i64(0xff7f_ffff_ffff_ffff_u64 as i64));
-    assert!(is_small_i64(0xff80_0000_0000_0000_u64 as i64));
-    assert!(is_small_i64(0x007f_ffff_ffff_ffff_u64 as i64));
-    assert!(!is_small_i64(0x0080_0000_0000_0000_u64 as i64));
-
-    assert!(is_small_u64(0x00ff_ffff_ffff_ffff_u64));
-    assert!(!is_small_u64(0x0100_0000_0000_0000_u64));
+    assert!(!is_small_i64(MIN_SMALL_I64 - 1));
+    assert!(is_small_i64(MIN_SMALL_I64));
+    assert!(is_small_i64(MAX_SMALL_I64));
+    assert!(!is_small_i64(MAX_SMALL_I64 + 1));
+
+    assert!(is_small_u64(MIN_SMALL_U64));
+    assert!(is_small_u64(MAX_SMALL_U64));
+    assert!(!is_small_u64(MAX_SMALL_U64 + 1));
+
+    assert!(!is_small_i128(MIN_SMALL_I128 - 1));
+    assert!(is_small_i128(MIN_SMALL_I128));
+    assert!(is_small_i128(MAX_SMALL_I128));
+    assert!(!is_small_i128(MAX_SMALL_I128 + 1));
+
+    assert!(is_small_u128(MIN_SMALL_U128));
+    assert!(is_small_u128(MAX_SMALL_U128));
+    assert!(!is_small_u128(MAX_SMALL_U128 + 1));
+
+    assert!(!is_small_i256(&(MIN_SMALL_I256 - 1)));
+    assert!(is_small_i256(&(MIN_SMALL_I256)));
+    assert!(is_small_i256(&(MAX_SMALL_I256)));
+    assert!(!is_small_i256(&(MAX_SMALL_I256 + 1)));
+
+    assert!(is_small_u256(&(MIN_SMALL_U256)));
+    assert!(is_small_u256(&(MAX_SMALL_U256)));
+    assert!(!is_small_u256(&(MAX_SMALL_U256 + 1)));
+
+    assert!(is_small_i64(-1_i64));
+    assert!(is_small_i64(-12345_i64));
+    assert!(is_small_i64(1_i64));
+    assert!(is_small_i64(12345_i64));
+
+    assert!(is_small_i128(-1_i128));
+    assert!(is_small_i128(-12345_i128));
+    assert!(is_small_i128(1_i128));
+    assert!(is_small_i128(12345_i128));
+
+    assert!(is_small_i256(&I256::new(-1_i128)));
+    assert!(is_small_i256(&I256::new(-12345_i128)));
+    assert!(is_small_i256(&I256::new(1_i128)));
+    assert!(is_small_i256(&I256::new(12345_i128)));
 }


Test patch:
diff --git a/soroban-env-host/src/test/num.rs b/soroban-env-host/src/test/num.rs
index bea11ca8..4fe68b1f 100644
--- a/soroban-env-host/src/test/num.rs
+++ b/soroban-env-host/src/test/num.rs
@@ -1,9 +1,10 @@
 use soroban_env_common::{
     xdr::{ScVal, Uint256},
-    TryIntoVal, I256,
+    TryFromVal, TryIntoVal, I256,
 };
 
 use crate::{Host, HostError, RawVal};
+use core::fmt::Debug;
 
 #[test]
 fn test_i64_roundtrip() -> Result<(), HostError> {
@@ -38,3 +39,72 @@ fn test_256_roundtrip() -> Result<(), HostError> {
 
     Ok(())
 }
+
+#[test]
+fn test_num_scval_roundtrips() {
+    use soroban_env_common::num::*;
+
+    fn check_roundtrip_ok<
+        T: Eq + Debug + Copy + TryIntoVal<Host, RawVal> + TryFromVal<Host, RawVal>,
+    >(
+        h: &Host,
+        input: T,
+        expect_object: bool,
+    ) {
+        let raw1: RawVal = input.try_into_val(h).unwrap();
+        let sc1: ScVal = raw1.try_into_val(h).unwrap();
+        let raw2: RawVal = sc1.try_into_val(h).unwrap();
+        let output: T = raw2.try_into_val(h).unwrap();
+        assert_eq!(input, output);
+        if expect_object {
+            assert!(raw1.is_object());
+            assert!(raw2.is_object());
+        } else {
+            assert!(!raw1.is_object());
+            assert!(!raw2.is_object());
+            assert_eq!(raw1.get_payload(), raw2.get_payload());
+        }
+    }
+
+    let host = Host::default();
+
+    check_roundtrip_ok::<i64>(&host, 0_i64, false);
+    check_roundtrip_ok::<i64>(&host, 1_i64, false);
+    check_roundtrip_ok::<i64>(&host, -1_i64, false);
+    check_roundtrip_ok::<i64>(&host, 12345_i64, false);
+    check_roundtrip_ok::<i64>(&host, -12345_i64, false);
+    check_roundtrip_ok::<i64>(&host, MIN_SMALL_I64, false);
+    check_roundtrip_ok::<i64>(&host, MAX_SMALL_I64, false);
+    check_roundtrip_ok::<i64>(&host, MIN_SMALL_I64 - 1, true);
+    check_roundtrip_ok::<i64>(&host, MAX_SMALL_I64 + 1, true);
+    check_roundtrip_ok::<i64>(&host, i64::MIN, true);
+    check_roundtrip_ok::<i64>(&host, i64::MAX, true);
+
+    check_roundtrip_ok::<u64>(&host, 0_u64, false);
+    check_roundtrip_ok::<u64>(&host, 1_u64, false);
+    check_roundtrip_ok::<u64>(&host, 12345_u64, false);
+    check_roundtrip_ok::<u64>(&host, MAX_SMALL_U64, false);
+    check_roundtrip_ok::<u64>(&host, MAX_SMALL_U64 + 1, true);
+    check_roundtrip_ok::<u64>(&host, u64::MAX, true);
+
+    check_roundtrip_ok::<i128>(&host, 0_i128, false);
+    check_roundtrip_ok::<i128>(&host, 1_i128, false);
+    check_roundtrip_ok::<i128>(&host, -1_i128, false);
+    check_roundtrip_ok::<i128>(&host, 12345_i128, false);
+    check_roundtrip_ok::<i128>(&host, -12345_i128, false);
+    check_roundtrip_ok::<i128>(&host, MIN_SMALL_I128, false);
+    check_roundtrip_ok::<i128>(&host, MAX_SMALL_I128, false);
+    check_roundtrip_ok::<i128>(&host, MIN_SMALL_I128 - 1, true);
+    check_roundtrip_ok::<i128>(&host, MAX_SMALL_I128 + 1, true);
+    check_roundtrip_ok::<i128>(&host, i128::MIN, true);
+    check_roundtrip_ok::<i128>(&host, i128::MAX, true);
+
+    check_roundtrip_ok::<u128>(&host, 0_u128, false);
+    check_roundtrip_ok::<u128>(&host, 1_u128, false);
+    check_roundtrip_ok::<u128>(&host, 12345_u128, false);
+    check_roundtrip_ok::<u128>(&host, MAX_SMALL_U128, false);
+    check_roundtrip_ok::<u128>(&host, MAX_SMALL_U128 + 1, true);
+    check_roundtrip_ok::<u128>(&host, u128::MAX, true);
+
+    // TODO: add roundtrips for {iu}256 when conversions exist.
+}