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

Unable to destroy Nomination Pools #4440

Closed
Ank4n opened this issue May 13, 2024 · 1 comment
Closed

Unable to destroy Nomination Pools #4440

Ank4n opened this issue May 13, 2024 · 1 comment
Labels
I2-bug The node fails to follow expected behavior.

Comments

@Ank4n
Copy link
Contributor

Ank4n commented May 13, 2024

First reported for Pool 231.

When the owner tries to withdraw_unbonded, they get a Token: Frozen error.

The issue seems to have started since the last polkadot runtime upgrade.

Issue

The pool seems to have an extra consumer reference. The pool is expected to have two consumer references

  • explicit consumer increment by pallet-staking.
  • applying lock on ledger increments consumer.

but most of the pool including #231 has 3 references. The destruction of pool fails when it tries to transfer all the remaining amount from pool account to the owner specifically on this condition. Since it has one remaining consumer, it cannot kill the account and expects ED to remain in the pool account.

This issue affects almost all the current pools if they try to destroy.

Cause

This PR seems to have led to this issue: #1976.
Earlier, applying a lock would increase consumer by 2, and similarly decrease by 2 when removed.

With the above fix, consumer references are only increased by 1. The older accounts with an extra consumer reference though are now in a bad state and cannot be killed.

Solution

There seems to be no easy way to detect on-chain which accounts have an extra consumer reference. For pool accounts, we could expect to only have 2 consumers and potentially this can be fixed in a migration.

A better solution would be fixing for all the accounts that have this extra reference.
The issue is mentioned and tracked at #2037.

Update

We could explicitly kill account when pools are destroyed. This can be either by force consumer decrement or a separate (new) fungible api that transfers and kills account.

@kianenigma
Copy link
Contributor

I suggest exploring a more short-sighted solution, even if it involved using governance, to fix all the current and prospect pools, even if a more systematic solution cannot be found. Not being able to destroy the pool implies a greater economic damage (500+ DOT) vs. an account (1, 0.01 DOT depending on the chain).

github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
…ce on the pool account (#4503)

addresses #4440 (will
close once we have this in prod runtimes).
related: #2037.

An extra consumer reference is preventing pools to be destroyed. When a
pool is ready to be destroyed, we
can safely clear the consumer references if any. Notably, I only check
for one extra consumer reference since that is a known bug. Anything
more indicates possibly another issue and we probably don't want to
silently absorb those errors as well.

After this change, pools with extra consumer reference should be able to
destroy normally.
Ank4n added a commit that referenced this issue May 20, 2024
…ce on the pool account (#4503)

addresses #4440 (will
close once we have this in prod runtimes).
related: #2037.

An extra consumer reference is preventing pools to be destroyed. When a
pool is ready to be destroyed, we
can safely clear the consumer references if any. Notably, I only check
for one extra consumer reference since that is a known bug. Anything
more indicates possibly another issue and we probably don't want to
silently absorb those errors as well.

After this change, pools with extra consumer reference should be able to
destroy normally.
fellowship-merge-bot bot pushed a commit to polkadot-fellows/runtimes that referenced this issue May 21, 2024
Changes:
- Fixes the issue [preventing nomination pools from getting
destroyed](paritytech/polkadot-sdk#4440).
- Adds a new staking runtime api to Kusama and Polkadot to help with
checking if era rewards are pending for a validator. Refer
paritytech/polkadot-sdk#4301.
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
…ce on the pool account (paritytech#4503)

addresses paritytech#4440 (will
close once we have this in prod runtimes).
related: paritytech#2037.

An extra consumer reference is preventing pools to be destroyed. When a
pool is ready to be destroyed, we
can safely clear the consumer references if any. Notably, I only check
for one extra consumer reference since that is a known bug. Anything
more indicates possibly another issue and we probably don't want to
silently absorb those errors as well.

After this change, pools with extra consumer reference should be able to
destroy normally.
@Ank4n Ank4n closed this as completed Jun 27, 2024
@github-project-automation github-project-automation bot moved this from 📕 Backlog to ✅ Done in (Nominated) Proof of Stake Jun 27, 2024
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
…ce on the pool account (paritytech#4503)

addresses paritytech#4440 (will
close once we have this in prod runtimes).
related: paritytech#2037.

An extra consumer reference is preventing pools to be destroyed. When a
pool is ready to be destroyed, we
can safely clear the consumer references if any. Notably, I only check
for one extra consumer reference since that is a known bug. Anything
more indicates possibly another issue and we probably don't want to
silently absorb those errors as well.

After this change, pools with extra consumer reference should be able to
destroy normally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior.
Projects
Status: Done
Development

No branches or pull requests

2 participants