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

Persistent Bonding State Issue with Zero Existential Deposit #4340

Closed
2 tasks done
benefacto opened this issue Apr 30, 2024 · 2 comments · Fixed by #4364
Closed
2 tasks done

Persistent Bonding State Issue with Zero Existential Deposit #4340

benefacto opened this issue Apr 30, 2024 · 2 comments · Fixed by #4364
Labels
I2-bug The node fails to follow expected behavior.

Comments

@benefacto
Copy link

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

We are building a new layer 1 using Frontier EVM alongside pallet_staking. We are using a custom Substrate token as its native token. We have set our existential deposit to zero to prevent dusting. However, we've identified an issue related to storage writes in the staking process, specifically with the Self::bonded entry. According to the documentation, this entry is not cleared unless the balance falls below the existential deposit and is removed as dust. With an existential deposit of zero, these entries are never cleaned, which leads to a situation where stakers remain indefinitely bonded with a stake of zero after they chill and unbond. This results in unnecessary storage overhead and potential unknown issues.

Expected behavior:
After an account is fully unbonded, it should not remain in a bonded state if it has a zero stake.

Actual behavior:
Accounts remain bonded with zero stake due to the non-removal of Self::bonded because the existential deposit is zero.

Potential impact:
This could lead to increased storage overhead and might cause other unknown issues in network behavior or staking mechanics.

Steps to reproduce

  1. Extend the frontier node template by configuring pallet_staking.
  2. Follow the staking, validating, chilling, and unbonding process with any account. Example steps:
    • Bond an account with a minimum stake value and set it as a validator.
    • After some operations and era transitions, chill and unbond the account.
    • Check the bonded state of the account after unbonding is completed.
    • The account remains bonded with a stake of zero which is not the expected behavior.
@benefacto benefacto added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Apr 30, 2024
@kianenigma
Copy link
Contributor

cc @Ank4n @gpestana

@Ank4n
Copy link
Contributor

Ank4n commented May 2, 2024

Yes, seems staking pallet has not been written with expectation that ED can be zero.
Fix is to do a <= check with ED or an explicit zero check (safer as it is noop for runtimes with ed > 0).

@Ank4n Ank4n removed the I10-unconfirmed Issue might be valid, but it's not yet known. label May 2, 2024
This was referenced May 3, 2024
Closed
Merged
github-merge-queue bot pushed a commit that referenced this issue May 3, 2024
@krisbitney
Fix: dust unbonded for zero existential deposit (#4364)
5198623 
When a staker unbonds and withdraws, it is possible that their stash
will contain less currency than the existential deposit. If that
happens, their stash is reaped. But if the existential deposit is zero,
the reap is not triggered. This PR adjusts `pallet_staking` to reap a
stash in the special case that the stash value is zero and the
existential deposit is zero.

This change is important for blockchains built on substrate that require
an existential deposit of zero, becuase it conserves valued storage
space.

There are two places in which ledgers are checked to determine if their
value is less than the existential deposit and they should be reaped: in
the methods `do_withdraw_unbonded` and `reap_stash`. When the check is
made, the condition `ledger_total == 0` is also checked. If
`ledger_total` is zero, then it must be below any existential deposit
greater than zero and equal to an existential deposit of 0.

I added a new test for each method to confirm the change behaves as
expected.

Closes #4340

---------

Co-authored-by: command-bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I2-bug The node fails to follow expected behavior.
Projects
(Nominated) Proof of Stake
Status: ✅ Done
Milestone
No milestone
3 participants
@kianenigma @Ank4n @benefacto