From 85740c083b55944e0adf461bc7489e803eacb19a Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 4 Dec 2023 17:08:55 -0500 Subject: [PATCH 01/76] Add to new branch --- developer-hub/Cargo.toml | 5 + .../src/guides/your_first_pallet/mod.rs | 6 +- .../reference_docs/defensive_programming.rs | 568 ++++++++++++++++++ developer-hub/src/reference_docs/mod.rs | 2 +- .../safe_defensive_programming.rs | 1 - 5 files changed, 577 insertions(+), 5 deletions(-) create mode 100644 developer-hub/src/reference_docs/defensive_programming.rs delete mode 100644 developer-hub/src/reference_docs/safe_defensive_programming.rs diff --git a/developer-hub/Cargo.toml b/developer-hub/Cargo.toml index 56f279c7e107..4206271334b2 100644 --- a/developer-hub/Cargo.toml +++ b/developer-hub/Cargo.toml @@ -28,6 +28,10 @@ kitchensink-runtime = { path = "../substrate/bin/node/runtime" } chain-spec-builder = { package = "staging-chain-spec-builder", path = "../substrate/bin/utils/chain-spec-builder" } subkey = { path = "../substrate/bin/utils/subkey" } +# Extra example pallets +pallet-referenda = { path = "../substrate/frame/referenda" } +pallet-broker = { path = "../substrate/frame/broker" } + # Substrate sc-network = { path = "../substrate/client/network" } sc-rpc-api = { path = "../substrate/client/rpc-api" } @@ -57,6 +61,7 @@ sp-api = { path = "../substrate/primitives/api" } sp-core = { path = "../substrate/primitives/core" } sp-keyring = { path = "../substrate/primitives/keyring" } sp-runtime = { path = "../substrate/primitives/runtime" } +sp-arithmetic = { path = "../substrate/primitives/arithmetic" } [dev-dependencies] parity-scale-codec = "3.6.5" diff --git a/developer-hub/src/guides/your_first_pallet/mod.rs b/developer-hub/src/guides/your_first_pallet/mod.rs index 0d2cc26d7576..3c0eef7c2d78 100644 --- a/developer-hub/src/guides/your_first_pallet/mod.rs +++ b/developer-hub/src/guides/your_first_pallet/mod.rs @@ -104,8 +104,8 @@ //! This macro will call `.into()` under the hood. #![doc = docify::embed!("./src/guides/your_first_pallet/mod.rs", transfer_better)] //! -//! Moreover, you will learn in the [Safe Defensive Programming -//! section](crate::reference_docs::safe_defensive_programming) that it is always recommended to use +//! Moreover, you will learn in the [Defensive Programming +//! section](crate::reference_docs::defensive_programming) that it is always recommended to use //! safe arithmetic operations in your runtime. By using [`frame::traits::CheckedSub`], we can not //! only take a step in that direction, but also improve the error handing and make it slightly more //! ergonomic. @@ -288,7 +288,7 @@ //! The following topics where used in this guide, but not covered in depth. It is suggested to //! study them subsequently: //! -//! - [`crate::reference_docs::safe_defensive_programming`]. +//! - [`crate::reference_docs::defensive_programming`]. //! - [`crate::reference_docs::frame_origin`]. //! - [`crate::reference_docs::frame_composite_enums`]. //! - The pallet we wrote in this guide was using `dev_mode`, learn more in diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs new file mode 100644 index 000000000000..7f186b89e714 --- /dev/null +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -0,0 +1,568 @@ +//! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] +//! types, eliminating the possibility of integer overflows, converting between number types, or +//! even handling floating point usage with fixed point arithmetic to mitigate issues that come with +//! floating point calculations. +//! +//! Intentional and predictable design should be our first and foremost +//! property for ensuring a well running, safely designed system. +//! +//! ## Defensive Programming +//! +//! Defensive programming is a design paradigm that enables a particular program to continue +//! running despite unexpected behavior. These unforseen circumstances may +//! cause the program to stop, (or in the Rust context, `panic!`) defensive practices allow for +//! these circumstances to be accounted for ahead of time, and for them to be handled in a more +//! graceful manner. +//! +//! The Polkadot SDK is both built to reflect these principles, and to be used accordingly. +//! +//! ## General Practices +//! +//! When developing within the context of the a runtime, there is one golden rule: +//! +//! ***DO NOT PANIC!*** +//! +//! Most of the time - there are some exceptions, such as critical operations being actually more +//! dangerous than allowing the node to continue running (block authoring, consensus, etc). +//! +//! - Directly using `unwrap()` for a [`Result`] shouldn't be used. +//! - This includes accessing indices of some collection type, which may implicitly `panic!` (i.e., +//! via `get()`) +//! - It may be acceptable to use `except()`, but only if one is completely certain (and has +//! performed a check beforehand) that a value won't panic upon unwrapping. +//! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) +//! - Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a +//! number of issues [(see more later in this document)](#integer-overflow). +//! +//! ### Defensive Traits +//! +//! To also aid in debugging and mitigating the above issues, there is a +//! [`Defensive`](frame::traits::Defensive) trait (and its companions, +//! [`DefensiveOption`](frame::traits::DefensiveOption), +//! [`DefensiveResult`](frame::traits::DefensiveResult)) that can be used to defensively unwrap +//! values. This can be used in place of +//! an `expect`, and again, only if the developer is sure about the unwrap in the first place. +//! +//! The primary difference of defensive implementations bring over vanilla ones is the usage of [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions). +//! `debug_assertions` allows for panics to occur in a testing context, but in +//! production/release, they will merely log an error (i.e., `log::error`). +//! +//! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which +//! provide an alternative to 'vanilla' Rust functions: +//! +//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) +//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) +//! +//! This traits are useful for catching issues in the development environment, without risking +//! panicking in production. +//! +//! ## Integer Overflow +//! +//! The Rust compiler prevents any sort of static overflow from happening at compile time. +//! The compiler panics in **debug** mode in the event of an integer overflow. In +//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion. +//! +//! However in the runtime context, we don't always have control over what is being supplied as a +//! parameter. For example, even this simple adding function could present one of two outcomes +//! depending on whether it is in **release** or **debug** mode: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] +//! +//! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in +//! release). +//! +//! ```ignore +//! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. +//! ``` +//! +//! It is actually the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in +//! the context of blockchain development, where unsafe arithmetic could produce unexpected +//! consequences. +//! +//! A quick example is a user's balance overflowing: the default behavior of wrapping could result +//! in the user's balance starting from zero, or vice versa, of a `0` balance turning into the +//! `MAX`. This could lead to various exploits and issues down the road, which if +//! failing silently, would be difficult to trace and rectify in production. +//! +//! Luckily, there are ways to both represent and handle these scenarios depending on our specific +//! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. +//! +//! ## Infallible Arithmetic +//! +//! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating +//! point arithmetic. +//! +//! A developer should use fixed-point arithmetic to mitigate the potential for inaccuracy, +//! rounding errors, or other unexpected behavior. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). +//! +//! Using **primitive** floating point number types in a blockchain context should be avoided, +//! as a single nondeterministic result could cause chaos for consensus along with the +//! aforementioned issues. +//! +//! The following methods represent different ways one can handle numbers safely natively in Rust, +//! without fear of panic or unexpected behavior from wrapping. +//! +//! ### Checked Arithmetic +//! +//! **Checked operations** utilize an `Option` as a return type. This allows for simple pattern +//! matching to catch any unexpected behavior in the event of an overflow. +//! +//! This is an example of a valid operation: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", checked_add_example)] +//! +//! This is an example of an invalid operation, in this case, a simulated integer overflow, which +//! would simply result in `None`: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + checked_add_handle_error_example +)] +//! +//! Typically, if you aren't sure about which operation to use for runtime math, **checked** +//! operations are a safe bet, as it presents two, predictable (and _erroring_) outcomes that can be +//! handled accordingly (`Some` and `None`). +//! +//! In a practical context, the resulting [`Option`] should be handled accordingly. The following +//! conventions can be seen from the within the Polkadot SDK, where it can be handled in one of +//! two ways: +//! +//! - As an [`Option`], using the `if let` / `if` or `match` +//! - As a [`Result`], via `ok_or` (or similar conversion to [`Result`] from [`Option`]) +//! +//! #### Handling via Option - More Verbose +//! +//! Because wrapped operations return `Option`, you can use a more verbose/explicit form of error +//! handling via `if` or `if let`: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance)] +//! +//! Optionally, `match` may also be directly used in a more concise manner: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + increase_balance_match +)] +//! +//! This is generally a useful convention for handling not only checked types, but most types that +//! return `Option`. +//! +//! #### Handling via Result - Less Verbose +//! +//! In the Polkadot SDK codebase, you may see checked operations being handled as a [`Result`] via +//! `ok_or`. This is a less verbose way of expressing the above. Which to use often boils down to +//! the developer's preference: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + increase_balance_result +)] +//! +//! +//! ### Saturating Operations +//! +//! Saturating a number limits it to the type's upper or lower bound, no matter the integer would +//! overflow in runtime. For example, adding to `u32::MAX` would simply limit itself to `u32::MAX`: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + saturated_add_example +)] +//! +//! Saturating calculations can be used if one is very sure that something won't overflow, but wants +//! to avoid introducing the notion of any potential-panic or wrapping behavior. +//! +//! There is also a series of defensive alternatives via +//! [`DefensiveSaturating`](frame::traits::DefensiveSaturating), which introduces the same behavior +//! of the [`Defensive`](frame::traits::Defensive) trait, only with saturating, mathematical +//! operations: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + saturated_defensive_example +)] +//! +//! ### Mathematical Operations in Substrate Development - Further Context +//! +//! As a recap, we covered the following concepts: +//! +//! 1. **Checked** operations - using [`Option`] or [`Result`], +//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type, +//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type, +//! +//! +//! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of +//! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a +//! `checked_` method in order to introduce safe arithmetic in their code. +//! +//! +//! #### The problem with 'default' wrapped operations +//! +//! **Wrapped operations** cause the overflow to wrap around to either the maximum or minimum of +//! that type. Imagine this in the context of a blockchain, where there are balances, voting +//! counters, nonces for transactions, and other aspects of a blockchain. +//! +//! Some of these mechanisms can be more critical than others. It's for this reason that we may +//! consider some other ways of dealing with runtime arithmetic, such as saturated or checked +//! operations, that won't carry these potential consequences. +//! +//! +//! While it may seem trivial, choosing how to handle numbers is quite important. As a thought +//! exercise, here are some scenarios of which will shed more light on when to use which. +//! +//! #### Bob's Overflowed Balance +//! +//! **Bob's** balance exceeds the `Balance` type on the `EduChain`. Because the pallet developer did +//! not handle the calculation to add to Bob's balance with any regard to this overflow, **Bob's** +//! balance is now essentially `0`, the operation **wrapped**. +//! +//!
+//! Solution: Saturating or Checked +//! For Bob's balance problems, using a `saturated_add` or `checked_add` could've mitigated this +//! issue. They simply would've reached the upper, or lower bounds, of the particular type for an +//! on-chain balance. In other words: Bob's balance would've stayed at the maximum of the Balance +//! type.
+//! +//! #### Alice's 'Underflowed' Balance +//! +//! Alice's balance has reached `0` after a transfer to Bob. Suddenly, she has been slashed on +//! `EduChain`, causing her balance to reach near the limit of `u32::MAX` - a very large amount - as +//! _wrapped operations_ can go both ways. Alice can now successfully vote using her new, +//! overpowered token balance, destroying the integrity of the chain. +//! +//!
+//! Solution: Saturating +//! For Alice's balance problem, using `saturated_sub` could've mitigated this issue. As debt or +//! having a negative balance is not a concept within blockchains, a saturating calculation +//! would've simply limited her balance to the lower bound of u32. +//! +//! In other words: Alice's balance would've stayed at "0", even after being slashed. +//! +//! This is also an example that while one system may work in isolation, shared interfaces, such +//! as the notion of balances, are often shared across multiple pallets - meaning these small +//! changes can make a big difference in outcome.
+//! +//! #### Proposals' ID Overwrite +//! +//! The type for counting the number of proposals on-chain is represented by a `u8` number, called +//! `proposals_count`. Every time a new proposal is added to the system, this number increases. With +//! the proposal pallet being high in usage, it has reached `u8::MAX`'s limit of `255`, causing +//! `proposals_count` to go to `0`. Unfortunately, this resulted in new proposals overwriting old +//! ones, effectively erasing any notion of past proposals! +//! +//!
+//! Solution: Checked +//! For the proposal IDs, proper handling via `checked` math would've been much more suitable, +//! Saturating could've been used - but it also would've 'failed' silently. Using `checked_add` to +//! ensure that the next proposal ID would've been valid would've been a viable way to let the user +//! know the state of their proposal: +//! +//! ```ignore +//! let next_proposal_id = current_count.checked_add(1).ok_or_else(|| Error::TooManyProposals)?; +//! ``` +//! +//!
+//! +//! +//! From the above, we can clearly see the problematic nature of seemingly simple operations in +//! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - +//! such as certain balance being never realistically attainable, or a number type being so large +//! that it could never realistically overflow unless one sent thousands of transactions to the +//! network. +//! +//! ### Decision Chart: When to use which? +#![doc = simple_mermaid::mermaid!("../../../docs/mermaid/integer_operation_decision.mmd")] +//! ## Fixed Point Arithmetic +//! +//! The following code uses types from [`sp_arithmetic`]. +//! +//! Fixed point arithmetic solves the aforementioned problems of dealing with the uncertain +//! nature of floating point numbers. Rather than use a radix point (`0.80`), a type which +//! _represents_ a floating point number in base 10, i.e., a **fixed point number**, can be used +//! instead. +//! +//! For use cases which operate within the range of `[0, 1]` types that implement +//! [`PerThing`](sp_arithmetic::PerThing) are used: +//! - **[`Perbill`](sp_arithmetic::Perbill), parts of a billion** +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", perbill_example)] +//! +//! - **[`Percent`](sp_arithmetic::Percent), parts of a hundred** +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_example)] +//! +//! Note that `190 / 400 = 0.475`, and that `Percent` represents it as a _rounded down_, fixed point +//! number (`47`). Unlike primitive types, types that implement +//! [`PerThing`](sp_arithmetic::PerThing) will also not overflow, and are therefore safe to use. +//! They adopt the same behavior that a saturated calculation would provide, meaning that if one is +//! to go over "100%", it wouldn't overflow, but simply stop at the upper or lower bound. +//! +//! For use cases which require precision beyond the range of `[0, 1]`, there are a number of other +//! fixed-point types to use: +//! +//! - [`FixedU64`](sp_arithmetic::FixedU64) and [`FixedI64`](sp_arithmetic::FixedI64) +//! - [`FixedI128`](sp_arithmetic::FixedU128) and [`FixedU128`](sp_arithmetic::FixedI128) +//! +//! Similar to types that implement [`PerThing`](sp_arithmetic::PerThing), these are also +//! fixed-point types, however, they are able to represent larger numbers: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", fixed_u64)] +//! +//! Let's now explore these types in practice, and how they may be used with pallets to perform +//! safer calculations in the runtime. +//! +//! ### 'PerThing' In Practice +//! +//! [`sp_arithmetic`] contains a trait called [`PerThing`](sp_arithmetic::PerThing), allowing a +//! custom type to be implemented specifically for fixed point arithmetic. While a number of +//! fixed-point types are introduced, let's focus on a few specific examples that implement +//! [`PerThing`](sp_arithmetic::PerThing): +//! +//! - [`Percent`](sp_arithmetic::Percent) - parts of one hundred. +//! - [`Permill`](sp_arithmetic::Permill) - parts of a million. +//! - [`Perbill`](sp_arithmetic::Perbill) - parts of a billion. +//! +//! Each of these can be used to construct and represent ratios within our runtime. +//! You will find types like [`Perbill`](sp_arithmetic::Perbill) being used often in pallet +//! development. [`pallet_referenda`] is a good example of a pallet which makes good use of fixed +//! point arithmetic to calculate. +//! +//! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating +//! point numbers in Substrate development. For this scenario, let's say we are demonstrating a +//! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be +//! deemed valid. +//! +//! For most cases, `Perbill` gives us a reasonable amount of precision for most applications, which +//! is why we're using it here. +//! +//! #### Fixed Point Arithmetic with [`PerThing`](sp_arithmetic::PerThing) +//! +//! As stated, one can also perform mathematics using these types directly. For example, finding the +//! percentage of a particular item: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_mult)] +//! +//! +//! ### Fixed Point Types in Practice +//! +//! As said earlier, if one needs to exceed the value of one, then +//! [`FixedU64`](sp_arithmetic::FixedU64) (and its signed and `u128` counterparts) can be utilized. +//! Take for example this very rudimentary pricing mechanism, where we wish to calculate the demand +//! / supply to get a price for some on-chain compute: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + fixed_u64_block_computation_example +)] +//! +//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`] +//! +//! #### Fixed Point Types in Practice +//! +//! Just as with [`PerThing`](sp_arithmetic::PerThing), you can also perform regular mathematical +//! expressions: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + fixed_u64_operation_example +)] +//! +//! +//! ## Other Resources +//! +//! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) + +#[cfg(test)] +mod tests { + enum BlockchainError { + Overflow, + } + + type Address = (); + + struct Runtime; + + impl Runtime { + fn get_balance(account: Address) -> u64 { + 0 + } + } + + #[docify::export] + #[test] + fn naive_add(x: u8, y: u8) -> u8 { + x + y + } + + #[docify::export] + #[test] + fn checked_add_example() { + // This is valid, as 20 is perfectly within the bounds of u32. + let add = (10u32).checked_add(10); + assert_eq!(add, Some(20)) + } + + #[docify::export] + #[test] + fn checked_add_handle_error_example() { + // This is invalid - we are adding something to the max of u32::MAX, which would overflow. + // Luckily, checked_add just marks this as None! + let add = u32::MAX.checked_add(10); + assert_eq!(add, None) + } + + #[docify::export] + #[test] + fn increase_balance(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + if let Some(new_balance) = balance.checked_add(amount) { + Runtime::set_balance(account, new_balance); + return Ok(()); + } else { + return Err(BlockchainError::Overflow); + } + } + + #[docify::export] + #[test] + fn increase_balance_match(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + let new_balance = match balance.checked_add(amount) { + Some(balance) => balance, + None => { + return Err(BlockchainError::Overflow); + }, + }; + Runtime::set_balance(account, new_balance); + Ok(()) + } + + #[docify::export] + #[test] + fn increase_balance_result(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount - this time, by using `ok_or` + let new_balance = balance.checked_add(amount).ok_or_else(|| BlockchainError::Overflow)?; + Runtime::set_balance(account, new_balance); + Ok(()) + } + + #[docify::export] + #[test] + fn saturated_add_example() { + // Saturating add simply saturates + // to the numeric bound of that type if it overflows. + let add = u32::MAX.saturating_add(10); + assert_eq!(add, u32::MAX) + } + #[docify::export] + #[test] + fn percent_mult() { + let percent = Percent::from_rational(5u32, 100u32); // aka, 5% + let five_percent_of_100 = percent * 100u32; // 5% of 100 is 5. + assert_eq!(five_percent_of_100, 5) + } + #[docify::export] + #[test] + fn perbill_example() { + let p = Perbill::from_percent(80); + // 800000000 bil, or a representative of 0.800000000. + // Precision is in the billions place. + assert_eq!(p.deconstruct(), 800000000); + } + + #[docify::export] + #[test] + fn percent_example() { + let percent = Percent::from_rational(190u32, 400u32); + assert_eq!(percent.deconstruct(), 47); + } + + #[docify::export] + #[test] + fn fixed_u64_block_computation_example() { + // Cores available per block + let supply = 10u128; + // Cores being ordered per block + let demand = 5u128; + // Calculate a very rudimentry on-chain price from supply / demand + let price = FixedU64::from_rational(demand, supply); + + // 0.5 DOT per core + assert_eq!(price, FixedU64::from_float(0.5)); + + // Now, the story has changed - lots of demand means we buy as many cores as there + // available. This also means that price goes up! For the sake of simplicity, we don't care + // about who gets a core - just about our very simple price model + + // Cores available per block + let supply = 10u128; + // Cores being ordered per block + let demand = 19u128; + // Calculate a very rudimentary on-chain price from supply / demand + let price = FixedU64::from_rational(demand, supply); + + // 1.9 DOT per core + assert_eq!(price, FixedU64::from_float(1.9)); + } + + #[docify::export] + #[test] + fn fixed_u64() { + // The difference between this and perthings is perthings operates within the relam of [0, + // 1] In cases where we need > 1, we can used fixed types such as FixedU64 + + let rational_1 = FixedU64::from_rational(10, 5); //" 200%" aka 2. + let rational_2 = + FixedU64::from_rational_with_rounding(5, 10, sp_arithmetic::Rounding::Down); // "50%" aka 0.50... + + assert_eq!(rational_1, (2u64).into()); + assert_eq!(rational_2.into_perbill(), Perbill::from_float(0.5)); + } + + #[docify::export] + #[test] + fn fixed_u64_operation_example() { + let rational_1 = FixedU64::from_rational(10, 5); // "200%" aka 2. + let rational_2 = FixedU64::from_rational(8, 5); // "160%" aka 1.6. + + let addition = rational_1 + rational_2; + let multiplication = rational_1 * rational_2; + let division = rational_1 / rational_2; + let subtraction = rational_1 - rational_2; + + assert_eq!(addition, FixedU64::from_float(3.6)); + assert_eq!(multiplication, FixedU64::from_float(3.2)); + assert_eq!(division, FixedU64::from_float(1.25)); + assert_eq!(subtraction, FixedU64::from_float(0.4)); + } + #[docify::export] + #[test] + fn bad_unwrap() { + let some_result: Result = Ok(10); + assert_eq!(some_result.unwrap(), 10); + } + + #[docify::export] + #[test] + fn good_unwrap() { + let some_result: Result = Err("Error"); + assert_eq!(some_result.unwrap_or_default(), 0); + assert_eq!(some_result.unwrap_or(10), 10); + } + + #[docify::export] + #[test] + fn bad_collection_retrieval() { + let my_list = vec![1, 2, 3, 4, 5]; + // THIS PANICS! + // Indexing on heap allocated values, i.e., vec, can be unsafe! + assert_eq!(my_list[5], 6) + } + + #[docify::export] + #[test] + fn good_collection_retrieval() { + let my_list = vec![1, 2, 3, 4, 5]; + // Rust includes `.get`, returning Option - so lets use that: + assert_eq!(my_list.get(5), None) + } + #[docify::export] + #[test] + #[should_panic(expected = "Defensive failure has been triggered!")] + fn saturated_defensive_example() { + let saturated_defensive = u32::MAX.defensive_saturating_add(10); + assert_eq!(saturated_defensive, u32::MAX); + } +} diff --git a/developer-hub/src/reference_docs/mod.rs b/developer-hub/src/reference_docs/mod.rs index 44284394000d..0fe2bd8b7ea0 100644 --- a/developer-hub/src/reference_docs/mod.rs +++ b/developer-hub/src/reference_docs/mod.rs @@ -49,7 +49,7 @@ pub mod frame_origin; /// Learn about how to write safe and defensive code in your FRAME runtime. // TODO: @CrackTheCode016 https://github.com/paritytech/polkadot-sdk-docs/issues/44 -pub mod safe_defensive_programming; +pub mod defensive_programming; /// Learn about composite enums in FRAME-based runtimes, such as "RuntimeEvent" and "RuntimeCall". pub mod frame_composite_enums; diff --git a/developer-hub/src/reference_docs/safe_defensive_programming.rs b/developer-hub/src/reference_docs/safe_defensive_programming.rs deleted file mode 100644 index 9d0f028e570d..000000000000 --- a/developer-hub/src/reference_docs/safe_defensive_programming.rs +++ /dev/null @@ -1 +0,0 @@ -//! From 9f76b7b9f878e15f4e1d12e2dddd715a71b31f29 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 1 Jan 2024 11:50:01 -0500 Subject: [PATCH 02/76] add decision chart --- Cargo.lock | 3 +++ docs/mermaid/integer_operation_decision.mmd | 7 +++++++ 2 files changed, 10 insertions(+) create mode 100644 docs/mermaid/integer_operation_decision.mmd diff --git a/Cargo.lock b/Cargo.lock index bbaa3d053a30..c0107a70337d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4393,8 +4393,10 @@ dependencies = [ "frame", "kitchensink-runtime", "pallet-aura", + "pallet-broker", "pallet-default-config-example", "pallet-examples", + "pallet-referenda", "pallet-timestamp", "parity-scale-codec", "sc-cli", @@ -4411,6 +4413,7 @@ dependencies = [ "scale-info", "simple-mermaid 0.1.0 (git+https://github.com/kianenigma/simple-mermaid.git?branch=main)", "sp-api", + "sp-arithmetic", "sp-core", "sp-io", "sp-keyring", diff --git a/docs/mermaid/integer_operation_decision.mmd b/docs/mermaid/integer_operation_decision.mmd new file mode 100644 index 000000000000..243a856fb694 --- /dev/null +++ b/docs/mermaid/integer_operation_decision.mmd @@ -0,0 +1,7 @@ +flowchart LR + CH["Checked"] + ST["Saturated"] + CH-->NEED["The user needs to know that the operation failed - and why"] + CH-->DOUBT["Unsure whether this operation could fail/overflow"] + ST-->SILENT["Silently reaching upper/lower bound will not result in any damage"] + ST-->REASON["In all reasonable circumstances, the number will not overflow, but safety is still desired"] From 6d232fceb90585d2c0943aa4b28944ed4ab61a0d Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 1 Jan 2024 12:03:47 -0500 Subject: [PATCH 03/76] add feedback --- .../reference_docs/defensive_programming.rs | 52 ++++++++----------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 7f186b89e714..198f159b9dc0 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -20,13 +20,13 @@ //! //! When developing within the context of the a runtime, there is one golden rule: //! -//! ***DO NOT PANIC!*** -//! -//! Most of the time - there are some exceptions, such as critical operations being actually more +//! ***DO NOT PANIC,*** most of the time. There are some exceptions, such as critical operations being actually more //! dangerous than allowing the node to continue running (block authoring, consensus, etc). //! -//! - Directly using `unwrap()` for a [`Result`] shouldn't be used. -//! - This includes accessing indices of some collection type, which may implicitly `panic!` (i.e., +//! General guidelines: +//! +//! - Avoid directly using `unwrap()` for a [`Result`]. +//! - This includes simple operations, such as accessing indices of some collection type, which may implicitly `panic!` (i.e., //! via `get()`) //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. @@ -36,25 +36,21 @@ //! //! ### Defensive Traits //! -//! To also aid in debugging and mitigating the above issues, there is a -//! [`Defensive`](frame::traits::Defensive) trait (and its companions, +//! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which +//! provide an alternative to 'vanilla' Rust functions, e.g.,: +//! +//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) +//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) +//! +//! The [`Defensive`](frame::traits::Defensive) trait (and its companions, //! [`DefensiveOption`](frame::traits::DefensiveOption), //! [`DefensiveResult`](frame::traits::DefensiveResult)) that can be used to defensively unwrap //! values. This can be used in place of //! an `expect`, and again, only if the developer is sure about the unwrap in the first place. //! -//! The primary difference of defensive implementations bring over vanilla ones is the usage of [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions). -//! `debug_assertions` allows for panics to occur in a testing context, but in +//! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in //! production/release, they will merely log an error (i.e., `log::error`). //! -//! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which -//! provide an alternative to 'vanilla' Rust functions: -//! -//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) -//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) -//! -//! This traits are useful for catching issues in the development environment, without risking -//! panicking in production. //! //! ## Integer Overflow //! @@ -62,9 +58,10 @@ //! The compiler panics in **debug** mode in the event of an integer overflow. In //! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion. //! -//! However in the runtime context, we don't always have control over what is being supplied as a +//! In the context of runtime development, we don't always have control over what is being supplied as a //! parameter. For example, even this simple adding function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: +//! #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! //! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in @@ -74,14 +71,9 @@ //! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. //! ``` //! -//! It is actually the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in +//! It is the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in //! the context of blockchain development, where unsafe arithmetic could produce unexpected -//! consequences. -//! -//! A quick example is a user's balance overflowing: the default behavior of wrapping could result -//! in the user's balance starting from zero, or vice versa, of a `0` balance turning into the -//! `MAX`. This could lead to various exploits and issues down the road, which if -//! failing silently, would be difficult to trace and rectify in production. +//! consequences like a user balance over or underflowing. //! //! Luckily, there are ways to both represent and handle these scenarios depending on our specific //! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. @@ -91,14 +83,14 @@ //! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating //! point arithmetic. //! -//! A developer should use fixed-point arithmetic to mitigate the potential for inaccuracy, -//! rounding errors, or other unexpected behavior. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). +//! A developer should use fixed-point instead of floating-point arithmetic to mitigate the potential for inaccuracy, +//! rounding errors, or other unexpected behavior. //! -//! Using **primitive** floating point number types in a blockchain context should be avoided, +//! Using floating point number types in the runtime should be avoided, //! as a single nondeterministic result could cause chaos for consensus along with the -//! aforementioned issues. +//! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0) //! -//! The following methods represent different ways one can handle numbers safely natively in Rust, +//! The following methods demonstrate different ways one can handle numbers safely natively in Rust, //! without fear of panic or unexpected behavior from wrapping. //! //! ### Checked Arithmetic From a7a4ee1e7060df9d6d8aa00124654b76f05399b4 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 1 Jan 2024 14:23:25 -0500 Subject: [PATCH 04/76] few tweaks --- .../src/reference_docs/defensive_programming.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 198f159b9dc0..54acad87a027 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -25,13 +25,11 @@ //! //! General guidelines: //! -//! - Avoid directly using `unwrap()` for a [`Result`]. -//! - This includes simple operations, such as accessing indices of some collection type, which may implicitly `panic!` (i.e., -//! via `get()`) +//! - Avoid panickable operations, such as directly using `unwrap()` for a [`Result`], common errors such as accessing collections out of bounds (using safer methods to access collection types, i.e., `get()`). //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) -//! - Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a +//! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a //! number of issues [(see more later in this document)](#integer-overflow). //! //! ### Defensive Traits @@ -42,10 +40,10 @@ //! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) //! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) //! -//! The [`Defensive`](frame::traits::Defensive) trait (and its companions, +//! The [`Defensive`](frame::traits::Defensive) trait and its companions, //! [`DefensiveOption`](frame::traits::DefensiveOption), -//! [`DefensiveResult`](frame::traits::DefensiveResult)) that can be used to defensively unwrap -//! values. This can be used in place of +//! [`DefensiveResult`](frame::traits::DefensiveResult) can be used to defensively unwrap +//! and handle values. This can be used in place of //! an `expect`, and again, only if the developer is sure about the unwrap in the first place. //! //! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in @@ -56,7 +54,7 @@ //! //! The Rust compiler prevents any sort of static overflow from happening at compile time. //! The compiler panics in **debug** mode in the event of an integer overflow. In -//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion. +//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion (from the `MAX` back to zero). //! //! In the context of runtime development, we don't always have control over what is being supplied as a //! parameter. For example, even this simple adding function could present one of two outcomes From 3014e206e0dd1df809891b83fcd33bf5d2262442 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:07:56 -0500 Subject: [PATCH 05/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 54acad87a027..f028011005bd 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -1,6 +1,6 @@ //! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] //! types, eliminating the possibility of integer overflows, converting between number types, or -//! even handling floating point usage with fixed point arithmetic to mitigate issues that come with +//! even replacing floating point usage with fixed point arithmetic to mitigate issues that come with //! floating point calculations. //! //! Intentional and predictable design should be our first and foremost From 48d2f163205d9b24f6f3f0d9ea83835d63bc3412 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:09:34 -0500 Subject: [PATCH 06/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index f028011005bd..bc5c77d3d299 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -86,7 +86,7 @@ //! //! Using floating point number types in the runtime should be avoided, //! as a single nondeterministic result could cause chaos for consensus along with the -//! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0) +//! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! //! The following methods demonstrate different ways one can handle numbers safely natively in Rust, //! without fear of panic or unexpected behavior from wrapping. From 8ac9f444be0c29cea58b34c3e4e6354d62920c23 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:09:51 -0500 Subject: [PATCH 07/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index bc5c77d3d299..09b85c428e33 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -320,7 +320,6 @@ //! percentage of a particular item: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_mult)] //! -//! //! ### Fixed Point Types in Practice //! //! As said earlier, if one needs to exceed the value of one, then From 8b20c600ce93cd1707347558703fd411110c8353 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:10:04 -0500 Subject: [PATCH 08/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 09b85c428e33..f1552de1595e 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -168,9 +168,9 @@ //! //! As a recap, we covered the following concepts: //! -//! 1. **Checked** operations - using [`Option`] or [`Result`], -//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type, -//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type, +//! 1. **Checked** operations - using [`Option`] or [`Result`] +//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type +//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type //! //! //! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of From 3a2c603b0b64d33bf8010ff5d8884867cace559e Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:10:18 -0500 Subject: [PATCH 09/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index f1552de1595e..175b4f5b1550 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -284,7 +284,7 @@ //! - [`FixedI128`](sp_arithmetic::FixedU128) and [`FixedU128`](sp_arithmetic::FixedI128) //! //! Similar to types that implement [`PerThing`](sp_arithmetic::PerThing), these are also -//! fixed-point types, however, they are able to represent larger numbers: +//! fixed-point types, however, they are able to represent larger fractions: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", fixed_u64)] //! //! Let's now explore these types in practice, and how they may be used with pallets to perform From 196290240698126aaedc772fb5160aa6cdb1e0d6 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:10:31 -0500 Subject: [PATCH 10/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 175b4f5b1550..5ac5cf27764c 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -516,6 +516,7 @@ mod tests { assert_eq!(division, FixedU64::from_float(1.25)); assert_eq!(subtraction, FixedU64::from_float(0.4)); } + #[docify::export] #[test] fn bad_unwrap() { From 263f5c70347c84e513f98c9e38721e321c1dd295 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:10:40 -0500 Subject: [PATCH 11/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 5ac5cf27764c..9605183b2c04 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -548,6 +548,7 @@ mod tests { // Rust includes `.get`, returning Option - so lets use that: assert_eq!(my_list.get(5), None) } + #[docify::export] #[test] #[should_panic(expected = "Defensive failure has been triggered!")] From e357c6050d782f8485345853d6fc2c2b47bc2116 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:10:48 -0500 Subject: [PATCH 12/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 9605183b2c04..bd9d2bed42bc 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -331,7 +331,7 @@ fixed_u64_block_computation_example )] //! -//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`] +//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`]. //! //! #### Fixed Point Types in Practice //! From 4341e28d56d1750d53a33a9132f63b1dff4a050b Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:11:33 -0500 Subject: [PATCH 13/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index bd9d2bed42bc..2a798491257c 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -10,7 +10,7 @@ //! //! Defensive programming is a design paradigm that enables a particular program to continue //! running despite unexpected behavior. These unforseen circumstances may -//! cause the program to stop, (or in the Rust context, `panic!`) defensive practices allow for +//! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for //! these circumstances to be accounted for ahead of time, and for them to be handled in a more //! graceful manner. //! From 36c2258b3dd5689b675775364e7d595c7a49b796 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:11:57 -0500 Subject: [PATCH 14/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 2a798491257c..8b7bf637e573 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -11,7 +11,7 @@ //! Defensive programming is a design paradigm that enables a particular program to continue //! running despite unexpected behavior. These unforseen circumstances may //! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for -//! these circumstances to be accounted for ahead of time, and for them to be handled in a more +//! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. //! //! The Polkadot SDK is both built to reflect these principles, and to be used accordingly. From 082c30bed1f51da12dab706f4bebc34bbe8b9c8e Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:12:09 -0500 Subject: [PATCH 15/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 8b7bf637e573..d8448bb9b185 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -14,7 +14,7 @@ //! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. //! -//! The Polkadot SDK is both built to reflect these principles, and to be used accordingly. +//! The Polkadot SDK is both built to reflect these principles and to be used accordingly. //! //! ## General Practices //! From cd1b33054e9fea88ff3c894ee893a7a6c22fed26 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:12:20 -0500 Subject: [PATCH 16/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index d8448bb9b185..517f39ec2e79 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -20,7 +20,7 @@ //! //! When developing within the context of the a runtime, there is one golden rule: //! -//! ***DO NOT PANIC,*** most of the time. There are some exceptions, such as critical operations being actually more +//! ***DO NOT PANIC***. There are some exceptions, such as critical operations being actually more //! dangerous than allowing the node to continue running (block authoring, consensus, etc). //! //! General guidelines: From 9894d281236a24104fa2e06cd7642cb6768cad1d Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:12:48 -0500 Subject: [PATCH 17/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 517f39ec2e79..8115795c5bf8 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -49,7 +49,6 @@ //! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in //! production/release, they will merely log an error (i.e., `log::error`). //! -//! //! ## Integer Overflow //! //! The Rust compiler prevents any sort of static overflow from happening at compile time. From 9d7e1ec3e0278e7287c68fc1cd983308c8edad15 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:12:54 -0500 Subject: [PATCH 18/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 8115795c5bf8..3349bebbe433 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -187,7 +187,6 @@ //! consider some other ways of dealing with runtime arithmetic, such as saturated or checked //! operations, that won't carry these potential consequences. //! -//! //! While it may seem trivial, choosing how to handle numbers is quite important. As a thought //! exercise, here are some scenarios of which will shed more light on when to use which. //! From 94de958643fa1d0b7ba985b16671219b9f2ddc48 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:13:01 -0500 Subject: [PATCH 19/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 3349bebbe433..ad0058a6d569 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -243,7 +243,6 @@ //! //! //! -//! //! From the above, we can clearly see the problematic nature of seemingly simple operations in //! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - //! such as certain balance being never realistically attainable, or a number type being so large From 9ae0bd369cd11ae7a2779676f7703b1fad73d170 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:14:29 -0500 Subject: [PATCH 20/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index ad0058a6d569..91109370ca5a 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -243,7 +243,7 @@ //! //! //! -//! From the above, we can clearly see the problematic nature of seemingly simple operations in +//! From the above, we can clearly see the problematic nature of seemingly simple operations in the //! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - //! such as certain balance being never realistically attainable, or a number type being so large //! that it could never realistically overflow unless one sent thousands of transactions to the From 98e1539abfcc6f93e85765edb2177e3514902bf8 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:15:23 -0500 Subject: [PATCH 21/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 91109370ca5a..dbaeae2088a7 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -141,7 +141,6 @@ increase_balance_result )] //! -//! //! ### Saturating Operations //! //! Saturating a number limits it to the type's upper or lower bound, no matter the integer would From 9b260f87c6aa664633b30ed798d8222ee1936a67 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:15:32 -0500 Subject: [PATCH 22/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index dbaeae2088a7..db0c18c4d8c1 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -263,7 +263,6 @@ //! [`PerThing`](sp_arithmetic::PerThing) are used: //! - **[`Perbill`](sp_arithmetic::Perbill), parts of a billion** #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", perbill_example)] -//! //! - **[`Percent`](sp_arithmetic::Percent), parts of a hundred** #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_example)] //! From 92412d97be8c7668849dc944e7a728d3f3cf4ddc Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:15:45 -0500 Subject: [PATCH 23/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index db0c18c4d8c1..d74a1148c9aa 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -175,7 +175,6 @@ //! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a //! `checked_` method in order to introduce safe arithmetic in their code. //! -//! //! #### The problem with 'default' wrapped operations //! //! **Wrapped operations** cause the overflow to wrap around to either the maximum or minimum of From 73b9e5d67ea71ff4c556adbf744eaaef83dd6725 Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:15:55 -0500 Subject: [PATCH 24/76] Update developer-hub/src/reference_docs/defensive_programming.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index d74a1148c9aa..8b1fcf996a53 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -173,7 +173,7 @@ //! //! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of //! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a -//! `checked_` method in order to introduce safe arithmetic in their code. +//! `checked_*` method in order to introduce safe arithmetic in their code. //! //! #### The problem with 'default' wrapped operations //! From 1738854a443a62ce9a84b259b0b96f0671e0ab9f Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:16:04 -0500 Subject: [PATCH 25/76] Update developer-hub/src/reference_docs/defensive_programming.rs Co-authored-by: Oliver Tale-Yazdi --- developer-hub/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer-hub/src/reference_docs/defensive_programming.rs b/developer-hub/src/reference_docs/defensive_programming.rs index 8b1fcf996a53..cd4c8ddf179c 100644 --- a/developer-hub/src/reference_docs/defensive_programming.rs +++ b/developer-hub/src/reference_docs/defensive_programming.rs @@ -545,7 +545,7 @@ mod tests { #[docify::export] #[test] - #[should_panic(expected = "Defensive failure has been triggered!")] + #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!")] fn saturated_defensive_example() { let saturated_defensive = u32::MAX.defensive_saturating_add(10); assert_eq!(saturated_defensive, u32::MAX); From 4adb25720b296a9e170ec537c6a63676c076360b Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 8 Jan 2024 10:16:39 -0500 Subject: [PATCH 26/76] Update developer-hub/src/reference_docs/mod.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- developer-hub/src/reference_docs/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/developer-hub/src/reference_docs/mod.rs b/developer-hub/src/reference_docs/mod.rs index 0fe2bd8b7ea0..03e65611ce26 100644 --- a/developer-hub/src/reference_docs/mod.rs +++ b/developer-hub/src/reference_docs/mod.rs @@ -48,7 +48,6 @@ pub mod signed_extensions; pub mod frame_origin; /// Learn about how to write safe and defensive code in your FRAME runtime. -// TODO: @CrackTheCode016 https://github.com/paritytech/polkadot-sdk-docs/issues/44 pub mod defensive_programming; /// Learn about composite enums in FRAME-based runtimes, such as "RuntimeEvent" and "RuntimeCall". From fdd26df91850b9ddf645205e4587ebdb46b1acb3 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 8 Jan 2024 10:40:40 -0500 Subject: [PATCH 27/76] new branch --- docs/sdk/src/guides/your_first_pallet/mod.rs | 6 +- .../reference_docs/defensive_programming.rs | 568 ++++++++++++++++++ docs/sdk/src/reference_docs/mod.rs | 2 +- .../safe_defensive_programming.rs | 1 - 4 files changed, 572 insertions(+), 5 deletions(-) create mode 100644 docs/sdk/src/reference_docs/defensive_programming.rs delete mode 100644 docs/sdk/src/reference_docs/safe_defensive_programming.rs diff --git a/docs/sdk/src/guides/your_first_pallet/mod.rs b/docs/sdk/src/guides/your_first_pallet/mod.rs index c886bc9af842..050592366c60 100644 --- a/docs/sdk/src/guides/your_first_pallet/mod.rs +++ b/docs/sdk/src/guides/your_first_pallet/mod.rs @@ -105,8 +105,8 @@ //! This macro will call `.into()` under the hood. #![doc = docify::embed!("./src/guides/your_first_pallet/mod.rs", transfer_better)] //! -//! Moreover, you will learn in the [Safe Defensive Programming -//! section](crate::reference_docs::safe_defensive_programming) that it is always recommended to use +//! Moreover, you will learn in the [Defensive Programming +//! section](crate::reference_docs::defensive_programming) that it is always recommended to use //! safe arithmetic operations in your runtime. By using [`frame::traits::CheckedSub`], we can not //! only take a step in that direction, but also improve the error handing and make it slightly more //! ergonomic. @@ -291,7 +291,7 @@ //! The following topics where used in this guide, but not covered in depth. It is suggested to //! study them subsequently: //! -//! - [`crate::reference_docs::safe_defensive_programming`]. +//! - [`crate::reference_docs::defensive_programming`]. //! - [`crate::reference_docs::frame_origin`]. //! - [`crate::reference_docs::frame_composite_enums`]. //! - The pallet we wrote in this guide was using `dev_mode`, learn more in diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs new file mode 100644 index 000000000000..7f186b89e714 --- /dev/null +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -0,0 +1,568 @@ +//! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] +//! types, eliminating the possibility of integer overflows, converting between number types, or +//! even handling floating point usage with fixed point arithmetic to mitigate issues that come with +//! floating point calculations. +//! +//! Intentional and predictable design should be our first and foremost +//! property for ensuring a well running, safely designed system. +//! +//! ## Defensive Programming +//! +//! Defensive programming is a design paradigm that enables a particular program to continue +//! running despite unexpected behavior. These unforseen circumstances may +//! cause the program to stop, (or in the Rust context, `panic!`) defensive practices allow for +//! these circumstances to be accounted for ahead of time, and for them to be handled in a more +//! graceful manner. +//! +//! The Polkadot SDK is both built to reflect these principles, and to be used accordingly. +//! +//! ## General Practices +//! +//! When developing within the context of the a runtime, there is one golden rule: +//! +//! ***DO NOT PANIC!*** +//! +//! Most of the time - there are some exceptions, such as critical operations being actually more +//! dangerous than allowing the node to continue running (block authoring, consensus, etc). +//! +//! - Directly using `unwrap()` for a [`Result`] shouldn't be used. +//! - This includes accessing indices of some collection type, which may implicitly `panic!` (i.e., +//! via `get()`) +//! - It may be acceptable to use `except()`, but only if one is completely certain (and has +//! performed a check beforehand) that a value won't panic upon unwrapping. +//! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) +//! - Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a +//! number of issues [(see more later in this document)](#integer-overflow). +//! +//! ### Defensive Traits +//! +//! To also aid in debugging and mitigating the above issues, there is a +//! [`Defensive`](frame::traits::Defensive) trait (and its companions, +//! [`DefensiveOption`](frame::traits::DefensiveOption), +//! [`DefensiveResult`](frame::traits::DefensiveResult)) that can be used to defensively unwrap +//! values. This can be used in place of +//! an `expect`, and again, only if the developer is sure about the unwrap in the first place. +//! +//! The primary difference of defensive implementations bring over vanilla ones is the usage of [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions). +//! `debug_assertions` allows for panics to occur in a testing context, but in +//! production/release, they will merely log an error (i.e., `log::error`). +//! +//! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which +//! provide an alternative to 'vanilla' Rust functions: +//! +//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) +//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) +//! +//! This traits are useful for catching issues in the development environment, without risking +//! panicking in production. +//! +//! ## Integer Overflow +//! +//! The Rust compiler prevents any sort of static overflow from happening at compile time. +//! The compiler panics in **debug** mode in the event of an integer overflow. In +//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion. +//! +//! However in the runtime context, we don't always have control over what is being supplied as a +//! parameter. For example, even this simple adding function could present one of two outcomes +//! depending on whether it is in **release** or **debug** mode: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] +//! +//! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in +//! release). +//! +//! ```ignore +//! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. +//! ``` +//! +//! It is actually the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in +//! the context of blockchain development, where unsafe arithmetic could produce unexpected +//! consequences. +//! +//! A quick example is a user's balance overflowing: the default behavior of wrapping could result +//! in the user's balance starting from zero, or vice versa, of a `0` balance turning into the +//! `MAX`. This could lead to various exploits and issues down the road, which if +//! failing silently, would be difficult to trace and rectify in production. +//! +//! Luckily, there are ways to both represent and handle these scenarios depending on our specific +//! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. +//! +//! ## Infallible Arithmetic +//! +//! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating +//! point arithmetic. +//! +//! A developer should use fixed-point arithmetic to mitigate the potential for inaccuracy, +//! rounding errors, or other unexpected behavior. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). +//! +//! Using **primitive** floating point number types in a blockchain context should be avoided, +//! as a single nondeterministic result could cause chaos for consensus along with the +//! aforementioned issues. +//! +//! The following methods represent different ways one can handle numbers safely natively in Rust, +//! without fear of panic or unexpected behavior from wrapping. +//! +//! ### Checked Arithmetic +//! +//! **Checked operations** utilize an `Option` as a return type. This allows for simple pattern +//! matching to catch any unexpected behavior in the event of an overflow. +//! +//! This is an example of a valid operation: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", checked_add_example)] +//! +//! This is an example of an invalid operation, in this case, a simulated integer overflow, which +//! would simply result in `None`: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + checked_add_handle_error_example +)] +//! +//! Typically, if you aren't sure about which operation to use for runtime math, **checked** +//! operations are a safe bet, as it presents two, predictable (and _erroring_) outcomes that can be +//! handled accordingly (`Some` and `None`). +//! +//! In a practical context, the resulting [`Option`] should be handled accordingly. The following +//! conventions can be seen from the within the Polkadot SDK, where it can be handled in one of +//! two ways: +//! +//! - As an [`Option`], using the `if let` / `if` or `match` +//! - As a [`Result`], via `ok_or` (or similar conversion to [`Result`] from [`Option`]) +//! +//! #### Handling via Option - More Verbose +//! +//! Because wrapped operations return `Option`, you can use a more verbose/explicit form of error +//! handling via `if` or `if let`: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance)] +//! +//! Optionally, `match` may also be directly used in a more concise manner: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + increase_balance_match +)] +//! +//! This is generally a useful convention for handling not only checked types, but most types that +//! return `Option`. +//! +//! #### Handling via Result - Less Verbose +//! +//! In the Polkadot SDK codebase, you may see checked operations being handled as a [`Result`] via +//! `ok_or`. This is a less verbose way of expressing the above. Which to use often boils down to +//! the developer's preference: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + increase_balance_result +)] +//! +//! +//! ### Saturating Operations +//! +//! Saturating a number limits it to the type's upper or lower bound, no matter the integer would +//! overflow in runtime. For example, adding to `u32::MAX` would simply limit itself to `u32::MAX`: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + saturated_add_example +)] +//! +//! Saturating calculations can be used if one is very sure that something won't overflow, but wants +//! to avoid introducing the notion of any potential-panic or wrapping behavior. +//! +//! There is also a series of defensive alternatives via +//! [`DefensiveSaturating`](frame::traits::DefensiveSaturating), which introduces the same behavior +//! of the [`Defensive`](frame::traits::Defensive) trait, only with saturating, mathematical +//! operations: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + saturated_defensive_example +)] +//! +//! ### Mathematical Operations in Substrate Development - Further Context +//! +//! As a recap, we covered the following concepts: +//! +//! 1. **Checked** operations - using [`Option`] or [`Result`], +//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type, +//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type, +//! +//! +//! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of +//! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a +//! `checked_` method in order to introduce safe arithmetic in their code. +//! +//! +//! #### The problem with 'default' wrapped operations +//! +//! **Wrapped operations** cause the overflow to wrap around to either the maximum or minimum of +//! that type. Imagine this in the context of a blockchain, where there are balances, voting +//! counters, nonces for transactions, and other aspects of a blockchain. +//! +//! Some of these mechanisms can be more critical than others. It's for this reason that we may +//! consider some other ways of dealing with runtime arithmetic, such as saturated or checked +//! operations, that won't carry these potential consequences. +//! +//! +//! While it may seem trivial, choosing how to handle numbers is quite important. As a thought +//! exercise, here are some scenarios of which will shed more light on when to use which. +//! +//! #### Bob's Overflowed Balance +//! +//! **Bob's** balance exceeds the `Balance` type on the `EduChain`. Because the pallet developer did +//! not handle the calculation to add to Bob's balance with any regard to this overflow, **Bob's** +//! balance is now essentially `0`, the operation **wrapped**. +//! +//!
+//! Solution: Saturating or Checked +//! For Bob's balance problems, using a `saturated_add` or `checked_add` could've mitigated this +//! issue. They simply would've reached the upper, or lower bounds, of the particular type for an +//! on-chain balance. In other words: Bob's balance would've stayed at the maximum of the Balance +//! type.
+//! +//! #### Alice's 'Underflowed' Balance +//! +//! Alice's balance has reached `0` after a transfer to Bob. Suddenly, she has been slashed on +//! `EduChain`, causing her balance to reach near the limit of `u32::MAX` - a very large amount - as +//! _wrapped operations_ can go both ways. Alice can now successfully vote using her new, +//! overpowered token balance, destroying the integrity of the chain. +//! +//!
+//! Solution: Saturating +//! For Alice's balance problem, using `saturated_sub` could've mitigated this issue. As debt or +//! having a negative balance is not a concept within blockchains, a saturating calculation +//! would've simply limited her balance to the lower bound of u32. +//! +//! In other words: Alice's balance would've stayed at "0", even after being slashed. +//! +//! This is also an example that while one system may work in isolation, shared interfaces, such +//! as the notion of balances, are often shared across multiple pallets - meaning these small +//! changes can make a big difference in outcome.
+//! +//! #### Proposals' ID Overwrite +//! +//! The type for counting the number of proposals on-chain is represented by a `u8` number, called +//! `proposals_count`. Every time a new proposal is added to the system, this number increases. With +//! the proposal pallet being high in usage, it has reached `u8::MAX`'s limit of `255`, causing +//! `proposals_count` to go to `0`. Unfortunately, this resulted in new proposals overwriting old +//! ones, effectively erasing any notion of past proposals! +//! +//!
+//! Solution: Checked +//! For the proposal IDs, proper handling via `checked` math would've been much more suitable, +//! Saturating could've been used - but it also would've 'failed' silently. Using `checked_add` to +//! ensure that the next proposal ID would've been valid would've been a viable way to let the user +//! know the state of their proposal: +//! +//! ```ignore +//! let next_proposal_id = current_count.checked_add(1).ok_or_else(|| Error::TooManyProposals)?; +//! ``` +//! +//!
+//! +//! +//! From the above, we can clearly see the problematic nature of seemingly simple operations in +//! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - +//! such as certain balance being never realistically attainable, or a number type being so large +//! that it could never realistically overflow unless one sent thousands of transactions to the +//! network. +//! +//! ### Decision Chart: When to use which? +#![doc = simple_mermaid::mermaid!("../../../docs/mermaid/integer_operation_decision.mmd")] +//! ## Fixed Point Arithmetic +//! +//! The following code uses types from [`sp_arithmetic`]. +//! +//! Fixed point arithmetic solves the aforementioned problems of dealing with the uncertain +//! nature of floating point numbers. Rather than use a radix point (`0.80`), a type which +//! _represents_ a floating point number in base 10, i.e., a **fixed point number**, can be used +//! instead. +//! +//! For use cases which operate within the range of `[0, 1]` types that implement +//! [`PerThing`](sp_arithmetic::PerThing) are used: +//! - **[`Perbill`](sp_arithmetic::Perbill), parts of a billion** +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", perbill_example)] +//! +//! - **[`Percent`](sp_arithmetic::Percent), parts of a hundred** +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_example)] +//! +//! Note that `190 / 400 = 0.475`, and that `Percent` represents it as a _rounded down_, fixed point +//! number (`47`). Unlike primitive types, types that implement +//! [`PerThing`](sp_arithmetic::PerThing) will also not overflow, and are therefore safe to use. +//! They adopt the same behavior that a saturated calculation would provide, meaning that if one is +//! to go over "100%", it wouldn't overflow, but simply stop at the upper or lower bound. +//! +//! For use cases which require precision beyond the range of `[0, 1]`, there are a number of other +//! fixed-point types to use: +//! +//! - [`FixedU64`](sp_arithmetic::FixedU64) and [`FixedI64`](sp_arithmetic::FixedI64) +//! - [`FixedI128`](sp_arithmetic::FixedU128) and [`FixedU128`](sp_arithmetic::FixedI128) +//! +//! Similar to types that implement [`PerThing`](sp_arithmetic::PerThing), these are also +//! fixed-point types, however, they are able to represent larger numbers: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", fixed_u64)] +//! +//! Let's now explore these types in practice, and how they may be used with pallets to perform +//! safer calculations in the runtime. +//! +//! ### 'PerThing' In Practice +//! +//! [`sp_arithmetic`] contains a trait called [`PerThing`](sp_arithmetic::PerThing), allowing a +//! custom type to be implemented specifically for fixed point arithmetic. While a number of +//! fixed-point types are introduced, let's focus on a few specific examples that implement +//! [`PerThing`](sp_arithmetic::PerThing): +//! +//! - [`Percent`](sp_arithmetic::Percent) - parts of one hundred. +//! - [`Permill`](sp_arithmetic::Permill) - parts of a million. +//! - [`Perbill`](sp_arithmetic::Perbill) - parts of a billion. +//! +//! Each of these can be used to construct and represent ratios within our runtime. +//! You will find types like [`Perbill`](sp_arithmetic::Perbill) being used often in pallet +//! development. [`pallet_referenda`] is a good example of a pallet which makes good use of fixed +//! point arithmetic to calculate. +//! +//! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating +//! point numbers in Substrate development. For this scenario, let's say we are demonstrating a +//! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be +//! deemed valid. +//! +//! For most cases, `Perbill` gives us a reasonable amount of precision for most applications, which +//! is why we're using it here. +//! +//! #### Fixed Point Arithmetic with [`PerThing`](sp_arithmetic::PerThing) +//! +//! As stated, one can also perform mathematics using these types directly. For example, finding the +//! percentage of a particular item: +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_mult)] +//! +//! +//! ### Fixed Point Types in Practice +//! +//! As said earlier, if one needs to exceed the value of one, then +//! [`FixedU64`](sp_arithmetic::FixedU64) (and its signed and `u128` counterparts) can be utilized. +//! Take for example this very rudimentary pricing mechanism, where we wish to calculate the demand +//! / supply to get a price for some on-chain compute: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + fixed_u64_block_computation_example +)] +//! +//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`] +//! +//! #### Fixed Point Types in Practice +//! +//! Just as with [`PerThing`](sp_arithmetic::PerThing), you can also perform regular mathematical +//! expressions: +#![doc = docify::embed!( + "./src/reference_docs/defensive_programming.rs", + fixed_u64_operation_example +)] +//! +//! +//! ## Other Resources +//! +//! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) + +#[cfg(test)] +mod tests { + enum BlockchainError { + Overflow, + } + + type Address = (); + + struct Runtime; + + impl Runtime { + fn get_balance(account: Address) -> u64 { + 0 + } + } + + #[docify::export] + #[test] + fn naive_add(x: u8, y: u8) -> u8 { + x + y + } + + #[docify::export] + #[test] + fn checked_add_example() { + // This is valid, as 20 is perfectly within the bounds of u32. + let add = (10u32).checked_add(10); + assert_eq!(add, Some(20)) + } + + #[docify::export] + #[test] + fn checked_add_handle_error_example() { + // This is invalid - we are adding something to the max of u32::MAX, which would overflow. + // Luckily, checked_add just marks this as None! + let add = u32::MAX.checked_add(10); + assert_eq!(add, None) + } + + #[docify::export] + #[test] + fn increase_balance(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + if let Some(new_balance) = balance.checked_add(amount) { + Runtime::set_balance(account, new_balance); + return Ok(()); + } else { + return Err(BlockchainError::Overflow); + } + } + + #[docify::export] + #[test] + fn increase_balance_match(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + let new_balance = match balance.checked_add(amount) { + Some(balance) => balance, + None => { + return Err(BlockchainError::Overflow); + }, + }; + Runtime::set_balance(account, new_balance); + Ok(()) + } + + #[docify::export] + #[test] + fn increase_balance_result(account: Address, amount: u64) -> Result<(), BlockchainError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount - this time, by using `ok_or` + let new_balance = balance.checked_add(amount).ok_or_else(|| BlockchainError::Overflow)?; + Runtime::set_balance(account, new_balance); + Ok(()) + } + + #[docify::export] + #[test] + fn saturated_add_example() { + // Saturating add simply saturates + // to the numeric bound of that type if it overflows. + let add = u32::MAX.saturating_add(10); + assert_eq!(add, u32::MAX) + } + #[docify::export] + #[test] + fn percent_mult() { + let percent = Percent::from_rational(5u32, 100u32); // aka, 5% + let five_percent_of_100 = percent * 100u32; // 5% of 100 is 5. + assert_eq!(five_percent_of_100, 5) + } + #[docify::export] + #[test] + fn perbill_example() { + let p = Perbill::from_percent(80); + // 800000000 bil, or a representative of 0.800000000. + // Precision is in the billions place. + assert_eq!(p.deconstruct(), 800000000); + } + + #[docify::export] + #[test] + fn percent_example() { + let percent = Percent::from_rational(190u32, 400u32); + assert_eq!(percent.deconstruct(), 47); + } + + #[docify::export] + #[test] + fn fixed_u64_block_computation_example() { + // Cores available per block + let supply = 10u128; + // Cores being ordered per block + let demand = 5u128; + // Calculate a very rudimentry on-chain price from supply / demand + let price = FixedU64::from_rational(demand, supply); + + // 0.5 DOT per core + assert_eq!(price, FixedU64::from_float(0.5)); + + // Now, the story has changed - lots of demand means we buy as many cores as there + // available. This also means that price goes up! For the sake of simplicity, we don't care + // about who gets a core - just about our very simple price model + + // Cores available per block + let supply = 10u128; + // Cores being ordered per block + let demand = 19u128; + // Calculate a very rudimentary on-chain price from supply / demand + let price = FixedU64::from_rational(demand, supply); + + // 1.9 DOT per core + assert_eq!(price, FixedU64::from_float(1.9)); + } + + #[docify::export] + #[test] + fn fixed_u64() { + // The difference between this and perthings is perthings operates within the relam of [0, + // 1] In cases where we need > 1, we can used fixed types such as FixedU64 + + let rational_1 = FixedU64::from_rational(10, 5); //" 200%" aka 2. + let rational_2 = + FixedU64::from_rational_with_rounding(5, 10, sp_arithmetic::Rounding::Down); // "50%" aka 0.50... + + assert_eq!(rational_1, (2u64).into()); + assert_eq!(rational_2.into_perbill(), Perbill::from_float(0.5)); + } + + #[docify::export] + #[test] + fn fixed_u64_operation_example() { + let rational_1 = FixedU64::from_rational(10, 5); // "200%" aka 2. + let rational_2 = FixedU64::from_rational(8, 5); // "160%" aka 1.6. + + let addition = rational_1 + rational_2; + let multiplication = rational_1 * rational_2; + let division = rational_1 / rational_2; + let subtraction = rational_1 - rational_2; + + assert_eq!(addition, FixedU64::from_float(3.6)); + assert_eq!(multiplication, FixedU64::from_float(3.2)); + assert_eq!(division, FixedU64::from_float(1.25)); + assert_eq!(subtraction, FixedU64::from_float(0.4)); + } + #[docify::export] + #[test] + fn bad_unwrap() { + let some_result: Result = Ok(10); + assert_eq!(some_result.unwrap(), 10); + } + + #[docify::export] + #[test] + fn good_unwrap() { + let some_result: Result = Err("Error"); + assert_eq!(some_result.unwrap_or_default(), 0); + assert_eq!(some_result.unwrap_or(10), 10); + } + + #[docify::export] + #[test] + fn bad_collection_retrieval() { + let my_list = vec![1, 2, 3, 4, 5]; + // THIS PANICS! + // Indexing on heap allocated values, i.e., vec, can be unsafe! + assert_eq!(my_list[5], 6) + } + + #[docify::export] + #[test] + fn good_collection_retrieval() { + let my_list = vec![1, 2, 3, 4, 5]; + // Rust includes `.get`, returning Option - so lets use that: + assert_eq!(my_list.get(5), None) + } + #[docify::export] + #[test] + #[should_panic(expected = "Defensive failure has been triggered!")] + fn saturated_defensive_example() { + let saturated_defensive = u32::MAX.defensive_saturating_add(10); + assert_eq!(saturated_defensive, u32::MAX); + } +} diff --git a/docs/sdk/src/reference_docs/mod.rs b/docs/sdk/src/reference_docs/mod.rs index c16122ee4287..1db7cd50d194 100644 --- a/docs/sdk/src/reference_docs/mod.rs +++ b/docs/sdk/src/reference_docs/mod.rs @@ -49,7 +49,7 @@ pub mod frame_origin; /// Learn about how to write safe and defensive code in your FRAME runtime. // TODO: @CrackTheCode016 https://github.com/paritytech/polkadot-sdk-docs/issues/44 -pub mod safe_defensive_programming; +pub mod defensive_programming; /// Learn about composite enums in FRAME-based runtimes, such as "RuntimeEvent" and "RuntimeCall". pub mod frame_composite_enums; diff --git a/docs/sdk/src/reference_docs/safe_defensive_programming.rs b/docs/sdk/src/reference_docs/safe_defensive_programming.rs deleted file mode 100644 index 9d0f028e570d..000000000000 --- a/docs/sdk/src/reference_docs/safe_defensive_programming.rs +++ /dev/null @@ -1 +0,0 @@ -//! From 8b1510f9613a262c61b2b58527411cc75fc9f87b Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 8 Jan 2024 11:58:02 -0500 Subject: [PATCH 28/76] reapply changes, fix test --- Cargo.lock | 45 +-------- docs/sdk/Cargo.toml | 6 ++ .../reference_docs/defensive_programming.rs | 95 ++++++++----------- docs/sdk/src/reference_docs/mod.rs | 1 - 4 files changed, 49 insertions(+), 98 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c8618a087781..061de8848499 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4611,48 +4611,6 @@ dependencies = [ "syn 1.0.109", ] -[[package]] -name = "developer-hub" -version = "0.0.1" -dependencies = [ - "cumulus-pallet-aura-ext", - "cumulus-pallet-parachain-system", - "docify", - "frame", - "kitchensink-runtime", - "pallet-aura", - "pallet-broker", - "pallet-default-config-example", - "pallet-examples", - "pallet-referenda", - "pallet-timestamp", - "parity-scale-codec", - "sc-cli", - "sc-client-db", - "sc-consensus-aura", - "sc-consensus-babe", - "sc-consensus-beefy", - "sc-consensus-grandpa", - "sc-consensus-manual-seal", - "sc-consensus-pow", - "sc-network", - "sc-rpc", - "sc-rpc-api", - "scale-info", - "simple-mermaid 0.1.0 (git+https://github.com/kianenigma/simple-mermaid.git?branch=main)", - "sp-api", - "sp-arithmetic", - "sp-core", - "sp-io", - "sp-keyring", - "sp-runtime", - "staging-chain-spec-builder", - "staging-node-cli", - "staging-parachain-info", - "subkey", - "substrate-wasm-builder", -] - [[package]] name = "diff" version = "0.1.13" @@ -13393,8 +13351,10 @@ dependencies = [ "frame", "kitchensink-runtime", "pallet-aura", + "pallet-broker", "pallet-default-config-example", "pallet-examples", + "pallet-referenda", "pallet-timestamp", "parity-scale-codec", "sc-cli", @@ -13411,6 +13371,7 @@ dependencies = [ "scale-info", "simple-mermaid", "sp-api", + "sp-arithmetic", "sp-core", "sp-io", "sp-keyring", diff --git a/docs/sdk/Cargo.toml b/docs/sdk/Cargo.toml index 246da2cd68c6..d4cc1de790bd 100644 --- a/docs/sdk/Cargo.toml +++ b/docs/sdk/Cargo.toml @@ -60,6 +60,12 @@ sp-api = { path = "../../substrate/primitives/api" } sp-core = { path = "../../substrate/primitives/core" } sp-keyring = { path = "../../substrate/primitives/keyring" } sp-runtime = { path = "../../substrate/primitives/runtime" } +sp-arithmetic = { path = "../../substrate/primitives/arithmetic" } + +# Misc pallet dependencies +pallet-referenda = { path = "../../substrate/frame/referenda" } +pallet-broker = { path = "../../substrate/frame/broker" } + [dev-dependencies] parity-scale-codec = "3.6.5" diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 7f186b89e714..a47d86bfe5e6 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -1,6 +1,6 @@ //! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] //! types, eliminating the possibility of integer overflows, converting between number types, or -//! even handling floating point usage with fixed point arithmetic to mitigate issues that come with +//! even replacing floating point usage with fixed point arithmetic to mitigate issues that come with //! floating point calculations. //! //! Intentional and predictable design should be our first and foremost @@ -10,61 +10,55 @@ //! //! Defensive programming is a design paradigm that enables a particular program to continue //! running despite unexpected behavior. These unforseen circumstances may -//! cause the program to stop, (or in the Rust context, `panic!`) defensive practices allow for -//! these circumstances to be accounted for ahead of time, and for them to be handled in a more +//! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for +//! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. //! -//! The Polkadot SDK is both built to reflect these principles, and to be used accordingly. +//! The Polkadot SDK is both built to reflect these principles and to be used accordingly. //! //! ## General Practices //! //! When developing within the context of the a runtime, there is one golden rule: //! -//! ***DO NOT PANIC!*** -//! -//! Most of the time - there are some exceptions, such as critical operations being actually more +//! ***DO NOT PANIC***. There are some exceptions, such as critical operations being actually more //! dangerous than allowing the node to continue running (block authoring, consensus, etc). //! -//! - Directly using `unwrap()` for a [`Result`] shouldn't be used. -//! - This includes accessing indices of some collection type, which may implicitly `panic!` (i.e., -//! via `get()`) +//! General guidelines: +//! +//! - Avoid panickable operations, such as directly using `unwrap()` for a [`Result`], common errors such as accessing collections out of bounds (using safer methods to access collection types, i.e., `get()`). //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) -//! - Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a +//! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a //! number of issues [(see more later in this document)](#integer-overflow). //! //! ### Defensive Traits //! -//! To also aid in debugging and mitigating the above issues, there is a -//! [`Defensive`](frame::traits::Defensive) trait (and its companions, -//! [`DefensiveOption`](frame::traits::DefensiveOption), -//! [`DefensiveResult`](frame::traits::DefensiveResult)) that can be used to defensively unwrap -//! values. This can be used in place of -//! an `expect`, and again, only if the developer is sure about the unwrap in the first place. -//! -//! The primary difference of defensive implementations bring over vanilla ones is the usage of [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions). -//! `debug_assertions` allows for panics to occur in a testing context, but in -//! production/release, they will merely log an error (i.e., `log::error`). -//! //! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which -//! provide an alternative to 'vanilla' Rust functions: +//! provide an alternative to 'vanilla' Rust functions, e.g.,: //! //! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) //! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) //! -//! This traits are useful for catching issues in the development environment, without risking -//! panicking in production. +//! The [`Defensive`](frame::traits::Defensive) trait and its companions, +//! [`DefensiveOption`](frame::traits::DefensiveOption), +//! [`DefensiveResult`](frame::traits::DefensiveResult) can be used to defensively unwrap +//! and handle values. This can be used in place of +//! an `expect`, and again, only if the developer is sure about the unwrap in the first place. +//! +//! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in +//! production/release, they will merely log an error (i.e., `log::error`). //! //! ## Integer Overflow //! //! The Rust compiler prevents any sort of static overflow from happening at compile time. //! The compiler panics in **debug** mode in the event of an integer overflow. In -//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion. +//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion (from the `MAX` back to zero). //! -//! However in the runtime context, we don't always have control over what is being supplied as a +//! In the context of runtime development, we don't always have control over what is being supplied as a //! parameter. For example, even this simple adding function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: +//! #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! //! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in @@ -74,14 +68,9 @@ //! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. //! ``` //! -//! It is actually the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in +//! It is the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in //! the context of blockchain development, where unsafe arithmetic could produce unexpected -//! consequences. -//! -//! A quick example is a user's balance overflowing: the default behavior of wrapping could result -//! in the user's balance starting from zero, or vice versa, of a `0` balance turning into the -//! `MAX`. This could lead to various exploits and issues down the road, which if -//! failing silently, would be difficult to trace and rectify in production. +//! consequences like a user balance over or underflowing. //! //! Luckily, there are ways to both represent and handle these scenarios depending on our specific //! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. @@ -91,14 +80,14 @@ //! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating //! point arithmetic. //! -//! A developer should use fixed-point arithmetic to mitigate the potential for inaccuracy, -//! rounding errors, or other unexpected behavior. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). +//! A developer should use fixed-point instead of floating-point arithmetic to mitigate the potential for inaccuracy, +//! rounding errors, or other unexpected behavior. //! -//! Using **primitive** floating point number types in a blockchain context should be avoided, +//! Using floating point number types in the runtime should be avoided, //! as a single nondeterministic result could cause chaos for consensus along with the -//! aforementioned issues. +//! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! -//! The following methods represent different ways one can handle numbers safely natively in Rust, +//! The following methods demonstrate different ways one can handle numbers safely natively in Rust, //! without fear of panic or unexpected behavior from wrapping. //! //! ### Checked Arithmetic @@ -152,7 +141,6 @@ increase_balance_result )] //! -//! //! ### Saturating Operations //! //! Saturating a number limits it to the type's upper or lower bound, no matter the integer would @@ -178,15 +166,14 @@ //! //! As a recap, we covered the following concepts: //! -//! 1. **Checked** operations - using [`Option`] or [`Result`], -//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type, -//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type, +//! 1. **Checked** operations - using [`Option`] or [`Result`] +//! 2. **Saturating** operations - limited to the lower and upper bounds of a number type +//! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type //! //! //! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of //! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a -//! `checked_` method in order to introduce safe arithmetic in their code. -//! +//! `checked_*` method in order to introduce safe arithmetic in their code. //! //! #### The problem with 'default' wrapped operations //! @@ -198,7 +185,6 @@ //! consider some other ways of dealing with runtime arithmetic, such as saturated or checked //! operations, that won't carry these potential consequences. //! -//! //! While it may seem trivial, choosing how to handle numbers is quite important. As a thought //! exercise, here are some scenarios of which will shed more light on when to use which. //! @@ -255,15 +241,14 @@ //! //! //! -//! -//! From the above, we can clearly see the problematic nature of seemingly simple operations in +//! From the above, we can clearly see the problematic nature of seemingly simple operations in the //! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - //! such as certain balance being never realistically attainable, or a number type being so large //! that it could never realistically overflow unless one sent thousands of transactions to the //! network. //! //! ### Decision Chart: When to use which? -#![doc = simple_mermaid::mermaid!("../../../docs/mermaid/integer_operation_decision.mmd")] +#![doc = simple_mermaid::mermaid!("../../../mermaid/integer_operation_decision.mmd")] //! ## Fixed Point Arithmetic //! //! The following code uses types from [`sp_arithmetic`]. @@ -277,7 +262,6 @@ //! [`PerThing`](sp_arithmetic::PerThing) are used: //! - **[`Perbill`](sp_arithmetic::Perbill), parts of a billion** #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", perbill_example)] -//! //! - **[`Percent`](sp_arithmetic::Percent), parts of a hundred** #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_example)] //! @@ -294,7 +278,7 @@ //! - [`FixedI128`](sp_arithmetic::FixedU128) and [`FixedU128`](sp_arithmetic::FixedI128) //! //! Similar to types that implement [`PerThing`](sp_arithmetic::PerThing), these are also -//! fixed-point types, however, they are able to represent larger numbers: +//! fixed-point types, however, they are able to represent larger fractions: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", fixed_u64)] //! //! Let's now explore these types in practice, and how they may be used with pallets to perform @@ -330,7 +314,6 @@ //! percentage of a particular item: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_mult)] //! -//! //! ### Fixed Point Types in Practice //! //! As said earlier, if one needs to exceed the value of one, then @@ -342,7 +325,7 @@ fixed_u64_block_computation_example )] //! -//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`] +//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`]. //! //! #### Fixed Point Types in Practice //! @@ -527,6 +510,7 @@ mod tests { assert_eq!(division, FixedU64::from_float(1.25)); assert_eq!(subtraction, FixedU64::from_float(0.4)); } + #[docify::export] #[test] fn bad_unwrap() { @@ -558,11 +542,12 @@ mod tests { // Rust includes `.get`, returning Option - so lets use that: assert_eq!(my_list.get(5), None) } + #[docify::export] #[test] - #[should_panic(expected = "Defensive failure has been triggered!")] + #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))] fn saturated_defensive_example() { let saturated_defensive = u32::MAX.defensive_saturating_add(10); assert_eq!(saturated_defensive, u32::MAX); } -} +} \ No newline at end of file diff --git a/docs/sdk/src/reference_docs/mod.rs b/docs/sdk/src/reference_docs/mod.rs index 1db7cd50d194..ab275d7ab8de 100644 --- a/docs/sdk/src/reference_docs/mod.rs +++ b/docs/sdk/src/reference_docs/mod.rs @@ -48,7 +48,6 @@ pub mod signed_extensions; pub mod frame_origin; /// Learn about how to write safe and defensive code in your FRAME runtime. -// TODO: @CrackTheCode016 https://github.com/paritytech/polkadot-sdk-docs/issues/44 pub mod defensive_programming; /// Learn about composite enums in FRAME-based runtimes, such as "RuntimeEvent" and "RuntimeCall". From 2e4e07698c31ce2c6b7d4d407d5341f90e7a73b1 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Thu, 11 Jan 2024 11:41:12 -0500 Subject: [PATCH 29/76] fmt + add license --- .../reference_docs/defensive_programming.rs | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index a47d86bfe5e6..6c32d98d4783 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -1,7 +1,22 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + //! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] //! types, eliminating the possibility of integer overflows, converting between number types, or -//! even replacing floating point usage with fixed point arithmetic to mitigate issues that come with -//! floating point calculations. +//! even replacing floating point usage with fixed point arithmetic to mitigate issues that come +//! with floating point calculations. //! //! Intentional and predictable design should be our first and foremost //! property for ensuring a well running, safely designed system. @@ -25,12 +40,15 @@ //! //! General guidelines: //! -//! - Avoid panickable operations, such as directly using `unwrap()` for a [`Result`], common errors such as accessing collections out of bounds (using safer methods to access collection types, i.e., `get()`). +//! - Avoid panickable operations, such as directly using `unwrap()` for a [`Result`], common errors +//! such as accessing collections out of bounds (using safer methods to access collection types, +//! i.e., `get()`). //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) -//! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as **arithmetic** in the runtime, could present a -//! number of issues [(see more later in this document)](#integer-overflow). +//! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as +//! **arithmetic** in the runtime, could present a number of issues [(see more later in this +//! document)](#integer-overflow). //! //! ### Defensive Traits //! @@ -53,12 +71,12 @@ //! //! The Rust compiler prevents any sort of static overflow from happening at compile time. //! The compiler panics in **debug** mode in the event of an integer overflow. In -//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion (from the `MAX` back to zero). +//! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion +//! (from the `MAX` back to zero). //! -//! In the context of runtime development, we don't always have control over what is being supplied as a -//! parameter. For example, even this simple adding function could present one of two outcomes +//! In the context of runtime development, we don't always have control over what is being supplied +//! as a parameter. For example, even this simple adding function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: -//! #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! //! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in @@ -68,9 +86,9 @@ //! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. //! ``` //! -//! It is the _silent_ portion of this behavior that presents a real issue. Such behavior should be made obvious, especially in -//! the context of blockchain development, where unsafe arithmetic could produce unexpected -//! consequences like a user balance over or underflowing. +//! It is the _silent_ portion of this behavior that presents a real issue. Such behavior should be +//! made obvious, especially in the context of blockchain development, where unsafe arithmetic could +//! produce unexpected consequences like a user balance over or underflowing. //! //! Luckily, there are ways to both represent and handle these scenarios depending on our specific //! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. @@ -80,8 +98,8 @@ //! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating //! point arithmetic. //! -//! A developer should use fixed-point instead of floating-point arithmetic to mitigate the potential for inaccuracy, -//! rounding errors, or other unexpected behavior. +//! A developer should use fixed-point instead of floating-point arithmetic to mitigate the +//! potential for inaccuracy, rounding errors, or other unexpected behavior. //! //! Using floating point number types in the runtime should be avoided, //! as a single nondeterministic result could cause chaos for consensus along with the @@ -550,4 +568,4 @@ mod tests { let saturated_defensive = u32::MAX.defensive_saturating_add(10); assert_eq!(saturated_defensive, u32::MAX); } -} \ No newline at end of file +} From 702990e5f1cbd3a0f2b7d20a502e6f9d54f6f894 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Thu, 11 Jan 2024 14:27:38 -0500 Subject: [PATCH 30/76] defensive list, clarification --- .../reference_docs/defensive_programming.rs | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 6c32d98d4783..0f3255802326 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -36,15 +36,24 @@ //! When developing within the context of the a runtime, there is one golden rule: //! //! ***DO NOT PANIC***. There are some exceptions, such as critical operations being actually more -//! dangerous than allowing the node to continue running (block authoring, consensus, etc). +//! dangerous than allowing the runtime to continue functioning (block authoring, consensus, etc). +//! +//! > It's important to make the differentiation between the **runtime** and **node**. The runtime +//! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the +//! > outer client which deals with telemetry and gossip from other nodes. For more information, +//! > read about Substrate's architecture. //! //! General guidelines: //! -//! - Avoid panickable operations, such as directly using `unwrap()` for a [`Result`], common errors -//! such as accessing collections out of bounds (using safer methods to access collection types, -//! i.e., `get()`). +//! - Avoid any writing functions that could explicitly panic. Directly using `unwrap()` for a +//! [`Result`] or common errors such as accessing an out of bounds index on a collection (using +//! safer methods to access collection types, i.e., `get()`) are both examples of what to +//! completely avoid in runtime development. //! - It may be acceptable to use `except()`, but only if one is completely certain (and has -//! performed a check beforehand) that a value won't panic upon unwrapping. +//! performed a check beforehand) that a value won't panic upon unwrapping. Even this is +//! discouraged, however, as future changes to that function could then cause that statement to +//! panic. It is better to ensure all errors are propagated and handled accordingly in some way. +//! - If a function *can* panic, it usually is prefaced with `unchecked_` to indicate its safety. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) //! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as //! **arithmetic** in the runtime, could present a number of issues [(see more later in this @@ -64,6 +73,18 @@ //! and handle values. This can be used in place of //! an `expect`, and again, only if the developer is sure about the unwrap in the first place. //! +//! Here is a full list of all defensive types: +//! +//! - [`DefensiveOption`](frame::traits::DefensiveOption) +//! - [`DefensiveResult`](frame::traits::DefensiveResult) +//! - [`DefensiveMax`](frame::traits::DefensiveMax) +//! - [`DefensiveSaturating`](frame::traits::DefensiveSaturating) +//! - [`DefensiveTruncateFrom`](frame::traits::DefensiveTruncateFrom) +//! +//! All of which can be used by importing +//! [`frame::traits::defensive_prelude`](frame::traits::defensive_prelude), which imports all +//! defensive traits at once. +//! //! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in //! production/release, they will merely log an error (i.e., `log::error`). //! From 1451cddb4d8f2460ab45387f32434051d34a8284 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Thu, 11 Jan 2024 14:37:08 -0500 Subject: [PATCH 31/76] spruce up tests --- .../reference_docs/defensive_programming.rs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0f3255802326..e22104527fd2 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -23,7 +23,7 @@ //! //! ## Defensive Programming //! -//! Defensive programming is a design paradigm that enables a particular program to continue +//! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a particular program to continue //! running despite unexpected behavior. These unforseen circumstances may //! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for //! these circumstances to be accounted for ahead of time and for them to be handled in a more @@ -494,12 +494,10 @@ mod tests { #[docify::export] #[test] fn fixed_u64_block_computation_example() { - // Cores available per block - let supply = 10u128; - // Cores being ordered per block - let demand = 5u128; - // Calculate a very rudimentry on-chain price from supply / demand - let price = FixedU64::from_rational(demand, supply); + // Calculate a very rudimentary on-chain price from supply / demand + // Supply: Cores available per block + // Demand: Cores being ordered per block + let price = FixedU64::from_rational(5u128, 10u128); // 0.5 DOT per core assert_eq!(price, FixedU64::from_float(0.5)); @@ -508,12 +506,10 @@ mod tests { // available. This also means that price goes up! For the sake of simplicity, we don't care // about who gets a core - just about our very simple price model - // Cores available per block - let supply = 10u128; - // Cores being ordered per block - let demand = 19u128; // Calculate a very rudimentary on-chain price from supply / demand - let price = FixedU64::from_rational(demand, supply); + // Supply: Cores available per block + // Demand: Cores being ordered per block + let price = FixedU64::from_rational(10u32, 19u32); // 1.9 DOT per core assert_eq!(price, FixedU64::from_float(1.9)); @@ -567,6 +563,7 @@ mod tests { #[docify::export] #[test] + #[should_panic] fn bad_collection_retrieval() { let my_list = vec![1, 2, 3, 4, 5]; // THIS PANICS! From 5cf16e3fca0384a8fdc32f66a7dd2f6ab3728acd Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:41:49 -0500 Subject: [PATCH 32/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index e22104527fd2..f93202b4014e 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -19,7 +19,7 @@ //! with floating point calculations. //! //! Intentional and predictable design should be our first and foremost -//! property for ensuring a well running, safely designed system. +//! priority for ensuring a well running, safely designed system. //! //! ## Defensive Programming //! From baab75555c549b8c2f855cab87cb46f521f7f169 Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:42:00 -0500 Subject: [PATCH 33/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index f93202b4014e..2068e9cb7852 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -344,7 +344,7 @@ //! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be //! deemed valid. //! -//! For most cases, `Perbill` gives us a reasonable amount of precision for most applications, which +//! For most applications, `Perbill` gives us a reasonable amount of precision, which //! is why we're using it here. //! //! #### Fixed Point Arithmetic with [`PerThing`](sp_arithmetic::PerThing) From 910cb9ddddf809a7525bdbf83ed393be11dc9030 Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:42:08 -0500 Subject: [PATCH 34/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 2068e9cb7852..8f52aea84dc6 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -340,7 +340,7 @@ //! point arithmetic to calculate. //! //! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating -//! point numbers in Substrate development. For this scenario, let's say we are demonstrating a +//! point numbers in development with Substrate. For this scenario, let's say we are demonstrating a //! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be //! deemed valid. //! From 5ff315953ada3dc9d6a92f429ee493190b17b637 Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:42:20 -0500 Subject: [PATCH 35/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 8f52aea84dc6..3fd2885a3533 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -235,7 +235,7 @@ //! //!
//! Solution: Saturating or Checked -//! For Bob's balance problems, using a `saturated_add` or `checked_add` could've mitigated this +//! For Bob's balance problems, using a `saturating_add` or `checked_add` could've mitigated this //! issue. They simply would've reached the upper, or lower bounds, of the particular type for an //! on-chain balance. In other words: Bob's balance would've stayed at the maximum of the Balance //! type.
From 7d64bb641d204a3864b7eb501ac3d9c196d0142b Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:42:30 -0500 Subject: [PATCH 36/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 3fd2885a3533..35ed00f1998b 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -53,7 +53,7 @@ //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to //! panic. It is better to ensure all errors are propagated and handled accordingly in some way. -//! - If a function *can* panic, it usually is prefaced with `unchecked_` to indicate its safety. +//! - If a function *can* panic, it usually is prefaced with `unchecked_` to indicate its unsafety. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) //! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as //! **arithmetic** in the runtime, could present a number of issues [(see more later in this From 6920c5863d8bb48a50938af5b7c780233e047337 Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:43:05 -0500 Subject: [PATCH 37/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 35ed00f1998b..6dffa9604f24 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -45,7 +45,7 @@ //! //! General guidelines: //! -//! - Avoid any writing functions that could explicitly panic. Directly using `unwrap()` for a +//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` for a //! [`Result`] or common errors such as accessing an out of bounds index on a collection (using //! safer methods to access collection types, i.e., `get()`) are both examples of what to //! completely avoid in runtime development. From 570f0cb42ba06ce639dc5c214e90db20417431ed Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:43:36 -0500 Subject: [PATCH 38/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 6dffa9604f24..b12bb358b3be 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -24,7 +24,7 @@ //! ## Defensive Programming //! //! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a particular program to continue -//! running despite unexpected behavior. These unforseen circumstances may +//! running despite unexpected behavior. Unforseen circumstances may //! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for //! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. From eec270055a5ec326082a585215d3fd8ac524b3a3 Mon Sep 17 00:00:00 2001 From: bader y Date: Fri, 12 Jan 2024 16:44:59 -0500 Subject: [PATCH 39/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index b12bb358b3be..0b4eb98bc4af 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -29,7 +29,7 @@ //! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. //! -//! The Polkadot SDK is both built to reflect these principles and to be used accordingly. +//! The Polkadot SDK is both built to reflect these principles and to facilitate their usage. //! //! ## General Practices //! From 2c8c78185db14fc84026a91171dc97fd83063032 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Fri, 12 Jan 2024 16:50:28 -0500 Subject: [PATCH 40/76] address feedback --- .../reference_docs/defensive_programming.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0b4eb98bc4af..2f5ed65e6c8c 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -24,7 +24,7 @@ //! ## Defensive Programming //! //! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a particular program to continue -//! running despite unexpected behavior. Unforseen circumstances may +//! running despite unexpected behavior. Unforeseen circumstances may //! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for //! these circumstances to be accounted for ahead of time and for them to be handled in a more //! graceful manner. @@ -45,10 +45,10 @@ //! //! General guidelines: //! -//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` for a -//! [`Result`] or common errors such as accessing an out of bounds index on a collection (using -//! safer methods to access collection types, i.e., `get()`) are both examples of what to -//! completely avoid in runtime development. +//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` for a +//! [`Result`], or common errors such as accessing an out of bounds index on a collection +//! should not be used. Safer methods to access collection types, i.e., `get()` are available, +//! upon which defensive handling of the resulting [`Option`] can occur. //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to @@ -235,10 +235,10 @@ //! //!
//! Solution: Saturating or Checked -//! For Bob's balance problems, using a `saturating_add` or `checked_add` could've mitigated this -//! issue. They simply would've reached the upper, or lower bounds, of the particular type for an -//! on-chain balance. In other words: Bob's balance would've stayed at the maximum of the Balance -//! type.
+//! For Bob's balance problems, using a `saturating_add` or `checked_add` could've mitigated +//! this issue. They simply would've reached the upper, or lower bounds, of the particular type for +//! an on-chain balance. In other words: Bob's balance would've stayed at the maximum of the +//! Balance type. //! //! #### Alice's 'Underflowed' Balance //! @@ -337,7 +337,7 @@ //! Each of these can be used to construct and represent ratios within our runtime. //! You will find types like [`Perbill`](sp_arithmetic::Perbill) being used often in pallet //! development. [`pallet_referenda`] is a good example of a pallet which makes good use of fixed -//! point arithmetic to calculate. +//! point arithmetic. //! //! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating //! point numbers in development with Substrate. For this scenario, let's say we are demonstrating a From a0a631f24af6c2866a3779d8988d031bce3233a6 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 15 Jan 2024 13:21:05 -0500 Subject: [PATCH 41/76] added example of valid panic --- Cargo.lock | 1 + docs/sdk/Cargo.toml | 1 + .../reference_docs/defensive_programming.rs | 47 +++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 061de8848499..232996198437 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13351,6 +13351,7 @@ dependencies = [ "frame", "kitchensink-runtime", "pallet-aura", + "pallet-babe", "pallet-broker", "pallet-default-config-example", "pallet-examples", diff --git a/docs/sdk/Cargo.toml b/docs/sdk/Cargo.toml index d4cc1de790bd..5596e62bfd32 100644 --- a/docs/sdk/Cargo.toml +++ b/docs/sdk/Cargo.toml @@ -65,6 +65,7 @@ sp-arithmetic = { path = "../../substrate/primitives/arithmetic" } # Misc pallet dependencies pallet-referenda = { path = "../../substrate/frame/referenda" } pallet-broker = { path = "../../substrate/frame/broker" } +pallet-babe = { path = "../../substrate/frame/babe" } [dev-dependencies] diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 2f5ed65e6c8c..048b85b68203 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -24,16 +24,18 @@ //! ## Defensive Programming //! //! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a particular program to continue -//! running despite unexpected behavior. Unforeseen circumstances may -//! cause the program to stop or, in the Rust context, `panic!`. Defensive practices allow for -//! these circumstances to be accounted for ahead of time and for them to be handled in a more -//! graceful manner. +//! running despite unexpected behavior, input or events which may arise in runtime. Normally, +//! unforeseen circumstances may cause the program to stop or, in the Rust context, `panic!`. +//! Defensive practices allow for these circumstances to be accounted for ahead of time and for them +//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant +//! behavior of blockchains. //! -//! The Polkadot SDK is both built to reflect these principles and to facilitate their usage. +//! The Polkadot SDK is both built to reflect these principles and to facilitate their usage +//! accordingly. //! //! ## General Practices //! -//! When developing within the context of the a runtime, there is one golden rule: +//! When developing within the context of the a runtime, there is *one* golden rule: //! //! ***DO NOT PANIC***. There are some exceptions, such as critical operations being actually more //! dangerous than allowing the runtime to continue functioning (block authoring, consensus, etc). @@ -42,13 +44,16 @@ //! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the //! > outer client which deals with telemetry and gossip from other nodes. For more information, //! > read about Substrate's architecture. +//! > It's also important to note that the behavior of the **node** may differ from that of the +//! > **runtime**, which is also why at times, you may see `unwrap()` or other "non-defensive" +//! > behavior taking place. //! //! General guidelines: //! //! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` for a -//! [`Result`], or common errors such as accessing an out of bounds index on a collection -//! should not be used. Safer methods to access collection types, i.e., `get()` are available, -//! upon which defensive handling of the resulting [`Option`] can occur. +//! [`Result`], or common errors such as accessing an out of bounds index on a collection should +//! not be used. Safer methods to access collection types, i.e., `get()` are available, upon which +//! defensive handling of the resulting [`Option`] can occur. //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to @@ -59,6 +64,30 @@ //! **arithmetic** in the runtime, could present a number of issues [(see more later in this //! document)](#integer-overflow). //! +//! ### Examples of when to `panic!` +//! +//! As you traverse through the codebase (particularly in `substrate/frame`, where the majority of +//! runtime code lives), you may notice that there occurrences where `panic!` is used explicitly. +//! This is used when the runtime should stall, rather than keep running, as that is considered +//! safer. Particularly when it comes to mission critical components, such as block authoring, +//! consensus, or other protocol-level dependencies, the unauthorized nature of a node may actually +//! cause harm to the network, and thus stalling would be the better option. +//! +//! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to +//! participate if it is disabled (see: [frame::traits::DisabledValidators]): +//! +//! ```rust +//! if T::DisabledValidators::is_disabled(authority_index) { +//! panic!( +//! "Validator with index {:?} is disabled and should not be attempting to author blocks.", +//! authority_index, +//! ); +//! } +//! ``` +//! +//! There are other such examples throughout various pallets, mostly those who are crucial to the +//! blockchain's function. +//! //! ### Defensive Traits //! //! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which From bec3672c634bcf97891f6c28fb3c77335d6e8bbf Mon Sep 17 00:00:00 2001 From: bader y Date: Mon, 15 Jan 2024 13:21:56 -0500 Subject: [PATCH 42/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Francisco Aguirre --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 048b85b68203..78c1810a297f 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! As our runtime should _never_ panic; this includes carefully handling [`Result`]/[`Option`] +//! As our runtime should _never_ panic, we should carefully handle [`Result`]/[`Option`] //! types, eliminating the possibility of integer overflows, converting between number types, or //! even replacing floating point usage with fixed point arithmetic to mitigate issues that come //! with floating point calculations. From 94b32c59b25d0068e4a72e86405ac9fa92465348 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:25:00 -0500 Subject: [PATCH 43/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 78c1810a297f..65508bd9bceb 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -27,7 +27,7 @@ //! running despite unexpected behavior, input or events which may arise in runtime. Normally, //! unforeseen circumstances may cause the program to stop or, in the Rust context, `panic!`. //! Defensive practices allow for these circumstances to be accounted for ahead of time and for them -//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant +//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant and deterministic //! behavior of blockchains. //! //! The Polkadot SDK is both built to reflect these principles and to facilitate their usage From fa707bb965e7894c2fc09b5cc703b22a7100263b Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:25:09 -0500 Subject: [PATCH 44/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 65508bd9bceb..3e60fe3a20a9 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -30,7 +30,7 @@ //! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant and deterministic //! behavior of blockchains. //! -//! The Polkadot SDK is both built to reflect these principles and to facilitate their usage +//! The Polkadot SDK is built to reflect these principles and to facilitate their usage //! accordingly. //! //! ## General Practices From 3cb2f6703ee42e626db44f71b078741f71a426a8 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:25:23 -0500 Subject: [PATCH 45/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 3e60fe3a20a9..815e5ae02ebb 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -37,8 +37,7 @@ //! //! When developing within the context of the a runtime, there is *one* golden rule: //! -//! ***DO NOT PANIC***. There are some exceptions, such as critical operations being actually more -//! dangerous than allowing the runtime to continue functioning (block authoring, consensus, etc). +//! ***DO NOT PANIC***. There are some exceptions, which will be covered later on in this doc. //! //! > It's important to make the differentiation between the **runtime** and **node**. The runtime //! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the From 142190530b4085ffcad8a0754c6a00c48682d79a Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:25:37 -0500 Subject: [PATCH 46/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 815e5ae02ebb..0636902845b4 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -43,9 +43,9 @@ //! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the //! > outer client which deals with telemetry and gossip from other nodes. For more information, //! > read about Substrate's architecture. -//! > It's also important to note that the behavior of the **node** may differ from that of the -//! > **runtime**, which is also why at times, you may see `unwrap()` or other "non-defensive" -//! > behavior taking place. +//! > It's also important to note that the criticality of the **node** is slightly lesser than that of the +//! > **runtime**, which is why in a few places of the node's code repository, you may see `unwrap()` or other "non-defensive" +//! > code instances. //! //! General guidelines: //! From 5ad43f9d49994a240a7e80fe643c16b19478fa22 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:25:53 -0500 Subject: [PATCH 47/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0636902845b4..7a4cad0b6cdf 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -49,10 +49,10 @@ //! //! General guidelines: //! -//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` for a -//! [`Result`], or common errors such as accessing an out of bounds index on a collection should -//! not be used. Safer methods to access collection types, i.e., `get()` are available, upon which -//! defensive handling of the resulting [`Option`] can occur. +//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` on a +//! [`Result`], or accessing an out-of-bounds index on a collection, should +//! be avoided. Safer methods to access collection types, i.e., `get()` which allow +//! defensive handling of the resulting [`Option`] are recommended to be used. //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to From 5a0393feecc7574c0ed5a213006ffa15c5f8dd2f Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:26:00 -0500 Subject: [PATCH 48/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 7a4cad0b6cdf..2a781d1fd003 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -245,7 +245,7 @@ //! #### The problem with 'default' wrapped operations //! //! **Wrapped operations** cause the overflow to wrap around to either the maximum or minimum of -//! that type. Imagine this in the context of a blockchain, where there are balances, voting +//! that type. Imagine this in the context of a blockchain, where there are account balances, voting //! counters, nonces for transactions, and other aspects of a blockchain. //! //! Some of these mechanisms can be more critical than others. It's for this reason that we may From 89926c1eafa78eb68493491704f26377a73fe192 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:26:12 -0500 Subject: [PATCH 49/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 2a781d1fd003..9f4a2e9e4e4f 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -248,9 +248,6 @@ //! that type. Imagine this in the context of a blockchain, where there are account balances, voting //! counters, nonces for transactions, and other aspects of a blockchain. //! -//! Some of these mechanisms can be more critical than others. It's for this reason that we may -//! consider some other ways of dealing with runtime arithmetic, such as saturated or checked -//! operations, that won't carry these potential consequences. //! //! While it may seem trivial, choosing how to handle numbers is quite important. As a thought //! exercise, here are some scenarios of which will shed more light on when to use which. From 023fa13368bf31036b9e45aa1d025a821099e1eb Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:26:19 -0500 Subject: [PATCH 50/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 9f4a2e9e4e4f..e88e1a3d4578 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -284,7 +284,7 @@ //! as the notion of balances, are often shared across multiple pallets - meaning these small //! changes can make a big difference in outcome. //! -//! #### Proposals' ID Overwrite +//! #### Proposal ID Overwrite //! //! The type for counting the number of proposals on-chain is represented by a `u8` number, called //! `proposals_count`. Every time a new proposal is added to the system, this number increases. With From f2da603bff2b8a2fcdea2b25014738e905f0dc62 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:26:29 -0500 Subject: [PATCH 51/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index e88e1a3d4578..0a992a5e50ea 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -289,7 +289,7 @@ //! The type for counting the number of proposals on-chain is represented by a `u8` number, called //! `proposals_count`. Every time a new proposal is added to the system, this number increases. With //! the proposal pallet being high in usage, it has reached `u8::MAX`'s limit of `255`, causing -//! `proposals_count` to go to `0`. Unfortunately, this resulted in new proposals overwriting old +//! `proposals_count` to go to `0`. Unfortunately, this results in new proposals overwriting old //! ones, effectively erasing any notion of past proposals! //! //!
From b07facb2ce51fe5ac169374f1eeb0d3f791209bf Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:27:40 -0500 Subject: [PATCH 52/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0a992a5e50ea..90042d8fd506 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -56,7 +56,7 @@ //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to -//! panic. It is better to ensure all errors are propagated and handled accordingly in some way. +//! panic. It is important to ensure all possible errors are propagated and handled effectively. //! - If a function *can* panic, it usually is prefaced with `unchecked_` to indicate its unsafety. //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) //! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as From f832eed0bdc4bb47584ec5ef63c324992eed86d2 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:27:57 -0500 Subject: [PATCH 53/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 90042d8fd506..ffc8ee84c919 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -61,7 +61,7 @@ //! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) //! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as //! **arithmetic** in the runtime, could present a number of issues [(see more later in this -//! document)](#integer-overflow). +//! document)](#integer-overflow). Use checked arithmetic wherever possible. //! //! ### Examples of when to `panic!` //! From 2114c715a1f0e19f358ffd1991a5bb2e001e27da Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:30:53 -0500 Subject: [PATCH 54/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index ffc8ee84c919..591f24212a55 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -84,8 +84,8 @@ //! } //! ``` //! -//! There are other such examples throughout various pallets, mostly those who are crucial to the -//! blockchain's function. +//! There are other such examples in various pallets, mostly those that are crucial to the +//! blockchain's functionality. //! //! ### Defensive Traits //! From b4b12e19c1ddb9c7bedb88ad9a3b1eefb742268e Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:31:07 -0500 Subject: [PATCH 55/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 591f24212a55..4d1c2b90da74 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -124,7 +124,7 @@ //! (from the `MAX` back to zero). //! //! In the context of runtime development, we don't always have control over what is being supplied -//! as a parameter. For example, even this simple adding function could present one of two outcomes +//! as a parameter. For example, even this simple add function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! From 0b25f7cc6f85c5e2f3ab2d21c803f39aa695c3a7 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:31:22 -0500 Subject: [PATCH 56/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 4d1c2b90da74..ddfef10f839c 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -139,7 +139,7 @@ //! made obvious, especially in the context of blockchain development, where unsafe arithmetic could //! produce unexpected consequences like a user balance over or underflowing. //! -//! Luckily, there are ways to both represent and handle these scenarios depending on our specific +//! Fortunately, there are ways to both represent and handle these scenarios depending on our specific //! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. //! //! ## Infallible Arithmetic From 07fe21fdb93b7794fe9a04571b12259a7525bcb9 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:31:53 -0500 Subject: [PATCH 57/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index ddfef10f839c..0043fe351edd 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -239,8 +239,8 @@ //! //! //! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of -//! dividing/modulo by zero at any point should be mitigated. One should be, instead, opting for a -//! `checked_*` method in order to introduce safe arithmetic in their code. +//! dividing/modulo by zero at any point should be mitigated. One should be opting for a +//! `checked_*` method to introduce safe arithmetic in their code. //! //! #### The problem with 'default' wrapped operations //! From d294b6a19327c14bcd476a90dbf9df56d5843004 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:32:07 -0500 Subject: [PATCH 58/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0043fe351edd..004ddb32e2e7 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -151,7 +151,7 @@ //! potential for inaccuracy, rounding errors, or other unexpected behavior. //! //! Using floating point number types in the runtime should be avoided, -//! as a single nondeterministic result could cause chaos for consensus along with the +//! as a single non-deterministic result could cause chaos for blockchain consensus along with the //! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! //! The following methods demonstrate different ways one can handle numbers safely natively in Rust, From 75ba359ac14c47ec2f8a5e8f7ddae58297c5e3f2 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:32:18 -0500 Subject: [PATCH 59/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 004ddb32e2e7..37cb2ab4b8f6 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -154,7 +154,7 @@ //! as a single non-deterministic result could cause chaos for blockchain consensus along with the //! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! -//! The following methods demonstrate different ways one can handle numbers safely natively in Rust, +//! The following methods demonstrate different ways one can handle numbers natively in Rust in a safe manner, //! without fear of panic or unexpected behavior from wrapping. //! //! ### Checked Arithmetic From f46c4ae3e19804676955b50555b7427eddb8f2ca Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:32:34 -0500 Subject: [PATCH 60/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 37cb2ab4b8f6..0892ba0911cc 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -159,8 +159,8 @@ //! //! ### Checked Arithmetic //! -//! **Checked operations** utilize an `Option` as a return type. This allows for simple pattern -//! matching to catch any unexpected behavior in the event of an overflow. +//! **Checked operations** utilize an `Option` as a return type. This allows for +//! catching any unexpected behavior in the event of an overflow through simple pattern matching. //! //! This is an example of a valid operation: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", checked_add_example)] From d3c4b7c2f0a50859e22279310414ddaf486fe502 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:32:49 -0500 Subject: [PATCH 61/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 0892ba0911cc..9746c87adce5 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -165,7 +165,7 @@ //! This is an example of a valid operation: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", checked_add_example)] //! -//! This is an example of an invalid operation, in this case, a simulated integer overflow, which +//! This is an example of an invalid operation. In this case, a simulated integer overflow, which //! would simply result in `None`: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", From ce75101bee1c0c0be150b0b9921c2c5bfe23f8b5 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:36:39 -0500 Subject: [PATCH 62/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 9746c87adce5..b3f5d5e07010 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -177,7 +177,7 @@ //! handled accordingly (`Some` and `None`). //! //! In a practical context, the resulting [`Option`] should be handled accordingly. The following -//! conventions can be seen from the within the Polkadot SDK, where it can be handled in one of +//! conventions can be seen within the Polkadot SDK, where it is handled in //! two ways: //! //! - As an [`Option`], using the `if let` / `if` or `match` From 3b68daa46314e93af296353bb0f34dd23cc97094 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:36:57 -0500 Subject: [PATCH 63/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index b3f5d5e07010..5a8c7acd6d59 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -201,7 +201,7 @@ //! #### Handling via Result - Less Verbose //! //! In the Polkadot SDK codebase, you may see checked operations being handled as a [`Result`] via -//! `ok_or`. This is a less verbose way of expressing the above. Which to use often boils down to +//! `ok_or`. This is a less verbose way of expressing the above. This usage often boils down to //! the developer's preference: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", From 679171bceabc90fe2271aef5a14689f5cfa69624 Mon Sep 17 00:00:00 2001 From: bader y Date: Tue, 13 Feb 2024 10:37:11 -0500 Subject: [PATCH 64/76] Update docs/sdk/src/reference_docs/defensive_programming.rs Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com> --- docs/sdk/src/reference_docs/defensive_programming.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 5a8c7acd6d59..58a4e5604f67 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -210,7 +210,7 @@ //! //! ### Saturating Operations //! -//! Saturating a number limits it to the type's upper or lower bound, no matter the integer would +//! Saturating a number limits it to the type's upper or lower bound, even if the integer were to //! overflow in runtime. For example, adding to `u32::MAX` would simply limit itself to `u32::MAX`: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", From fc45d7015620285803438ed8fe88c20fc70abc5c Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Thu, 7 Mar 2024 15:38:20 -0500 Subject: [PATCH 65/76] (inital) move things around --- Cargo.lock | 7 +- .../reference_docs/defensive_programming.rs | 223 +---------------- substrate/primitives/arithmetic/Cargo.toml | 1 + .../primitives/arithmetic/src/fixed_point.rs | 26 ++ substrate/primitives/arithmetic/src/lib.rs | 226 +++++++++++++----- .../primitives/arithmetic/src/per_things.rs | 48 ++++ 6 files changed, 259 insertions(+), 272 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 917179c781e7..184f7ee6dc39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13411,19 +13411,19 @@ dependencies = [ "frame-system", "kitchensink-runtime", "pallet-aura", - "pallet-babe", - "pallet-broker", "pallet-authorship", + "pallet-babe", "pallet-balances", + "pallet-broker", "pallet-collective", "pallet-default-config-example", "pallet-democracy", "pallet-example-offchain-worker", "pallet-example-single-block-migrations", "pallet-examples", - "pallet-referenda", "pallet-multisig", "pallet-proxy", + "pallet-referenda", "pallet-scheduler", "pallet-timestamp", "pallet-transaction-payment", @@ -18452,6 +18452,7 @@ name = "sp-arithmetic" version = "23.0.0" dependencies = [ "criterion 0.4.0", + "docify 0.2.7", "integer-sqrt", "num-traits", "parity-scale-codec", diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 58a4e5604f67..3867cf307ec5 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -27,8 +27,8 @@ //! running despite unexpected behavior, input or events which may arise in runtime. Normally, //! unforeseen circumstances may cause the program to stop or, in the Rust context, `panic!`. //! Defensive practices allow for these circumstances to be accounted for ahead of time and for them -//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant and deterministic -//! behavior of blockchains. +//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant and +//! deterministic behavior of blockchains. //! //! The Polkadot SDK is built to reflect these principles and to facilitate their usage //! accordingly. @@ -43,16 +43,18 @@ //! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the //! > outer client which deals with telemetry and gossip from other nodes. For more information, //! > read about Substrate's architecture. -//! > It's also important to note that the criticality of the **node** is slightly lesser than that of the -//! > **runtime**, which is why in a few places of the node's code repository, you may see `unwrap()` or other "non-defensive" +//! > It's also important to note that the criticality of the **node** is slightly lesser than that +//! > of the +//! > **runtime**, which is why in a few places of the node's code repository, you may see +//! > `unwrap()` or other "non-defensive" //! > code instances. //! //! General guidelines: //! //! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` on a -//! [`Result`], or accessing an out-of-bounds index on a collection, should -//! be avoided. Safer methods to access collection types, i.e., `get()` which allow -//! defensive handling of the resulting [`Option`] are recommended to be used. +//! [`Result`], or accessing an out-of-bounds index on a collection, should be avoided. Safer +//! methods to access collection types, i.e., `get()` which allow defensive handling of the +//! resulting [`Option`] are recommended to be used. //! - It may be acceptable to use `except()`, but only if one is completely certain (and has //! performed a check beforehand) that a value won't panic upon unwrapping. Even this is //! discouraged, however, as future changes to that function could then cause that statement to @@ -139,8 +141,8 @@ //! made obvious, especially in the context of blockchain development, where unsafe arithmetic could //! produce unexpected consequences like a user balance over or underflowing. //! -//! Fortunately, there are ways to both represent and handle these scenarios depending on our specific -//! use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. +//! Fortunately, there are ways to both represent and handle these scenarios depending on our +//! specific use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. //! //! ## Infallible Arithmetic //! @@ -154,8 +156,8 @@ //! as a single non-deterministic result could cause chaos for blockchain consensus along with the //! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! -//! The following methods demonstrate different ways one can handle numbers natively in Rust in a safe manner, -//! without fear of panic or unexpected behavior from wrapping. +//! The following methods demonstrate different ways one can handle numbers natively in Rust in a +//! safe manner, without fear of panic or unexpected behavior from wrapping. //! //! ### Checked Arithmetic //! @@ -237,7 +239,6 @@ //! 2. **Saturating** operations - limited to the lower and upper bounds of a number type //! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type //! -//! //! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of //! dividing/modulo by zero at any point should be mitigated. One should be opting for a //! `checked_*` method to introduce safe arithmetic in their code. @@ -313,94 +314,6 @@ //! //! ### Decision Chart: When to use which? #![doc = simple_mermaid::mermaid!("../../../mermaid/integer_operation_decision.mmd")] -//! ## Fixed Point Arithmetic -//! -//! The following code uses types from [`sp_arithmetic`]. -//! -//! Fixed point arithmetic solves the aforementioned problems of dealing with the uncertain -//! nature of floating point numbers. Rather than use a radix point (`0.80`), a type which -//! _represents_ a floating point number in base 10, i.e., a **fixed point number**, can be used -//! instead. -//! -//! For use cases which operate within the range of `[0, 1]` types that implement -//! [`PerThing`](sp_arithmetic::PerThing) are used: -//! - **[`Perbill`](sp_arithmetic::Perbill), parts of a billion** -#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", perbill_example)] -//! - **[`Percent`](sp_arithmetic::Percent), parts of a hundred** -#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_example)] -//! -//! Note that `190 / 400 = 0.475`, and that `Percent` represents it as a _rounded down_, fixed point -//! number (`47`). Unlike primitive types, types that implement -//! [`PerThing`](sp_arithmetic::PerThing) will also not overflow, and are therefore safe to use. -//! They adopt the same behavior that a saturated calculation would provide, meaning that if one is -//! to go over "100%", it wouldn't overflow, but simply stop at the upper or lower bound. -//! -//! For use cases which require precision beyond the range of `[0, 1]`, there are a number of other -//! fixed-point types to use: -//! -//! - [`FixedU64`](sp_arithmetic::FixedU64) and [`FixedI64`](sp_arithmetic::FixedI64) -//! - [`FixedI128`](sp_arithmetic::FixedU128) and [`FixedU128`](sp_arithmetic::FixedI128) -//! -//! Similar to types that implement [`PerThing`](sp_arithmetic::PerThing), these are also -//! fixed-point types, however, they are able to represent larger fractions: -#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", fixed_u64)] -//! -//! Let's now explore these types in practice, and how they may be used with pallets to perform -//! safer calculations in the runtime. -//! -//! ### 'PerThing' In Practice -//! -//! [`sp_arithmetic`] contains a trait called [`PerThing`](sp_arithmetic::PerThing), allowing a -//! custom type to be implemented specifically for fixed point arithmetic. While a number of -//! fixed-point types are introduced, let's focus on a few specific examples that implement -//! [`PerThing`](sp_arithmetic::PerThing): -//! -//! - [`Percent`](sp_arithmetic::Percent) - parts of one hundred. -//! - [`Permill`](sp_arithmetic::Permill) - parts of a million. -//! - [`Perbill`](sp_arithmetic::Perbill) - parts of a billion. -//! -//! Each of these can be used to construct and represent ratios within our runtime. -//! You will find types like [`Perbill`](sp_arithmetic::Perbill) being used often in pallet -//! development. [`pallet_referenda`] is a good example of a pallet which makes good use of fixed -//! point arithmetic. -//! -//! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating -//! point numbers in development with Substrate. For this scenario, let's say we are demonstrating a -//! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be -//! deemed valid. -//! -//! For most applications, `Perbill` gives us a reasonable amount of precision, which -//! is why we're using it here. -//! -//! #### Fixed Point Arithmetic with [`PerThing`](sp_arithmetic::PerThing) -//! -//! As stated, one can also perform mathematics using these types directly. For example, finding the -//! percentage of a particular item: -#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", percent_mult)] -//! -//! ### Fixed Point Types in Practice -//! -//! As said earlier, if one needs to exceed the value of one, then -//! [`FixedU64`](sp_arithmetic::FixedU64) (and its signed and `u128` counterparts) can be utilized. -//! Take for example this very rudimentary pricing mechanism, where we wish to calculate the demand -//! / supply to get a price for some on-chain compute: -#![doc = docify::embed!( - "./src/reference_docs/defensive_programming.rs", - fixed_u64_block_computation_example -)] -//! -//! For a much more comprehensive example, be sure to look at the source for [`pallet_broker`]. -//! -//! #### Fixed Point Types in Practice -//! -//! Just as with [`PerThing`](sp_arithmetic::PerThing), you can also perform regular mathematical -//! expressions: -#![doc = docify::embed!( - "./src/reference_docs/defensive_programming.rs", - fixed_u64_operation_example -)] -//! -//! //! ## Other Resources //! //! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) @@ -493,116 +406,6 @@ mod tests { let add = u32::MAX.saturating_add(10); assert_eq!(add, u32::MAX) } - #[docify::export] - #[test] - fn percent_mult() { - let percent = Percent::from_rational(5u32, 100u32); // aka, 5% - let five_percent_of_100 = percent * 100u32; // 5% of 100 is 5. - assert_eq!(five_percent_of_100, 5) - } - #[docify::export] - #[test] - fn perbill_example() { - let p = Perbill::from_percent(80); - // 800000000 bil, or a representative of 0.800000000. - // Precision is in the billions place. - assert_eq!(p.deconstruct(), 800000000); - } - - #[docify::export] - #[test] - fn percent_example() { - let percent = Percent::from_rational(190u32, 400u32); - assert_eq!(percent.deconstruct(), 47); - } - - #[docify::export] - #[test] - fn fixed_u64_block_computation_example() { - // Calculate a very rudimentary on-chain price from supply / demand - // Supply: Cores available per block - // Demand: Cores being ordered per block - let price = FixedU64::from_rational(5u128, 10u128); - - // 0.5 DOT per core - assert_eq!(price, FixedU64::from_float(0.5)); - - // Now, the story has changed - lots of demand means we buy as many cores as there - // available. This also means that price goes up! For the sake of simplicity, we don't care - // about who gets a core - just about our very simple price model - - // Calculate a very rudimentary on-chain price from supply / demand - // Supply: Cores available per block - // Demand: Cores being ordered per block - let price = FixedU64::from_rational(10u32, 19u32); - - // 1.9 DOT per core - assert_eq!(price, FixedU64::from_float(1.9)); - } - - #[docify::export] - #[test] - fn fixed_u64() { - // The difference between this and perthings is perthings operates within the relam of [0, - // 1] In cases where we need > 1, we can used fixed types such as FixedU64 - - let rational_1 = FixedU64::from_rational(10, 5); //" 200%" aka 2. - let rational_2 = - FixedU64::from_rational_with_rounding(5, 10, sp_arithmetic::Rounding::Down); // "50%" aka 0.50... - - assert_eq!(rational_1, (2u64).into()); - assert_eq!(rational_2.into_perbill(), Perbill::from_float(0.5)); - } - - #[docify::export] - #[test] - fn fixed_u64_operation_example() { - let rational_1 = FixedU64::from_rational(10, 5); // "200%" aka 2. - let rational_2 = FixedU64::from_rational(8, 5); // "160%" aka 1.6. - - let addition = rational_1 + rational_2; - let multiplication = rational_1 * rational_2; - let division = rational_1 / rational_2; - let subtraction = rational_1 - rational_2; - - assert_eq!(addition, FixedU64::from_float(3.6)); - assert_eq!(multiplication, FixedU64::from_float(3.2)); - assert_eq!(division, FixedU64::from_float(1.25)); - assert_eq!(subtraction, FixedU64::from_float(0.4)); - } - - #[docify::export] - #[test] - fn bad_unwrap() { - let some_result: Result = Ok(10); - assert_eq!(some_result.unwrap(), 10); - } - - #[docify::export] - #[test] - fn good_unwrap() { - let some_result: Result = Err("Error"); - assert_eq!(some_result.unwrap_or_default(), 0); - assert_eq!(some_result.unwrap_or(10), 10); - } - - #[docify::export] - #[test] - #[should_panic] - fn bad_collection_retrieval() { - let my_list = vec![1, 2, 3, 4, 5]; - // THIS PANICS! - // Indexing on heap allocated values, i.e., vec, can be unsafe! - assert_eq!(my_list[5], 6) - } - - #[docify::export] - #[test] - fn good_collection_retrieval() { - let my_list = vec![1, 2, 3, 4, 5]; - // Rust includes `.get`, returning Option - so lets use that: - assert_eq!(my_list.get(5), None) - } #[docify::export] #[test] diff --git a/substrate/primitives/arithmetic/Cargo.toml b/substrate/primitives/arithmetic/Cargo.toml index 301821ad6893..120edd06a660 100644 --- a/substrate/primitives/arithmetic/Cargo.toml +++ b/substrate/primitives/arithmetic/Cargo.toml @@ -27,6 +27,7 @@ scale-info = { version = "2.10.0", default-features = false, features = ["derive serde = { features = ["alloc", "derive"], optional = true, workspace = true } static_assertions = "1.1.0" sp-std = { path = "../std", default-features = false } +docify = "0.2.7" [dev-dependencies] criterion = "0.4.0" diff --git a/substrate/primitives/arithmetic/src/fixed_point.rs b/substrate/primitives/arithmetic/src/fixed_point.rs index ce14d2957b5e..4ab22537b5b0 100644 --- a/substrate/primitives/arithmetic/src/fixed_point.rs +++ b/substrate/primitives/arithmetic/src/fixed_point.rs @@ -16,6 +16,32 @@ // limitations under the License. //! Decimal Fixed Point implementations for Substrate runtime. +//! Similar to types that implement [`PerThing`](PerThing), these are also +//! fixed-point types, however, they are able to represent larger fractions: +#![doc = docify::embed!("./src/lib.rs", fixed_u64)] +//! +//! ### Fixed Point Types in Practice +//! +//! As said earlier, if one needs to exceed the value of one, then +//! [`FixedU64`](FixedU64) (and its signed and `u128` counterparts) can be utilized. +//! Take for example this very rudimentary pricing mechanism, where we wish to calculate the demand +//! / supply to get a price for some on-chain compute: +#![doc = docify::embed!( + "./src/lib.rs", + fixed_u64_block_computation_example +)] +//! +//! For a much more comprehensive example, be sure to look at the source for pallet_broker. +//! +//! #### Fixed Point Types in Practice +//! +//! Just as with [`PerThing`](PerThing), you can also perform regular mathematical +//! expressions: +#![doc = docify::embed!( + "./src/lib.rs", + fixed_u64_operation_example +)] +//! use crate::{ helpers_128bit::{multiply_by_rational_with_rounding, sqrt}, diff --git a/substrate/primitives/arithmetic/src/lib.rs b/substrate/primitives/arithmetic/src/lib.rs index 900f0b75c3bf..4891da767b56 100644 --- a/substrate/primitives/arithmetic/src/lib.rs +++ b/substrate/primitives/arithmetic/src/lib.rs @@ -41,22 +41,38 @@ pub mod rational; pub mod traits; pub use fixed_point::{ - FixedI128, FixedI64, FixedPointNumber, FixedPointOperand, FixedU128, FixedU64, + FixedI128, + FixedI64, + FixedPointNumber, + FixedPointOperand, + FixedU128, + FixedU64, }; pub use per_things::{ - InnerOf, MultiplyArg, PerThing, PerU16, Perbill, Percent, Permill, Perquintill, RationalArg, - ReciprocalArg, Rounding, SignedRounding, UpperOf, + InnerOf, + MultiplyArg, + PerThing, + PerU16, + Perbill, + Percent, + Permill, + Perquintill, + RationalArg, + ReciprocalArg, + Rounding, + SignedRounding, + UpperOf, }; -pub use rational::{MultiplyRational, Rational128, RationalInfinite}; +pub use rational::{ MultiplyRational, Rational128, RationalInfinite }; -use sp_std::{cmp::Ordering, fmt::Debug, prelude::*}; -use traits::{BaseArithmetic, One, SaturatedConversion, Unsigned, Zero}; +use sp_std::{ cmp::Ordering, fmt::Debug, prelude::* }; +use traits::{ BaseArithmetic, One, SaturatedConversion, Unsigned, Zero }; -use codec::{Decode, Encode, MaxEncodedLen}; +use codec::{ Decode, Encode, MaxEncodedLen }; use scale_info::TypeInfo; #[cfg(feature = "serde")] -use serde::{Deserialize, Serialize}; +use serde::{ Deserialize, Serialize }; /// Arithmetic errors. #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)] @@ -91,14 +107,14 @@ pub trait ThresholdOrd { fn tcmp(&self, other: &T, threshold: T) -> Ordering; } -impl ThresholdOrd for T -where - T: Ord + PartialOrd + Copy + Clone + traits::Zero + traits::Saturating, +impl ThresholdOrd + for T + where T: Ord + PartialOrd + Copy + Clone + traits::Zero + traits::Saturating { fn tcmp(&self, other: &T, threshold: T) -> Ordering { // early exit. if threshold.is_zero() { - return self.cmp(other) + return self.cmp(other); } let upper_bound = other.saturating_add(threshold); @@ -146,15 +162,22 @@ impl_normalize_for_numeric!(u8, u16, u32, u64, u128); impl Normalizable

for Vec

{ fn normalize(&self, targeted_sum: P) -> Result, &'static str> { - let uppers = self.iter().map(|p| >::from(p.deconstruct())).collect::>(); - - let normalized = - normalize(uppers.as_ref(), >::from(targeted_sum.deconstruct()))?; - - Ok(normalized - .into_iter() - .map(|i: UpperOf

| P::from_parts(i.saturated_into::())) - .collect()) + let uppers = self + .iter() + .map(|p| >::from(p.deconstruct())) + .collect::>(); + + let normalized = normalize( + uppers.as_ref(), + >::from(targeted_sum.deconstruct()) + )?; + + Ok( + normalized + .into_iter() + .map(|i: UpperOf

| P::from_parts(i.saturated_into::())) + .collect() + ) } } @@ -188,8 +211,7 @@ impl Normalizable

for Vec

{ /// /// * This proof is used in the implementation as well. pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str> -where - T: Clone + Copy + Ord + BaseArithmetic + Unsigned + Debug, + where T: Clone + Copy + Ord + BaseArithmetic + Unsigned + Debug { // compute sum and return error if failed. let mut sum = T::zero(); @@ -203,12 +225,12 @@ where // Nothing to do here. if count.is_zero() { - return Ok(Vec::::new()) + return Ok(Vec::::new()); } let diff = targeted_sum.max(sum) - targeted_sum.min(sum); if diff.is_zero() { - return Ok(input.to_vec()) + return Ok(input.to_vec()); } let needs_bump = targeted_sum > sum; @@ -230,8 +252,7 @@ where if !per_round.is_zero() { for _ in 0..count { - output_with_idx[min_index].1 = output_with_idx[min_index] - .1 + output_with_idx[min_index].1 = output_with_idx[min_index].1 .checked_add(&per_round) .expect("Proof provided in the module doc; qed."); if output_with_idx[min_index].1 >= threshold { @@ -243,15 +264,14 @@ where // continue with the previous min_index while !leftover.is_zero() { - output_with_idx[min_index].1 = output_with_idx[min_index] - .1 + output_with_idx[min_index].1 = output_with_idx[min_index].1 .checked_add(&T::one()) .expect("Proof provided in the module doc; qed."); if output_with_idx[min_index].1 >= threshold { min_index += 1; min_index %= count; } - leftover -= One::one() + leftover -= One::one(); } } else { // must decrease the stakes a bit. decrement from the max element. index of maximum is now @@ -261,13 +281,13 @@ where // at this threshold we move to next index. let threshold = output_with_idx .first() - .expect("length of input is greater than zero; it must have a first; qed") - .1; + .expect("length of input is greater than zero; it must have a first; qed").1; if !per_round.is_zero() { for _ in 0..count { - output_with_idx[max_index].1 = - output_with_idx[max_index].1.checked_sub(&per_round).unwrap_or_else(|| { + output_with_idx[max_index].1 = output_with_idx[max_index].1 + .checked_sub(&per_round) + .unwrap_or_else(|| { let remainder = per_round - output_with_idx[max_index].1; leftover += remainder; output_with_idx[max_index].1.saturating_sub(per_round) @@ -285,7 +305,7 @@ where if output_with_idx[max_index].1 <= threshold { max_index = max_index.checked_sub(1).unwrap_or(count - 1); } - leftover -= One::one() + leftover -= One::one(); } else { max_index = max_index.checked_sub(1).unwrap_or(count - 1); } @@ -297,12 +317,17 @@ where targeted_sum, "sum({:?}) != {:?}", output_with_idx, - targeted_sum, + targeted_sum ); // sort again based on the original index. output_with_idx.sort_by_key(|x| x.0); - Ok(output_with_idx.into_iter().map(|(_, t)| t).collect()) + Ok( + output_with_idx + .into_iter() + .map(|(_, t)| t) + .collect() + ) } #[cfg(test)] @@ -353,7 +378,7 @@ mod normalize_tests { vec![ Perbill::from_parts(333333334), Perbill::from_parts(333333333), - Perbill::from_parts(333333333), + Perbill::from_parts(333333333) ] ); @@ -364,7 +389,7 @@ mod normalize_tests { vec![ Perbill::from_parts(316666668), Perbill::from_parts(383333332), - Perbill::from_parts(300000000), + Perbill::from_parts(300000000) ] ); } @@ -375,13 +400,13 @@ mod normalize_tests { // could have a situation where the sum cannot be calculated in the inner type. Calculating // using the upper type of the per_thing should assure this to be okay. assert_eq!( - vec![PerU16::from_percent(40), PerU16::from_percent(40), PerU16::from_percent(40),] + vec![PerU16::from_percent(40), PerU16::from_percent(40), PerU16::from_percent(40)] .normalize(PerU16::one()) .unwrap(), vec![ PerU16::from_parts(21845), // 33% PerU16::from_parts(21845), // 33% - PerU16::from_parts(21845), // 33% + PerU16::from_parts(21845) // 33% ] ); } @@ -425,6 +450,89 @@ mod normalize_tests { } } +mod per_and_fixed_examples { + #[docify::export] + #[test] + fn percent_mult() { + let percent = Percent::from_rational(5u32, 100u32); // aka, 5% + let five_percent_of_100 = percent * 100u32; // 5% of 100 is 5. + assert_eq!(five_percent_of_100, 5) + } + #[docify::export] + #[test] + fn perbill_example() { + let p = Perbill::from_percent(80); + // 800000000 bil, or a representative of 0.800000000. + // Precision is in the billions place. + assert_eq!(p.deconstruct(), 800000000); + } + + #[docify::export] + #[test] + fn percent_example() { + let percent = Percent::from_rational(190u32, 400u32); + assert_eq!(percent.deconstruct(), 47); + } + + #[docify::export] + #[test] + fn fixed_u64_block_computation_example() { + // Calculate a very rudimentary on-chain price from supply / demand + // Supply: Cores available per block + // Demand: Cores being ordered per block + let price = FixedU64::from_rational(5u128, 10u128); + + // 0.5 DOT per core + assert_eq!(price, FixedU64::from_float(0.5)); + + // Now, the story has changed - lots of demand means we buy as many cores as there + // available. This also means that price goes up! For the sake of simplicity, we don't care + // about who gets a core - just about our very simple price model + + // Calculate a very rudimentary on-chain price from supply / demand + // Supply: Cores available per block + // Demand: Cores being ordered per block + let price = FixedU64::from_rational(10u32, 19u32); + + // 1.9 DOT per core + assert_eq!(price, FixedU64::from_float(1.9)); + } + + #[docify::export] + #[test] + fn fixed_u64() { + // The difference between this and perthings is perthings operates within the relam of [0, + // 1] In cases where we need > 1, we can used fixed types such as FixedU64 + + let rational_1 = FixedU64::from_rational(10, 5); //" 200%" aka 2. + let rational_2 = FixedU64::from_rational_with_rounding( + 5, + 10, + Rounding::Down + ); // "50%" aka 0.50... + + assert_eq!(rational_1, (2u64).into()); + assert_eq!(rational_2.into_perbill(), Perbill::from_float(0.5)); + } + + #[docify::export] + #[test] + fn fixed_u64_operation_example() { + let rational_1 = FixedU64::from_rational(10, 5); // "200%" aka 2. + let rational_2 = FixedU64::from_rational(8, 5); // "160%" aka 1.6. + + let addition = rational_1 + rational_2; + let multiplication = rational_1 * rational_2; + let division = rational_1 / rational_2; + let subtraction = rational_1 - rational_2; + + assert_eq!(addition, FixedU64::from_float(3.6)); + assert_eq!(multiplication, FixedU64::from_float(3.2)); + assert_eq!(division, FixedU64::from_float(1.25)); + assert_eq!(subtraction, FixedU64::from_float(0.4)); + } +} + #[cfg(test)] mod threshold_compare_tests { use super::*; @@ -437,15 +545,15 @@ mod threshold_compare_tests { let e = Perbill::from_percent(10).mul_ceil(b); // [115 - 11,5 (103,5), 115 + 11,5 (126,5)] is all equal - assert_eq!(103u32.tcmp(&b, e), Ordering::Equal); - assert_eq!(104u32.tcmp(&b, e), Ordering::Equal); - assert_eq!(115u32.tcmp(&b, e), Ordering::Equal); - assert_eq!(120u32.tcmp(&b, e), Ordering::Equal); - assert_eq!(126u32.tcmp(&b, e), Ordering::Equal); - assert_eq!(127u32.tcmp(&b, e), Ordering::Equal); - - assert_eq!(128u32.tcmp(&b, e), Ordering::Greater); - assert_eq!(102u32.tcmp(&b, e), Ordering::Less); + assert_eq!((103u32).tcmp(&b, e), Ordering::Equal); + assert_eq!((104u32).tcmp(&b, e), Ordering::Equal); + assert_eq!((115u32).tcmp(&b, e), Ordering::Equal); + assert_eq!((120u32).tcmp(&b, e), Ordering::Equal); + assert_eq!((126u32).tcmp(&b, e), Ordering::Equal); + assert_eq!((127u32).tcmp(&b, e), Ordering::Equal); + + assert_eq!((128u32).tcmp(&b, e), Ordering::Greater); + assert_eq!((102u32).tcmp(&b, e), Ordering::Less); } #[test] @@ -455,15 +563,15 @@ mod threshold_compare_tests { let e = Perbill::from_parts(100) * b; // [115 - 11,5 (103,5), 115 + 11,5 (126,5)] is all equal - assert_eq!(103u32.tcmp(&b, e), 103u32.cmp(&b)); - assert_eq!(104u32.tcmp(&b, e), 104u32.cmp(&b)); - assert_eq!(115u32.tcmp(&b, e), 115u32.cmp(&b)); - assert_eq!(120u32.tcmp(&b, e), 120u32.cmp(&b)); - assert_eq!(126u32.tcmp(&b, e), 126u32.cmp(&b)); - assert_eq!(127u32.tcmp(&b, e), 127u32.cmp(&b)); - - assert_eq!(128u32.tcmp(&b, e), 128u32.cmp(&b)); - assert_eq!(102u32.tcmp(&b, e), 102u32.cmp(&b)); + assert_eq!((103u32).tcmp(&b, e), (103u32).cmp(&b)); + assert_eq!((104u32).tcmp(&b, e), (104u32).cmp(&b)); + assert_eq!((115u32).tcmp(&b, e), (115u32).cmp(&b)); + assert_eq!((120u32).tcmp(&b, e), (120u32).cmp(&b)); + assert_eq!((126u32).tcmp(&b, e), (126u32).cmp(&b)); + assert_eq!((127u32).tcmp(&b, e), (127u32).cmp(&b)); + + assert_eq!((128u32).tcmp(&b, e), (128u32).cmp(&b)); + assert_eq!((102u32).tcmp(&b, e), (102u32).cmp(&b)); } #[test] diff --git a/substrate/primitives/arithmetic/src/per_things.rs b/substrate/primitives/arithmetic/src/per_things.rs index fe88b72e24c2..35c77e074a90 100644 --- a/substrate/primitives/arithmetic/src/per_things.rs +++ b/substrate/primitives/arithmetic/src/per_things.rs @@ -15,6 +15,54 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! For use cases which operate within the range of `[0, 1]` types that implement +//! [`PerThing`](PerThing) are used: +//! - **[`Perbill`](Perbill), parts of a billion** +#![doc = docify::embed!("./src/lib.rs", perbill_example)] +//! - **[`Percent`](Percent), parts of a hundred** +#![doc = docify::embed!("./src/lib.rs", percent_example)] +//! +//! Note that `190 / 400 = 0.475`, and that `Percent` represents it as a _rounded down_, fixed point +//! number (`47`). Unlike primitive types, types that implement +//! [`PerThing`](PerThing) will also not overflow, and are therefore safe to use. +//! They adopt the same behavior that a saturated calculation would provide, meaning that if one is +//! to go over "100%", it wouldn't overflow, but simply stop at the upper or lower bound. +//! +//! For use cases which require precision beyond the range of `[0, 1]`, there are fixed-point types which can be used. +//! +//! Let's now explore these types in practice, and how they may be used with pallets to perform +//! safer calculations in the runtime. +//! +//! ### 'PerThing' In Practice +//! +//! A notable trait called, called [`PerThing`](PerThing), allowing a +//! custom type to be implemented specifically for fixed point arithmetic. While a number of +//! fixed-point types are introduced, let's focus on a few specific examples that implement +//! [`PerThing`](PerThing): +//! +//! - [`Percent`](Percent) - parts of one hundred. +//! - [`Permill`](Permill) - parts of a million. +//! - [`Perbill`](Perbill) - parts of a billion. +//! +//! Each of these can be used to construct and represent ratios within our runtime. +//! You will find types like [`Perbill`](Perbill) being used often in pallet +//! development. pallet_referenda is a good example of a pallet which makes good use of fixed +//! point arithmetic. +//! +//! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating +//! point numbers in development with Substrate. For this scenario, let's say we are demonstrating a +//! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be +//! deemed valid. +//! +//! For most applications, `Perbill` gives us a reasonable amount of precision, which +//! is why we're using it here. +//! +//! #### Fixed Point Arithmetic with [`PerThing`](PerThing) +//! +//! As stated, one can also perform mathematics using these types directly. For example, finding the +//! percentage of a particular item: +#![doc = docify::embed!("./src/lib.rs", percent_mult)] + #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; From 2966925afa0d5c4df25a4ce86b82b279818e2f18 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Fri, 8 Mar 2024 15:53:12 -0500 Subject: [PATCH 66/76] start moving things around (unfinished) --- .../primitives/arithmetic/src/fixed_point.rs | 5 +- substrate/primitives/arithmetic/src/lib.rs | 95 +++++++------------ .../primitives/arithmetic/src/per_things.rs | 37 ++++---- 3 files changed, 53 insertions(+), 84 deletions(-) diff --git a/substrate/primitives/arithmetic/src/fixed_point.rs b/substrate/primitives/arithmetic/src/fixed_point.rs index 4ab22537b5b0..95e3f75c9de8 100644 --- a/substrate/primitives/arithmetic/src/fixed_point.rs +++ b/substrate/primitives/arithmetic/src/fixed_point.rs @@ -22,7 +22,7 @@ //! //! ### Fixed Point Types in Practice //! -//! As said earlier, if one needs to exceed the value of one, then +//! If one needs to exceed the value of one (1), then //! [`FixedU64`](FixedU64) (and its signed and `u128` counterparts) can be utilized. //! Take for example this very rudimentary pricing mechanism, where we wish to calculate the demand //! / supply to get a price for some on-chain compute: @@ -31,7 +31,8 @@ fixed_u64_block_computation_example )] //! -//! For a much more comprehensive example, be sure to look at the source for pallet_broker. +//! For a much more comprehensive example, be sure to look at the source for broker (the "coretime") +//! pallet. //! //! #### Fixed Point Types in Practice //! diff --git a/substrate/primitives/arithmetic/src/lib.rs b/substrate/primitives/arithmetic/src/lib.rs index 4891da767b56..ad42298dc0d2 100644 --- a/substrate/primitives/arithmetic/src/lib.rs +++ b/substrate/primitives/arithmetic/src/lib.rs @@ -41,38 +41,22 @@ pub mod rational; pub mod traits; pub use fixed_point::{ - FixedI128, - FixedI64, - FixedPointNumber, - FixedPointOperand, - FixedU128, - FixedU64, + FixedI128, FixedI64, FixedPointNumber, FixedPointOperand, FixedU128, FixedU64, }; pub use per_things::{ - InnerOf, - MultiplyArg, - PerThing, - PerU16, - Perbill, - Percent, - Permill, - Perquintill, - RationalArg, - ReciprocalArg, - Rounding, - SignedRounding, - UpperOf, + InnerOf, MultiplyArg, PerThing, PerU16, Perbill, Percent, Permill, Perquintill, RationalArg, + ReciprocalArg, Rounding, SignedRounding, UpperOf, }; -pub use rational::{ MultiplyRational, Rational128, RationalInfinite }; +pub use rational::{MultiplyRational, Rational128, RationalInfinite}; -use sp_std::{ cmp::Ordering, fmt::Debug, prelude::* }; -use traits::{ BaseArithmetic, One, SaturatedConversion, Unsigned, Zero }; +use sp_std::{cmp::Ordering, fmt::Debug, prelude::*}; +use traits::{BaseArithmetic, One, SaturatedConversion, Unsigned, Zero}; -use codec::{ Decode, Encode, MaxEncodedLen }; +use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; #[cfg(feature = "serde")] -use serde::{ Deserialize, Serialize }; +use serde::{Deserialize, Serialize}; /// Arithmetic errors. #[derive(Eq, PartialEq, Clone, Copy, Encode, Decode, Debug, TypeInfo, MaxEncodedLen)] @@ -107,9 +91,9 @@ pub trait ThresholdOrd { fn tcmp(&self, other: &T, threshold: T) -> Ordering; } -impl ThresholdOrd - for T - where T: Ord + PartialOrd + Copy + Clone + traits::Zero + traits::Saturating +impl ThresholdOrd for T +where + T: Ord + PartialOrd + Copy + Clone + traits::Zero + traits::Saturating, { fn tcmp(&self, other: &T, threshold: T) -> Ordering { // early exit. @@ -162,22 +146,15 @@ impl_normalize_for_numeric!(u8, u16, u32, u64, u128); impl Normalizable

for Vec

{ fn normalize(&self, targeted_sum: P) -> Result, &'static str> { - let uppers = self - .iter() - .map(|p| >::from(p.deconstruct())) - .collect::>(); - - let normalized = normalize( - uppers.as_ref(), - >::from(targeted_sum.deconstruct()) - )?; - - Ok( - normalized - .into_iter() - .map(|i: UpperOf

| P::from_parts(i.saturated_into::())) - .collect() - ) + let uppers = self.iter().map(|p| >::from(p.deconstruct())).collect::>(); + + let normalized = + normalize(uppers.as_ref(), >::from(targeted_sum.deconstruct()))?; + + Ok(normalized + .into_iter() + .map(|i: UpperOf

| P::from_parts(i.saturated_into::())) + .collect()) } } @@ -211,7 +188,8 @@ impl Normalizable

for Vec

{ /// /// * This proof is used in the implementation as well. pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str> - where T: Clone + Copy + Ord + BaseArithmetic + Unsigned + Debug +where + T: Clone + Copy + Ord + BaseArithmetic + Unsigned + Debug, { // compute sum and return error if failed. let mut sum = T::zero(); @@ -252,7 +230,8 @@ pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str if !per_round.is_zero() { for _ in 0..count { - output_with_idx[min_index].1 = output_with_idx[min_index].1 + output_with_idx[min_index].1 = output_with_idx[min_index] + .1 .checked_add(&per_round) .expect("Proof provided in the module doc; qed."); if output_with_idx[min_index].1 >= threshold { @@ -264,7 +243,8 @@ pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str // continue with the previous min_index while !leftover.is_zero() { - output_with_idx[min_index].1 = output_with_idx[min_index].1 + output_with_idx[min_index].1 = output_with_idx[min_index] + .1 .checked_add(&T::one()) .expect("Proof provided in the module doc; qed."); if output_with_idx[min_index].1 >= threshold { @@ -281,13 +261,13 @@ pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str // at this threshold we move to next index. let threshold = output_with_idx .first() - .expect("length of input is greater than zero; it must have a first; qed").1; + .expect("length of input is greater than zero; it must have a first; qed") + .1; if !per_round.is_zero() { for _ in 0..count { - output_with_idx[max_index].1 = output_with_idx[max_index].1 - .checked_sub(&per_round) - .unwrap_or_else(|| { + output_with_idx[max_index].1 = + output_with_idx[max_index].1.checked_sub(&per_round).unwrap_or_else(|| { let remainder = per_round - output_with_idx[max_index].1; leftover += remainder; output_with_idx[max_index].1.saturating_sub(per_round) @@ -322,12 +302,7 @@ pub fn normalize(input: &[T], targeted_sum: T) -> Result, &'static str // sort again based on the original index. output_with_idx.sort_by_key(|x| x.0); - Ok( - output_with_idx - .into_iter() - .map(|(_, t)| t) - .collect() - ) + Ok(output_with_idx.into_iter().map(|(_, t)| t).collect()) } #[cfg(test)] @@ -406,7 +381,7 @@ mod normalize_tests { vec![ PerU16::from_parts(21845), // 33% PerU16::from_parts(21845), // 33% - PerU16::from_parts(21845) // 33% + PerU16::from_parts(21845) // 33% ] ); } @@ -505,11 +480,7 @@ mod per_and_fixed_examples { // 1] In cases where we need > 1, we can used fixed types such as FixedU64 let rational_1 = FixedU64::from_rational(10, 5); //" 200%" aka 2. - let rational_2 = FixedU64::from_rational_with_rounding( - 5, - 10, - Rounding::Down - ); // "50%" aka 0.50... + let rational_2 = FixedU64::from_rational_with_rounding(5, 10, Rounding::Down); // "50%" aka 0.50... assert_eq!(rational_1, (2u64).into()); assert_eq!(rational_2.into_perbill(), Perbill::from_float(0.5)); diff --git a/substrate/primitives/arithmetic/src/per_things.rs b/substrate/primitives/arithmetic/src/per_things.rs index 35c77e074a90..f826986874a3 100644 --- a/substrate/primitives/arithmetic/src/per_things.rs +++ b/substrate/primitives/arithmetic/src/per_things.rs @@ -15,34 +15,30 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! For use cases which operate within the range of `[0, 1]` types that implement -//! [`PerThing`](PerThing) are used: -//! - **[`Perbill`](Perbill), parts of a billion** +//! Types that implement [`PerThing`](PerThing) can be used as a floating-point alternative for +//! numbers that operate within the realm of `[0, 1]`. The primary types may you encounter in +//! Substrate would be the following: +//! - [`Percent`](Percent) - parts of one hundred. +//! - [`Permill`](Permill) - parts of a million. +//! - [`Perbill`](Perbill) - parts of a billion. +//! +//! In use, you may see them being used as follows: +//! +//! > **[`Perbill`](Perbill), parts of a billion** #![doc = docify::embed!("./src/lib.rs", perbill_example)] -//! - **[`Percent`](Percent), parts of a hundred** +//! > **[`Percent`](Percent), parts of a hundred** #![doc = docify::embed!("./src/lib.rs", percent_example)] //! -//! Note that `190 / 400 = 0.475`, and that `Percent` represents it as a _rounded down_, fixed point -//! number (`47`). Unlike primitive types, types that implement +//! Note that `Percent` is represented as a _rounded down_, fixed point +//! number (see the example above). Unlike primitive types, types that implement //! [`PerThing`](PerThing) will also not overflow, and are therefore safe to use. //! They adopt the same behavior that a saturated calculation would provide, meaning that if one is //! to go over "100%", it wouldn't overflow, but simply stop at the upper or lower bound. //! -//! For use cases which require precision beyond the range of `[0, 1]`, there are fixed-point types which can be used. -//! -//! Let's now explore these types in practice, and how they may be used with pallets to perform -//! safer calculations in the runtime. +//! For use cases which require precision beyond the range of `[0, 1]`, there are fixed-point types +//! which can be used. //! -//! ### 'PerThing' In Practice -//! -//! A notable trait called, called [`PerThing`](PerThing), allowing a -//! custom type to be implemented specifically for fixed point arithmetic. While a number of -//! fixed-point types are introduced, let's focus on a few specific examples that implement -//! [`PerThing`](PerThing): -//! -//! - [`Percent`](Percent) - parts of one hundred. -//! - [`Permill`](Permill) - parts of a million. -//! - [`Perbill`](Perbill) - parts of a billion. +//! Let's now explore these types in practice to perform safer calculations in the runtime. //! //! Each of these can be used to construct and represent ratios within our runtime. //! You will find types like [`Perbill`](Perbill) being used often in pallet @@ -61,6 +57,7 @@ //! //! As stated, one can also perform mathematics using these types directly. For example, finding the //! percentage of a particular item: + #![doc = docify::embed!("./src/lib.rs", percent_mult)] #[cfg(feature = "serde")] From 83a13d7fc8674c7a02119ccdcc869402c257c8e9 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 11 Mar 2024 14:48:54 -0400 Subject: [PATCH 67/76] polish + grammar --- .../reference_docs/defensive_programming.rs | 197 ++++++++---------- .../primitives/arithmetic/src/fixed_point.rs | 2 +- 2 files changed, 93 insertions(+), 106 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 3867cf307ec5..7385ce04e98e 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -13,136 +13,101 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! As our runtime should _never_ panic, we should carefully handle [`Result`]/[`Option`] -//! types, eliminating the possibility of integer overflows, converting between number types, or -//! even replacing floating point usage with fixed point arithmetic to mitigate issues that come -//! with floating point calculations. +//! Learn about how to write safe and defensive code in your FRAME runtime. +//! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a program to continue +//! running despite unexpected behavior, input, or events that may arise in runtime. +//! Usually, unforeseen circumstances may cause the program to stop or, in the Rust context, +//! panic!. Defensive practices allow for these circumstances to be accounted for ahead of time +//! and for them to be handled gracefully, which is in line with the intended fault-tolerant and +//! deterministic nature of blockchains. //! -//! Intentional and predictable design should be our first and foremost -//! priority for ensuring a well running, safely designed system. +//! The Polkadot SDK is built to reflect these principles and to facilitate their usage accordingly. //! -//! ## Defensive Programming +//! ## General Overview //! -//! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a particular program to continue -//! running despite unexpected behavior, input or events which may arise in runtime. Normally, -//! unforeseen circumstances may cause the program to stop or, in the Rust context, `panic!`. -//! Defensive practices allow for these circumstances to be accounted for ahead of time and for them -//! to be handled in a graceful manner, which is in the line of the intended, fault-tolerant and -//! deterministic behavior of blockchains. +//! When developing within the context of the Substrate runtime, there is one golden rule: //! -//! The Polkadot SDK is built to reflect these principles and to facilitate their usage -//! accordingly. +//! ***DO NOT PANIC***. There are some exceptions, but generally, this is the default precedent. //! -//! ## General Practices +//! > It’s important to differentiate between the runtime and node. The runtime refers to the core +//! > business logic of a Substrate-based chain, whereas the node refers to the outer client, which +//! > deals with telemetry and gossip from other nodes. For more information, read about +//! > [Substrate's node +//! > architecture](crate::reference_docs::wasm_meta_protocol#node-vs-runtime). It’s also important +//! > to note that the criticality of the node is slightly lesser +//! > than that of the runtime, which is why you may see `unwrap()` or other “non-defensive” +//! > approaches +//! in a few places of the node's code repository. //! -//! When developing within the context of the a runtime, there is *one* golden rule: -//! -//! ***DO NOT PANIC***. There are some exceptions, which will be covered later on in this doc. -//! -//! > It's important to make the differentiation between the **runtime** and **node**. The runtime -//! > refers to the core business logic of a Substrate-based chain, whereas the node refers to the -//! > outer client which deals with telemetry and gossip from other nodes. For more information, -//! > read about Substrate's architecture. -//! > It's also important to note that the criticality of the **node** is slightly lesser than that -//! > of the -//! > **runtime**, which is why in a few places of the node's code repository, you may see -//! > `unwrap()` or other "non-defensive" -//! > code instances. +//! Most of these practices fall within Rust's +//! colloquial usage of proper error propagation, handling, and arithmetic-based edge cases. //! //! General guidelines: //! -//! - Avoid writing functions that could explicitly panic. Directly using `unwrap()` on a -//! [`Result`], or accessing an out-of-bounds index on a collection, should be avoided. Safer -//! methods to access collection types, i.e., `get()` which allow defensive handling of the -//! resulting [`Option`] are recommended to be used. -//! - It may be acceptable to use `except()`, but only if one is completely certain (and has -//! performed a check beforehand) that a value won't panic upon unwrapping. Even this is -//! discouraged, however, as future changes to that function could then cause that statement to +//! - **Avoid writing functions that could explicitly panic,** such as directly using `unwrap()` on +//! a [`Result`], or accessing an out-of-bounds index on a collection. Safer methods to access +//! collection types, i.e., `get()` which allow defensive handling of the resulting [`Option`] are +//! recommended to be used. +//! - **It may be acceptable to use `except()`,** but only if one is completely certain (and has +//! performed a check beforehand) that a value won't panic upon unwrapping. *Even this is +//! discouraged*, however, as future changes to that function could then cause that statement to //! panic. It is important to ensure all possible errors are propagated and handled effectively. -//! - If a function *can* panic, it usually is prefaced with `unchecked_` to indicate its unsafety. -//! - If you are writing a function that could panic, [be sure to document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) -//! - Carefully handle mathematical operations. Many seemingly, simplistic operations, such as +//! - **If a function *can* panic,** it usually is prefaced with `unchecked_` to indicate its +//! unsafety. +//! - **If you are writing a function that could panic,** [document it!](https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html#documenting-components) +//! - **Carefully handle mathematical operations.** Many seemingly, simplistic operations, such as //! **arithmetic** in the runtime, could present a number of issues [(see more later in this //! document)](#integer-overflow). Use checked arithmetic wherever possible. //! -//! ### Examples of when to `panic!` -//! -//! As you traverse through the codebase (particularly in `substrate/frame`, where the majority of -//! runtime code lives), you may notice that there occurrences where `panic!` is used explicitly. -//! This is used when the runtime should stall, rather than keep running, as that is considered -//! safer. Particularly when it comes to mission critical components, such as block authoring, -//! consensus, or other protocol-level dependencies, the unauthorized nature of a node may actually -//! cause harm to the network, and thus stalling would be the better option. -//! -//! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to -//! participate if it is disabled (see: [frame::traits::DisabledValidators]): +//! These guidelines could be summarized in the following example, where `bad_pop` is prone to +//! panicking, and `good_pop` allows for proper error handling to take place: //! -//! ```rust -//! if T::DisabledValidators::is_disabled(authority_index) { -//! panic!( -//! "Validator with index {:?} is disabled and should not be attempting to author blocks.", -//! authority_index, -//! ); -//! } +//!```rust +//! // Bad pop always requires that we return something, even if vector/array is empty. +//! fn bad_pop(v: Vec) -> T {} +//! // Good pop allows us to return None from the Option if need be. +//! fn good_pop(v: Vec) -> Option {} //! ``` //! -//! There are other such examples in various pallets, mostly those that are crucial to the -//! blockchain's functionality. -//! //! ### Defensive Traits //! //! The [`Defensive`](frame::traits::Defensive) trait provides a number of functions, all of which //! provide an alternative to 'vanilla' Rust functions, e.g.,: //! -//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) -//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) -//! -//! The [`Defensive`](frame::traits::Defensive) trait and its companions, -//! [`DefensiveOption`](frame::traits::DefensiveOption), -//! [`DefensiveResult`](frame::traits::DefensiveResult) can be used to defensively unwrap -//! and handle values. This can be used in place of -//! an `expect`, and again, only if the developer is sure about the unwrap in the first place. -//! -//! Here is a full list of all defensive types: -//! -//! - [`DefensiveOption`](frame::traits::DefensiveOption) -//! - [`DefensiveResult`](frame::traits::DefensiveResult) -//! - [`DefensiveMax`](frame::traits::DefensiveMax) -//! - [`DefensiveSaturating`](frame::traits::DefensiveSaturating) -//! - [`DefensiveTruncateFrom`](frame::traits::DefensiveTruncateFrom) -//! -//! All of which can be used by importing -//! [`frame::traits::defensive_prelude`](frame::traits::defensive_prelude), which imports all -//! defensive traits at once. +//! - [`defensive_unwrap_or()`](frame::traits::Defensive::defensive_unwrap_or) instead of +//! `unwrap_or()` +//! - [`defensive_ok_or()`](frame::traits::DefensiveOption::defensive_ok_or) instead of `ok_or()` //! //! Defensive methods use [`debug_assertions`](https://doc.rust-lang.org/reference/conditional-compilation.html#debug_assertions), which panic in development, but in //! production/release, they will merely log an error (i.e., `log::error`). //! +//! The [`Defensive`](frame::traits::Defensive) trait and its various implementations can be found +//! [here](frame::traits::Defensive). +//! //! ## Integer Overflow //! -//! The Rust compiler prevents any sort of static overflow from happening at compile time. +//! The Rust compiler prevents static overflow from happening at compile time. //! The compiler panics in **debug** mode in the event of an integer overflow. In //! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion //! (from the `MAX` back to zero). //! -//! In the context of runtime development, we don't always have control over what is being supplied +//! In runtime development, we don't always have control over what is being supplied //! as a parameter. For example, even this simple add function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! -//! If we passed in overflow-able values at runtime, this could actually panic (or wrap, if in -//! release). +//! If we passed overflow-able values at runtime, this could panic (or wrap if in release). //! //! ```ignore //! naive_add(250u8, 10u8); // In debug mode, this would panic. In release, this would return 4. //! ``` //! -//! It is the _silent_ portion of this behavior that presents a real issue. Such behavior should be -//! made obvious, especially in the context of blockchain development, where unsafe arithmetic could -//! produce unexpected consequences like a user balance over or underflowing. +//! It is the silent portion of this behavior that presents a real issue. Such behavior should be +//! made obvious, especially in blockchain development, where unsafe arithmetic could produce +//! unexpected consequences like a user balance over or underflowing. //! //! Fortunately, there are ways to both represent and handle these scenarios depending on our -//! specific use case natively built into Rust, as well as libraries like [`sp_arithmetic`]. +//! specific use case natively built into Rust and libraries like [`sp_arithmetic`]. //! //! ## Infallible Arithmetic //! @@ -152,12 +117,13 @@ //! A developer should use fixed-point instead of floating-point arithmetic to mitigate the //! potential for inaccuracy, rounding errors, or other unexpected behavior. //! -//! Using floating point number types in the runtime should be avoided, -//! as a single non-deterministic result could cause chaos for blockchain consensus along with the -//! aforementioned issues. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). +//! - [Fixed point types](sp_arithmetic::fixed_point) and their associated usage can be found here. +//! - [PerThing](sp_arithmetic::per_things) and its associated types can be found here. +//! +//! Using floating point number types (i.e., f32. f64) in the runtime should be avoided, as a single non-deterministic result could cause chaos for blockchain consensus along with the issues above. For more on the specifics of the peculiarities of floating point calculations, [watch this video by the Computerphile](https://www.youtube.com/watch?v=PZRI1IfStY0). //! -//! The following methods demonstrate different ways one can handle numbers natively in Rust in a -//! safe manner, without fear of panic or unexpected behavior from wrapping. +//! The following methods demonstrate different ways to handle numbers natively in Rust safely, +//! without fear of panic or unexpected behavior from wrapping. //! //! ### Checked Arithmetic //! @@ -174,13 +140,12 @@ checked_add_handle_error_example )] //! -//! Typically, if you aren't sure about which operation to use for runtime math, **checked** -//! operations are a safe bet, as it presents two, predictable (and _erroring_) outcomes that can be -//! handled accordingly (`Some` and `None`). +//! Suppose you aren’t sure which operation to use for runtime math. In that case, checked +//! operations are the safest bet, presenting two predictable (and erroring) outcomes that can be +//! handled accordingly (Some and None). //! -//! In a practical context, the resulting [`Option`] should be handled accordingly. The following -//! conventions can be seen within the Polkadot SDK, where it is handled in -//! two ways: +//! The following conventions can be seen within the Polkadot SDK, where it is +//! handled in two ways: //! //! - As an [`Option`], using the `if let` / `if` or `match` //! - As a [`Result`], via `ok_or` (or similar conversion to [`Result`] from [`Option`]) @@ -198,7 +163,7 @@ )] //! //! This is generally a useful convention for handling not only checked types, but most types that -//! return `Option`. +//! return `Option` (such as some storage constructs in Substrate). //! //! #### Handling via Result - Less Verbose //! @@ -295,7 +260,7 @@ //! //!

//! Solution: Checked -//! For the proposal IDs, proper handling via `checked` math would've been much more suitable, +//! For the proposal IDs, proper handling via `checked` math would've been suitable, //! Saturating could've been used - but it also would've 'failed' silently. Using `checked_add` to //! ensure that the next proposal ID would've been valid would've been a viable way to let the user //! know the state of their proposal: @@ -307,13 +272,35 @@ //!
//! //! From the above, we can clearly see the problematic nature of seemingly simple operations in the -//! runtime. Of course, it may be that using unchecked math is perfectly fine under some scenarios - -//! such as certain balance being never realistically attainable, or a number type being so large -//! that it could never realistically overflow unless one sent thousands of transactions to the -//! network. +//! runtime, and care should be given to ensure a defensive approach is taken. //! //! ### Decision Chart: When to use which? #![doc = simple_mermaid::mermaid!("../../../mermaid/integer_operation_decision.mmd")] +//! ### Edge cases of `panic!`-able instances in Substrate +//! +//! As you traverse through the codebase (particularly in `substrate/frame`, where the majority of +//! runtime code lives), you may notice that there occurrences where `panic!` is used explicitly. +//! This is used when the runtime should stall, rather than keep running, as that is considered +//! safer. Particularly when it comes to mission critical components, such as block authoring, +//! consensus, or other protocol-level dependencies, the unauthorized nature of a node may actually +//! cause harm to the network, and thus stalling would be the better option. +//! +//! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to +//! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): +//! +//! ```rust +//! if T::DisabledValidators::is_disabled(authority_index) { +//! panic!( +//! "Validator with index {:?} is disabled and should not be attempting to author blocks.", +//! authority_index, +//! ); +//! } +//! ``` +//! +//! There are other such examples in various pallets, mostly those that are crucial to the +//! blockchain's functionality at the consensus level. Most of the time, you will not be writing +//! pallets which operate at this level. +//! //! ## Other Resources //! //! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) diff --git a/substrate/primitives/arithmetic/src/fixed_point.rs b/substrate/primitives/arithmetic/src/fixed_point.rs index 95e3f75c9de8..a5cfac76ab15 100644 --- a/substrate/primitives/arithmetic/src/fixed_point.rs +++ b/substrate/primitives/arithmetic/src/fixed_point.rs @@ -16,7 +16,7 @@ // limitations under the License. //! Decimal Fixed Point implementations for Substrate runtime. -//! Similar to types that implement [`PerThing`](PerThing), these are also +//! Similar to types that implement [`PerThing`](crate::per_things), these are also //! fixed-point types, however, they are able to represent larger fractions: #![doc = docify::embed!("./src/lib.rs", fixed_u64)] //! From 114b677daa2778c3611e5ca25a3584467ec645c2 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 11 Mar 2024 16:40:02 -0400 Subject: [PATCH 68/76] fmt + other checks --- .../reference_docs/defensive_programming.rs | 74 +++++++++---------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 7385ce04e98e..ac74e48fe388 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -13,7 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Learn about how to write safe and defensive code in your FRAME runtime. //! [Defensive programming](https://en.wikipedia.org/wiki/Defensive_programming) is a design paradigm that enables a program to continue //! running despite unexpected behavior, input, or events that may arise in runtime. //! Usually, unforeseen circumstances may cause the program to stop or, in the Rust context, @@ -114,6 +113,10 @@ //! Both Rust and Substrate provide safe ways to deal with numbers and alternatives to floating //! point arithmetic. //! +//! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of +//! dividing/modulo by zero at any point should be mitigated. One should be opting for a +//! `checked_*` method to introduce safe arithmetic in their code in most cases. +//! //! A developer should use fixed-point instead of floating-point arithmetic to mitigate the //! potential for inaccuracy, rounding errors, or other unexpected behavior. //! @@ -142,7 +145,7 @@ //! //! Suppose you aren’t sure which operation to use for runtime math. In that case, checked //! operations are the safest bet, presenting two predictable (and erroring) outcomes that can be -//! handled accordingly (Some and None). +//! handled accordingly (Some and None). //! //! The following conventions can be seen within the Polkadot SDK, where it is //! handled in two ways: @@ -156,20 +159,20 @@ //! handling via `if` or `if let`: #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance)] //! -//! Optionally, `match` may also be directly used in a more concise manner: +//! Optionally, match may also be directly used in a more concise manner: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", increase_balance_match )] //! -//! This is generally a useful convention for handling not only checked types, but most types that -//! return `Option` (such as some storage constructs in Substrate). +//! This is generally a useful convention for handling checked types and most types that return +//! `Option`. //! //! #### Handling via Result - Less Verbose //! -//! In the Polkadot SDK codebase, you may see checked operations being handled as a [`Result`] via -//! `ok_or`. This is a less verbose way of expressing the above. This usage often boils down to -//! the developer's preference: +//! In the Polkadot SDK codebase, checked operations are handled as a `Result` via `ok_or`. This is +//! a less verbose way of expressing the above. This usage often boils down to the developer’s +//! preference: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", increase_balance_result @@ -177,8 +180,9 @@ //! //! ### Saturating Operations //! -//! Saturating a number limits it to the type's upper or lower bound, even if the integer were to -//! overflow in runtime. For example, adding to `u32::MAX` would simply limit itself to `u32::MAX`: +//! Saturating a number limits it to the type’s upper or lower bound, even if the integer type +//! overflowed in runtime. For example, adding to `u32::MAX` would simply limit itself to +//! `u32::MAX`: #![doc = docify::embed!( "./src/reference_docs/defensive_programming.rs", saturated_add_example @@ -204,17 +208,12 @@ //! 2. **Saturating** operations - limited to the lower and upper bounds of a number type //! 3. **Wrapped** operations (the default) - wrap around to above or below the bounds of a type //! -//! Known scenarios that could be fallible should be avoided: i.e., avoiding the possibility of -//! dividing/modulo by zero at any point should be mitigated. One should be opting for a -//! `checked_*` method to introduce safe arithmetic in their code. -//! //! #### The problem with 'default' wrapped operations //! //! **Wrapped operations** cause the overflow to wrap around to either the maximum or minimum of //! that type. Imagine this in the context of a blockchain, where there are account balances, voting //! counters, nonces for transactions, and other aspects of a blockchain. //! -//! //! While it may seem trivial, choosing how to handle numbers is quite important. As a thought //! exercise, here are some scenarios of which will shed more light on when to use which. //! @@ -233,30 +232,29 @@ //! //! #### Alice's 'Underflowed' Balance //! -//! Alice's balance has reached `0` after a transfer to Bob. Suddenly, she has been slashed on -//! `EduChain`, causing her balance to reach near the limit of `u32::MAX` - a very large amount - as -//! _wrapped operations_ can go both ways. Alice can now successfully vote using her new, -//! overpowered token balance, destroying the integrity of the chain. +//! Alice’s balance has reached `0` after a transfer to Bob. Suddenly, she has been slashed on +//! EduChain, causing her balance to reach near the limit of `u32::MAX` - a very large amount - as +//! wrapped operations can go both ways. Alice can now successfully vote using her new, overpowered +//! token balance, destroying the chain's integrity. //! //!
//! Solution: Saturating -//! For Alice's balance problem, using `saturated_sub` could've mitigated this issue. As debt or -//! having a negative balance is not a concept within blockchains, a saturating calculation -//! would've simply limited her balance to the lower bound of u32. -//! -//! In other words: Alice's balance would've stayed at "0", even after being slashed. +//! For Alice's balance problem, using `saturated_sub` could've mitigated this issue. A saturating +//! calculation would've simply limited her balance to the lower bound of u32, as having a negative +//! balance is not a concept within blockchains. In other words: Alice's balance would've stayed +//! at "0", even after being slashed. //! //! This is also an example that while one system may work in isolation, shared interfaces, such //! as the notion of balances, are often shared across multiple pallets - meaning these small -//! changes can make a big difference in outcome.
+//! changes can make a big difference depending on the scenario.
//! //! #### Proposal ID Overwrite //! -//! The type for counting the number of proposals on-chain is represented by a `u8` number, called -//! `proposals_count`. Every time a new proposal is added to the system, this number increases. With -//! the proposal pallet being high in usage, it has reached `u8::MAX`'s limit of `255`, causing -//! `proposals_count` to go to `0`. Unfortunately, this results in new proposals overwriting old -//! ones, effectively erasing any notion of past proposals! +//! A `u8` parameter, called `proposals_count`, represents the type for counting the number of +//! proposals on-chain. Every time a new proposal is added to the system, this number increases. +//! With the proposal pallet's high usage, it has reached `u8::MAX`’s limit of 255, causing +//! `proposals_count` to go to 0. Unfortunately, this results in new proposals overwriting old ones, +//! effectively erasing any notion of past proposals! //! //!
//! Solution: Checked @@ -279,11 +277,11 @@ //! ### Edge cases of `panic!`-able instances in Substrate //! //! As you traverse through the codebase (particularly in `substrate/frame`, where the majority of -//! runtime code lives), you may notice that there occurrences where `panic!` is used explicitly. -//! This is used when the runtime should stall, rather than keep running, as that is considered -//! safer. Particularly when it comes to mission critical components, such as block authoring, -//! consensus, or other protocol-level dependencies, the unauthorized nature of a node may actually -//! cause harm to the network, and thus stalling would be the better option. +//! runtime code lives), you may notice that there (only a few!) occurrences where `panic!` is used +//! explicitly. This is used when the runtime should stall, rather than keep running, as that is +//! considered safer. Particularly when it comes to mission-critical components, such as block +//! authoring, consensus, or other protocol-level dependencies, going through with an action may +//! actually cause harm to the network, and thus stalling would be the better option. //! //! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to //! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): @@ -297,9 +295,9 @@ //! } //! ``` //! -//! There are other such examples in various pallets, mostly those that are crucial to the -//! blockchain's functionality at the consensus level. Most of the time, you will not be writing -//! pallets which operate at this level. +//! There are other examples in various pallets, mostly those crucial to the blockchain’s +//! functionality. Most of the time, you will not be writing pallets which operate at this level, +//! but these exceptions should be noted regardless. //! //! ## Other Resources //! From 81f366ffd2b72869db1f43463d003dc61296f113 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 11 Mar 2024 16:44:42 -0400 Subject: [PATCH 69/76] make perthings more concise --- substrate/primitives/arithmetic/src/per_things.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/substrate/primitives/arithmetic/src/per_things.rs b/substrate/primitives/arithmetic/src/per_things.rs index f826986874a3..21272e583a5d 100644 --- a/substrate/primitives/arithmetic/src/per_things.rs +++ b/substrate/primitives/arithmetic/src/per_things.rs @@ -38,20 +38,10 @@ //! For use cases which require precision beyond the range of `[0, 1]`, there are fixed-point types //! which can be used. //! -//! Let's now explore these types in practice to perform safer calculations in the runtime. -//! //! Each of these can be used to construct and represent ratios within our runtime. //! You will find types like [`Perbill`](Perbill) being used often in pallet -//! development. pallet_referenda is a good example of a pallet which makes good use of fixed -//! point arithmetic. -//! -//! Let's examine the usage of `Perbill` and how exactly we can use it as an alternative to floating -//! point numbers in development with Substrate. For this scenario, let's say we are demonstrating a -//! _voting_ system which depends on reaching a certain threshold, or percentage, before it can be -//! deemed valid. -//! -//! For most applications, `Perbill` gives us a reasonable amount of precision, which -//! is why we're using it here. +//! development. `pallet_referenda` is a good example of a pallet which makes good use of fixed +//! point arithmetic, as it relies on representing various curves and thresholds relating to governance. //! //! #### Fixed Point Arithmetic with [`PerThing`](PerThing) //! From 2a6521e0e788d43214c0fe8d6b9940bb93db2cfc Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Mon, 11 Mar 2024 16:55:36 -0400 Subject: [PATCH 70/76] fmt and clippy --- substrate/primitives/arithmetic/src/per_things.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/arithmetic/src/per_things.rs b/substrate/primitives/arithmetic/src/per_things.rs index 21272e583a5d..5e5ffed7a9ba 100644 --- a/substrate/primitives/arithmetic/src/per_things.rs +++ b/substrate/primitives/arithmetic/src/per_things.rs @@ -41,7 +41,8 @@ //! Each of these can be used to construct and represent ratios within our runtime. //! You will find types like [`Perbill`](Perbill) being used often in pallet //! development. `pallet_referenda` is a good example of a pallet which makes good use of fixed -//! point arithmetic, as it relies on representing various curves and thresholds relating to governance. +//! point arithmetic, as it relies on representing various curves and thresholds relating to +//! governance. //! //! #### Fixed Point Arithmetic with [`PerThing`](PerThing) //! From 5987b6d410c9b7561aa32c909b6de657a90a4b65 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Wed, 13 Mar 2024 12:36:08 -0400 Subject: [PATCH 71/76] make sure tests pass --- substrate/primitives/arithmetic/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/arithmetic/src/lib.rs b/substrate/primitives/arithmetic/src/lib.rs index ad42298dc0d2..3786944b5578 100644 --- a/substrate/primitives/arithmetic/src/lib.rs +++ b/substrate/primitives/arithmetic/src/lib.rs @@ -425,7 +425,10 @@ mod normalize_tests { } } +#[cfg(test)] mod per_and_fixed_examples { + use super::*; + #[docify::export] #[test] fn percent_mult() { @@ -467,7 +470,7 @@ mod per_and_fixed_examples { // Calculate a very rudimentary on-chain price from supply / demand // Supply: Cores available per block // Demand: Cores being ordered per block - let price = FixedU64::from_rational(10u32, 19u32); + let price = FixedU64::from_rational(19u128, 10u128); // 1.9 DOT per core assert_eq!(price, FixedU64::from_float(1.9)); From 110c23c4314b1b124af2c823c266ec661ad087f0 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Tue, 19 Mar 2024 15:50:17 -0400 Subject: [PATCH 72/76] lock update --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index dbce2a4a23fb..faeb854099d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18407,6 +18407,7 @@ dependencies = [ "scale-info", "serde", "sp-crypto-hashing", + "sp-std 14.0.0", "static_assertions", ] From bd4c7453bd2244ba0bc34d67226d64d02c6988dd Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Tue, 19 Mar 2024 16:02:17 -0400 Subject: [PATCH 73/76] attempt to make clippy happy --- docs/sdk/src/reference_docs/defensive_programming.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index ac74e48fe388..509a27b72641 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -5,7 +5,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -284,14 +284,14 @@ //! actually cause harm to the network, and thus stalling would be the better option. //! //! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to -//! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): +//! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): //! //! ```rust //! if T::DisabledValidators::is_disabled(authority_index) { -//! panic!( -//! "Validator with index {:?} is disabled and should not be attempting to author blocks.", -//! authority_index, -//! ); +//! panic!( +//! "Validator with index {:?} is disabled and should not be attempting to author blocks.", +//! authority_index, +//! ); //! } //! ``` //! From 5ee7b1a6cf8e18548100855cb1d8207f2f75d8c7 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Tue, 19 Mar 2024 16:43:27 -0400 Subject: [PATCH 74/76] attempt 2 --- docs/mermaid/integer_operation_decision.mmd | 7 -- .../reference_docs/defensive_programming.rs | 106 ++++++++---------- 2 files changed, 49 insertions(+), 64 deletions(-) delete mode 100644 docs/mermaid/integer_operation_decision.mmd diff --git a/docs/mermaid/integer_operation_decision.mmd b/docs/mermaid/integer_operation_decision.mmd deleted file mode 100644 index 243a856fb694..000000000000 --- a/docs/mermaid/integer_operation_decision.mmd +++ /dev/null @@ -1,7 +0,0 @@ -flowchart LR - CH["Checked"] - ST["Saturated"] - CH-->NEED["The user needs to know that the operation failed - and why"] - CH-->DOUBT["Unsure whether this operation could fail/overflow"] - ST-->SILENT["Silently reaching upper/lower bound will not result in any damage"] - ST-->REASON["In all reasonable circumstances, the number will not overflow, but safety is still desired"] diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 509a27b72641..9f07914b1ef7 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -61,7 +61,7 @@ //! These guidelines could be summarized in the following example, where `bad_pop` is prone to //! panicking, and `good_pop` allows for proper error handling to take place: //! -//!```rust +//!```ignore //! // Bad pop always requires that we return something, even if vector/array is empty. //! fn bad_pop(v: Vec) -> T {} //! // Good pop allows us to return None from the Option if need be. @@ -85,7 +85,7 @@ //! //! ## Integer Overflow //! -//! The Rust compiler prevents static overflow from happening at compile time. +//! The Rust compiler prevents static overflow from happening at compile time. //! The compiler panics in **debug** mode in the event of an integer overflow. In //! **release** mode, it resorts to silently _wrapping_ the overflowed amount in a modular fashion //! (from the `MAX` back to zero). @@ -93,8 +93,12 @@ //! In runtime development, we don't always have control over what is being supplied //! as a parameter. For example, even this simple add function could present one of two outcomes //! depending on whether it is in **release** or **debug** mode: -#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", naive_add)] //! +//! ```ignore +//! fn naive_add(x: u8, y: u8) -> u8 { +//! x + y +//! } +//! ``` //! If we passed overflow-able values at runtime, this could panic (or wrap if in release). //! //! ```ignore @@ -160,10 +164,7 @@ #![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance)] //! //! Optionally, match may also be directly used in a more concise manner: -#![doc = docify::embed!( - "./src/reference_docs/defensive_programming.rs", - increase_balance_match -)] +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance_match)] //! //! This is generally a useful convention for handling checked types and most types that return //! `Option`. @@ -173,20 +174,14 @@ //! In the Polkadot SDK codebase, checked operations are handled as a `Result` via `ok_or`. This is //! a less verbose way of expressing the above. This usage often boils down to the developer’s //! preference: -#![doc = docify::embed!( - "./src/reference_docs/defensive_programming.rs", - increase_balance_result -)] +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", increase_balance_result)] //! //! ### Saturating Operations //! //! Saturating a number limits it to the type’s upper or lower bound, even if the integer type //! overflowed in runtime. For example, adding to `u32::MAX` would simply limit itself to //! `u32::MAX`: -#![doc = docify::embed!( - "./src/reference_docs/defensive_programming.rs", - saturated_add_example -)] +#![doc = docify::embed!("./src/reference_docs/defensive_programming.rs", saturated_add_example)] //! //! Saturating calculations can be used if one is very sure that something won't overflow, but wants //! to avoid introducing the notion of any potential-panic or wrapping behavior. @@ -272,8 +267,6 @@ //! From the above, we can clearly see the problematic nature of seemingly simple operations in the //! runtime, and care should be given to ensure a defensive approach is taken. //! -//! ### Decision Chart: When to use which? -#![doc = simple_mermaid::mermaid!("../../../mermaid/integer_operation_decision.mmd")] //! ### Edge cases of `panic!`-able instances in Substrate //! //! As you traverse through the codebase (particularly in `substrate/frame`, where the majority of @@ -286,7 +279,7 @@ //! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to //! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): //! -//! ```rust +//! ```ignore //! if T::DisabledValidators::is_disabled(authority_index) { //! panic!( //! "Validator with index {:?} is disabled and should not be attempting to author blocks.", @@ -302,49 +295,29 @@ //! ## Other Resources //! //! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) - -#[cfg(test)] -mod tests { - enum BlockchainError { +#![allow(dead_code)] +mod fake_runtime_types { + // Note: The following types are purely for the purpose of example, and do not contain any + // *real* use case other than demonstrating various concepts. + pub enum RuntimeError { Overflow, + UserDoesntExist, } - type Address = (); + pub type Address = (); - struct Runtime; + pub struct Runtime; impl Runtime { - fn get_balance(account: Address) -> u64 { - 0 + fn get_balance(account: Address) -> Result { + Ok(0u64) } - } - #[docify::export] - #[test] - fn naive_add(x: u8, y: u8) -> u8 { - x + y + fn set_balance(account: Address, new_balance: u64) {} } #[docify::export] - #[test] - fn checked_add_example() { - // This is valid, as 20 is perfectly within the bounds of u32. - let add = (10u32).checked_add(10); - assert_eq!(add, Some(20)) - } - - #[docify::export] - #[test] - fn checked_add_handle_error_example() { - // This is invalid - we are adding something to the max of u32::MAX, which would overflow. - // Luckily, checked_add just marks this as None! - let add = u32::MAX.checked_add(10); - assert_eq!(add, None) - } - - #[docify::export] - #[test] - fn increase_balance(account: Address, amount: u64) -> Result<(), BlockchainError> { + fn increase_balance(account: Address, amount: u64) -> Result<(), RuntimeError> { // Get a user's current balance let balance = Runtime::get_balance(account)?; // SAFELY increase the balance by some amount @@ -352,20 +325,19 @@ mod tests { Runtime::set_balance(account, new_balance); return Ok(()); } else { - return Err(BlockchainError::Overflow); + return Err(RuntimeError::Overflow); } } #[docify::export] - #[test] - fn increase_balance_match(account: Address, amount: u64) -> Result<(), BlockchainError> { + fn increase_balance_match(account: Address, amount: u64) -> Result<(), RuntimeError> { // Get a user's current balance let balance = Runtime::get_balance(account)?; // SAFELY increase the balance by some amount let new_balance = match balance.checked_add(amount) { Some(balance) => balance, None => { - return Err(BlockchainError::Overflow); + return Err(RuntimeError::Overflow); }, }; Runtime::set_balance(account, new_balance); @@ -373,15 +345,35 @@ mod tests { } #[docify::export] - #[test] - fn increase_balance_result(account: Address, amount: u64) -> Result<(), BlockchainError> { + fn increase_balance_result(account: Address, amount: u64) -> Result<(), RuntimeError> { // Get a user's current balance let balance = Runtime::get_balance(account)?; // SAFELY increase the balance by some amount - this time, by using `ok_or` - let new_balance = balance.checked_add(amount).ok_or_else(|| BlockchainError::Overflow)?; + let new_balance = balance.checked_add(amount).ok_or_else(|| RuntimeError::Overflow)?; Runtime::set_balance(account, new_balance); Ok(()) } +} + +#[cfg(test)] +mod tests { + use frame::traits::DefensiveSaturating; + #[docify::export] + #[test] + fn checked_add_example() { + // This is valid, as 20 is perfectly within the bounds of u32. + let add = (10u32).checked_add(10); + assert_eq!(add, Some(20)) + } + + #[docify::export] + #[test] + fn checked_add_handle_error_example() { + // This is invalid - we are adding something to the max of u32::MAX, which would overflow. + // Luckily, checked_add just marks this as None! + let add = u32::MAX.checked_add(10); + assert_eq!(add, None) + } #[docify::export] #[test] From 562b2b48e0193f188f37d3bc3f10147e70aa1448 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Tue, 19 Mar 2024 16:56:08 -0400 Subject: [PATCH 75/76] more clippy (facepalm) --- .../reference_docs/defensive_programming.rs | 163 +++++++++--------- 1 file changed, 82 insertions(+), 81 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index 9f07914b1ef7..e06af0ed8d50 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -277,7 +277,7 @@ //! actually cause harm to the network, and thus stalling would be the better option. //! //! Take the example of the BABE pallet ([`pallet_babe`]), which doesn't allow for a validator to -//! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): +//! participate if it is disabled (see: [`frame::traits::DisabledValidators`]): //! //! ```ignore //! if T::DisabledValidators::is_disabled(authority_index) { @@ -296,99 +296,100 @@ //! //! - [PBA Book - FRAME Tips & Tricks](https://polkadot-blockchain-academy.github.io/pba-book/substrate/tips-tricks/page.html?highlight=perthing#substrate-and-frame-tips-and-tricks) #![allow(dead_code)] +#[allow(unused_variables)] mod fake_runtime_types { - // Note: The following types are purely for the purpose of example, and do not contain any - // *real* use case other than demonstrating various concepts. - pub enum RuntimeError { - Overflow, - UserDoesntExist, - } + // Note: The following types are purely for the purpose of example, and do not contain any + // *real* use case other than demonstrating various concepts. + pub enum RuntimeError { + Overflow, + UserDoesntExist, + } - pub type Address = (); + pub type Address = (); - pub struct Runtime; + pub struct Runtime; - impl Runtime { - fn get_balance(account: Address) -> Result { - Ok(0u64) - } + impl Runtime { + fn get_balance(account: Address) -> Result { + Ok(0u64) + } - fn set_balance(account: Address, new_balance: u64) {} - } + fn set_balance(account: Address, new_balance: u64) {} + } - #[docify::export] - fn increase_balance(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - if let Some(new_balance) = balance.checked_add(amount) { - Runtime::set_balance(account, new_balance); - return Ok(()); - } else { - return Err(RuntimeError::Overflow); - } - } + #[docify::export] + fn increase_balance(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + if let Some(new_balance) = balance.checked_add(amount) { + Runtime::set_balance(account, new_balance); + Ok(()) + } else { + Err(RuntimeError::Overflow) + } + } - #[docify::export] - fn increase_balance_match(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - let new_balance = match balance.checked_add(amount) { - Some(balance) => balance, - None => { - return Err(RuntimeError::Overflow); - }, - }; - Runtime::set_balance(account, new_balance); - Ok(()) - } + #[docify::export] + fn increase_balance_match(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + let new_balance = match balance.checked_add(amount) { + Some(balance) => balance, + None => { + return Err(RuntimeError::Overflow); + } + }; + Runtime::set_balance(account, new_balance); + Ok(()) + } - #[docify::export] - fn increase_balance_result(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - this time, by using `ok_or` - let new_balance = balance.checked_add(amount).ok_or_else(|| RuntimeError::Overflow)?; - Runtime::set_balance(account, new_balance); - Ok(()) - } + #[docify::export] + fn increase_balance_result(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount - this time, by using `ok_or` + let new_balance = balance.checked_add(amount).ok_or(RuntimeError::Overflow)?; + Runtime::set_balance(account, new_balance); + Ok(()) + } } #[cfg(test)] mod tests { - use frame::traits::DefensiveSaturating; - #[docify::export] - #[test] - fn checked_add_example() { - // This is valid, as 20 is perfectly within the bounds of u32. - let add = (10u32).checked_add(10); - assert_eq!(add, Some(20)) - } + use frame::traits::DefensiveSaturating; + #[docify::export] + #[test] + fn checked_add_example() { + // This is valid, as 20 is perfectly within the bounds of u32. + let add = (10u32).checked_add(10); + assert_eq!(add, Some(20)) + } - #[docify::export] - #[test] - fn checked_add_handle_error_example() { - // This is invalid - we are adding something to the max of u32::MAX, which would overflow. - // Luckily, checked_add just marks this as None! - let add = u32::MAX.checked_add(10); - assert_eq!(add, None) - } + #[docify::export] + #[test] + fn checked_add_handle_error_example() { + // This is invalid - we are adding something to the max of u32::MAX, which would overflow. + // Luckily, checked_add just marks this as None! + let add = u32::MAX.checked_add(10); + assert_eq!(add, None) + } - #[docify::export] - #[test] - fn saturated_add_example() { - // Saturating add simply saturates - // to the numeric bound of that type if it overflows. - let add = u32::MAX.saturating_add(10); - assert_eq!(add, u32::MAX) - } + #[docify::export] + #[test] + fn saturated_add_example() { + // Saturating add simply saturates + // to the numeric bound of that type if it overflows. + let add = u32::MAX.saturating_add(10); + assert_eq!(add, u32::MAX) + } - #[docify::export] - #[test] - #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))] - fn saturated_defensive_example() { - let saturated_defensive = u32::MAX.defensive_saturating_add(10); - assert_eq!(saturated_defensive, u32::MAX); - } + #[docify::export] + #[test] + #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))] + fn saturated_defensive_example() { + let saturated_defensive = u32::MAX.defensive_saturating_add(10); + assert_eq!(saturated_defensive, u32::MAX); + } } From 75265e3ba00f8c539f59208922504fbaddf17ed3 Mon Sep 17 00:00:00 2001 From: Bader Youssef Date: Tue, 19 Mar 2024 17:02:19 -0400 Subject: [PATCH 76/76] fmt --- .../reference_docs/defensive_programming.rs | 160 +++++++++--------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/docs/sdk/src/reference_docs/defensive_programming.rs b/docs/sdk/src/reference_docs/defensive_programming.rs index e06af0ed8d50..9828e1b50918 100644 --- a/docs/sdk/src/reference_docs/defensive_programming.rs +++ b/docs/sdk/src/reference_docs/defensive_programming.rs @@ -298,98 +298,98 @@ #![allow(dead_code)] #[allow(unused_variables)] mod fake_runtime_types { - // Note: The following types are purely for the purpose of example, and do not contain any - // *real* use case other than demonstrating various concepts. - pub enum RuntimeError { - Overflow, - UserDoesntExist, - } + // Note: The following types are purely for the purpose of example, and do not contain any + // *real* use case other than demonstrating various concepts. + pub enum RuntimeError { + Overflow, + UserDoesntExist, + } - pub type Address = (); + pub type Address = (); - pub struct Runtime; + pub struct Runtime; - impl Runtime { - fn get_balance(account: Address) -> Result { - Ok(0u64) - } + impl Runtime { + fn get_balance(account: Address) -> Result { + Ok(0u64) + } - fn set_balance(account: Address, new_balance: u64) {} - } + fn set_balance(account: Address, new_balance: u64) {} + } - #[docify::export] - fn increase_balance(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - if let Some(new_balance) = balance.checked_add(amount) { - Runtime::set_balance(account, new_balance); - Ok(()) - } else { - Err(RuntimeError::Overflow) - } - } + #[docify::export] + fn increase_balance(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + if let Some(new_balance) = balance.checked_add(amount) { + Runtime::set_balance(account, new_balance); + Ok(()) + } else { + Err(RuntimeError::Overflow) + } + } - #[docify::export] - fn increase_balance_match(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - let new_balance = match balance.checked_add(amount) { - Some(balance) => balance, - None => { - return Err(RuntimeError::Overflow); - } - }; - Runtime::set_balance(account, new_balance); - Ok(()) - } + #[docify::export] + fn increase_balance_match(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount + let new_balance = match balance.checked_add(amount) { + Some(balance) => balance, + None => { + return Err(RuntimeError::Overflow); + }, + }; + Runtime::set_balance(account, new_balance); + Ok(()) + } - #[docify::export] - fn increase_balance_result(account: Address, amount: u64) -> Result<(), RuntimeError> { - // Get a user's current balance - let balance = Runtime::get_balance(account)?; - // SAFELY increase the balance by some amount - this time, by using `ok_or` - let new_balance = balance.checked_add(amount).ok_or(RuntimeError::Overflow)?; - Runtime::set_balance(account, new_balance); - Ok(()) - } + #[docify::export] + fn increase_balance_result(account: Address, amount: u64) -> Result<(), RuntimeError> { + // Get a user's current balance + let balance = Runtime::get_balance(account)?; + // SAFELY increase the balance by some amount - this time, by using `ok_or` + let new_balance = balance.checked_add(amount).ok_or(RuntimeError::Overflow)?; + Runtime::set_balance(account, new_balance); + Ok(()) + } } #[cfg(test)] mod tests { - use frame::traits::DefensiveSaturating; - #[docify::export] - #[test] - fn checked_add_example() { - // This is valid, as 20 is perfectly within the bounds of u32. - let add = (10u32).checked_add(10); - assert_eq!(add, Some(20)) - } + use frame::traits::DefensiveSaturating; + #[docify::export] + #[test] + fn checked_add_example() { + // This is valid, as 20 is perfectly within the bounds of u32. + let add = (10u32).checked_add(10); + assert_eq!(add, Some(20)) + } - #[docify::export] - #[test] - fn checked_add_handle_error_example() { - // This is invalid - we are adding something to the max of u32::MAX, which would overflow. - // Luckily, checked_add just marks this as None! - let add = u32::MAX.checked_add(10); - assert_eq!(add, None) - } + #[docify::export] + #[test] + fn checked_add_handle_error_example() { + // This is invalid - we are adding something to the max of u32::MAX, which would overflow. + // Luckily, checked_add just marks this as None! + let add = u32::MAX.checked_add(10); + assert_eq!(add, None) + } - #[docify::export] - #[test] - fn saturated_add_example() { - // Saturating add simply saturates - // to the numeric bound of that type if it overflows. - let add = u32::MAX.saturating_add(10); - assert_eq!(add, u32::MAX) - } + #[docify::export] + #[test] + fn saturated_add_example() { + // Saturating add simply saturates + // to the numeric bound of that type if it overflows. + let add = u32::MAX.saturating_add(10); + assert_eq!(add, u32::MAX) + } - #[docify::export] - #[test] - #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))] - fn saturated_defensive_example() { - let saturated_defensive = u32::MAX.defensive_saturating_add(10); - assert_eq!(saturated_defensive, u32::MAX); - } + #[docify::export] + #[test] + #[cfg_attr(debug_assertions, should_panic(expected = "Defensive failure has been triggered!"))] + fn saturated_defensive_example() { + let saturated_defensive = u32::MAX.defensive_saturating_add(10); + assert_eq!(saturated_defensive, u32::MAX); + } }