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

Linter: Warn when storage can never be freed up #1932

Merged
merged 18 commits into from
Oct 5, 2023

Conversation

jubnzv
Copy link
Member

@jubnzv jubnzv commented Oct 4, 2023

Summary

Closes #1431

  • [n] Does it introduce breaking changes?
  • [n] Is it dependant on the specific version of cargo-contract or pallet-contracts?

Description

This PR introduces a new lint for ink_linting that finds storage fields of Mapping and Vec types that are used to insert elements, however there is no way to remove their elements.

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, and this lint helps developers to find these cases.

By default, it has the Allow warning level, which means, that that lint must be explicitly enabled by the developer. The reason for that is that there are many cases where adding elements without removing them is necessary and should not be considered as an error.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@jubnzv jubnzv merged commit 1b84f8a into use-ink:master Oct 5, 2023
22 checks passed
jubnzv added a commit to jubnzv/ink that referenced this pull request Oct 5, 2023
cmichi pushed a commit that referenced this pull request Oct 25, 2023
* feat(linter): strict balance equality lint

The current implementation works only with intraprocedural MIR and does
not support taint propagation across function calls.

Closes #1811

* feat(lint): Handle temporary values resulted after Rvalue::Use

* fix(lint): spans to emit diagnostics

Previously, diagnostics did not work, since `terminator.span` is
resulted after macro expansion

* feat(tests): more tests

* feat(lint): Manually traverse functions in user-defined code

This is required to implement interprocedural analysis

* feat(lint): interprocedural analysis that finds tainted returns

* fix(lint): recursive calls in interprocedural analysis

* fix(lint): false negative on `CheckedBinaryOp`

* feat(lint): propagation through references

* feat(lint): Propagate tainted values through `&mut` arguments

* chore(lint): docstring, comments

* feat(lint): handle comparison of references in functions

* chore(tests): comments

* feat(lint+tests): updated `pass` test, fixed binop conditions

* feat(tests): test for lint suppressions

* chore(tests): fmt

* chore(tests): fmt

* chore: Add changelog entry

* chore(lint): Reuse utility functions introduced in #1932

* chore: Fix changelog

* chore: Fix comments
@SkymanOne SkymanOne mentioned this pull request Nov 30, 2023
@SkymanOne SkymanOne mentioned this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linter: Warn when storage can never be freed up
2 participants