Merge branch 'main' into release-v0.5.0

docs/STYLE_GUIDE.md

@@ -3,22 +3,21 @@
 As a basis, the
 [Substrate Style Guide](https://docs.substrate.io/build/troubleshoot-your-code/)
 should be taken into account. In addition to that, the following sections
-further elaborate the style guide used in this repository.
+further elaborate on the style guide used in this repository.
 
 ## Comments
 
 - Comments **must** be wrapped at 100 chars per line.
-- Comments **must** be formulated in markdown.
 
-## Doc comments
+## Comments and Docstrings
 
 - Documentation is written using Markdown syntax.
 - Function documentation should be kept lean and mean. Try to avoid documenting
   the parameters, and instead choose self-documenting parameter names. If
   parameters interact in a complex manner (for example, if two arguments of type
   `Vec<T>` must have the same length), then add a paragraph explaining this.
-- Begin every docstring with a meaningful one sentence description of the
-  function in third person.
+- Begin every docstring with a meaningful one-sentence description of the
+  function in the third person.
 - Avoid WET documentation such as this:
 
   ```rust
@@ -45,8 +44,9 @@ further elaborate the style guide used in this repository.
   (to make understanding the benchmarks easier).
 - Docstrings for dispatchables need not document the `origin` parameter, but
   should specify what origins the dispatchable may be called by.
-- Docstrings for dispatchables **must** include the events that the dispatchable
-  emits.
+- Docstrings for dispatchables **must** include the high-level events that the
+  dispatchable emits and state under which conditions these events are emitted.
+- Document side-effects of functions.
 - Use `#![doc = include_str!("../README.md")]`.
 - Detail non-trivial algorithms in comments inside the function.
 
@@ -58,6 +58,10 @@ An example of a good docstring would be this:
 ///
 /// May only be (successfully) called by `RejectOrigin`. The fraction of the advisory bond that is
 /// slashed is determined by `AdvisoryBondSlashPercentage`.
+///
+/// # Emits
+///
+/// - `MarketRejected` on success.
 pub fn reject_market(
     origin: OriginFor<T>,
     #[pallet::compact] market_id: MarketIdOf<T>,
@@ -76,32 +80,29 @@ duplicating documentation.
 - Format code contained in macro invocations (`impl_benchmarks!`,
   `decl_runtime_apis!`, homebrew macros in `runtime/`, etc.) and attributes
   (`#[pallet::weight(...)`, etc.) manually.
-- Add trailing commas in macro invocations manually, as rustfmt won't add them
-  automatically.
-
-  ```rust
-  ensure!(
-      a_very_very_very_very_very_very_very_long_variable,
-      b_very_very_very_very_very_very_very_long_variable, // This comma is not ensured by rustfmt.
-  )
-  ```
 
 ## Code Style
 
 - Never use panickers.
-- Prefer double turbofish `Vec::<T>::new()` over single turbofish
-  `<Vec<T>>::new()`.
 - All branches of match expressions **should** be explicit. Avoid using the
   catch-all `_ =>`.
-- When changing enums, maintain the existing order and add variants only at the
-  end of the enum to prevent messing up indices.
-- Maintain lexicographical ordering of traits in `#[derive(...)]` attributes.
+- When removing variants from enums that are used in storage or emitted, then
+  explicitly state the scale index of each variant:
+  ```rust
+  enum E {
+      #[codec(index = 0)]
+      V1,
+      #[codec(index = 1)]
+      V2,
+      /// --- snip! ---
+  }
+  ```
 
 ## Crate and Pallet Structure
 
-- Don't dump all code into `lib.rs`. Split code multiple files (`types.rs`,
+- Don't dump all code into `lib.rs`. Split code into multiple files (`types.rs`,
   `traits.rs`, etc.) or even modules (`types/`, `traits/`, etc.).
-- Changes to pallets **must** retain order of dispatchables.
+- Changes to pallets **must** retain the order of dispatchables.
 - Sort pallet contents in the following order:
   - `Config` trait
   - Type values
@@ -113,4 +114,45 @@ duplicating documentation.
   - Hooks
   - Dispatchables
   - Pallet's public and private functions
-  - Trait impelmentations
+  - Trait implementations
+
+## Code Style and Design
+
+- Exceed 70 lines of code per function only in exceptional circumstances. Aim
+  for less.
+- No `while` in production. All `for` loops must have a maximum number of
+  passes.
+- Use depth checks when using recursion in production. Use recursion only if the
+  algorithm is defined using recursion.
+- Avoid `mut` in production code if possible without much pain.
+- Mark all extrinsics `transactional`, even if they satisfy
+  the verify-first/write-later principle.
+- Avoid indentation over five levels; never go over seven levels.
+- All public functions must be documented. Documentation of `pub(crate)` and
+  private functions is optional but encouraged.
+- Keep modules lean. Only exceed 1,000 lines of code per file in exceptional
+  circumstances. Aim for less (except in `lib.rs`). Consider splitting modules
+  into separate files. Auto-generated files are excluded.
+
+## Workflow
+
+- Merges require one review. Additional reviews may be requested.
+- Every merge into a feature branch requires a review.
+- Aim for at most 500 LOC added per PR. Only exceed 1,000 LOC lines added in a
+  PR in exceptional circumstances. Plan ahead and break a large PR into smaller
+  PRs targeting a feature branch. Feature branches are exempt from this rule.
+- Reviews take priority over most other tasks.
+- Reviewing a PR should not take longer than two business days. Aim for shorter
+  PRs if the changes are complex.
+- A PR should not be in flight (going from first `s:ready-for-review` to
+  `s:accepted`) for longer than two weeks. Aim for shorter PRs if the changes
+  are complex.
+
+## Testing
+
+- Aim for 100% code coverage, excluding only logic errors that are raised on
+  inconsistent state. In other words: All execution paths **should** be tested.
+  There should be a clear justification for every LOC without test coverage.
+- For larger modules, use one test file per extrinsic for unit tests. Make unit
+  tests as decoupled as possible from other modules. Place end-to-end and
+  integration tests in extra files.

docs/review_checklist.md

@@ -53,6 +53,7 @@
     - [ ] The try-runtime passes without any warnings (substrate storage
           operations often just log a warning instead of failing, so these
           warnings usually point to problem which could break the storage).
+- [ ] Ensure that the [STYLE_GUIDE] is observed.
 
 ## Events
 
@@ -85,3 +86,4 @@ Additional info (similar to the remark emitted by
 anywhere in the chain storage.
 
 [docs.zeitgeist.pm]: docs.zeitgeist.pm
+[STYLE_GUIDE]: ./STYLE_GUIDE.md

runtime/battery-station/src/lib.rs

@@ -58,9 +58,9 @@ use zrml_prediction_markets::Call::{
 };
 use zrml_rikiddo::types::{EmaMarketVolume, FeeSigmoid, RikiddoSigmoidMV};
 use zrml_swaps::Call::{
-    pool_exit, pool_exit_with_exact_asset_amount, pool_exit_with_exact_pool_amount, pool_join,
-    pool_join_with_exact_asset_amount, pool_join_with_exact_pool_amount, swap_exact_amount_in,
-    swap_exact_amount_out,
+    force_pool_exit, pool_exit, pool_exit_with_exact_asset_amount,
+    pool_exit_with_exact_pool_amount, pool_join, pool_join_with_exact_asset_amount,
+    pool_join_with_exact_pool_amount, swap_exact_amount_in, swap_exact_amount_out,
 };
 #[cfg(feature = "parachain")]
 use {
@@ -164,7 +164,6 @@ impl Contains<RuntimeCall> for ContractsCallfilter {
 #[derive(scale_info::TypeInfo)]
 pub struct IsCallable;
 
-// Currently disables Rikiddo.
 impl Contains<RuntimeCall> for IsCallable {
     fn contains(call: &RuntimeCall) -> bool {
         #[allow(clippy::match_like_matches_macro)]
@@ -182,6 +181,10 @@ impl Contains<RuntimeCall> for IsCallable {
                 } => false,
                 _ => true,
             },
+            RuntimeCall::Swaps(inner_call) => match inner_call {
+                force_pool_exit { .. } => true,
+                _ => false,
+            },
             _ => true,
         }
     }

runtime/zeitgeist/src/lib.rs

@@ -123,6 +123,7 @@ impl Contains<RuntimeCall> for IsCallable {
         use zrml_prediction_markets::Call::{
             admin_move_market_to_closed, admin_move_market_to_resolved, create_market, edit_market,
         };
+        use zrml_swaps::Call::force_pool_exit;
 
         #[allow(clippy::match_like_matches_macro)]
         match runtime_call {
@@ -170,6 +171,10 @@ impl Contains<RuntimeCall> for IsCallable {
                 }
             }
             RuntimeCall::SimpleDisputes(_) => false,
+            RuntimeCall::Swaps(inner_call) => match inner_call {
+                force_pool_exit { .. } => true,
+                _ => false,
+            },
             RuntimeCall::System(inner_call) => {
                 match inner_call {
                     // Some "waste" storage will never impact proper operation.

scripts/benchmarks/configuration.sh

@@ -30,8 +30,8 @@ export ZEITGEIST_PALLETS=(
     zrml_authorized zrml_court zrml_global_disputes zrml_liquidity_mining zrml_neo_swaps \
     zrml_orderbook zrml_parimutuel zrml_prediction_markets zrml_swaps zrml_styx \
 )
-export ZEITGEIST_PALLETS_RUNS="${ZEITGEIST_PALLETS_RUNS:-1000}"
-export ZEITGEIST_PALLETS_STEPS="${ZEITGEIST_PALLETS_STEPS:-10}"
+export ZEITGEIST_PALLETS_RUNS="${ZEITGEIST_PALLETS_RUNS:-20}"
+export ZEITGEIST_PALLETS_STEPS="${ZEITGEIST_PALLETS_STEPS:-50}"
 export ZEITGEIST_WEIGHT_TEMPLATE="./misc/weight_template.hbs"
 
 export PROFILE="${PROFILE:-production}"

zrml/court/src/benchmarks.rs

@@ -27,7 +27,7 @@ use crate::{
     types::{CourtParticipantInfo, CourtPoolItem, CourtStatus, Draw, Vote},
     AppealInfo, BalanceOf, Call, Config, CourtId, CourtPool, Courts, DelegatedStakesOf,
     MarketIdToCourtId, MarketOf, Pallet as Court, Pallet, Participants, RequestBlock,
-    SelectedDraws, VoteItem,
+    SelectedDraws, VoteItem, YearlyInflation,
 };
 use alloc::{vec, vec::Vec};
 use frame_benchmarking::{account, benchmarks, whitelisted_caller};
@@ -672,6 +672,7 @@ benchmarks! {
 
         <frame_system::Pallet<T>>::set_block_number(T::InflationPeriod::get());
         let now = <frame_system::Pallet<T>>::block_number();
+        YearlyInflation::<T>::put(Perbill::from_percent(2));
     }: {
         Court::<T>::handle_inflation(now);
     }

zrml/swaps/src/lib.rs

@@ -129,36 +129,7 @@ mod pallet {
             min_assets_out: Vec<BalanceOf<T>>,
         ) -> DispatchResult {
             let who = ensure_signed(origin)?;
-            ensure!(pool_amount != Zero::zero(), Error::<T>::ZeroAmount);
-            let who_clone = who.clone();
-            let pool = Self::pool_by_id(pool_id)?;
-            // If the pool is still in use, prevent a pool drain.
-            Self::ensure_minimum_liquidity_shares(pool_id, &pool, pool_amount)?;
-            let pool_account_id = Pallet::<T>::pool_account_id(&pool_id);
-            let params = PoolParams {
-                asset_bounds: min_assets_out,
-                event: |evt| Self::deposit_event(Event::PoolExit(evt)),
-                pool_account_id: &pool_account_id,
-                pool_amount,
-                pool_id,
-                pool: &pool,
-                transfer_asset: |amount, amount_bound, asset| {
-                    Self::ensure_minimum_balance(pool_id, &pool, asset, amount)?;
-                    ensure!(amount >= amount_bound, Error::<T>::LimitOut);
-                    T::AssetManager::transfer(asset, &pool_account_id, &who, amount)?;
-                    Ok(())
-                },
-                transfer_pool: || {
-                    Self::burn_pool_shares(pool_id, &who, pool_amount)?;
-                    Ok(())
-                },
-                fee: |amount: BalanceOf<T>| {
-                    let exit_fee_amount = amount.bmul(Self::calc_exit_fee(&pool))?;
-                    Ok(exit_fee_amount)
-                },
-                who: who_clone,
-            };
-            crate::utils::pool::<_, _, _, _, T>(params)
+            Self::do_pool_exit(who, pool_id, pool_amount, min_assets_out)
         }
 
         /// Pool - Exit with exact pool amount
@@ -512,6 +483,22 @@ mod pallet {
             )?;
             Ok(Some(weight).into())
         }
+
+        #[pallet::call_index(11)]
+        #[pallet::weight(T::WeightInfo::pool_exit(
+            min_assets_out.len().min(T::MaxAssets::get().into()) as u32
+        ))]
+        #[transactional]
+        pub fn force_pool_exit(
+            origin: OriginFor<T>,
+            who: T::AccountId,
+            #[pallet::compact] pool_id: PoolId,
+            #[pallet::compact] pool_amount: BalanceOf<T>,
+            min_assets_out: Vec<BalanceOf<T>>,
+        ) -> DispatchResult {
+            let _ = ensure_signed(origin)?;
+            Self::do_pool_exit(who, pool_id, pool_amount, min_assets_out)
+        }
     }
 
     #[pallet::config]
@@ -747,6 +734,44 @@ mod pallet {
     pub(crate) type NextPoolId<T> = StorageValue<_, PoolId, ValueQuery>;
 
     impl<T: Config> Pallet<T> {
+        fn do_pool_exit(
+            who: T::AccountId,
+            pool_id: PoolId,
+            pool_amount: BalanceOf<T>,
+            min_assets_out: Vec<BalanceOf<T>>,
+        ) -> DispatchResult {
+            ensure!(pool_amount != Zero::zero(), Error::<T>::ZeroAmount);
+            let who_clone = who.clone();
+            let pool = Self::pool_by_id(pool_id)?;
+            // If the pool is still in use, prevent a pool drain.
+            Self::ensure_minimum_liquidity_shares(pool_id, &pool, pool_amount)?;
+            let pool_account_id = Pallet::<T>::pool_account_id(&pool_id);
+            let params = PoolParams {
+                asset_bounds: min_assets_out,
+                event: |evt| Self::deposit_event(Event::PoolExit(evt)),
+                pool_account_id: &pool_account_id,
+                pool_amount,
+                pool_id,
+                pool: &pool,
+                transfer_asset: |amount, amount_bound, asset| {
+                    Self::ensure_minimum_balance(pool_id, &pool, asset, amount)?;
+                    ensure!(amount >= amount_bound, Error::<T>::LimitOut);
+                    T::AssetManager::transfer(asset, &pool_account_id, &who, amount)?;
+                    Ok(())
+                },
+                transfer_pool: || {
+                    Self::burn_pool_shares(pool_id, &who, pool_amount)?;
+                    Ok(())
+                },
+                fee: |amount: BalanceOf<T>| {
+                    let exit_fee_amount = amount.bmul(Self::calc_exit_fee(&pool))?;
+                    Ok(exit_fee_amount)
+                },
+                who: who_clone,
+            };
+            crate::utils::pool::<_, _, _, _, T>(params)
+        }
+
         pub fn get_spot_price(
             pool_id: &PoolId,
             asset_in: &AssetOf<T>,