Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ref docs frame currency #2683

Closed
wants to merge 9 commits into from
Closed

Ref docs frame currency #2683

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8c94c41
initial currency outline
juangirini Dec 8, 2023
c94350a
intial frame currency
juangirini Dec 11, 2023
14f11bb
reorganize fungibles docs
juangirini Dec 14, 2023
9a94042
remove fungibles and assets
juangirini Dec 15, 2023
3fe82bc
Merge branch 'master' into jg/ref-docs-frame-currency
juangirini Dec 18, 2023
cbb7291
Merge branch 'master' into jg/ref-docs-frame-currency
juangirini Dec 26, 2023
6babc75
fix rustdocs links
juangirini Jan 8, 2024
933a8e2
fix rustdocs links
juangirini Jan 8, 2024
80ff2ce
review improvements
juangirini Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 49 additions & 6 deletions docs/sdk/src/reference_docs/frame_currency.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,51 @@
//! FRAME Currency Abstractions and Traits
//! # FRAME Currency Abstractions and Traits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be useful to have a table with the comparing the Currency vs Fungible/s terms and a description on how they differ (e.g. locks vs holds, etc). This is usually a place where people get confused and it would be great if we could have a single place to refresh the memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we can improve this document by adding some more information, maybe we can keep that for another PR, wdyt?

//!
//! Notes:
//! Currency related traits in FRAME provide standardized interfaces for implementing various
//! currency functionalities. These traits enable developers to create, transfer, and manage
//! different forms of digital assets within the blockchain environment, ensuring that the economic
//! aspects of the chain are robust and adaptable to different use cases.
juangirini marked this conversation as resolved.
Show resolved Hide resolved
//!
//! - History, `Currency` trait.
//! - `Hold` and `Freeze` with diagram.
//! - `HoldReason` and `FreezeReason`
//! - This footgun: <https://github.com/paritytech/polkadot-sdk/pull/1900#discussion_r1363783609>
//! ## The Currency Trait
//!
//! The [`Currency`](../../../frame_support/traits/tokens/currency/index.html) trait was initially
juangirini marked this conversation as resolved.
Show resolved Hide resolved
//! introduced in Substrate to manage the native token balances. This trait was later deprecated in
juangirini marked this conversation as resolved.
Show resolved Hide resolved
//! favor of the [`fungible`](../../../frame_support/traits/tokens/fungible/index.html) traits in
//! Substrate's PR [#12951](https://github.com/paritytech/substrate/pull/12951). This shift is part
//! of a broader initiative to enhance token management capabilities within the framework. This
//! deprecation is aimed at providing improved safety and more flexibility for managing assets,
//! beyond the capabilities of the original
//! [`Currency`](../../../frame_support/traits/tokens/currency/index.html) trait. This transition
//! enables more diverse economic models in Substrate. For more details, you can view the discussion
//! on the [Tokens Horizon issue](https://github.com/paritytech/polkadot-sdk/issues/327). The
//! [`Currency`](../../../frame_support/traits/tokens/currency/index.html) trait is still available
//! in Substrate, but it is recommended to use the **fungible** traits instead. The
//! [deprecation PR](https://github.com/paritytech/substrate/pull/12951) has a dedicated section on
//! upgrading from **Currency** to **fungible**. Besides, this [issue](https://github.com/paritytech/polkadot-sdk/issues/226)
//! lists the pallets that have been upgraded to the **fungible** traits, and the ones that are
//! still using the [`Currency`](../../../frame_support/traits/tokens/currency/index.html) trait.
//! There one could find the relevant code examples for upgrading.
//!
//! ## Fungible Traits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not going to briefly talk about Fungibles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see fungibles making sense in multiple assets scenario but not here, there is this other PR that covers more generic tokens topics

//!
//! The [`fungible`](../../../frame_support/traits/tokens/fungible/index.html) traits are designed
//! for managing currency types, providing a streamlined approach for single-asset operations.
//! Fungible is currently preferred over currency as the latter is deprecated.
//!
//! #### Holds and Freezes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! #### Holds and Freezes
//! ### Holds and Freezes

//!
//! Learn more about this two concepts in
//! [`frame_support::traits::tokens::fungible::hold`][1] and
//! [`frame_support::traits::tokens::fungible::freeze`][2].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! [`frame_support::traits::tokens::fungible::freeze`][2].
//! [`frame_support::traits::tokens::fungible::freeze`].

I believe the link is added automatically in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really as frame_support is not a dependency of polkadot-sdk-docs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be able to add it as a dep?

//!
//! ## Pallet Balances
juangirini marked this conversation as resolved.
Show resolved Hide resolved
//! The [`pallet_balances`](../../../pallet_balances/index.html) is a key component in FRAME. It
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This linking looks better to me, maybe we can stick with that and remove the references in the other cases?

Copy link
Contributor Author

@juangirini juangirini Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed them where they made the lines too long or difficult to read

//! is designed for managing native token balances. It plays a crucial role in tracking and
//! manipulating the balance of accounts, providing functionalities essential for a wide range of
//! financial operations. The key functions of
//! [`pallet_balances`](../../../pallet_balances/index.html) include transferring tokens between
//! accounts, checking account balances, and adjusting balances for staking or fees. This pallet
//! implements the [`fungible`](../../../frame_support/traits/tokens/fungible/index.html)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the most important detail came last; What I think is the most important piece of info here is that pallet-balances and pallet-assets provide implementations for all of the aforementioned traits. We can list those here (while linking to the proper impl block in the rust-docs).

//! traits, aligning with the standardized approach for asset management in Substrate.
//!
//! [1]: ../../../frame_support/traits/tokens/fungible/hold/index.html
//! [2]: ../../../frame_support/traits/tokens/fungible/freeze/index.html
19 changes: 19 additions & 0 deletions substrate/frame/support/src/traits/tokens/fungible/freeze.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,26 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Freeze
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed the right place to document this. Any methods that can also be improved?

//!
//! The traits for putting freezes within a single fungible token class.
//!
//! Freezes can overlap with [Holds](crate::traits::fungible::hold). Since Holds are designed to be
//! infallibly slashed, this means that any logic using a `Freeze` must handle the possibility of
//! the frozen amount being reduced, potentially to zero. A permissionless function should be
//! provided in order to allow bookkeeping to be updated in this instance. E.g. some balance is
//! frozen when it is used for voting, one could use held balance for voting, but nothing prevents
//! this frozen balance from being reduced if the overlapping hold is slashed.
//!
//! ```
//! |__total__________________________________|
//! |__on_hold__|_____________free____________|
//! |__________frozen___________|
//! |__on_hold__|__ed__|
//! ```
//!
//! Freezes require a `Reason`, which is configurable and is expected to be an enum aggregated
Copy link
Contributor

@kianenigma kianenigma Jan 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly mention or link to the exact keyword of RuntimeFreezeReason.

//! across all pallet instances of the runtime.

use scale_info::TypeInfo;
use sp_arithmetic::{
Expand Down
11 changes: 11 additions & 0 deletions substrate/frame/support/src/traits/tokens/fungible/hold.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Holds
//!
//! The traits for putting holds within a single fungible token class.
//!
//! Holds are explicitly designed to be infallibly slashed. They do not contribute to the ED but
//! do require a provider reference, removing any possibility of account reference counting from
//! being problematic for a slash. They are also always named, ensuring different holds do not
//! accidentally slash each other's balances. E.g. some balance is held when it is put to staking,
//! it does not contribute to the ED, but it is slashed when the staker misbehaves.
//!
//! Holds require a `Reason`, which is configurable and is expected to be an enum aggregated across
//! all pallet instances of the runtime.

use crate::{
ensure,
Expand Down
8 changes: 7 additions & 1 deletion substrate/frame/support/src/traits/tokens/fungible/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! The traits for dealing with a single fungible token class and any associated types.
//! # Fungible Traits
//!
//! **The traits for dealing with a single fungible token class and any associated types.**
//!
//! This trait includes key methods for asset management, like transfer, balance check, and minting.
//! It's particularly useful in scenarios involving a single currency type, simplifying the
//! implementation and management process.
//!
//! ### User-implememted traits
//! - `Inspect`: Regular balance inspector functions.
Expand Down
4 changes: 4 additions & 0 deletions substrate/frame/support/src/traits/tokens/fungibles/freeze.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Freeze
//!
//! The traits for putting freezes within a single fungible token class.
//!
//! Refer to [`frame_support::traits::tokens::fungible::freeze`] for more info.

use crate::{ensure, traits::tokens::Fortitude};
use scale_info::TypeInfo;
Expand Down
4 changes: 4 additions & 0 deletions substrate/frame/support/src/traits/tokens/fungibles/hold.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Hold
//!
//! The traits for putting holds within a single fungible token class.
//!
//! Refer to [`frame_support::traits::tokens::fungible::hold`] for more info.

use crate::{
ensure,
Expand Down
9 changes: 8 additions & 1 deletion substrate/frame/support/src/traits/tokens/fungibles/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! The traits for sets of fungible tokens and any associated types.
//! # Fungibles Traits
//!
//! **The traits for sets of fungible tokens and any associated types.**
//!
//! Offers a comprehensive solution for managing multiple asset types within a single system.
//! It's more complex than the [`fungible`](crate::traits::tokens::fungible) trait, suited for
//! environments where diverse asset types coexist and interact. This trait is essential in
//! multi-currency contexts, providing the necessary tools for intricate asset management.

pub mod approvals;
mod enumerable;
Expand Down