Skip to content

Conversation

teodanciu
Copy link
Contributor

@teodanciu teodanciu commented Sep 29, 2025

Description

Store, in each pool's StakePoolState, the stake credentials that have delegated to that pool.
This set is updated in DELEG rule, for both shelley and conway:

  • on (re)delegation
  • when the stake credential is unregistered

The set is used to improve the performance of removing delegations when retiring the pool.
Based on PR: #5330

Closes: #4990

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch 12 times, most recently from b86f5cb to e358c21 Compare October 2, 2025 16:13
@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch 13 times, most recently from 4dea23b to f44c36b Compare October 10, 2025 19:10
@teodanciu teodanciu changed the title [WIP] - Store delegators in pool state Store delegators in pool state Oct 10, 2025
@teodanciu teodanciu marked this pull request as ready for review October 10, 2025 20:32
@teodanciu teodanciu requested a review from a team as a code owner October 10, 2025 20:32
@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch 2 times, most recently from 7cf8b76 to 47d319c Compare October 15, 2025 12:13
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

This is a great piece of work. There is only one real problem in this PR and the rest of the comments are things that I've notice that can be improved thanks to your new changes.

cState
& certDStateL . accountsL
%~ adjustAccountState (stakePoolDelegationAccountStateL ?~ stakePool) stakeCred
& certPStateL %~ adjustPState stakePool
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will require processDelegationInternal to accept Maybe (AccountState era), instead of Maybe Delegatee, which should be totally doable

Suggested change
& certPStateL %~ adjustPState stakePool
& certPStateL %~ unDelegReDelegStakePool cred accountState (Just stakePool)

DelegStakeVote oldPool _ | Just oldPool /= mNewPool -> Just oldPool
_ -> Nothing

unDelegDRep ::
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of unDelegDRep I propose to add the exact same function as I suggested for stake pools in the Shelley DELEG rule, but for DReps and move it to where VState is defined:

-- Update reverse DRep delegation Set whenever stake credential unregisters or changes its delegation. Whenever new delegation matches the old one this is a noop
unDelegReDelegDRep ::
  Credential 'Staking ->
  AccountState era ->
  -- ^ Account that is loosing its current delegation and/or acquiring a new one
  Maybe DRep ->
  -- ^ Potential new delegation. In case when stake credential unregisters this must be `Nothing`.
  VState era ->
  VState era
unDelegReDelegDRep stakeCred accountState mNewDRep = 
  fromMaybe (vsDRepsL %~ addNewDelegation) $ do
    dRep@(DRepCredential dRepCred) <- accountState ^. dRepDelegationAccountStateL
    pure $ 
      -- There is no need to update set of delegations if delegation is unchanged
      if Just dRep == mNewStakePool
      then id
      else 
        vsDRepsL %~ addNewDelegation . Map.adjust (drepDelegsL %~ Set.delete stakeCred) dRepCred)
  where
    addNewDelegation = 
      case mNewDRep of
        Just (DRepCredential dRepCred) ->
          Map.adjust (drepDelegsL %~ Set.insert stakeCred) dRepCred
        Nothing -> id

Comment on lines 334 to 336
adjustPState newPool =
(psStakePoolsL %~ Map.adjust (spsDelegsL %~ Set.insert stakeCred) newPool)
. unDelegStakePool stakeCred mCurDelegatee (Just newPool)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No longer needed

Suggested change
adjustPState newPool =
(psStakePoolsL %~ Map.adjust (spsDelegsL %~ Set.insert stakeCred) newPool)
. unDelegStakePool stakeCred mCurDelegatee (Just newPool)

cState
& certDStateL . accountsL
%~ adjustAccountState (dRepDelegationAccountStateL ?~ dRep) stakeCred
& certVStateL %~ unDelegDRep stakeCred mCurDelegatee
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
& certVStateL %~ unDelegDRep stakeCred mCurDelegatee
& certVStateL %~ unDelegReDelDRep stakeCred mAccountState (Just dRep)

Comment on lines 325 to 327
dReps
| preserveIncorrectDelegation = cState ^. certVStateL . vsDRepsL
| otherwise = cState' ^. certVStateL . vsDRepsL
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will need to be slightly rewritten:

Suggested change
dReps
| preserveIncorrectDelegation = cState ^. certVStateL . vsDRepsL
| otherwise = cState' ^. certVStateL . vsDRepsL
in if preserveIncorrectDelegation
then cState & certVStateL %~ case dRep of
DRepCredential dRepCred ->
vsDRepsL %~ Map.adjust (drepDelegsL %~ Set.insert stakeCred) dRepCred
_ -> id
else cState'

And all the stuff below is no longer needed.

@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch 2 times, most recently from f803f7e to 75acf46 Compare October 21, 2025 14:13
@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch from 75acf46 to 4f3f4ba Compare October 21, 2025 14:31
@teodanciu teodanciu force-pushed the td/store-delegs-in-stakepoolstate branch from 4f3f4ba to e8348a9 Compare October 21, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve performance of undelegation when reaping stake pools

2 participants