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.

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

Defensive Programming in Substrate Reference Document #2615

Merged

Conversation

CrackTheCode016
Copy link
Contributor

@CrackTheCode016 CrackTheCode016 commented Dec 4, 2023

This PR is being continued from #2206, which was closed when the developer_hub was merged.
closes paritytech/polkadot-sdk-docs#44


Description

This PR adds a reference document to the developer-hub crate (see #2102). This specific reference document covers defensive programming practices common within the context of developing a runtime with Substrate.

In particular, this covers the following areas:

  • Default behavior of how Rust deals with numbers in general
  • How to deal with floating point numbers in runtime / fixed point arithmetic
  • How to deal with Integer overflows
  • General "safe math" / defensive programming practices for common pallet development scenarios
  • Defensive traits that exist within Substrate, i.e., defensive_saturating_add , defensive_unwrap_or
  • More general defensive programming examples (keep it concise)
  • Link to relevant examples where these practices are actually in production / being used
  • Unwrapping (or rather lack thereof) 101

todo

  • Apply feedback from previous PR
  • This may warrant a PR to append some of these docs to sp_arithmetic

@CrackTheCode016 CrackTheCode016 marked this pull request as draft December 4, 2023 22:11
@CrackTheCode016 CrackTheCode016 marked this pull request as ready for review January 1, 2024 19:24
@juangirini juangirini requested a review from a team January 2, 2024 09:34
@juangirini
Copy link
Contributor

the base of the PR has changed, it shouldn't create/update the developer-hub dir, but rather docs/sdk

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Thanks! This should give us a very solid basis for future doc extensions.

developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
//!
//! ## General Practices
//!
//! When developing within the context of the a runtime, there is one golden rule:
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this ref doc is describing safe programming in Substrate in general, I think it would be interesting to elaborate why the runtime should not panic (no fee DoS and, in some cases, stalled runtime) and where it's OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one example, it may need more elaboration.

developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
developer-hub/src/reference_docs/defensive_programming.rs Outdated Show resolved Hide resolved
CrackTheCode016 and others added 16 commits January 8, 2024 10:07
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
@CrackTheCode016
Copy link
Contributor Author

@kianenigma I have hopefully addressed your feedback, added documentation to the supporting files in sp_arithmetic, and severely reduced the size of the reference doc without sacrificing the content. It may still be verbose in some areas; let me know if this is good for merging. Thanks!

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Yes, great!

Can be merged once the CI is green.

@kianenigma kianenigma added the R0-silent Changes should not be mentioned in any release notes label Mar 18, 2024
auto-merge was automatically disabled March 19, 2024 19:55

Head branch was pushed to by a user without write access

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5575670

@CrackTheCode016
Copy link
Contributor Author

@kianenigma (sorry for the ping) CI looks all good and green now.

@kianenigma kianenigma added this pull request to the merge queue Mar 20, 2024
@kianenigma
Copy link
Contributor

@kianenigma (sorry for the ping) CI looks all good and green now.

You need to fix the toml formats it seems.

Merged via the queue into paritytech:master with commit b686bfe Mar 20, 2024
130 of 132 checks passed
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Mar 24, 2024
_This PR is being continued from
paritytech#2206, which was closed
when the developer_hub was merged._
closes paritytech/polkadot-sdk-docs#44

---
# Description

This PR adds a reference document to the `developer-hub` crate (see
paritytech#2102). This specific
reference document covers defensive programming practices common within
the context of developing a runtime with Substrate.

In particular, this covers the following areas: 

- Default behavior of how Rust deals with numbers in general
- How to deal with floating point numbers in runtime / fixed point
arithmetic
- How to deal with Integer overflows
- General "safe math" / defensive programming practices for common
pallet development scenarios
- Defensive traits that exist within Substrate, i.e.,
`defensive_saturating_add `, `defensive_unwrap_or`
- More general defensive programming examples (keep it concise)
- Link to relevant examples where these practices are actually in
production / being used
- Unwrapping (or rather lack thereof) 101

todo
-- 
- [x] Apply feedback from previous PR
- [x] This may warrant a PR to append some of these docs to
`sp_arithmetic`

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T11-documentation This PR/Issue is related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ref docs for safe_defensive_programming
8 participants