Update documentation of Random (#19609)

## Description 

Describe the changes or additions included in this PR.

## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

---------

Co-authored-by: ronny-mysten <118224482+ronny-mysten@users.noreply.github.com>

crates/sui-framework/docs/sui-framework/random.md

@@ -404,6 +404,11 @@ transaction.
 
 Create a generator. Can be used to derive up to MAX_U16 * 32 random bytes.
 
+Using randomness can be error-prone if you don't observe the subtleties in its correct use, for example, randomness
+dependent code might be exploitable to attacks that carefully set the gas budget
+in a way that breaks security. For more information, see:
+https://docs.sui.io/guides/developer/advanced/randomness-onchain
+
 
 <pre><code><b>public</b> <b>fun</b> <a href="../sui-framework/random.md#0x2_random_new_generator">new_generator</a>(r: &<a href="../sui-framework/random.md#0x2_random_Random">random::Random</a>, ctx: &<b>mut</b> <a href="../sui-framework/tx_context.md#0x2_tx_context_TxContext">tx_context::TxContext</a>): <a href="../sui-framework/random.md#0x2_random_RandomGenerator">random::RandomGenerator</a>
 </code></pre>

crates/sui-framework/packages/sui-framework/sources/random.move

@@ -135,6 +135,11 @@ public struct RandomGenerator has drop {
 }
 
 /// Create a generator. Can be used to derive up to MAX_U16 * 32 random bytes.
+///
+/// Using randomness can be error-prone if you don't observe the subtleties in its correct use, for example, randomness
+/// dependent code might be exploitable to attacks that carefully set the gas budget
+/// in a way that breaks security. For more information, see:
+/// https://docs.sui.io/guides/developer/advanced/randomness-onchain
 public fun new_generator(r: &Random, ctx: &mut TxContext): RandomGenerator {
  let inner = load_inner(r);
  let seed = hmac_sha3_256(

docs/content/guides/developer/advanced/randomness-onchain.mdx

@@ -20,76 +20,149 @@ Although `Random` is a shared object, it is inaccessible for mutable operations,
 
 :::
 
-Having access to random numbers is only one part of designing secure applications, you should also pay careful attention to how you use that randomness. To securely access randomness:
+Having access to random numbers is only one part of designing secure applications, you should also pay careful attention to how you use that randomness.
+To securely access randomness:
 
 - Define your function as (private) `entry`.
 - Prefer generating randomness using function-local `RandomGenerator`.
 - Make sure that the "unhappy path" of your function does not charge more gas than the "happy path".
 
+## Limited resources and `Random` dependent flows
+
+Be aware that some resources that are available to transactions are limited.
+If you are not careful, an attacker can break or exploit your application by deliberately controlling the point where your function runs out of resources.
+
+Concretely, gas is such a resource.
+Consider the following vulnerable code:
+
+```move
+// Insecure implementation, do not use!
+entry fun insecure_play(r: &Random, payment: Coin<SUI>, ...) {
+ ...
+ let mut generator = new_generator(r, ctx);
+ let win = generator.generate_bool();
+ if (win) { // happy flow
+ ... cheap computation ...
+ } else {
+ ... very expensive computation ...
+ }
+}
+```
+
+Observe that the gas costs of a transaction that calls `insecure_play` depends on the value of `win`.
+An attacker could call this function with a gas budget that is sufficient for the "happy flow" but not the "unhappy one", resulting in it either winning or reverting the transaction (but never losing the payment).
+
+:::warning
+
+The `Random` API does not automatically prevent this kind of attack, and you must be aware of this subtlety when designing your contracts.
+
+:::
+
+
+Other limited resources per transaction that you should consider are:
+
+- The number of new objects.
+- The number of objects that can be used (including dynamic fields).
+- Number of events emitted.
+- Number of UIDs generated, or deleted, or transferred.
+
+
+
+For many use cases this attack is not an issue, like when selecting a raffle winner, or lottery numbers, as the code running is independent of the randomness.
+However, in the cases where it can be problematic, you can consider one of the following:
+
+- Use two steps:
+Split the logic to two functions that must be called by different transactions.
+The first function, called by transaction `tx1`, fetches a random value and stores it in an object that is unreadable by other commands in `tx1` (for example, by transferring the object to the caller, or, by storing the tx digest and checking it is different on read).
+A second function, called by transaction `tx2`, reads the stored value and completes the operation.
+`tx2` might indeed fail, but now the random value is fixed and cannot be modified using repeated calls.
+It is important that the inputs to the second function are fixed and cannot be modified after `tx1` (otherwise an attacker can modify them after seeing the randomness committed by `tx1`).
+Also, it is important to gracefully handle the case in which the second step is never completed (for example, charge a fee in the first step).
+See [this](https://github.com/MystenLabs/sui/blob/main/examples/move/random/random_nft/sources/example.move#L117-L142) for example implementation.
+- Write the function in a way that the happy flow consumes more gas than the unhappy one.
+ - Keep in mind that external functions or native ones can change in the future, potentially resulting in different costs compared to the time you conducted your tests.
+ - [profile-transaction](../../../references/cli/client.mdx#profile-a-transaction) can be used to profile the costs of a transaction.
+
+
+
 ## Use (non-public) `entry` functions
 
-While composition is very powerful for smart contracts, it opens the door to attacks on functions that use randomness. Consider for example the next module:
+While composition is very powerful for smart contracts, it opens the door to attacks on functions that use randomness.
+Consider for example a betting game that uses randomness for rolling dice:
 
 ```move
 module games::dice {
-...
- struct GuessedCorrectly has drop { ... };
+ ...
+ public enum Ticket has drop {
+ Lost,
+ Won,
+ }
+
+ public fun is_winner(t: &Ticket): bool {
+ match (t) {
+ Ticket::Won => true,
+ Ticket::Lost => false,
+ }
+ }
 
- /// If you guess correctly the output you get a GuessedCorrectly object.
- public fun play_dice(guess: u8, fee: Coin<SUI>, r: &Random, ctx: &mut TxContext): Option<GuessedCorrectly> {
+ /// If you guess correctly the output, then you get a GuessedCorrectly object.
+ /// Otherwise you get nothing.
+ public fun play_dice(guess: u8, fee: Coin<SUI>, r: &Random, ctx: &mut TxContext): Ticket {
  // Pay for the turn
  assert!(coin::value(&fee) == 1000000, EInvalidAmount);
  transfer::public_transfer(fee, CREATOR_ADDRESS);
+
  // Roll the dice
  let mut generator = new_generator(r, ctx);
- if (guess == random::generate_u8_in_range(&mut generator, 1, 6)) {
- option::some(GuessedCorrectly {})
+ if (guess == generator.generate_u8_in_range(1, 6)) {
+ Ticket::Won
  } else {
- option::none()
+ Ticket::Lost
  }
  }
-...
+ ...
 }
 ```
 
 An attacker can deploy the next function:
 
 ```move
-public fun attack(guess: u8, r: &Random, ctx: &mut TxContext): GuessedCorrectly {
- let output = dice::play_dice(guess, r, ctx);
- option::extract(output) // reverts the transaction if roll_dice returns option::none()
+public fun attack(guess: u8, r: &Random, ctx: &mut TxContext): Ticket {
+ let t = dice::play_dice(guess, r, ctx);
+ // revert the transaction if play_dice lost
+ assert!(!dice::is_winner(&t), 0);
+ t
 }
 ```
 
-The attacker can now call `attack` with a guess, and always revert the fee transfer if the guess is incorrect.
+The attacker can now call `attack` with a guess, and **always** revert the fee transfer if the guess is incorrect.
 
-To protect against composition attacks in this example, define `play_dice` as a private `entry` function so functions from other modules cannot call it, such as,
+To protect against composition attacks, define your function as a private `entry` function so functions from other modules cannot call it.
 
-```move
-entry fun play_dice(guess: u8, fee: Coin<SUI>, r: &Random, ctx: &mut TxContext): Option<GuessedCorrectly> {
- ...
-}
-```
-
-:::note
+:::tip
 
 The Move compiler enforces this behavior by rejecting `public` functions with `Random` as an argument.
 
 :::
 
 ## Programmable transaction block (PTB) restrictions
 
-A similar attack to the one previously described involves PTBs _even_ when `play_dice` is defined as a private `entry` function. For example, consider the `entry play_dice(guess: u8, fee: Coin<SUI>, r: &Random, ctx: &mut TxContext): Option<GuessedCorrectly> { … }` function defined earlier, the attacker can publish the function
+A similar attack to the one previously described involves PTBs _even_ when `play_dice` is defined as a private `entry` function.
+For example, consider the `entry play_dice(guess: u8, fee: Coin<SUI>, r: &Random, ctx: &mut TxContext): Ticket { … }` function defined earlier, the attacker can publish the function
 
 ```move
-public fun attack(output: Option<GuessedCorrectly>): GuessedCorrectly {
- option::extract(output)
+public fun attack(t: Ticket): Ticket {
+ assert!(!dice::is_winner(&t), 0);
+ t
 }
 ```
 
-and send a PTB with commands `play_dice(...), attack(Result(0))` where `Result(0)` is the output of the first command. As before, the attack takes advantage of the atomic nature of PTBs and always reverts the _entire transaction_ if the guess was incorrect, without paying the fee. Sending multiple transactions can repeat the attck, each one executed with different randomness and reverted if the guess is incorrect.
+and send a PTB with commands `play_dice(...), attack(Result(0))` where `Result(0)` is the output of the first command.
+As before, the attack takes advantage of the atomic nature of PTBs and always reverts the _entire transaction_ if the
+guess was incorrect, without paying the fee. Sending multiple transactions can repeat the attack, each one executed with
+different randomness and reverted if the guess is incorrect.
 
-:::note
+:::tip
 
 To protect against PTB-based composition attacks, Sui rejects PTBs that have commands that are not `TransferObjects` or `MergeCoins` following a `MoveCall` command that uses `Random` as an input.
 
@@ -99,46 +172,12 @@ To protect against PTB-based composition attacks, Sui rejects PTBs that have com
 
 `RandomGenerator` is secure as long as it's created by the consuming module. If passed as an argument, the caller might be able to predict the outputs of that `RandomGenerator` instance (for example, by calling `bcs::to_bytes(&generator)` and parsing its internal state).
 
-:::note
+:::tip
 
 The Move compiler enforces this behavior by rejecting `public` functions with `RandomGenerator` as an argument.
 
 :::
 
-## Limited resources and `Random` dependent flows
-
-Developers should be aware that some resources that are available to transactions are limited. If a function that reads `Random` consumes more resources in the unhappy path flow than in the happy path flow, an attacker can use that difference to revert the transaction in the unhappy flow as previously demonstrated. Concretely, gas is such a resource. Consider the following code:
-
-```move
-// Insecure implementation, do not use.
-entry fun insecure_play(r: &Random, payment: Coin<SUI>, ...) {
- ...
- let mut generator = new_generator(r, ctx);
- let win = random::generate_bool(&mut generator);
- if (win) { // happy flow
- ... cheap computation ...
- } else {
- ... very expensive computation ...
- }
-}
-```
-
-Observe that the gas costs of a transaction that calls `insecure_play` depends on the value of `win` - An attacker could call this function with a gas object that has enough balance to cover the happy flow but not the unhappy one, resulting in it either winning or reverting the transaction (but never losing the payment).
-
-In many cases this is not an issue, like when selecting a raffle winner, lottery numbers, or a random NFT. However, in the cases where it can be problematic, you can do one of the following:
-
-- Write the function in a way that the happy flow consumes more gas than the unhappy one.
- - Keep in mind that external functions or native ones can change in the future, potentially resulting in different costs compared to the time you conducted your tests.
- - Use [profile-transaction](../../../references/cli/client.mdx#profile-a-transaction) on Testnet transactions to verify the costs of different flows.
-- Split the logic in two: one function that fetches a random value and stores it in an object, and another function that reads that stored value and completes the operation. The latter function might indeed fail, but now the random value is fixed and cannot be modified using repeated calls.
-
-See [random_nft](https://github.com/MystenLabs/sui/blob/main/examples/move/random/random_nft) for examples.
-
-Other limited resources per transaction are:
-
-- The number of new objects.
-- The number of objects that can be used (including dynamic fields).
-
 ## Accessing `Random` from TypeScript
 
 If you want to call `roll_dice(r: &Random, ctx: &mut TxContext)` in module `example`, use the following code snippet:
@@ -158,6 +197,5 @@ txb.moveCall({
  - <a href="/references/framework/sui-framework/random" data-noBrokenLinkCheck="true">
  random.move
  </a>
-- [Randomness NFT example](https://github.com/MystenLabs/sui/blob/main/examples/move/random/random_nft)
 - [Raffle example](https://github.com/MystenLabs/sui/blob/main/examples/move/random/raffles)
 - [Sui Client CLI](../../../references/cli/client)