Skip to content

Add tolerance to nom pool pending rewards try-state#1236

Merged
liamaharon merged 5 commits into
masterfrom
liam/fix-nom-pool-try-state
Sep 14, 2023
Merged

Add tolerance to nom pool pending rewards try-state#1236
liamaharon merged 5 commits into
masterfrom
liam/fix-nom-pool-try-state

Conversation

@liamaharon

@liamaharon liamaharon commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Closes #158

In our last FRAME call it was discussed that a likely solution to the ED imbalances is lazily fixing the pools as they are interacted with.

So, we should add some tiny tolerance to the try-state checks so next time there's an ED change they don't start failing until they've all been interacted with.

Update 12 Sept

Rather than adding tolerance, have replaced the ensure with a warning.

@liamaharon liamaharon requested a review from Ank4n August 29, 2023 04:51
@liamaharon liamaharon added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Aug 29, 2023
@paritytech-ci paritytech-ci requested review from a team August 29, 2023 04:52
Comment thread substrate/frame/nomination-pools/src/lib.rs Outdated

@kianenigma kianenigma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the convo in https://github.com/paritytech/polkadot-sdk/pull/1236/files#r1308580829, I think this is one of those problem for which no ideal solution exists so I hope to accelerate either of the fixes by approving.

FWIW though, I think given the current way that this pallet is coded, the reward pools always having ED is actually NOT enforced, so instead of adding a tolerance to the check, the check should be strict, but it should only emit a warning instead of panicking.

@liamaharon liamaharon requested review from a team September 12, 2023 03:09
@liamaharon

liamaharon commented Sep 12, 2023

Copy link
Copy Markdown
Contributor Author

I have replaced the ensure check with a warning as suggested.

@liamaharon liamaharon merged commit cc39edd into master Sep 14, 2023
@liamaharon liamaharon deleted the liam/fix-nom-pool-try-state branch September 14, 2023 00:25
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
Closes paritytech#158

In our last FRAME call it was discussed that a likely solution to the ED
imbalances is lazily fixing the pools as they are interacted with.

So, we should add some tiny tolerance to the try-state checks so next
time there's an ED change they don't start failing until they've all
been interacted with.

### Update 12 Sept

Rather than adding tolerance, have replaced the `ensure` with a warning.

---------

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T1-FRAME This PR/Issue is related to core FRAME, the framework.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nomination-pools try-state hook fails on Kusama

5 participants