Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
4153: CBR-525: Fix failure to sync issue when switching to OBFT r=disassembler a=erikd

## Description

Fix issue that caused the node to loose sync with the chain in OBFT era and then fail to re-sync once out of sync.

## Linked issue

https://iohk.myjetbrains.com/youtrack/issue/CBR-525

- [x] CHANGELOG entry has been added and is linked to the correct PR on GitHub.

## Testing checklist
<!-- If you aren't providing any tests as part of this PR, use this section to state clearly why. It needs to be a strong motivation and definitely the exception, not the rule. -->
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.

## QA Steps
Synced the OBFT testnet numerous times.


Co-authored-by: Erik de Castro Lopo <[email protected]>
Co-authored-by: Michael Hueschen <[email protected]>
Co-authored-by: Samuel Leathers <[email protected]>
  • Loading branch information
4 people committed May 31, 2019
2 parents a566b51 + be04da9 commit 72985e0
Show file tree
Hide file tree
Showing 27 changed files with 716 additions and 266 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG

## Cardano SL 3.0.2

### Fixes

- Fix issue that caused the node to lose sync with the chain in the OBFT era and then fail to re-sync once out of sync ([CBR-525]https://iohk.myjetbrains.com/youtrack/issue/CBR-525) [#4153](https://github.com/input-output-hk/cardano-sl/pull/4153))

## Cardano SL 3.0.1

### Fixes
Expand Down
2 changes: 2 additions & 0 deletions chain/cardano-sl-chain.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ library
hs-source-dirs: src
exposed-modules:
Pos.Chain.Block
Pos.Chain.Block.Slog.LastBlkSlots
Pos.Chain.Delegation
Pos.Chain.Genesis
Pos.Chain.Lrc
Expand Down Expand Up @@ -247,6 +248,7 @@ test-suite chain-test
Test.Pos.Chain.Block.CborSpec
Test.Pos.Chain.Block.Gen
Test.Pos.Chain.Block.SafeCopySpec
Test.Pos.Chain.Block.Slog.LastBlkSlots
Test.Pos.Chain.Delegation.Arbitrary
Test.Pos.Chain.Delegation.Bi
Test.Pos.Chain.Delegation.Example
Expand Down
20 changes: 18 additions & 2 deletions chain/src/Pos/Chain/Block/Header.hs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ import Pos.Chain.Update.BlockVersion (BlockVersion,
import Pos.Chain.Update.SoftwareVersion (HasSoftwareVersion (..),
SoftwareVersion)
import Pos.Core.Attributes (mkAttributes)
import Pos.Core.Common (ChainDifficulty, HasDifficulty (..))
import Pos.Core.Common (ChainDifficulty, HasDifficulty (..),
addressHash)
import Pos.Core.Slotting (EpochIndex (..), EpochOrSlot (..),
HasEpochIndex (..), HasEpochOrSlot (..), SlotCount (..),
SlotId (..), flattenSlotId, slotIdF)
Expand Down Expand Up @@ -185,7 +186,11 @@ headerLastSlotInfo slotCount = \case
convert bh =
LastSlotInfo
(flattenSlotId slotCount . _mcdSlot $ _gbhConsensus bh)
(_mcdLeaderKey $ _gbhConsensus bh)
-- Since _mcdLeaderKey is confirmed (see comment in function
-- `mkMainHeaderExplicit`) to be the delegator's key, we will be
-- enforcing block-minting-count thresholds by the delegator's key,
-- which is the desired behavior.
(addressHash . _mcdLeaderKey $ _gbhConsensus bh)

--------------------------------------------------------------------------------
-- HeaderHash
Expand Down Expand Up @@ -427,6 +432,17 @@ mkMainHeaderExplicit pm prevHash difficulty slotId sk pske body extra =
(BlockSignature $ sign pm SignMainBlock sk toSign)
(makeSignature toSign)
pske
-- If `pske :: ProxySKBlockInfo` is Just, indicating delegation, we use
-- the PublicKey of the delegator (in Original era, this is the Stakeholder
-- whose stake was selected by FTS. In OBFT era, this is the Genesis
-- Stakeholder, selected via round-robin. In both cases, the delegator is
-- delegating the right to mint a block to the delegatee).
--
-- If `pske :: ProxySKBlockInfo` is Nothing, we use `toPublic` of the `sk`
-- provided, which should be provided by the node issuing the block who was
-- thus selected themselves. See `db/src/Pos/DB/Block/Logic/Creation.hs`
-- for the top of the function call chain which provides the `sk` which
-- makes its way down to here.
leaderPk = maybe (toPublic sk) snd pske
consensus =
MainConsensusData
Expand Down
18 changes: 9 additions & 9 deletions chain/src/Pos/Chain/Block/Logic/Integrity.hs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ import Pos.Chain.Block.Header (BlockHeader (..), HasHeaderHash (..),
mainHeaderLeaderKey, verifyBlockHeader)
import Pos.Chain.Block.IsHeader (headerSlotL)
import Pos.Chain.Block.Main (mebAttributes, mehAttributes)
import Pos.Chain.Block.Slog (ConsensusEraLeaders (..),
LastSlotInfo (..))
import Pos.Chain.Block.Slog (ConsensusEraLeaders (..), LastBlkSlots)
import qualified Pos.Chain.Block.Slog.LastBlkSlots as LastBlkSlots
import Pos.Chain.Genesis as Genesis (Config (..))
import Pos.Chain.Txp (TxValidationRules)
import Pos.Chain.Update (BlockVersionData (..), ConsensusEra (..))
Expand Down Expand Up @@ -211,7 +211,7 @@ verifyHeader pm VerifyHeaderParams {..} h =
, ( obftLeaderCanMint blockSlotLeader blkSecurityParam lastBlkSlots
, sformat ("ObftLenient: slot leader who published block, "%build%", has minted too many blocks ("% build %") in the past "%build%" slots.")
blockSlotLeader
(blocksMintedByLeaderInLastKSlots blockSlotLeader $ getOldestFirst lastBlkSlots)
(blocksMintedByLeaderInLastKSlots blockSlotLeader lastBlkSlots)
(getBlockCount blkSecurityParam)
)
]
Expand Down Expand Up @@ -240,22 +240,22 @@ verifyHeader pm VerifyHeaderParams {..} h =
where
-- Determine whether the leader is allowed to mint a block based on
-- whether blocksMintedByLeaderInLastKSlots <= floor (k * t)
obftLeaderCanMint :: AddressHash PublicKey -> BlockCount -> OldestFirst [] LastSlotInfo -> Bool
obftLeaderCanMint leaderAddrHash blkSecurityParam (OldestFirst lastBlkSlots) =
obftLeaderCanMint :: AddressHash PublicKey -> BlockCount -> LastBlkSlots -> Bool
obftLeaderCanMint leaderAddrHash blkSecurityParam lastBlkSlots =
blocksMintedByLeaderInLastKSlots leaderAddrHash lastBlkSlots
<= leaderMintThreshold blkSecurityParam

blocksMintedByLeaderInLastKSlots :: AddressHash PublicKey -> [LastSlotInfo] -> Int
blocksMintedByLeaderInLastKSlots :: AddressHash PublicKey -> LastBlkSlots -> Int
blocksMintedByLeaderInLastKSlots leaderAddrHash lastBlkSlots =
length $
filter (\lsi -> leaderAddrHash == (addressHash $ lsiLeaderPubkeyHash lsi))
lastBlkSlots
LastBlkSlots.getKeyCount lastBlkSlots leaderAddrHash

leaderMintThreshold :: BlockCount -> Int
leaderMintThreshold blkSecurityParam =
let k = getBlockCount blkSecurityParam
in floor $ (fromIntegral k :: Double) * t

-- This value was arrived at by deciding on the upper bound of the number of
-- blocks in the last 'k' blocks (not slots) signed by the same signing key.
t :: Double
t = 0.22

Expand Down
152 changes: 152 additions & 0 deletions chain/src/Pos/Chain/Block/Slog/LastBlkSlots.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
-- | Slog-related types.

module Pos.Chain.Block.Slog.LastBlkSlots
( LastBlkSlots
, LastSlotInfo (..)

-- * Create LastBlkSlots
, create
, fromList

-- * Access LastBlkSlots components
, getList
, lbsCount
, lbsList
, lbsMap

-- * LastBlkSlots operations
, getKeyCount
, isFull
, listLength
, mapSize
, totalKeyCount
, update
, updateMany
, updateManyR
) where

import Universum

import qualified Data.List as List
import qualified Data.Map.Strict as Map
import Formatting (bprint, build, int, (%))
import qualified Formatting.Buildable as Buildable

import Pos.Binary.Class (Cons (..), Field (..), deriveSimpleBi)
import Pos.Core (AddressHash, FlatSlotId)
import Pos.Core.Chrono (OldestFirst (..))
import Pos.Crypto (PublicKey (..))


-- Make sure its the actual genesis keys that are being counted.

data LastSlotInfo = LastSlotInfo
{ lsiFlatSlotId :: !FlatSlotId
-- ^ The flattened SlotId of this block.
, lsiLeaderPubkeyHash :: !(AddressHash PublicKey)
-- ^ The hash of the public key of the slot leader for this slot.
} deriving (Eq, Show, Generic)

instance Buildable LastSlotInfo where
build (LastSlotInfo i ahpk) =
bprint ( "LastSlotInfo "% int %" "% build) i ahpk

instance NFData LastSlotInfo

-- | This type contains 'FlatSlotId's of the blocks whose depth is
-- less than 'blkSecurityParam'. 'FlatSlotId' is chosen in favor of
-- 'SlotId', because the main use case is chain quality calculation,
-- for which flat slot is more convenient.
-- Version 1 of this data type was:
-- type LastBlkSlots = OldestFirst [] FlatSlotId
-- Version 2 of this data type was:
-- data LastBlkSlots = LastBlkSlots
-- { lbsList :: !(OldestFirst [] LastSlotInfo)
-- , lbsMap :: !(Map (AddressHash PublicKey) Int)
-- } deriving (Eq, Show, Generic)
data LastBlkSlots = LastBlkSlots
{ lbsCount :: !Int
, lbsList :: !(OldestFirst [] LastSlotInfo)
, lbsMap :: !(Map (AddressHash PublicKey) Int)
} deriving (Eq, Show, Generic)

instance NFData LastBlkSlots

create :: Int -> LastBlkSlots
create k = LastBlkSlots k (OldestFirst []) mempty

getKeyCount :: LastBlkSlots -> AddressHash PublicKey -> Int
getKeyCount lbs key =
fromMaybe 0 $ Map.lookup key (lbsMap lbs)

totalKeyCount :: LastBlkSlots -> Int
totalKeyCount =
sum . map snd . Map.toList . lbsMap

isFull :: LastBlkSlots -> Bool
isFull lbs =
length (getList lbs) == lbsCount lbs

-- | Update LastBlkSlots with a single LastSlotInfo
update :: LastBlkSlots -> LastSlotInfo -> LastBlkSlots
update (LastBlkSlots k (OldestFirst lst) mp) lsi =
if length lst < k
then LastBlkSlots k (OldestFirst $ lst ++ [lsi]) (increment mp $ lsiLeaderPubkeyHash lsi)
else case lst of
[] -> error "Pos.Chain.Block.Slog.LastBlkSlots: Impossible empty list"
(x:xs) ->
LastBlkSlots k
(OldestFirst $ xs ++ [lsi])
(increment (decrement mp (lsiLeaderPubkeyHash x)) $ lsiLeaderPubkeyHash lsi)

-- | Update 'LastBlkSlots' with the elements from the list (head first).
updateMany :: LastBlkSlots -> OldestFirst [] LastSlotInfo -> LastBlkSlots
updateMany lbs = List.foldl' update lbs . getOldestFirst

-- | Like 'updateMany` but returns a tuple of the new 'LastBlkSlots' and a list
-- of the elements removed.
updateManyR :: LastBlkSlots -> OldestFirst [] LastSlotInfo -> (LastBlkSlots, OldestFirst [] LastSlotInfo)
updateManyR lbs (OldestFirst xs) =
let removed = List.take (length xs + listLength lbs - lbsCount lbs) (getList lbs ++ xs)
in (List.foldl' update lbs xs, OldestFirst removed)

getList :: LastBlkSlots -> [LastSlotInfo]
getList = getOldestFirst . lbsList

listLength :: LastBlkSlots -> Int
listLength = length . lbsList

mapSize :: LastBlkSlots -> Int
mapSize = Map.size . lbsMap

fromList :: Int -> OldestFirst [] LastSlotInfo -> LastBlkSlots
fromList k = List.foldl' update (create k) . getOldestFirst

-- -----------------------------------------------------------------------------
-- Private

increment :: Map (AddressHash PublicKey) Int -> AddressHash PublicKey -> Map (AddressHash PublicKey) Int
increment m k =
Map.alter incr k m
where
incr Nothing = Just 1
incr (Just x) = Just $ x + 1

decrement :: Map (AddressHash PublicKey) Int -> AddressHash PublicKey -> Map (AddressHash PublicKey) Int
decrement m k =
Map.alter decr k m
where
decr Nothing = Nothing
decr (Just x)
| x > 1 = Just $ x - 1
| otherwise = Nothing

-- -----------------------------------------------------------------------------
-- TH derived instances at the end of the file.

deriveSimpleBi ''LastSlotInfo [
Cons 'LastSlotInfo [
Field [| lsiFlatSlotId :: FlatSlotId |],
Field [| lsiLeaderPubkeyHash :: AddressHash PublicKey |]
]
]
44 changes: 3 additions & 41 deletions chain/src/Pos/Chain/Block/Slog/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
module Pos.Chain.Block.Slog.Types
( LastBlkSlots
, LastSlotInfo (..)
, noLastBlkSlots

, ConsensusEraLeaders (..)

Expand All @@ -19,50 +18,20 @@ module Pos.Chain.Block.Slog.Types

import Universum

import qualified Cardano.Crypto.Wallet as CC
import Control.Lens (makeClassy)
import qualified Data.ByteString.Base16 as B16
import qualified Data.ByteString.Char8 as BS
import Formatting (Format, bprint, int, later, string, (%))
import qualified Formatting.Buildable as Buildable
import Formatting (Format, bprint, later)

import System.Metrics.Label (Label)

import Pos.Binary.Class (Cons (..), Field (..), deriveSimpleBi)
import Pos.Chain.Block.Slog.LastBlkSlots (LastBlkSlots,
LastSlotInfo (..))
import Pos.Core (BlockCount, ChainDifficulty, EpochIndex, FlatSlotId,
LocalSlotIndex, SlotCount, SlotLeaders, StakeholderId,
slotIdF, unflattenSlotId)
import Pos.Core.Chrono (OldestFirst (..))
import Pos.Core.Reporting (MetricMonitorState)
import Pos.Crypto (PublicKey (..))


data LastSlotInfo = LastSlotInfo
{ lsiFlatSlotId :: !FlatSlotId
-- ^ The flattened SlotId of this block.
, lsiLeaderPubkeyHash :: !PublicKey
-- ^ The hash of the public key of the slot leader for this slot.
} deriving (Eq, Show, Generic)

instance Buildable LastSlotInfo where
build (LastSlotInfo i (PublicKey pk)) =
bprint ( "LastSlotInfo "% int %" "% string)
i (take 16 . BS.unpack . B16.encode $ CC.xpubPublicKey pk)

instance NFData LastSlotInfo

-- | This type contains 'FlatSlotId's of the blocks whose depth is
-- less than 'blkSecurityParam'. 'FlatSlotId' is chosen in favor of
-- 'SlotId', because the main use case is chain quality calculation,
-- for which flat slot is more convenient.
-- Version 1 of this data type was:
-- type LastBlkSlots = OldestFirst [] FlatSlotId
type LastBlkSlots = OldestFirst [] LastSlotInfo


noLastBlkSlots :: LastBlkSlots
noLastBlkSlots = OldestFirst []

-- | This data type is used for block verification. It specifies which slot
-- leader verification algorithm to use and the parameters required to do so.
data ConsensusEraLeaders
Expand Down Expand Up @@ -146,10 +115,3 @@ deriveSimpleBi ''SlogUndo [
Cons 'SlogUndo [
Field [| getSlogUndo :: Maybe FlatSlotId |]
]]

deriveSimpleBi ''LastSlotInfo [
Cons 'LastSlotInfo [
Field [| lsiFlatSlotId :: FlatSlotId |],
Field [| lsiLeaderPubkeyHash :: PublicKey |]
]
]
3 changes: 2 additions & 1 deletion chain/test/Test/Pos/Chain/Block/Arbitrary.hs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import Pos.Binary.Class (biSize)
import Pos.Chain.Block (ConsensusEraLeaders (..), HeaderHash,
headerLastSlotInfo, mkMainBlock, mkMainBlockExplicit)
import qualified Pos.Chain.Block as Block
import qualified Pos.Chain.Block.Slog.LastBlkSlots as LastBlkSlots
import qualified Pos.Chain.Delegation as Core
import Pos.Chain.Genesis (GenesisHash (..))
import Pos.Chain.Update (ConsensusEra (..),
Expand Down Expand Up @@ -505,7 +506,7 @@ genHeaderAndParams pm era = do
pure $ ObftLenientLeaders
(Set.fromList $ mapMaybe (fmap Core.addressHash . Block.headerLeaderKey) headers)
blkSecurityParam
(OldestFirst $ mapMaybe (headerLastSlotInfo slotsPerEpoch) headers)
(LastBlkSlots.updateMany (LastBlkSlots.create (fromIntegral $ getBlockCount blkSecurityParam)) . OldestFirst $ mapMaybe (headerLastSlotInfo slotsPerEpoch) headers)
, Block.vhpMaxSize = Just (biSize header)
, Block.vhpVerifyNoUnknown = not hasUnknownAttributes
, Block.vhpConsensusEra = era
Expand Down
Loading

0 comments on commit 72985e0

Please sign in to comment.