address comments

MystenLabs · Mar 17, 2024 · ba5ce51 · ba5ce51
1 parent 817695a
commit ba5ce51
Show file tree

Hide file tree

Showing 10 changed files with 223 additions and 168 deletions.
diff --git a/crates/sui-framework/docs/deepbook/dependencies/sui-framework/random.md b/crates/sui-framework/docs/deepbook/dependencies/sui-framework/random.md
@@ -197,6 +197,15 @@ The actual state is stored in a versioned inner field.
 
 
 
+<a name="0x2_random_EInvalidLength"></a>
+
+
+
+<pre><code><b>const</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidLength">EInvalidLength</a>: u64 = 4;
+</code></pre>
+
+
+
 <a name="0x2_random_EInvalidRandomnessUpdate"></a>
 
 
@@ -224,6 +233,15 @@ The actual state is stored in a versioned inner field.
 
 
 
+<a name="0x2_random_U16_MAX"></a>
+
+
+
+<pre><code><b>const</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_U16_MAX">U16_MAX</a>: u64 = 65535;
+</code></pre>
+
+
+
 <a name="0x2_random_create"></a>
 
 ## Function `create`
@@ -360,7 +378,7 @@ transaction.
     <b>let</b> epoch = <a href="../../dependencies/sui-framework/tx_context.md#0x2_tx_context_epoch">tx_context::epoch</a>(ctx);
     <b>let</b> inner = <a href="../../dependencies/sui-framework/random.md#0x2_random_load_inner_mut">load_inner_mut</a>(self);
     <b>if</b> (inner.randomness_round == 0 && inner.epoch == 0 &&
-            <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
+        <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
         // First <b>update</b> should be for round zero.
         <b>assert</b>!(new_round == 0, <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidRandomnessUpdate">EInvalidRandomnessUpdate</a>);
     } <b>else</b> {
@@ -486,11 +504,12 @@ transaction.
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_append">vector::append</a>(&<b>mut</b> result, <a href="../../dependencies/sui-framework/random.md#0x2_random_derive_next_block">derive_next_block</a>(g));
         num_of_blocks = num_of_blocks - 1;
     };
-    // Take remaining bytes from the generator's buffer.
+    // Fill the generator's buffer <b>if</b> needed.
     <b>let</b> num_of_bytes = (num_of_bytes <b>as</b> u64);
     <b>if</b> (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&g.buffer) &lt; (num_of_bytes - <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result))) {
         <a href="../../dependencies/sui-framework/random.md#0x2_random_fill_buffer">fill_buffer</a>(g);
     };
+    // Take remaining bytes from the generator's buffer.
     <b>while</b> (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result) &lt; num_of_bytes) {
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_push_back">vector::push_back</a>(&<b>mut</b> result, <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_pop_back">vector::pop_back</a>(&<b>mut</b> g.buffer));
     };
@@ -724,9 +743,12 @@ transaction.
     <b>if</b> (<b>min</b> == max) {
         <b>return</b> <b>min</b>
     };
-    <b>let</b> diff = ((max - <b>min</b>) <b>as</b> u256) + 1;
+    // Pick a <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number in [0, max - <b>min</b>] by generating a <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number that is larger than max-<b>min</b>, and taking
+    // the modulo of the <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number by the range size. Then add the <b>min</b> <b>to</b> the result <b>to</b> get a number in
+    // [<b>min</b>, max].
+    <b>let</b> range_size = ((max - <b>min</b>) <b>as</b> u256) + 1;
     <b>let</b> rand = <a href="../../dependencies/sui-framework/random.md#0x2_random_u256_from_bytes">u256_from_bytes</a>(g, num_of_bytes);
-    <b>min</b> + ((rand % diff) <b>as</b> u128)
+    <b>min</b> + ((rand % range_size) <b>as</b> u128)
 }
 </code></pre>
 
@@ -870,13 +892,16 @@ transaction.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_shuffle">shuffle</a>&lt;T&gt;(g: &<b>mut</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_RandomGenerator">RandomGenerator</a>, v: &<b>mut</b> <a href="../../dependencies/move-stdlib/vector.md#0x1_vector">vector</a>&lt;T&gt;) {
-    <b>let</b> n = (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v) <b>as</b> u32);
+    <b>let</b> n = <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v);
     <b>if</b> (n == 0) {
         <b>return</b>
     };
-    <b>let</b> i: u32 = 0;
-    <b>while</b> (i &lt; (n - 1)) {
-        <b>let</b> j = <a href="../../dependencies/sui-framework/random.md#0x2_random_generate_u32_in_range">generate_u32_in_range</a>(g, i, n-1);
+    <b>assert</b>!(n &lt;= <a href="../../dependencies/sui-framework/random.md#0x2_random_U16_MAX">U16_MAX</a>, <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidLength">EInvalidLength</a>);
+    <b>let</b> n = (n <b>as</b> u16);
+    <b>let</b> i: u16 = 0;
+    <b>let</b> end = n - 1;
+    <b>while</b> (i &lt; end) {
+        <b>let</b> j = <a href="../../dependencies/sui-framework/random.md#0x2_random_generate_u16_in_range">generate_u16_in_range</a>(g, i, end);
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_swap">vector::swap</a>(v, (i <b>as</b> u64), (j <b>as</b> u64));
         i = i + 1;
     };

diff --git a/crates/sui-framework/docs/sui-framework/random.md b/crates/sui-framework/docs/sui-framework/random.md
@@ -199,6 +199,15 @@ Unique randomness generator, derived from the global randomness.
 
 
 
+<a name="0x2_random_EInvalidLength"></a>
+
+
+
+<pre><code><b>const</b> <a href="random.md#0x2_random_EInvalidLength">EInvalidLength</a>: u64 = 4;
+</code></pre>
+
+
+
 <a name="0x2_random_EInvalidRandomnessUpdate"></a>
 
 
@@ -226,6 +235,15 @@ Unique randomness generator, derived from the global randomness.
 
 
 
+<a name="0x2_random_U16_MAX"></a>
+
+
+
+<pre><code><b>const</b> <a href="random.md#0x2_random_U16_MAX">U16_MAX</a>: u64 = 65535;
+</code></pre>
+
+
+
 <a name="0x2_random_create"></a>
 
 ## Function `create`
@@ -362,7 +380,7 @@ transaction.
     <b>let</b> epoch = <a href="tx_context.md#0x2_tx_context_epoch">tx_context::epoch</a>(ctx);
     <b>let</b> inner = <a href="random.md#0x2_random_load_inner_mut">load_inner_mut</a>(self);
     <b>if</b> (inner.randomness_round == 0 && inner.epoch == 0 &&
-            <a href="dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
+        <a href="dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
         // First <b>update</b> should be for round zero.
         <b>assert</b>!(new_round == 0, <a href="random.md#0x2_random_EInvalidRandomnessUpdate">EInvalidRandomnessUpdate</a>);
     } <b>else</b> {
@@ -490,11 +508,12 @@ Generate n random bytes.
         <a href="dependencies/move-stdlib/vector.md#0x1_vector_append">vector::append</a>(&<b>mut</b> result, <a href="random.md#0x2_random_derive_next_block">derive_next_block</a>(g));
         num_of_blocks = num_of_blocks - 1;
     };
-    // Take remaining bytes from the generator's buffer.
+    // Fill the generator's buffer <b>if</b> needed.
     <b>let</b> num_of_bytes = (num_of_bytes <b>as</b> u64);
     <b>if</b> (<a href="dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&g.buffer) &lt; (num_of_bytes - <a href="dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result))) {
         <a href="random.md#0x2_random_fill_buffer">fill_buffer</a>(g);
     };
+    // Take remaining bytes from the generator's buffer.
     <b>while</b> (<a href="dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result) &lt; num_of_bytes) {
         <a href="dependencies/move-stdlib/vector.md#0x1_vector_push_back">vector::push_back</a>(&<b>mut</b> result, <a href="dependencies/move-stdlib/vector.md#0x1_vector_pop_back">vector::pop_back</a>(&<b>mut</b> g.buffer));
     };
@@ -735,9 +754,12 @@ Generate a boolean.
     <b>if</b> (<b>min</b> == max) {
         <b>return</b> <b>min</b>
     };
-    <b>let</b> diff = ((max - <b>min</b>) <b>as</b> u256) + 1;
+    // Pick a <a href="random.md#0x2_random">random</a> number in [0, max - <b>min</b>] by generating a <a href="random.md#0x2_random">random</a> number that is larger than max-<b>min</b>, and taking
+    // the modulo of the <a href="random.md#0x2_random">random</a> number by the range size. Then add the <b>min</b> <b>to</b> the result <b>to</b> get a number in
+    // [<b>min</b>, max].
+    <b>let</b> range_size = ((max - <b>min</b>) <b>as</b> u256) + 1;
     <b>let</b> rand = <a href="random.md#0x2_random_u256_from_bytes">u256_from_bytes</a>(g, num_of_bytes);
-    <b>min</b> + ((rand % diff) <b>as</b> u128)
+    <b>min</b> + ((rand % range_size) <b>as</b> u128)
 }
 </code></pre>
 
@@ -873,7 +895,7 @@ Generate a random u8 in [min, max] (with a bias of 2^{-64}).
 
 ## Function `shuffle`
 
-Shuffle a vector using the random generator.
+Shuffle a vector using the random generator (Fisher–Yates/Knuth shuffle).
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="random.md#0x2_random_shuffle">shuffle</a>&lt;T&gt;(g: &<b>mut</b> <a href="random.md#0x2_random_RandomGenerator">random::RandomGenerator</a>, v: &<b>mut</b> <a href="dependencies/move-stdlib/vector.md#0x1_vector">vector</a>&lt;T&gt;)
@@ -886,13 +908,16 @@ Shuffle a vector using the random generator.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="random.md#0x2_random_shuffle">shuffle</a>&lt;T&gt;(g: &<b>mut</b> <a href="random.md#0x2_random_RandomGenerator">RandomGenerator</a>, v: &<b>mut</b> <a href="dependencies/move-stdlib/vector.md#0x1_vector">vector</a>&lt;T&gt;) {
-    <b>let</b> n = (<a href="dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v) <b>as</b> u32);
+    <b>let</b> n = <a href="dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v);
     <b>if</b> (n == 0) {
         <b>return</b>
     };
-    <b>let</b> i: u32 = 0;
-    <b>while</b> (i &lt; (n - 1)) {
-        <b>let</b> j = <a href="random.md#0x2_random_generate_u32_in_range">generate_u32_in_range</a>(g, i, n-1);
+    <b>assert</b>!(n &lt;= <a href="random.md#0x2_random_U16_MAX">U16_MAX</a>, <a href="random.md#0x2_random_EInvalidLength">EInvalidLength</a>);
+    <b>let</b> n = (n <b>as</b> u16);
+    <b>let</b> i: u16 = 0;
+    <b>let</b> end = n - 1;
+    <b>while</b> (i &lt; end) {
+        <b>let</b> j = <a href="random.md#0x2_random_generate_u16_in_range">generate_u16_in_range</a>(g, i, end);
         <a href="dependencies/move-stdlib/vector.md#0x1_vector_swap">vector::swap</a>(v, (i <b>as</b> u64), (j <b>as</b> u64));
         i = i + 1;
     };

diff --git a/crates/sui-framework/docs/sui-system/dependencies/sui-framework/random.md b/crates/sui-framework/docs/sui-system/dependencies/sui-framework/random.md
@@ -197,6 +197,15 @@ The actual state is stored in a versioned inner field.
 
 
 
+<a name="0x2_random_EInvalidLength"></a>
+
+
+
+<pre><code><b>const</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidLength">EInvalidLength</a>: u64 = 4;
+</code></pre>
+
+
+
 <a name="0x2_random_EInvalidRandomnessUpdate"></a>
 
 
@@ -224,6 +233,15 @@ The actual state is stored in a versioned inner field.
 
 
 
+<a name="0x2_random_U16_MAX"></a>
+
+
+
+<pre><code><b>const</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_U16_MAX">U16_MAX</a>: u64 = 65535;
+</code></pre>
+
+
+
 <a name="0x2_random_create"></a>
 
 ## Function `create`
@@ -360,7 +378,7 @@ transaction.
     <b>let</b> epoch = <a href="../../dependencies/sui-framework/tx_context.md#0x2_tx_context_epoch">tx_context::epoch</a>(ctx);
     <b>let</b> inner = <a href="../../dependencies/sui-framework/random.md#0x2_random_load_inner_mut">load_inner_mut</a>(self);
     <b>if</b> (inner.randomness_round == 0 && inner.epoch == 0 &&
-            <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
+        <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_is_empty">vector::is_empty</a>(&inner.random_bytes)) {
         // First <b>update</b> should be for round zero.
         <b>assert</b>!(new_round == 0, <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidRandomnessUpdate">EInvalidRandomnessUpdate</a>);
     } <b>else</b> {
@@ -486,11 +504,12 @@ transaction.
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_append">vector::append</a>(&<b>mut</b> result, <a href="../../dependencies/sui-framework/random.md#0x2_random_derive_next_block">derive_next_block</a>(g));
         num_of_blocks = num_of_blocks - 1;
     };
-    // Take remaining bytes from the generator's buffer.
+    // Fill the generator's buffer <b>if</b> needed.
     <b>let</b> num_of_bytes = (num_of_bytes <b>as</b> u64);
     <b>if</b> (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&g.buffer) &lt; (num_of_bytes - <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result))) {
         <a href="../../dependencies/sui-framework/random.md#0x2_random_fill_buffer">fill_buffer</a>(g);
     };
+    // Take remaining bytes from the generator's buffer.
     <b>while</b> (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(&result) &lt; num_of_bytes) {
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_push_back">vector::push_back</a>(&<b>mut</b> result, <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_pop_back">vector::pop_back</a>(&<b>mut</b> g.buffer));
     };
@@ -724,9 +743,12 @@ transaction.
     <b>if</b> (<b>min</b> == max) {
         <b>return</b> <b>min</b>
     };
-    <b>let</b> diff = ((max - <b>min</b>) <b>as</b> u256) + 1;
+    // Pick a <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number in [0, max - <b>min</b>] by generating a <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number that is larger than max-<b>min</b>, and taking
+    // the modulo of the <a href="../../dependencies/sui-framework/random.md#0x2_random">random</a> number by the range size. Then add the <b>min</b> <b>to</b> the result <b>to</b> get a number in
+    // [<b>min</b>, max].
+    <b>let</b> range_size = ((max - <b>min</b>) <b>as</b> u256) + 1;
     <b>let</b> rand = <a href="../../dependencies/sui-framework/random.md#0x2_random_u256_from_bytes">u256_from_bytes</a>(g, num_of_bytes);
-    <b>min</b> + ((rand % diff) <b>as</b> u128)
+    <b>min</b> + ((rand % range_size) <b>as</b> u128)
 }
 </code></pre>
 
@@ -870,13 +892,16 @@ transaction.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_shuffle">shuffle</a>&lt;T&gt;(g: &<b>mut</b> <a href="../../dependencies/sui-framework/random.md#0x2_random_RandomGenerator">RandomGenerator</a>, v: &<b>mut</b> <a href="../../dependencies/move-stdlib/vector.md#0x1_vector">vector</a>&lt;T&gt;) {
-    <b>let</b> n = (<a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v) <b>as</b> u32);
+    <b>let</b> n = <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_length">vector::length</a>(v);
     <b>if</b> (n == 0) {
         <b>return</b>
     };
-    <b>let</b> i: u32 = 0;
-    <b>while</b> (i &lt; (n - 1)) {
-        <b>let</b> j = <a href="../../dependencies/sui-framework/random.md#0x2_random_generate_u32_in_range">generate_u32_in_range</a>(g, i, n-1);
+    <b>assert</b>!(n &lt;= <a href="../../dependencies/sui-framework/random.md#0x2_random_U16_MAX">U16_MAX</a>, <a href="../../dependencies/sui-framework/random.md#0x2_random_EInvalidLength">EInvalidLength</a>);
+    <b>let</b> n = (n <b>as</b> u16);
+    <b>let</b> i: u16 = 0;
+    <b>let</b> end = n - 1;
+    <b>while</b> (i &lt; end) {
+        <b>let</b> j = <a href="../../dependencies/sui-framework/random.md#0x2_random_generate_u16_in_range">generate_u16_in_range</a>(g, i, end);
         <a href="../../dependencies/move-stdlib/vector.md#0x1_vector_swap">vector::swap</a>(v, (i <b>as</b> u64), (j <b>as</b> u64));
         i = i + 1;
     };

diff --git a/crates/sui-framework/packages/sui-framework/sources/random.move b/crates/sui-framework/packages/sui-framework/sources/random.move
@@ -17,9 +17,11 @@ module sui::random {
     const EWrongInnerVersion: u64 = 1;
     const EInvalidRandomnessUpdate: u64 = 2;
     const EInvalidRange: u64 = 3;
+    const EInvalidLength: u64 = 4;
 
     const CURRENT_VERSION: u64 = 1;
     const RAND_OUTPUT_LEN: u16 = 32;
+    const U16_MAX: u64 = 0xFFFF;
 
     /// Singleton shared object which stores the global randomness state.
     /// The actual state is stored in a versioned inner field.
@@ -173,11 +175,12 @@ module sui::random {
             vector::append(&mut result, derive_next_block(g));
             num_of_blocks = num_of_blocks - 1;
         };
-        // Take remaining bytes from the generator's buffer.
+        // Fill the generator's buffer if needed.
         let num_of_bytes = (num_of_bytes as u64);
         if (vector::length(&g.buffer) < (num_of_bytes - vector::length(&result))) {
             fill_buffer(g);
         };
+        // Take remaining bytes from the generator's buffer.
         while (vector::length(&result) < num_of_bytes) {
             vector::push_back(&mut result, vector::pop_back(&mut g.buffer));
         };
@@ -186,6 +189,7 @@ module sui::random {
 
     // Helper function that extracts the given number of bytes from the random generator and returns it as u256.
     // Assumes that the caller has already checked that num_of_bytes is valid.
+    // TODO: Replace with a macro when we have support for it.
     fun u256_from_bytes(g: &mut RandomGenerator, num_of_bytes: u8): u256 {
         if (vector::length(&g.buffer) < (num_of_bytes as u64)) {
             fill_buffer(g);
@@ -236,15 +240,20 @@ module sui::random {
     }
 
     // Helper function to generate a random u128 in [min, max] using a random number with num_of_bytes bytes.
-    // Assumes that the caller verified the inputs, and uses num_of_bytes to control the bias.
+    // Assumes that the caller verified the inputs, and uses num_of_bytes to control the bias (e.g., 8 bytes larger
+    // than the actual type used by the caller function to limit the bias by 2^{-64}).
+    // TODO: Replace with a macro when we have support for it.
     fun u128_in_range(g: &mut RandomGenerator, min: u128, max: u128, num_of_bytes: u8): u128 {
         assert!(min <= max, EInvalidRange);
         if (min == max) {
             return min
         };
-        let diff = ((max - min) as u256) + 1;
+        // Pick a random number in [0, max - min] by generating a random number that is larger than max-min, and taking
+        // the modulo of the random number by the range size. Then add the min to the result to get a number in
+        // [min, max].
+        let range_size = ((max - min) as u256) + 1;
         let rand = u256_from_bytes(g, num_of_bytes);
-        min + ((rand % diff) as u128)
+        min + ((rand % range_size) as u128)
     }
 
     /// Generate a random u128 in [min, max] (with a bias of 2^{-64}).
@@ -274,13 +283,16 @@ module sui::random {
 
     /// Shuffle a vector using the random generator (Fisher–Yates/Knuth shuffle).
     public fun shuffle<T>(g: &mut RandomGenerator, v: &mut vector<T>) {
-        let n = (vector::length(v) as u32);
+        let n = vector::length(v);
         if (n == 0) {
             return
         };
-        let i: u32 = 0;
-        while (i < (n - 1)) {
-            let j = generate_u32_in_range(g, i, n - 1);
+        assert!(n <= U16_MAX, EInvalidLength);
+        let n = (n as u16);
+        let i: u16 = 0;
+        let end = n - 1;
+        while (i < end) {
+            let j = generate_u16_in_range(g, i, end);
             vector::swap(v, (i as u64), (j as u64));
             i = i + 1;
         };