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

Jump to bottom

Investigate fungible trait Holds and Freezes overlap. #1819

Closed
4 of 6 tasks
mattheworris opened this issue Dec 19, 2023 · 0 comments · Fixed by #1873
Closed
4 of 6 tasks

Investigate fungible trait Holds and Freezes overlap. #1819

mattheworris opened this issue Dec 19, 2023 · 0 comments · Fixed by #1873
Assignees
@mattheworris
Labels
Spike research / spike tech-debt

Comments

@mattheworris
Copy link
Collaborator

mattheworris commented Dec 19, 2023 •
edited

As referenced here: Deprecate Currency; introduce holds and freezing into fungible traits
and implemented in the following PRs: #1779 #1818

The following comment indicates that we need to investigate some test cases where Holds are slashed and ensure that the affected pallets handle their bookkeeping for Freezes correctly:

Freezes are essentially the same as locks, except that they overlap with Holds. 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.

Discussion

  • Implement e2e tests that exercise the overlap between Holds and Freezes
    • Build a test where an account with a Freeze and a Hold and the Hold is slashed
    • Ensure the capacity pallet handles the logic correctly without any exploits
      • Check if amount staked is reduced to correlate with the amount slashed. Users should not be able to continue receiving the same capacity when tokens are slashed.
    • Ensure that a user can stake token to network and participate in governance with the same token amount (frozen amount).
    • Ensure the time-release pallet handles the logic correctly without any exploits

Changes

  • Add e2e test with simultaneous Hold and Freeze.

How to Test

  • Confirm that the new e2e tests pass.
@mattheworris mattheworris added the Spike research / spike label Dec 19, 2023
@mattheworris mattheworris self-assigned this Dec 19, 2023
@enddynayn enddynayn mentioned this issue Jan 9, 2024
Closed
2 tasks
@mattheworris mattheworris mentioned this issue Feb 12, 2024
Merged
6 tasks
@mattheworris mattheworris linked a pull request Feb 12, 2024 that will close this issue
Investigate fungible trait Holds and Freezes overlap #1873
Merged
6 tasks
mattheworris added a commit that referenced this issue Feb 14, 2024
@mattheworris
Investigate fungible trait Holds and Freezes overlap (#1873)
03bc60e 
# Goal
The goal of this PR is to investigate the overlap of `hold` and `frozen`
in the frame system account, used by the balances pallet and the
fungible trait (which replaced the Currency trait).

Closes #1819

# Discussion
Investigation indicates that it is not currently possible to create a
situation where `frozen` overlaps with `hold` [currently indicated by
`reserved` in the frame system account] and that `hold` can be slashed.

- Use the `Democracy` pallet to create a proposal that would result in
some tokens being placed on `hold`. The tokens on `hold` would always be
returned to the user at some point in the future, they are not able to
be slashed, as near as I can tell.
- Use the `treasury` pallet to create a spend proposal that would result
in a minimum of 100 tokens being used as a bond and placed on `hold`.
These tokens will be slashed if the proposal is rejected. We can
simulate a rejection using `sudo` and see that the tokens are slashed.
- Validators and their Nominators can have their stake slashed. This is
done using `hold`.

However, experimentation with `hold` shows that conditions where `hold`
and `frozen` overlap cause the transactions to fail.

Tests were added to make sure this behavior does not change in the
future.

# Changes
Two tests were added: 
1. Create a treasury spend proposal and then attempt to stake with an
amount that would overlap the `hold`. This transaction fails.
2. Create a stake and then attempt to create a treasury spend proposal
that would require an amount of tokens that would overlap with the
existing stake. This transaction fails.

# How to Test

- Confirm that the e2e tests pass.


# Checklist
- [ ] Chain spec updated
- [ ] Custom RPC OR Runtime API added/changed? Updated js/api-augment.
- [ ] Design doc(s) updated
- [x] Tests added
- [ ] Benchmarks added
- [ ] Weights updated

---------

Co-authored-by: Matthew Orris <--help>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattheworris mattheworris

Labels
Spike research / spike tech-debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Investigate fungible trait Holds and Freezes overlap frequency-chain/frequency
2 participants
@wilwade @mattheworris