Skip to content

Commit

Permalink
Merge #2957
Browse files Browse the repository at this point in the history
2957: Add advice about how to add new argument in Ouroboros.Consensus.Node r=nfrisby a=nfrisby

One commit. Only adds comments.

Co-authored-by: Nicolas Frisby <nick.frisby@iohk.io>
  • Loading branch information
iohk-bors[bot] and nfrisby authored Aug 24, 2021
2 parents 877ce05 + ed7312c commit 6d00ff7
Showing 1 changed file with 28 additions and 0 deletions.
28 changes: 28 additions & 0 deletions ouroboros-consensus/src/Ouroboros/Consensus/Node.hs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,30 @@ import Ouroboros.Consensus.Storage.LedgerDB.DiskPolicy
import Ouroboros.Consensus.Storage.VolatileDB
(BlockValidationPolicy (..))

{-------------------------------------------------------------------------------
The arguments to the Consensus Layer node functionality
-------------------------------------------------------------------------------}

-- How to add a new argument
--
-- 1) As a Consensus Layer maintainer, use your judgement to determine whether
-- the new argument belongs in 'RunNodeArgs' or 'LowLevelArgs'. Give it the type
-- that seems most " natural ", future-proof, and useful for the whole range of
-- invocations: our tests, our own benchmarks, deployment on @mainnet@, etc. The
-- major litmus test is: it only belongs in 'RunNodeArgs' if /every/ invocation
-- of our node code must specify it.
--
-- 2) If you add it to 'LowLevelArgs', you'll have type errors in
-- 'stdLowLevelRunNodeArgsIO'. To fix them, you'll need to either hard-code a
-- default value or else extend 'StdRunNodeArgs' with a new sufficient field.
--
-- 3) When extending either 'RunNodeArgs' or 'StdRunNodeArgs', the
-- @cardano-node@ will have to be updated, so consider the Node Team's
-- preferences when choosing the new field's type. As an oversimplification,
-- Consensus /owns/ 'RunNodeArgs' while Node /owns/ 'StdRunNodeArgs', but it's
-- always worth spending some effort to try to find a type that satisfies both
-- teams.

-- | Arguments expected from any invocation of 'runWith', whether by deployed
-- code, tests, etc.
data RunNodeArgs m addrNTN addrNTC blk = RunNodeArgs {
Expand Down Expand Up @@ -201,6 +225,10 @@ data LowLevelRunNodeArgs m addrNTN addrNTC versionDataNTN versionDataNTC blk = L
, llrnMaxClockSkew :: ClockSkew
}

{-------------------------------------------------------------------------------
Entrypoints to the Consensus Layer node functionality
-------------------------------------------------------------------------------}

-- | Combination of 'runWith' and 'stdLowLevelRunArgsIO'
run :: forall blk.
RunNode blk
Expand Down

0 comments on commit 6d00ff7

Please sign in to comment.