Skip to content

Revert tie breaker change for small pools #1548

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Revert tie breaker change for small pools #1548

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e50f05
Praos: use leader VRF for tiebreaker
Cerkoryn Jun 3, 2025
b85a489
Merge pull request #1 from Cerkoryn/codex/revert-tie-breaker-change-f…
Cerkoryn Jun 3, 2025
3bdc3cb
Add LANGUAGE ScopedTypeVariables
Cerkoryn Jun 4, 2025
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
23 changes: 16 additions & 7 deletions ouroboros-consensus-cardano/src/shelley/Ouroboros/Consensus/Shelley/Protocol/Praos.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE TypeApplications #-}
Expand All @@ -7,7 +8,7 @@
module Ouroboros.Consensus.Shelley.Protocol.Praos (PraosEnvelopeError (..)) where

import qualified Cardano.Crypto.KES as KES
import Cardano.Crypto.VRF (certifiedOutput)
import Cardano.Crypto.VRF (certifiedOutput, mkTestOutputVRF)
import Cardano.Ledger.BHeaderView
import Cardano.Ledger.BaseTypes (ProtVer (ProtVer), Version)
import Cardano.Ledger.Keys (hashKey)
Expand All @@ -26,6 +27,7 @@ import Ouroboros.Consensus.Protocol.Praos
import Ouroboros.Consensus.Protocol.Praos.Common
( MaxMajorProtVer (MaxMajorProtVer)
)
import Ouroboros.Consensus.Protocol.Praos.VRF (vrfLeaderValue)
import Ouroboros.Consensus.Protocol.Praos.Header
( Header (..)
, HeaderBody (..)
Expand All @@ -44,6 +46,8 @@ import Ouroboros.Consensus.Shelley.Protocol.Abstract
, ShelleyProtocol
, ShelleyProtocolHeader
)
import Cardano.Protocol.TPraos.BHeader (BoundedNatural (bvValue))
import Data.Proxy (Proxy (..))

type instance ProtoCrypto (Praos c) = c

Expand Down Expand Up @@ -184,12 +188,17 @@ instance PraosCrypto c => ProtocolHeaderSupportsProtocol (Praos c) where
pHeaderIssuer = hbVk . headerBody
pHeaderIssueNo = SL.ocertN . hbOCert . headerBody

-- This is the "unified" VRF value, prior to range extension which yields e.g.
-- the leader VRF value used for slot election.
--
-- In the future, we might want to use a dedicated range-extended VRF value
-- here instead.
pTieBreakVRFValue = certifiedOutput . hbVrfRes . headerBody
-- Use the leader VRF value derived via range extension.
-- This mirrors the behaviour of 'TPraos', giving a slight
-- advantage to smaller pools in slot battles.
-- See https://github.com/IntersectMBO/ouroboros-network/issues/4051
-- for the discussion around this tie breaker.
pTieBreakVRFValue =
mkTestOutputVRF
. bvValue
. vrfLeaderValue (Proxy @c)
. hbVrfRes
. headerBody
Comment on lines +196 to +201
Copy link
Member

Choose a reason for hiding this comment

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

For a proof-of-concept implementation, this seems fine; however, for an actual implementation, it makes sense to avoid mkTestOutputVRF and going through Natural.

Also, one needs to add

{-# LANGUAGE ScopedTypeVariables #-}

at the top to get this to compile.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your response @amesgen.

One natural place would be the Consensus Working Group call (next iteration on the 19th of June, see the Intersect Discord)

I was not aware of the existence of this working group, but if we are able to get this issue added as a discussion item I'm sure one of us from the SPO Incentives Working Group would be willing to show up and make the case for reverting back to using the L hash for slot battles.

To a certain degree, we want to discourage small pools, in order to avoid Sybil attacks.

I think regarding this and the explanation in @dcoutts comment that pledge was designed to be the limiting factor for both pool splitting and Sybil attacks.

If we can address that separately then we can empower small pools to our hearts' content without having to worry about exploitation by bad actors. Consequently, we have just re-submitted a CIP with simulation research and a built tool to model it that aims to solve exactly this issue.

The PR to merge it into the CIP-repository is here: cardano-foundation/CIPs#1042. CIP Editors will be reviewing it at their next meeting on 10 June.

Copy link
Member

Choose a reason for hiding this comment

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

One natural place would be the Consensus Working Group call (next iteration on the 19th of June, see the Intersect Discord)

I was not aware of the existence of this working group, but if we are able to get this issue added as a discussion item I'm sure one of us from the SPO Incentives Working Group would be willing to show up and make the case for reverting back to using the L hash for slot battles.

Sounds great, added to the agenda (also pinged you(?) on the Intersect Discord), thanks 👍

To a certain degree, we want to discourage small pools, in order to avoid Sybil attacks.

I think regarding this and the explanation in @dcoutts comment that pledge was designed to be the limiting factor for both pool splitting and Sybil attacks.

If we can address that separately then we can empower small pools to our hearts' content without having to worry about exploitation by bad actors. Consequently, we have just re-submitted a CIP with simulation research and a built tool to model it that aims to solve exactly this issue.

The PR to merge it into the CIP-repository is here: cardano-foundation/CIPs#1042. CIP Editors will be reviewing it at their next meeting on 10 June.

Yeah, these general efforts on changing the reward system are definitely closely related and should be studied together. It seems to me that pool splitting is inherently related to Sybil attacks, but it indeed sounds cool if they could be somehow (at least partially) separated.

Also cc @CarlosLopezDeLara

Copy link
Member

Choose a reason for hiding this comment

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

FYI some notes on how much non-orphaned blocks/rewards would change for small/large pools with this change compared to the status quo: https://hackmd.io/hX7q5s8JSKSP-j3525J0bA


instance PraosCrypto c => ProtocolHeaderSupportsLedger (Praos c) where
mkHeaderView hdr@Header{headerBody} =
Expand Down