Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
35 changes: 19 additions & 16 deletions linting/extra/src/primitive_topic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,42 +54,45 @@ use rustc_session::{
};

declare_lint! {
/// **What it does:** Checks for ink! contracts that use
/// the [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive
/// number types. Topics are discrete events for which it makes sense to filter. Typical
/// examples of fields that should be filtered are `AccountId`, `bool` or `enum` variants.
/// ## What it does
/// Checks for ink! contracts that use the
/// [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive number
/// types. Topics are discrete events for which it makes sense to filter. Typical examples of
/// fields that should be filtered are `AccountId`, `bool` or `enum` variants.
///
/// **Why is this bad?** It typically doesn't make sense to annotate types like `u32` or `i32`
/// as a topic, if those fields can take continuous values that could be anywhere between
/// `::MIN` and `::MAX`. An example of a case where it doesn't make sense at all to have a
/// topic on the storage field is something like `value: Balance` in the examle below.
///
/// **Example:**
/// ## Why is this bad?
/// It typically doesn't make sense to annotate types like `u32` or `i32` as a topic, if those
/// fields can take continuous values that could be anywhere between `::MIN` and `::MAX`. An
/// example of a case where it doesn't make sense at all to have a topic on the storage field
/// is something like `value: Balance` in the examle below.
///
/// ## Example
/// ```rust
/// // Good
/// // Filtering transactions based on source and destination addresses.
/// // Bad
/// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
/// // to `::MIN`.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// #[ink(topic)]
/// value: Balance,
/// }
/// ```
///
/// Use instead:
///
/// ```rust
/// // Bad
/// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
/// // to `::MIN`.
/// // Good
/// // Filtering transactions based on source and destination addresses.
/// #[ink(event)]
/// pub struct Transaction {
/// #[ink(topic)]
/// src: Option<AccountId>,
/// #[ink(topic)]
/// dst: Option<AccountId>,
/// #[ink(topic)]
/// value: Balance,
/// }
/// ```
Expand Down
7 changes: 3 additions & 4 deletions linting/extra/src/storage_never_freed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,17 @@ use std::collections::{
};

declare_lint! {
/// **What it does:**
/// ## What it does
/// This lint ensures that for every storage field with a collection type, when there is an
/// operation to insert new elements, there's also an operation for removing elements.
///
/// **Why is this bad?**
/// ## Why is this bad?
/// When a user executes a contract function that writes to storage, they have to put a
/// deposit down for the amount of storage space used. Whoever frees up that storage at some
/// later point gets the deposit back. Therefore, it is always a good idea to make it possible
/// for users to free up their storage space.
///
/// **Example:**
///
/// ## Example
/// In the following example there is a storage field with the `Mapping` type that has an
/// function that inserts new elements:
///
Expand Down
21 changes: 13 additions & 8 deletions linting/extra/src/strict_balance_equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,25 @@ use std::collections::{
};

declare_lint! {
/// **What it does:** Looks for strict equalities with balance in ink! contracts.
/// ## What it does
/// Looks for strict equalities with balance in ink! contracts.
///
/// **Why is this bad?** The problem with strict balance equality is that it is always possible
/// to forcibly send tokens to a contract. For example, using
/// ## Why is this bad?
/// The problem with strict balance equality is that it is always possible to forcibly send
/// tokens to a contract. For example, using
/// [`terminate_contract`](https://paritytech.github.io/ink/ink_env/fn.terminate_contract.html).
/// In such a case, the condition involving the contract balance will work incorrectly, what
/// may lead to security issues, including DoS attacks and draining contract's gas.
///
/// **Known problems**: There are many ways to implement balance comparison in ink! contracts.
/// This lint is not trying to be exhaustive. Instead, it addresses the most common cases that
/// may occur in real-world contracts and focuses on precision and lack of false positives.
///
/// **Example:**
/// ## Known problems
/// There are many ways to implement balance comparison in ink! contracts. This lint is not
/// trying to be exhaustive. Instead, it addresses the most common cases that may occur in
/// real-world contracts and focuses on precision and lack of false positives.
///
/// ## Example
/// Assume, there is an attacker contract that sends all its funds to the target contract when
/// terminated:
///
/// ```rust
/// #[ink::contract]
/// pub mod attacker {
Expand All @@ -101,6 +104,7 @@ declare_lint! {
///
/// If the target contains a condition with strict balance equality, this may be manipulated by
/// the attacker:
///
/// ```rust
/// #[ink::contract]
/// pub mod target {
Expand All @@ -116,6 +120,7 @@ declare_lint! {
///
/// This could be mitigated using non-strict equality operators in the condition with the
/// balance:
///
/// ```rust
/// #[ink::contract]
/// pub mod target {
Expand Down
3 changes: 2 additions & 1 deletion linting/mandatory/src/no_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ declare_lint! {
/// ## Example
///
/// ```rust
/// // Bad: Contract does not contain the `no_main` attribute, so it cannot be compiled to Wasm
/// // Bad: Contract does not contain the `no_main` attribute,
/// // so it cannot be compiled to Wasm
/// #![cfg_attr(not(feature = "std"), no_std)]
/// #[ink::contract]
/// mod my_contract { /* ... */ }
Expand Down
55 changes: 55 additions & 0 deletions scripts/generate_linter_docs.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#!/usr/bin/env bash

usage() {
echo "Usage: $0 PATH"
echo "Creates or updates the documentation for ink_linting hosted at use.ink."
echo
echo "Arguments:"
echo " PATH Path to the local clone of https://github.com/paritytech/ink-docs"
echo
echo "Examples:"
echo " $0 ~/ink-docs"
}

if [ ! $# -eq 1 ]; then
usage
exit 1
fi

ink_docs_dir="$1"
tmp_file=$(mktemp /tmp/linting_docs.XXXXXX.md)
had_update=0
for lint_src in linting/{mandatory,extra}/src/*.rs; do
lint_name="$(basename "${lint_src%.*}")"
lint_doc_file="$ink_docs_dir/docs/linter/rules/$lint_name.md"
lint_doc_filestring="$(sed -n '/^declare_lint! {$/,/^}$/p' "$lint_src")"
[[ -z "$lint_doc_filestring" ]] && continue

# Save the extracted documentation to the temporary file
truncate -s 0 "$tmp_file"
{
echo "---"
echo "title: $lint_name"
echo "hide_title: true"
echo "description: $lint_name lint documentation"
echo "---"
echo "# $lint_name"
printf "%s" "$lint_doc_filestring" | sed -n 's/^\s*\/\/\/\s\?\(.*\)$/\1/;T;p'
} >> "$tmp_file"

if [[ ! -f "$lint_doc_file" ]]; then
echo "$lint_name: created documentation"
had_update=1
mv "$tmp_file" "$lint_doc_file"
continue
fi

if cmp -s "$tmp_file" "$lint_doc_file"; then
echo "$lint_name: no changes"
else
echo "$lint_name: updated"
mv "$tmp_file" "$lint_doc_file"
fi
done

[[ $had_update -eq 1 ]] && echo -e "\nPlease add created documentation sections to $ink_docs_dir/sidebars.js"