Optimise query stake snapshot command

[newhoggy]

Depends on IntersectMBO/ouroboros-network#3898
Resolves #2717

[newhoggy]

Before:

$ time CARDANO_NODE_SOCKET_PATH=/Users/jky/wrk/iohk/mainnet/cardano-node.socket cardano-cli query stake-snapshot --cardano-mode --mainnet --stake-pool-id fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4

{
    "activeStakeGo": 24456415918186222,
    "activeStakeMark": 24453012636354912,
    "activeStakeSet": 24441961174746539,
    "poolStakeGo": 8595693051,
    "poolStakeMark": 8595693051,
    "poolStakeSet": 8595693051
}
CARDANO_NODE_SOCKET_PATH=/Users/jky/wrk/iohk/mainnet/cardano-node.socket       54.74s user 4.08s system 63% cpu 1:32.24 total

After:

$ time CARDANO_NODE_SOCKET_PATH=/Users/jky/wrk/iohk/mainnet/cardano-node.socket $(./scripts/bin-path.sh cardano-cli) query stake-snapshot --cardano-mode --mainnet --stake-pool-id fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4

{
    "activeStakeGo": 24456415918186222,
    "activeStakeMark": 24453012636354912,
    "activeStakeSet": 24441961174746539,
    "poolStakeGo": 8595693051,
    "poolStakeMark": 8595693051,
    "poolStakeSet": 8595693051
}
CARDANO_NODE_SOCKET_PATH=/Users/jky/wrk/iohk/mainnet/cardano-node.socket       0.01s user 0.01s system 1% cpu 1.014 total

[Jimbo4350]

Couple of minor changes

comments addressed

[Jimbo4350]

LGTM! Please squash the commits and you have some build errors

[Jimbo4350]

Please squash the commits

Squashed commits

[Jimbo4350]

Why don't we implement something similar to:

data QueryUTxOFilter =
     -- | /O(n) time and space/ for utxo size n
     QueryUTxOWhole

     -- | /O(n) time, O(m) space/ for utxo size n, and address set size m
   | QueryUTxOByAddress (Set AddressAny)

     -- | /O(m log n) time, O(m) space/ for utxo size n, and address set size m
   | QueryUTxOByTxIn (Set TxIn)
  deriving (Eq, Show)

We can then account for the case where we are interested in single or multiple pool stake snapshots.

e.g

data QueryStakeSnapshotFilter = 
     QuerySinglePoolStakeSnapshot PoolId
   | QueryMutiplePoolStakeSnapshots (Set PoolId) 

And then we can pipe that through in a similar fashion to QueryUTxOFilter

[Jimbo4350]

I think this logic shouldn't be in the JSON instance but at the point of rendering. The JSON instance should only be concerned with rendering JSON. In the cli we should determine how we render depending on if we get a Map of length 1 or more.

[newhoggy]

There's not a good way to do the fact there is a Map at all breaks backwards compatibility.

This PR is meant to preserve backwards compatibility.

A follow up PR simplifies this code but breaks backwards compatibility. #4279

It would be good to get an optimised version of this command merged first before merging the change that breaks compatibility.

[Jimbo4350]

I've just re-discovered data Stakes. This is what we were rendering. What we should do to maintain backwards compatibility is create a function with type signature Consensus.StakeSnapshots crypto -> Stakes. Imposing our cli logic on a JSON instance seems wrong, which should just be concerned with rendering JSON and should not be dictated by the needs of our query.

[newhoggy]

The data doesn't fit into data Stakes either because the map can be empty and I would not have anything to fill the pool fields in that case.

I've rendered the JSON manually instead.

Comments addressed.

[newhoggy]

data QueryStakeSnapshotFilter =
     QuerySinglePoolStakeSnapshot PoolId
   | QueryMutiplePoolStakeSnapshots (Set PoolId)

I feel that QueryMutiplePoolStakeSnapshots subsumes QuerySinglePoolStakeSnapshot.

What's the case for adding an extra single pool id constructor?