Repository: stellar/rs-soroban-env
Pull request #1091 (2023-10-03 21:34:18)

Feature: Expose `rem_euclid` functions on 256 bit numbers
### What

Add `rem_euclid` host functions to both the `I256` and `U256` bit numbers. 

### Why

Fixed point math implementations need to compute a remainder to round results properly. Euclidean remainder chosen for consistent positive results, but I can make the standard one work if that would be more fitting.

See ->
OpenZepplin: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/Math.sol#L207
Soroban verison: https://github.com/mootz12/soroban-fixed-point-math

### Known limitations

At the moment, I can only support "forced panic" fixed point math with `I256` and `U256` (host forces panic if something overflows during a host call to, for example, `i256_add`).

This can be worked around by changing the way errors are handled for bignum host fn calls here: https://github.com/stellar/rs-soroban-env/blob/main/soroban-env-host/src/host/num.rs#L28

I don't imagine people will want to manually handle phantom overflows in contracts anyway, but, there might be some fuzzing implications if it's hard to discern there overflows from other errors.


mootz12: SDK Implementation here: https://github.com/stellar/rs-soroban-sdk/pull/1102
jayz22: Can you add some tests in https://github.com/stellar/rs-soroban-env/blob/main/soroban-env-host/src/test/num.rs. Doesn't have to cover every corner case but just making sure the operation is covered. 
graydon: - Why did example_contract_data.wasm shrink by 80%? Is that a mistake?
- When you say "forced panic", you mean a _guest_ panic (a wasm trap)? I think that's mostly how we want to handle arithmetic errors, yeah, though we _could_ add special `wrapping_`, `checked_` or `saturating_` variants if guests want finer control. Default shouldn't be something that's ignorable.

mootz12: > * Why did example_contract_data.wasm shrink by 80%? Is that a mistake?

Looks like it lost 800 bytes (from ~4800 to ~4000). I'm guessing the 80% number on the diff is the amount left unchanged?

Inspecting the WAT files doesn't really make it clear (to me) what optimization occurred, but the WASM looks to be well tested here: https://github.com/stellar/rs-soroban-env/blob/main/soroban-env-host/src/test/storage.rs#L316

> * When you say "forced panic", you mean a _guest_ panic (a wasm trap)? I think that's mostly how we want to handle arithmetic errors, yeah, though we _could_ add special `wrapping_`, `checked_` or `saturating_` variants if guests want finer control. Default shouldn't be something that's ignorable.

I believe so, yeah. Basically if any overflow occurs during the calculation things would panic. There is a discord thread with a bit more context: https://discord.com/channels/897514728459468821/1158458172181319841
dmkozh: This looks good to me, thanks for your contribution!

Patch:
diff --git a/soroban-env-common/env.json b/soroban-env-common/env.json
index c70d7696..b69d417e 100644
--- a/soroban-env-common/env.json
+++ b/soroban-env-common/env.json
@@ -506,6 +506,22 @@
                 },
                 {
                     "export": "r",
+                    "name": "u256_rem_euclid",
+                    "args": [
+                        {
+                            "name": "lhs",
+                            "type": "U256Val"
+                        },
+                        {
+                            "name": "rhs",
+                            "type": "U256Val"
+                        }
+                    ],
+                    "return": "U256Val",
+                    "docs": "Performs checked Euclidean modulo. Computes `lhs % rhs`, returning `ScError` if `rhs == 0` or overflow occurred. "
+                },
+                {
+                    "export": "s",
                     "name": "u256_pow",
                     "args": [
                         {
@@ -521,7 +537,7 @@
                     "docs": "Performs checked exponentiation. Computes `lhs.exp(rhs)`, returning `ScError` if overflow occurred. "
                 },
                 {
-                    "export": "s",
+                    "export": "t",
                     "name": "u256_shl",
                     "args": [
                         {
@@ -537,7 +553,7 @@
                     "docs": "Performs checked shift left. Computes `lhs << rhs`, returning `ScError` if `rhs` is larger than or equal to the number of bits in `lhs`."
                 },
                 {
-                    "export": "t",
+                    "export": "u",
                     "name": "u256_shr",
                     "args": [
                         {
@@ -553,7 +569,7 @@
                     "docs": "Performs checked shift right. Computes `lhs >> rhs`, returning `ScError` if `rhs` is larger than or equal to the number of bits in `lhs`."
                 },
                 {
-                    "export": "u",
+                    "export": "v",
                     "name": "i256_add",
                     "args": [
                         {
@@ -569,7 +585,7 @@
                     "docs": "Performs checked integer addition. Computes `lhs + rhs`, returning `ScError` if overflow occurred. "
                 },
                 {
-                    "export": "v",
+                    "export": "w",
                     "name": "i256_sub",
                     "args": [
                         {
@@ -585,7 +601,7 @@
                     "docs": "Performs checked integer subtraction. Computes `lhs - rhs`, returning `ScError` if overflow occurred. "
                 },
                 {
-                    "export": "w",
+                    "export": "x",
                     "name": "i256_mul",
                     "args": [
                         {
@@ -601,7 +617,7 @@
                     "docs": "Performs checked integer multiplication. Computes `lhs * rhs`, returning `ScError` if overflow occurred. "
                 },
                 {
-                    "export": "x",
+                    "export": "y",
                     "name": "i256_div",
                     "args": [
                         {
@@ -617,7 +633,23 @@
                     "docs": "Performs checked integer division. Computes `lhs / rhs`, returning `ScError` if `rhs == 0` or overflow occurred. "
                 },
                 {
-                    "export": "y",
+                    "export": "z",
+                    "name": "i256_rem_euclid",
+                    "args": [
+                        {
+                            "name": "lhs",
+                            "type": "I256Val"
+                        },
+                        {
+                            "name": "rhs",
+                            "type": "I256Val"
+                        }
+                    ],
+                    "return": "I256Val",
+                    "docs": "Performs checked Euclidean modulo. Computes `lhs % rhs`, returning `ScError` if `rhs == 0` or overflow occurred. "
+                },
+                {
+                    "export": "A",
                     "name": "i256_pow",
                     "args": [
                         {
@@ -633,7 +665,7 @@
                     "docs": "Performs checked exponentiation. Computes `lhs.exp(rhs)`, returning `ScError` if overflow occurred. "
                 },
                 {
-                    "export": "z",
+                    "export": "B",
                     "name": "i256_shl",
                     "args": [
                         {
@@ -649,7 +681,7 @@
                     "docs": "Performs checked shift left. Computes `lhs << rhs`, returning `ScError` if `rhs` is larger than or equal to the number of bits in `lhs`."
                 },
                 {
-                    "export": "A",
+                    "export": "C",
                     "name": "i256_shr",
                     "args": [
                         {
@@ -665,7 +697,7 @@
                     "docs": "Performs checked shift right. Computes `lhs >> rhs`, returning `ScError` if `rhs` is larger than or equal to the number of bits in `lhs`."
                 },
                 {
-                    "export": "B",
+                    "export": "D",
                     "name": "timepoint_obj_from_u64",
                     "args": [
                         {
@@ -677,7 +709,7 @@
                     "docs": "Convert a `u64` to a `Timepoint` object."
                 },
                 {
-                    "export": "C",
+                    "export": "E",
                     "name": "timepoint_obj_to_u64",
                     "args": [
                         {
@@ -689,7 +721,7 @@
                     "docs": "Convert a `Timepoint` object to a `u64`."
                 },
                 {
-                    "export": "D",
+                    "export": "F",
                     "name": "duration_obj_from_u64",
                     "args": [
                         {
@@ -701,7 +733,7 @@
                     "docs": "Convert a `u64` to a `Duration` object."
                 },
                 {
-                    "export": "E",
+                    "export": "G",
                     "name": "duration_obj_to_u64",
                     "args": [
                         {
@@ -711,7 +743,7 @@
                     ],
                     "return": "u64",
                     "docs": "Convert a `Duration` object a `u64`."
-                }                
+                }
             ]
         },
         {
diff --git a/soroban-env-common/src/meta.rs b/soroban-env-common/src/meta.rs
index 9d08cb63..010734cd 100644
--- a/soroban-env-common/src/meta.rs
+++ b/soroban-env-common/src/meta.rs
@@ -58,7 +58,7 @@ soroban_env_macros::generate_env_meta_consts!(
 #[cfg(not(feature = "next"))]
 soroban_env_macros::generate_env_meta_consts!(
     ledger_protocol_version: 20,
-    pre_release_version: 63,
+    pre_release_version: 64,
 );
 
 pub const fn get_ledger_protocol_version(interface_version: u64) -> u32 {
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index 6fcf3177..9ac7abad 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -1079,6 +1079,13 @@ impl VmCallerEnv for Host {
     impl_bignum_host_fns!(u256_sub, checked_sub, U256, U256Val, Int256AddSub);
     impl_bignum_host_fns!(u256_mul, checked_mul, U256, U256Val, Int256Mul);
     impl_bignum_host_fns!(u256_div, checked_div, U256, U256Val, Int256Div);
+    impl_bignum_host_fns!(
+        u256_rem_euclid,
+        checked_rem_euclid,
+        U256,
+        U256Val,
+        Int256Div
+    );
     impl_bignum_host_fns_rhs_u32!(u256_pow, checked_pow, U256, U256Val, Int256Pow);
     impl_bignum_host_fns_rhs_u32!(u256_shl, checked_shl, U256, U256Val, Int256Shift);
     impl_bignum_host_fns_rhs_u32!(u256_shr, checked_shr, U256, U256Val, Int256Shift);
@@ -1087,6 +1094,13 @@ impl VmCallerEnv for Host {
     impl_bignum_host_fns!(i256_sub, checked_sub, I256, I256Val, Int256AddSub);
     impl_bignum_host_fns!(i256_mul, checked_mul, I256, I256Val, Int256Mul);
     impl_bignum_host_fns!(i256_div, checked_div, I256, I256Val, Int256Div);
+    impl_bignum_host_fns!(
+        i256_rem_euclid,
+        checked_rem_euclid,
+        I256,
+        I256Val,
+        Int256Div
+    );
     impl_bignum_host_fns_rhs_u32!(i256_pow, checked_pow, I256, I256Val, Int256Pow);
     impl_bignum_host_fns_rhs_u32!(i256_shl, checked_shl, I256, I256Val, Int256Shift);
     impl_bignum_host_fns_rhs_u32!(i256_shr, checked_shr, I256, I256Val, Int256Shift);


Test patch:
diff --git a/soroban-env-host/src/test/num.rs b/soroban-env-host/src/test/num.rs
index 730d6b40..aead903f 100644
--- a/soroban-env-host/src/test/num.rs
+++ b/soroban-env-host/src/test/num.rs
@@ -320,6 +320,16 @@ fn test_u256_arith() -> Result<(), HostError> {
     )?;
     check_num_arith_expect_err(&host, U256::new(1), U256::ZERO, Host::u256_div)?;
 
+    // rem_euclid
+    check_num_arith_ok(
+        &host,
+        U256::new(7),
+        U256::new(4),
+        Host::u256_rem_euclid,
+        U256::new(3),
+    )?;
+    check_num_arith_expect_err(&host, U256::new(1), U256::ZERO, Host::u256_rem_euclid)?;
+
     // pow
     check_num_arith_rhs_u32_ok(&host, U256::new(2), 5, Host::u256_pow, U256::new(32))?;
     check_num_arith_rhs_u32_expect_err(&host, U256::MAX, 2, Host::u256_pow)?;
@@ -373,6 +383,16 @@ fn test_i256_arith() -> Result<(), HostError> {
     check_num_arith_expect_err(&host, I256::MIN, I256::new(-1), Host::i256_div)?;
     check_num_arith_expect_err(&host, I256::new(1), I256::new(0), Host::i256_div)?;
 
+    // rem_euclid
+    check_num_arith_ok(
+        &host,
+        I256::new(-7),
+        I256::new(4),
+        Host::i256_rem_euclid,
+        I256::new(1),
+    )?;
+    check_num_arith_expect_err(&host, I256::new(1), I256::ZERO, Host::i256_rem_euclid)?;
+
     // pow
     check_num_arith_rhs_u32_ok(&host, I256::new(8), 2, Host::i256_pow, I256::new(64))?;
     check_num_arith_rhs_u32_expect_err(&host, I256::MAX, 2, Host::i256_pow)?;