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

Fix consumer handling in pallet balances update_locks #1970

Closed
bkchr opened this issue Oct 21, 2023 · 7 comments · Fixed by #1976
Closed

Fix consumer handling in pallet balances update_locks #1970

bkchr opened this issue Oct 21, 2023 · 7 comments · Fixed by #1976
Assignees
Labels
I2-bug The node fails to follow expected behavior.

Comments

@bkchr
Copy link
Member

bkchr commented Oct 21, 2023

When removing all locks of an account there is a bug that leads to reducing the consumer count twice.

First it happens here:

frame_system::Pallet::<T>::dec_consumers(who);

And then the second time here:

system::Pallet::<T>::dec_consumers(who);

@bkchr bkchr added the I2-bug The node fails to follow expected behavior. label Oct 21, 2023
@liamaharon liamaharon self-assigned this Oct 21, 2023
@liamaharon
Copy link
Contributor

Self-assigned, I need to spend some time understanding this for writing docs anyway.

@xlc
Copy link
Contributor

xlc commented Oct 21, 2023

we need a script to check if this bug have happened on any parachain and generate a list of accounts that needs fix up

@xlc
Copy link
Contributor

xlc commented Oct 21, 2023

and this is something should be identified by a fuzzer so I guess we don’t have enough of them

@xlc
Copy link
Contributor

xlc commented Oct 23, 2023

I can image it could be hard to write such script that works on all the parachains. That should spark another question: What can we improve to make such task easier in future? e.g. better event system or better indexer or better invariant checker

@bkchr
Copy link
Member Author

bkchr commented Oct 23, 2023

Writing a script for every chain should be possible. You just need to monitor pallet_balances::Lock and the changes to consumers. We would probably only require teams to set the starting block for when the runtime was activated that contained the broken logic.

@michalisFr
Copy link

@bkchr Does this relate to this issue? #1404 (comment)

@bkchr
Copy link
Member Author

bkchr commented Oct 23, 2023

@bkchr Does this relate to this issue? #1404 (comment)

Could probably be related.

liamaharon added a commit that referenced this issue Oct 30, 2023
…ng consumers (#1976)

Closes #1970

Follow up issue to tackle, once the erroneous double
incrementing/decrementing has stopped:
#2037
@github-project-automation github-project-automation bot moved this from Backlog to Done in Runtime / FRAME Oct 30, 2023
girazoki pushed a commit to moondance-labs/polkadot-sdk that referenced this issue Dec 1, 2023
…ng consumers (paritytech#1976)

Closes paritytech#1970

Follow up issue to tackle, once the erroneous double
incrementing/decrementing has stopped:
paritytech#2037
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
…ng consumers (paritytech#1976)

Closes paritytech#1970

Follow up issue to tackle, once the erroneous double
incrementing/decrementing has stopped:
paritytech#2037
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
4 participants