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.

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

ChainSync client: allow to prevent historical MsgRollBackward/MsgAwaitReply #1186

Merged
merged 4 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Breaking

- Adapted to ChainSync client changes due to new message historicity check.
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,15 @@ mkHandlers ::
-> NodeKernel m addrNTN addrNTC blk
-> Handlers m addrNTN blk
mkHandlers
NodeKernelArgs {chainSyncFutureCheck, keepAliveRng, miniProtocolParameters}
NodeKernel {getChainDB, getMempool, getTopLevelConfig, getTracers = tracers, getPeerSharingAPI} =
NodeKernelArgs {chainSyncFutureCheck, chainSyncHistoricityCheck, keepAliveRng, miniProtocolParameters}
NodeKernel {getChainDB, getMempool, getTopLevelConfig, getTracers = tracers, getPeerSharingAPI, getGsmState} =
Handlers {
hChainSyncClient = \peer _isBigLedgerpeer dynEnv ->
CsClient.chainSyncClient
CsClient.ConfigEnv {
CsClient.cfg = getTopLevelConfig
, CsClient.someHeaderInFutureCheck = chainSyncFutureCheck
, CsClient.historicityCheck = chainSyncHistoricityCheck (atomically getGsmState)
, CsClient.chainDbView =
CsClient.defaultChainDbView getChainDB
, CsClient.mkPipelineDecision0 = pipelineDecisionLowHighMark
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ import Ouroboros.Consensus.Fragment.InFuture (CheckInFuture,
ClockSkew)
import qualified Ouroboros.Consensus.Fragment.InFuture as InFuture
import Ouroboros.Consensus.Ledger.Extended (ExtLedgerState (..))
import Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck
(HistoricityCheck)
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck as HistoricityCheck
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck as InFutureCheck
import qualified Ouroboros.Consensus.Network.NodeToClient as NTC
import qualified Ouroboros.Consensus.Network.NodeToNode as NTN
Expand Down Expand Up @@ -491,6 +494,11 @@ runWith RunNodeArgs{..} encAddrNtN decAddrNtN LowLevelRunNodeArgs{..} =
let gsmMarkerFileView =
case ChainDB.cdbsHasFSGsmDB $ ChainDB.cdbsArgs finalArgs of
SomeHasFS x -> GSM.realMarkerFileView chainDB x
historicityCheck getGsmState =
case gcHistoricityCutoff llrnGenesisConfig of
Nothing -> HistoricityCheck.noCheck
Just historicityCutoff ->
HistoricityCheck.mkCheck systemTime getGsmState historicityCutoff
fmap (nodeKernelArgsEnforceInvariants . llrnCustomiseNodeKernelArgs)
$ mkNodeKernelArgs
registry
Expand All @@ -501,6 +509,7 @@ runWith RunNodeArgs{..} encAddrNtN decAddrNtN LowLevelRunNodeArgs{..} =
rnTraceConsensus
btime
(InFutureCheck.realHeaderInFutureCheck llrnMaxClockSkew systemTime)
historicityCheck
chainDB
llrnMaxCaughtUpAge
(Just durationUntilTooOld)
Expand Down Expand Up @@ -744,6 +753,7 @@ mkNodeKernelArgs ::
-> Tracers m (ConnectionId addrNTN) (ConnectionId addrNTC) blk
-> BlockchainTime m
-> InFutureCheck.SomeHeaderInFutureCheck m blk
-> (m GSM.GsmState -> HistoricityCheck m blk)
-> ChainDB m blk
-> NominalDiffTime
-> Maybe (GSM.WrapDurationUntilTooOld m blk)
Expand All @@ -761,6 +771,7 @@ mkNodeKernelArgs
tracers
btime
chainSyncFutureCheck
chainSyncHistoricityCheck
chainDB
maxCaughtUpAge
gsmDurationUntilTooOld
Expand All @@ -778,6 +789,7 @@ mkNodeKernelArgs
, chainDB
, initChainDB = nodeInitChainDB
, chainSyncFutureCheck
, chainSyncHistoricityCheck
, blockFetchSize = estimateBlockSize
, mempoolCapacityOverride = NoMempoolCapacityBytesOverride
, miniProtocolParameters = defaultMiniProtocolParameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import Ouroboros.Consensus.MiniProtocol.ChainSync.Client
(CSJConfig (..), CSJEnabledConfig (..),
ChainSyncLoPBucketConfig (..),
ChainSyncLoPBucketEnabledConfig (..))
import Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck
(HistoricityCutoff (..))
import qualified Ouroboros.Consensus.Node.GsmState as GSM
import Ouroboros.Consensus.Storage.ChainDB (ChainDbArgs)
import qualified Ouroboros.Consensus.Storage.ChainDB as ChainDB
Expand All @@ -45,6 +47,7 @@ data GenesisConfig = GenesisConfig {
gcChainSyncLoPBucketConfig :: !ChainSyncLoPBucketConfig
, gcCSJConfig :: !CSJConfig
, gcLoEAndGDDConfig :: !(LoEAndGDDConfig ())
, gcHistoricityCutoff :: !(Maybe HistoricityCutoff)
}

-- TODO justification/derivation from other parameters
Expand All @@ -58,6 +61,10 @@ enableGenesisConfigDefault = GenesisConfig {
csjcJumpSize = 3 * 2160 * 20 -- mainnet forecast range
}
, gcLoEAndGDDConfig = LoEAndGDDEnabled ()
-- Duration in seconds of one Cardano mainnet Shelley stability window
-- (3k/f slots times one second per slot) plus one extra hour as a
-- safety margin.
, gcHistoricityCutoff = Just $ HistoricityCutoff $ 3 * 2160 * 20 + 3600
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this too Cardano specific? And somewhat related, is it ok to have these constants hardcoded here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is specific to Cardano mainnet; like many other constants in this area (e.g. the timeouts, protocol-level size limits, the GSM MaxCaughtUpAge).

}

-- | Disable all Genesis components, yielding Praos behavior.
Expand All @@ -66,6 +73,7 @@ disableGenesisConfig = GenesisConfig {
gcChainSyncLoPBucketConfig = ChainSyncLoPBucketDisabled
, gcCSJConfig = CSJDisabled
, gcLoEAndGDDConfig = LoEAndGDDDisabled
, gcHistoricityCutoff = Nothing
}

-- | Genesis-related arguments needed by the NodeKernel initialization logic.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ import qualified Ouroboros.Consensus.MiniProtocol.BlockFetch.ClientInterface as
import Ouroboros.Consensus.MiniProtocol.ChainSync.Client
(ChainSyncClientHandle (..), ChainSyncState (..),
viewChainSyncState)
import Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck
(HistoricityCheck)
import Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck
(SomeHeaderInFutureCheck)
import Ouroboros.Consensus.Node.Genesis (GenesisNodeKernelArgs (..),
Expand Down Expand Up @@ -171,6 +173,9 @@ data NodeKernelArgs m addrNTN addrNTC blk = NodeKernelArgs {
, chainDB :: ChainDB m blk
, initChainDB :: StorageConfig blk -> InitChainDB m blk -> m ()
, chainSyncFutureCheck :: SomeHeaderInFutureCheck m blk
-- | See 'HistoricityCheck' for details.
, chainSyncHistoricityCheck
amesgen marked this conversation as resolved.
Show resolved Hide resolved
:: m GSM.GsmState -> HistoricityCheck m blk
, blockFetchSize :: Header blk -> SizeInBytes
, mempoolCapacityOverride :: MempoolCapacityBytesOverride
, miniProtocolParameters :: MiniProtocolParameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import Ouroboros.Consensus.Ledger.SupportsMempool
import Ouroboros.Consensus.Ledger.SupportsProtocol
import Ouroboros.Consensus.Mempool
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client as CSClient
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck as HistoricityCheck
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck as InFutureCheck
import qualified Ouroboros.Consensus.Network.NodeToNode as NTN
import Ouroboros.Consensus.Node.ExitPolicy
Expand Down Expand Up @@ -1003,6 +1004,7 @@ runThreadNetwork systemTime ThreadNetworkArgs
InFutureCheck.realHeaderInFutureCheck
InFuture.defaultClockSkew
(OracularClock.finiteSystemTime clock)
, chainSyncHistoricityCheck = \_getGsmState -> HistoricityCheck.noCheck
, blockFetchSize = estimateBlockSize
, mempoolCapacityOverride = NoMempoolCapacityBytesOverride
, keepAliveRng = kaRng
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Data.Map.Strict (Map)
import Data.Proxy (Proxy (..))
import Network.TypedProtocol.Codec (AnyMessage)
import Ouroboros.Consensus.Block (Header, Point)
import Ouroboros.Consensus.BlockchainTime (RelativeTime (..))
import Ouroboros.Consensus.Config (TopLevelConfig (..))
import Ouroboros.Consensus.Ledger.SupportsProtocol
(LedgerSupportsProtocol)
Expand All @@ -25,6 +26,7 @@ import Ouroboros.Consensus.MiniProtocol.ChainSync.Client
ChainSyncLoPBucketConfig, ChainSyncStateView (..),
Consensus, bracketChainSyncClient, chainSyncClient)
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client as CSClient
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck as HistoricityCheck
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck as InFutureCheck
import Ouroboros.Consensus.Node.GsmState (GsmState (Syncing))
import Ouroboros.Consensus.Util (ShowProxy)
Expand Down Expand Up @@ -84,6 +86,10 @@ basicChainSyncClient
, CSClient.cfg
, CSClient.chainDbView
, CSClient.someHeaderInFutureCheck = dummyHeaderInFutureCheck
-- Preventing historical MsgRollBack and MsgAwaitReply messages is
-- motivated by preventing additional load from CSJ-disengaged peers; we
-- do not care about this in these tests.
, CSClient.historicityCheck = HistoricityCheck.noCheck
}
CSClient.DynamicEnv {
CSClient.version = maxBound
Expand All @@ -103,7 +109,9 @@ basicChainSyncClient
{ InFutureCheck.proxyArrival = Proxy
, InFutureCheck.recordHeaderArrival = \_ -> pure ()
, InFutureCheck.judgeHeaderArrival = \_ _ _ -> pure ()
, InFutureCheck.handleHeaderArrival = \_ -> pure Nothing
, InFutureCheck.handleHeaderArrival = \_ ->
nfrisby marked this conversation as resolved.
Show resolved Hide resolved
-- We are not inspecting header slot time in the Genesis tests.
pure $ pure $ RelativeTime 0
}

-- | Create and run a ChainSync client using 'bracketChainSyncClient' and
Expand Down
17 changes: 10 additions & 7 deletions ouroboros-consensus/bench/ChainSync-client-bench/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import qualified Ouroboros.Consensus.HeaderStateHistory as HeaderStateHistory
import qualified Ouroboros.Consensus.HeaderValidation as HV
import qualified Ouroboros.Consensus.Ledger.Extended as Extended
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client as CSClient
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck as HistoricityCheck
import qualified Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck as InFutureCheck
import Ouroboros.Consensus.MiniProtocol.ChainSync.Server
(chainSyncServerForFollower)
Expand All @@ -38,10 +39,10 @@ import Ouroboros.Consensus.Util.STM (Fingerprint (..),
import Ouroboros.Consensus.Util.Time (secondsToNominalDiffTime)
import Ouroboros.Network.AnchoredFragment (AnchoredFragment)
import qualified Ouroboros.Network.AnchoredFragment as AF
import qualified Ouroboros.Network.AnchoredSeq as AS
import Ouroboros.Network.Block (ChainUpdate (AddBlock, RollBack),
Tip (TipGenesis), tipFromHeader)
import Ouroboros.Network.ControlMessage (ControlMessage (Continue))
import qualified Ouroboros.Network.Mock.Chain as Chain
import Ouroboros.Network.Protocol.ChainSync.ClientPipelined
import Ouroboros.Network.Protocol.ChainSync.Codec (codecChainSyncId)
import Ouroboros.Network.Protocol.ChainSync.PipelineDecision
Expand Down Expand Up @@ -105,11 +106,12 @@ oneBenchRun
CSClient.getCurrentChain = pure $ AF.Empty AF.AnchorGenesis
, CSClient.getHeaderStateHistory =
pure
$ HeaderStateHistory.HeaderStateHistory
$ AS.Empty
$ HV.genesisHeaderState ()
$ HeaderStateHistory.fromChain
topConfig
(oracularLedgerDB GenesisPoint)
Chain.Genesis
, CSClient.getIsInvalidBlock = pure invalidBlock
, CSClient.getPastLedger = pure . oracularLedgerDB
, CSClient.getPastLedger = pure . Just . oracularLedgerDB
}

headerInFutureCheck ::
Expand All @@ -127,6 +129,7 @@ oneBenchRun
, CSClient.cfg = topConfig
, CSClient.tracer = nullTracer `asTypeOf` contramap show debugTracer
, CSClient.someHeaderInFutureCheck = headerInFutureCheck
, CSClient.historicityCheck = HistoricityCheck.noCheck
, CSClient.mkPipelineDecision0 =
pipelineDecisionLowHighMark 10 20
}
Expand Down Expand Up @@ -174,9 +177,9 @@ inTheYearOneBillion = SystemTime {
* 1e9
}

oracularLedgerDB :: Point B -> Maybe (Extended.ExtLedgerState B)
oracularLedgerDB :: Point B -> Extended.ExtLedgerState B
oracularLedgerDB p =
Just Extended.ExtLedgerState {
Extended.ExtLedgerState {
Extended.headerState = HV.HeaderState {
HV.headerStateTip = case pointToWithOriginRealPoint p of
Origin -> Origin
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Breaking

- Added functionality to disallow historical `MsgRollBackward`s and
`MsgAwaitReply`s in the ChainSync client.
2 changes: 2 additions & 0 deletions ouroboros-consensus/ouroboros-consensus.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ library
Ouroboros.Consensus.MiniProtocol.BlockFetch.ClientInterface
Ouroboros.Consensus.MiniProtocol.BlockFetch.Server
Ouroboros.Consensus.MiniProtocol.ChainSync.Client
Ouroboros.Consensus.MiniProtocol.ChainSync.Client.HistoricityCheck
Ouroboros.Consensus.MiniProtocol.ChainSync.Client.InFutureCheck
Ouroboros.Consensus.MiniProtocol.ChainSync.Client.Jumping
Ouroboros.Consensus.MiniProtocol.ChainSync.Client.State
Expand Down Expand Up @@ -723,6 +724,7 @@ benchmark ChainSync-client-bench
contra-tracer,
ouroboros-consensus,
ouroboros-network-api,
ouroboros-network-mock,
ouroboros-network-protocols,
time,
typed-protocols-examples,
Expand Down
Loading
Loading