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

New query GetPoolDistr #3932

Merged
merged 1 commit into from
Aug 17, 2022
Merged

New query GetPoolDistr #3932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/NetworkProtocolVersion.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ data ShelleyNodeToClientVersion =
-- | New queries introduced: GetRewardInfoPools
| ShelleyNodeToClientVersion5

-- | New queries introduced: GetPoolState, GetStakeSnapshots
-- | New queries introduced: GetPoolDistr, GetPoolState, GetStakeSnapshots
| ShelleyNodeToClientVersion6
deriving (Show, Eq, Ord, Enum, Bounded)

Expand Down
25 changes: 25 additions & 0 deletions ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/Query.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ data instance BlockQuery (ShelleyBlock proto era) :: Type -> Type where
-> BlockQuery (ShelleyBlock proto era)
(StakeSnapshots (EraCrypto era))

GetPoolDistr
:: Maybe (Set (SL.KeyHash 'SL.StakePool (EraCrypto era)))
-> BlockQuery (ShelleyBlock proto era)
(SL.PoolDistr (EraCrypto era))
nfrisby marked this conversation as resolved.
Show resolved Hide resolved

-- WARNING: please add new queries to the end of the list and stick to this
-- order in all other pattern matches on queries. This helps in particular
-- with the en/decoders, as we want the CBOR tags to be ordered.
Expand Down Expand Up @@ -330,6 +335,12 @@ instance (ShelleyCompatible proto era, ProtoCrypto proto ~ crypto) => QueryLedge
, ssSetTotal = getAllStake _pstakeSet
, ssGoTotal = getAllStake _pstakeGo
}
GetPoolDistr mPoolIds ->
let poolDistr = SL.calculatePoolDistr . SL._pstakeSet . SL.esSnapshots $ getEpochState st in
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this calculatePoolDistr perhaps too expensive?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be, but I don't know that we have any clear requirements on what is too expensive. Note that SL._pstakeSet is one of the data structure that we anticipate being moved to disk.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is for query leadership-schedule, so I think we have a good amount of headroom -- I wouldn't expect SPOs etc to be spamming this command (though it's easy enough to do that accidentally within a script).

Copy link
Contributor

Choose a reason for hiding this comment

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

We could micro-benchmark the function, and see where we are at, if we are worried.

For a general idea as to what calculatePoolDistr does:

https://github.com/input-output-hk/cardano-ledger/blob/dd4b6e31a05e3c02c6f00f0d53cf20e3b8b74468/eras/shelley/impl/src/Cardano/Ledger/Shelley/Rules/NewEpoch.hs#L204-L220

  • convert the very large stake distribution into an association list
    • un-compacting the coins in the process
    • lookup the stake creds in the delegation map in the process
  • Map.fromListWith (+) on the big association list
  • Map.intersectionWith on the condensed map to add in the VRF keys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to make this cheaper if we're only interested in a subset of pool ids?

Copy link
Contributor

Choose a reason for hiding this comment

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

great question! The following might be faster:

Make a new version of calculatePoolDistr with signature

calculatePoolDistr' :: SnapShot crypto -> Set (KeyHash 'Staking crypto) -> PoolDistr crypto

which is identical to calculatePoolDistr except that after looking up the credential in the delegation map (see here), it also looks up the pool ID in the set of pool IDs provided by the user (and discards the result if not found).

Copy link

Choose a reason for hiding this comment

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

I'd look at use cases before optimising too much. I think the two likely cases are:

  • all pools
  • a specific pool (and then repeat for multi-pool operators)

The command is likely to be used occasionally rather than frequently, I expect

case mPoolIds of
Just poolIds -> SL.PoolDistr $ Map.restrictKeys (SL.unPoolDistr poolDistr) poolIds
Nothing -> poolDistr

where
lcfg = configLedger $ getExtLedgerCfg cfg
globals = shelleyLedgerGlobals lcfg
Expand Down Expand Up @@ -451,6 +462,13 @@ instance SameDepIndex (BlockQuery (ShelleyBlock proto era)) where
= Nothing
sameDepIndex (GetStakeSnapshots _) _
= Nothing
sameDepIndex (GetPoolDistr poolids) (GetPoolDistr poolids')
| poolids == poolids'
= Just Refl
| otherwise
= Nothing
sameDepIndex (GetPoolDistr _) _
= Nothing

deriving instance Eq (BlockQuery (ShelleyBlock proto era) result)
deriving instance Show (BlockQuery (ShelleyBlock proto era) result)
Expand Down Expand Up @@ -478,6 +496,7 @@ instance ShelleyCompatible proto era => ShowQuery (BlockQuery (ShelleyBlock prot
GetRewardInfoPools -> show
GetPoolState {} -> show
GetStakeSnapshots {} -> show
GetPoolDistr {} -> show

-- | Is the given query supported by the given 'ShelleyNodeToClientVersion'?
querySupportedVersion :: BlockQuery (ShelleyBlock proto era) result -> ShelleyNodeToClientVersion -> Bool
Expand All @@ -503,6 +522,7 @@ querySupportedVersion = \case
GetRewardInfoPools -> (>= v5)
GetPoolState {} -> (>= v6)
GetStakeSnapshots {} -> (>= v6)
GetPoolDistr {} -> (>= v6)
-- WARNING: when adding a new query, a new @ShelleyNodeToClientVersionX@
-- must be added. See #2830 for a template on how to do this.
where
Expand Down Expand Up @@ -592,6 +612,8 @@ encodeShelleyQuery query = case query of
CBOR.encodeListLen 2 <> CBOR.encodeWord8 19 <> toCBOR poolids
GetStakeSnapshots poolId ->
CBOR.encodeListLen 2 <> CBOR.encodeWord8 20 <> toCBOR poolId
GetPoolDistr poolids ->
CBOR.encodeListLen 2 <> CBOR.encodeWord8 21 <> toCBOR poolids

decodeShelleyQuery ::
ShelleyBasedEra era
Expand Down Expand Up @@ -621,6 +643,7 @@ decodeShelleyQuery = do
(1, 18) -> return $ SomeSecond GetRewardInfoPools
(2, 19) -> SomeSecond . GetPoolState <$> fromCBOR
(2, 20) -> SomeSecond . GetStakeSnapshots <$> fromCBOR
(2, 21) -> SomeSecond . GetPoolDistr <$> fromCBOR
_ -> fail $
"decodeShelleyQuery: invalid (len, tag): (" <>
show len <> ", " <> show tag <> ")"
Expand Down Expand Up @@ -650,6 +673,7 @@ encodeShelleyResult query = case query of
GetRewardInfoPools -> toCBOR
GetPoolState {} -> toCBOR
GetStakeSnapshots {} -> toCBOR
GetPoolDistr {} -> toCBOR

decodeShelleyResult ::
ShelleyCompatible proto era
Expand Down Expand Up @@ -677,6 +701,7 @@ decodeShelleyResult query = case query of
GetRewardInfoPools -> fromCBOR
GetPoolState {} -> fromCBOR
GetStakeSnapshots {} -> fromCBOR
GetPoolDistr {} -> fromCBOR

-- | The stake snapshot returns information about the mark, set, go ledger snapshots for a pool,
-- plus the total active stake for each snapshot that can be used in a 'sigma' calculation.
Expand Down
7 changes: 6 additions & 1 deletion ouroboros-consensus/docs/interface-CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ may appear out of chronological order.
The internals of each entry are organized similar to
https://keepachangelog.com/en/1.1.0/, adapted to our plan explained above.

## Circa 2022-08-08

### Added

- `GetPoolDistr`: Get the pool distribution for the given stake pool ids

## Circa 2022-08-03
### Added

Expand All @@ -66,7 +72,6 @@ https://keepachangelog.com/en/1.1.0/, adapted to our plan explained above.
### Removed
- `ouroboros-consensus-cardano/tools/db-analyser` has been removed.


## Circa 2022-07-26

### Added
Expand Down
2 changes: 1 addition & 1 deletion ouroboros-network/src/Ouroboros/Network/NodeToClient/Version.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ data NodeToClientVersion
| NodeToClientV_13
-- ^ enabled @CardanoNodeToClientVersion9@, i.e., Babbage
| NodeToClientV_14
-- ^ added @GetPoolState, @GetSnapshots
-- ^ added @GetPoolDistr, @GetPoolState, @GetSnapshots
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 NodeToClientV_14 hasn't been yet released. node-1.35.x comes with NodeToClientV_13 as the latest version.

deriving (Eq, Ord, Enum, Bounded, Show, Typeable)

-- | We set 16ths bit to distinguish `NodeToNodeVersion` and
Expand Down