Addressing reviewer comments

Author: wrwg

apps/nextra/pages/en/build/smart-contracts/book/functions.mdx

@@ -773,10 +773,10 @@ struct S has key {
 
 ### Operations on Functions
 
-A function is evaluated by providing the corresponding number of parameters, similar as when calling a named function. When a function is evaluated, the function value is *consumed*. Hence if the value needs to be evaluated multiple times, it's type must have the `copy` ability:
+A function value is evaluated by providing the corresponding number of parameters, similar as when calling a named function. During evaluation, the function value is *consumed*. Hence if the value needs to be evaluated multiple times, it's type must have the `copy` ability:
 
 ```move
-let f: |u64|bool has copy = ...;
+let f: |u64|bool has copy = |x| x > 0;
 assert!(f(1) == f(2))
 ```
 
@@ -788,21 +788,14 @@ Function values support equality and ordering. Note that those relations are bas
 Function types, specifically if they come together with abilities, can be verbose, and if the same type of function type is used many times in the code, repetitive. For this purpose, Move recognizes struct wrappers around function types as a special case. They can be used to effectively create named function types:
 
 ```move
-struct Predicate<T>(|T|bool) has copy;
+struct Predicate<T>(|&T|bool) has copy;
 ```
 
 Move supports this feature by automatically converting function values into the wrapper type and vice versa. Examples:
 
 ```move
-fun is_true(op: Predicate<u64>, param: u64) {
-    // Automatically converts into wrapped function and evaluates
-    op(param)
-}
-
-fun is_true_on_value(f: |u64|bool has copy): bool {
-    // Automatically converts into function wrapper
-    is_true(f, 42)
-}
+let f: Predicate<u64> = |x| *x > 0; // lambda converts to Predicate
+assert!(f(&22)) // Predicate callable
 ```
 
 ### Denoting Function Values
@@ -817,16 +810,19 @@ let f: |u64|bool has copy+drop+store = is_even;
 let g: |u64|bool has copy+drop = is_odd;
 ```
 
-A public function is required to build a storable function value because it needs to be guaranteed that the underlying function persists so the function value can be safely restored from storage at any point in the future. However, code upgrade may change the underlying implementation of the function, while its signature is persistent.
+A _persistent_ function is required to build a storable function value because it needs to be guaranteed that the underlying function exists and can be safely restored from storage at any point in the future. However, code upgrade may change the underlying implementation of the function, while its signature is persistent.
 
-If a function should storable which is non-public, an attribute can be used to mark this function as persistent, with the same upgrade behavior than public functions. This avoids exposing such a function outside of a package which can be a security risk:
+While `public` and `entry` functions are persistent by default, a none-public function needs to be marked with the attribute `#[persistent]` to become storable:
 
 ```move
 #[persistent] fun is_odd(x: u64): bool { x % 2 == 1 }
 ...
 let g: |u64|bool has copy+drop+store = is_odd;
 ```
 
+Using the `#[persistent]` attribute is preferred if the only objective is to make a function storable, avoiding security implications with public or entry visibility.
+
+
 ### Lambda Expressions and Closures
 
 Function values can be denoted by _lambda expressions_ (as also available as parameters for [inline functions](#function-parameters-and-lambda-expressions)). Lambda expressions can capture context variables _by value_: those values are moved (or copied) into a _closure_, from where they are produced when the function is evaluated. Examples:
@@ -839,36 +835,48 @@ let add = |y| { let S(x) = s; x + y }; // s will be moved into the closure
 assert!(add(2) == 3)
 ```
 
-Notice it is not possible to capture reference values at this point of time in Move. Similarly, it is not possible to mutate any locals in the context of a lambda. Specifically, the following pattern as known from lambdas in inline functions, is not supported:
+Closures with captured values are lexicographical ordered using first the name of the underlying function (which maybe generated from lambda lifting), and then the captured values.
+
+The type of the closure constructed by a lambda expression is inferred from the expression (for example, the type of `add` in the example above is inferred as `|u64|u64`). The abilities of this function type are derived as follows. By default, the function underlying a closure is a private function, so the function itself is `copy+drop` (and not `store`). This is intersected with the abilities of all the captured context variables. However, there is a special case for lambdas where instead of a private function an underlying persistent function can be identified, such that the lambda just 'delays' certain arguments of this function. This pattern is also called 'currying' in functional programming (named after the mathematician Curry). Here are some examples:
+
+```move
+#[persistent] fun add(x: u64, y: u64) { x + y }
+...
+let x = 22;
+let f: |u64|u64 has copy+drop+store = |y| add(x, y);  // 1st argument captured, 2nd argument delayed
+let f: |u64|u64 has copy+drop+store = |y| add(y, x);  // 1st argument delayed, 2nd argument captured
+```
+
+Notice it is not possible to _capture_ reference values at this point of time in Move. Thus, the following code does not compile:
+
+```move
+let x = &22;
+let f = |y| add(*x, y) // DOES NOT COMPILE
+```
+
+Related, it is not possible to mutate any locals in the context of a lambda. Specifically, the following pattern as known from lambdas with inline functions, is not supported:
 
 ```move
 let x = 0;
 collection.for_each(|e| x += e) // DOES NOT COMPILE
 ```
 
-The type of the closure constructed by a lambda expression is inferred from the expression in the usual way (for example, the type of `add` in the example above is inferred as `|u64|u64`). The abilities of this function type are derived as follows. By default, the function underlying a closure is a private function, so the function itself is `copy+drop` (and not `store`). This is intersected with the abilities of all the captured context variables.
-
-A closure can have the `store` ability under the condition that the underlying function is persistent. This is the case if the lambda denoting the closure is based on solely delaying some of the parameters of an underlying persistent function. Example:
+However, the actual parameters of lambdas can be references, only captured values are restricted. For example:
 
 ```move
-#[persistent] fun add(x: u64, y: u64) { x + y }
-...
 let x = 22;
-let f: |u64|u64 has copy+drop+store = |y| add(x, y)
-let f: |u64|u64 has copy+drop+store = |y| add(y, x)
+let f : |&u64|u64 = |y| add(x, *y)
 ```
 
-For reference, this mechanism is also called 'currying' of `add` in functional programming languages.
-
 ### Reentrancy Check
 
-Via dynamic dispatch of function values, reentrancy of modules in a chain of function calls is possible. If module `m1`  uses module `m2`, and `m1` calls `m2::f` passing a function value to it, this function value can callback into `m1`. This situation is called _reentrancy_, and is not possible in Move without function values, since the module usage relation is acyclic.
+Via dynamic dispatch of function values, reentrancy of modules in a chain of function calls is possible. If module `m1` uses module `m2`, and `m1` calls `m2::f` passing a function value to it, this function value can callback into `m1`. This situation is called _reentrancy_, and is not possible in Move without function values, since the module usage relation is acyclic.
 
-The Move VM dynamically detects reentrancy for a module, and _locks_ all resources declared in this module from being accessed. Thus during reentrancy, calling the functions `borrow_global`, `borrow_global_mut`, and `move_from` lead to an abort. Here is an example:
+The Move VM dynamically detects reentrancy of a module, and _locks_ all resources declared in this module from being accessed. Thus during reentrancy of `m`, calling resource operations like `&m::R[addr]`, `&mut m::R[addr]`, and `move_from<m::R>` lead to an abort. Here is an example:
 
 ```move
-module caller {
-  use addr::callee;
+module 0x42::caller {
+  use 0x42::callee;
   struct R{ count: u64 } has key;
   fun calling() acquires R {
      let r = &mut R[@addr];
@@ -881,7 +889,7 @@ module caller {
   fun do_something(r: &mut R) { .. }
 }
 
-module callee {
+module 0x42::callee {
   fun call_me<T(x: &mut T, action: |&mut T|) {
     action(x)
   }
@@ -890,16 +898,23 @@ module callee {
 
 Notice that dispatching a function value to a concrete function in the same module is also considered to be reentrancy. If the function `callee::call_me` would be moved into the module `caller`, the same semantics is in effect.
 
-The default reentrancy check ensures consistency of Move's reference semantics and suppresses side-effects for the resources in a reentered module. But it does not protect from side-effects on other state of the app. For this purposes, the `#[module_lock]` attribute can be attached to a function. In a calling context where this attribute is active, reentrancy will be fully suppressed, causing an abort at the moment a module is reentered. In the example below, the state effected is owned by another module `account` instead of the module `caller` for which the can potentially happen via the `notify` callback:
+The default reentrancy check ensures consistency of Move's reference semantics and suppresses side effects of reentrancy for the resources owned by the re-entered module. However, re-entered code is allowed to still access resource state managed by modules outside the reentrancy path. Such state accesses can be considered bad design, but they exist. 
+For these purposes, the `#[module_lock]` attribute can be attached to a function:
 
 ```move
-module account { ... }
-module caller {
-  #[module_lock] // without this lock, the notify call could withdraw unlimited amount
+module 0x42::account { ... }
+module 0x42::caller {
+  #[module_lock] // without this lock, the notify call could withdraw more than intended.
   fun transfer(from: address, to: address, amount: u64, notify: |u64|) {
+    // Oops. This should be really differently designed, using `Coin` type and moving it.
+    assert!(account::balance(from) - MIN_BALANCE >= amount);
     account::deposit(to, amount)
-    notify(amount); // attempt to reentrer `transfer` is blocked
+    notify(amount); // attempt to re-entrer `transfer` is blocked
     account::withtdraw(from, amout);
   }
 }
 ```
+
+While a function with this attribute is running, all calls reentering any module will lead to an abort, given a stronger protection. 
+
+The attribute `#[module_lock]` restriction is not the default behavior since it is too strong for typical patterns of higher-order programming. For example, `collection.find(|x| cond(x))` will lead to a reentrancy of the module which contains this expression, from the module which defines the collection type. 

apps/nextra/pages/en/build/smart-contracts/book/move-2.mdx

@@ -6,7 +6,7 @@ The Move 2 language releases are described on this page. The reference documenta
 
 The Move 2.2 language release adds the following features to Move:
 
-- **Acquires Optional**: the `acquires` annotation on function declarations can be omitted, to be inferred by the compiler
+- **Optional Acquires**: The `acquires` annotation on function declarations can be omitted, to be inferred by the compiler.
 - **Function Values**: Move now supports function values, which can be passed around as parameters and stored in resources. See the [reference doc here](functions.mdx#function-values).
 ## Move 2.1