branch bound limits

src/Chainweb/BlockHeaderDB/RestAPI/Server.hs

@@ -1,6 +1,8 @@
 {-# LANGUAGE BangPatterns #-}
 {-# LANGUAGE DataKinds #-}
+{-# LANGUAGE DerivingStrategies #-}
 {-# LANGUAGE FlexibleContexts #-}
+{-# LANGUAGE GeneralizedNewtypeDeriving #-}
 {-# LANGUAGE LambdaCase #-}
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE ScopedTypeVariables #-}
@@ -43,6 +45,7 @@ import Data.Functor.Of
 import Data.IORef
 import Data.Proxy
 import Data.Text.Encoding (decodeUtf8)
+import Numeric.Natural(Natural)
 
 import Network.Wai.EventSource (ServerEvent(..), eventSourceAppIO)
 
@@ -106,6 +109,14 @@ checkBounds
 checkBounds db b = b
     <$ traverse_ (checkKey db . _getUpperBound) (_branchBoundsUpper b)
 
+err400Msg :: ToJSON msg  => msg -> ServerError
+err400Msg msg = ServerError
+    { errHTTPCode = 400
+    , errReasonPhrase = "Bad request"
+    , errBody = encode msg
+    , errHeaders = []
+    }
+
 -- -------------------------------------------------------------------------- --
 -- Handlers
 
@@ -118,6 +129,21 @@ defaultEntryLimit = 360
 p2pEntryLimit :: Num a => a
 p2pEntryLimit = 20
 
+newtype BranchBoundsLimit
+    = BranchBoundsLimit { getBranchBoundsLimit :: Natural }
+    deriving newtype (Show, Eq, Ord)
+
+-- | Default limit for the number of bounds in the request of a branch query
+--
+defaultBoundsLimit :: BranchBoundsLimit
+defaultBoundsLimit = BranchBoundsLimit 32
+
+-- | Limit for the number of bounds in the request of a branch query on the P2P
+-- API.
+--
+p2pBoundsLimit :: BranchBoundsLimit
+p2pBoundsLimit = BranchBoundsLimit 4
+
 -- | Query Branch Hashes of the database.
 --
 -- Cf. "Chainweb.BlockHeaderDB.RestAPI" for more details
@@ -132,14 +158,19 @@ branchHashesHandler
     -> Maybe MaxRank
     -> BranchBounds db
     -> Handler (Page (NextItem (DbKey db)) (DbKey db))
-branchHashesHandler db limit next minr maxr bounds = do
-    nextChecked <- traverse (traverse $ checkKey db) next
-    checkedBounds <- checkBounds db bounds
-    liftIO
-        $ branchKeys db nextChecked (succ <$> effectiveLimit) minr maxr
-            (_branchBoundsLower checkedBounds)
-            (_branchBoundsUpper checkedBounds)
-        $ finiteStreamToPage id effectiveLimit . void
+branchHashesHandler db limit next minr maxr bounds
+    | fromIntegral (length (_branchBoundsUpper bounds)) > getBranchBoundsLimit defaultBoundsLimit = throwError $ err400Msg $
+        "upper branch bound limit exceeded. Only " <> show defaultBoundsLimit <> " values are supported."
+    | fromIntegral (length (_branchBoundsLower bounds)) > getBranchBoundsLimit defaultBoundsLimit = throwError $ err400Msg $
+        "lower branch bound limit exceeded. Only " <> show defaultBoundsLimit <> " values are supported."
+    | otherwise = do
+        nextChecked <- traverse (traverse $ checkKey db) next
+        checkedBounds <- checkBounds db bounds
+        liftIO
+            $ branchKeys db nextChecked (succ <$> effectiveLimit) minr maxr
+                (_branchBoundsLower checkedBounds)
+                (_branchBoundsUpper checkedBounds)
+            $ finiteStreamToPage id effectiveLimit . void
   where
     effectiveLimit = min defaultKeyLimit <$> (limit <|> Just defaultKeyLimit)
 
@@ -151,6 +182,7 @@ branchHeadersHandler
     :: TreeDb db
     => ToJSON (DbKey db)
     => db
+    -> BranchBoundsLimit
     -> Limit
         -- ^ max limit
     -> Maybe Limit
@@ -159,14 +191,19 @@ branchHeadersHandler
     -> Maybe MaxRank
     -> BranchBounds db
     -> Handler (Page (NextItem (DbKey db)) (DbEntry db))
-branchHeadersHandler db maxLimit limit next minr maxr bounds = do
-    nextChecked <- traverse (traverse $ checkKey db) next
-    checkedBounds <- checkBounds db bounds
-    liftIO
-        $ branchEntries db nextChecked (succ <$> effectiveLimit) minr maxr
-            (_branchBoundsLower checkedBounds)
-            (_branchBoundsUpper checkedBounds)
-        $ finiteStreamToPage key effectiveLimit . void
+branchHeadersHandler db (BranchBoundsLimit boundsLimit) maxLimit limit next minr maxr bounds
+    | fromIntegral (length (_branchBoundsUpper bounds)) > boundsLimit = throwError $ err400Msg $
+        "upper branch bound limit exceeded. Only " <> show boundsLimit <> " values are supported."
+    | fromIntegral (length (_branchBoundsLower bounds)) > boundsLimit = throwError $ err400Msg $
+        "lower branch bound limit exceeded. Only " <> show boundsLimit <> " values are supported."
+    | otherwise = do
+        nextChecked <- traverse (traverse $ checkKey db) next
+        checkedBounds <- checkBounds db bounds
+        liftIO
+            $ branchEntries db nextChecked (succ <$> effectiveLimit) minr maxr
+                (_branchBoundsLower checkedBounds)
+                (_branchBoundsUpper checkedBounds)
+            $ finiteStreamToPage key effectiveLimit . void
   where
     effectiveLimit = min maxLimit <$> (limit <|> Just maxLimit)
 
@@ -243,15 +280,15 @@ blockHeaderDbServer (BlockHeaderDb_ db)
     :<|> headersHandler db defaultEntryLimit
     :<|> headerHandler db
     :<|> branchHashesHandler db
-    :<|> branchHeadersHandler db defaultEntryLimit
+    :<|> branchHeadersHandler db defaultBoundsLimit defaultEntryLimit
 
 -- Restricted P2P BlockHeader DB API
 --
 p2pBlockHeaderDbServer :: BlockHeaderDb_ v c -> Server (P2pBlockHeaderDbApi v c)
 p2pBlockHeaderDbServer (BlockHeaderDb_ db)
     = headersHandler db p2pEntryLimit
     :<|> headerHandler db
-    :<|> branchHeadersHandler db p2pEntryLimit
+    :<|> branchHeadersHandler db p2pBoundsLimit p2pEntryLimit
 
 -- -------------------------------------------------------------------------- --
 -- Multichain Server
@@ -284,7 +321,7 @@ headerStreamServer
     .  CanReadablePayloadCas tbl
     => CutDb tbl
     -> Server (HeaderStreamApi v)
-headerStreamServer cdb = headerStreamHandler cdb
+headerStreamServer = headerStreamHandler
 
 headerStreamHandler :: forall tbl. CanReadablePayloadCas tbl => CutDb tbl -> Tagged Handler Application
 headerStreamHandler db = Tagged $ \req resp -> do

src/Chainweb/Chainweb.hs

@@ -668,7 +668,7 @@ runChainweb cw = do
             . throttle (_chainwebPutPeerThrottler cw)
             . throttle (_chainwebMempoolThrottler cw)
             . throttle (_chainwebThrottler cw)
-            . requestSizeLimit
+            . p2pRequestSizeLimit
             . p2pValidationMiddleware
 
         -- 2. Start Clients (with a delay of 500ms)
@@ -678,7 +678,7 @@ runChainweb cw = do
         , threadDelay 500000 >> do
             serveServiceApi
                 $ serviceHttpLog
-                . requestSizeLimit
+                . serviceRequestSizeLimit
                 . serviceApiValidationMiddleware
         ]
 
@@ -786,11 +786,26 @@ runChainweb cw = do
                 mw)
             (monitorConnectionsClosedByClient clientClosedConnectionsCounter)
 
-    requestSizeLimit :: Middleware
-    requestSizeLimit = requestSizeLimitMiddleware $
+    -- Request size limit for the service API
+    --
+    serviceRequestSizeLimit :: Middleware
+    serviceRequestSizeLimit = requestSizeLimitMiddleware $
         setMaxLengthForRequest (\_req -> pure $ Just $ 2 * 1024 * 1024) -- 2MB
         defaultRequestSizeLimitSettings
 
+    -- Request size limit for the P2P API
+    --
+    -- NOTE: this may need to have to be adjusted if the p2p limits for batch
+    -- sizes or number of branch bound change. It may also need adjustment for
+    -- other protocol changes, like additional HTTP request headers or changes
+    -- in the mempool protocol.
+    --
+    -- FIXME: can we make this smaller and still let the mempool work?
+    --
+    p2pRequestSizeLimit :: Middleware
+    p2pRequestSizeLimit = requestSizeLimitMiddleware $
+        setMaxLengthForRequest (\_req -> pure $ Just $ 2 * 1024 * 1024) -- 2MB
+        defaultRequestSizeLimitSettings
 
     httpLog :: Middleware
     httpLog = requestResponseLogger $ setComponent "http:p2p-api" (_chainwebLogger cw)

test/Chainweb/Test/RestAPI.hs

@@ -2,6 +2,9 @@
 {-# LANGUAGE FlexibleContexts #-}
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE PatternSynonyms #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+
+{-# options_ghc -fno-warn-unused-local-binds -fno-warn-unused-imports #-}
 
 -- |
 -- Module: Chainweb.Test.RestAPI
@@ -19,6 +22,7 @@ module Chainweb.Test.RestAPI
 import Control.Monad
 import Control.Monad.IO.Class
 
+import Data.Bifunctor (first)
 import qualified Data.ByteString.Char8 as B8
 import Data.Either
 import Data.Foldable
@@ -40,6 +44,7 @@ import Text.Read (readEither)
 
 -- internal modules
 
+import Chainweb.BlockHash (BlockHash)
 import Chainweb.BlockHeader
 import Chainweb.BlockHeaderDB
 import Chainweb.BlockHeaderDB.Internal (unsafeInsertBlockHeaderDb)
@@ -48,6 +53,7 @@ import Chainweb.Graph
 import Chainweb.Mempool.Mempool (MempoolBackend, MockTx)
 import Chainweb.RestAPI
 import Chainweb.Test.RestAPI.Client_
+import Chainweb.Test.RestAPI.Utils (isFailureResponse, clientErrorStatusCode)
 import Chainweb.Test.Utils
 import Chainweb.Test.Utils.BlockHeader
 import Chainweb.Test.TestVersions (barebonesTestVersion)
@@ -168,6 +174,7 @@ simpleClientSession envIO cid =
         assertBool ("test failed: " <> sshow res) (isRight res)
   where
 
+    session :: [(ChainId, BlockHeaderDb)] -> (String -> IO a) -> ClientM ()
     session dbs step = do
 
         let gbh0 = genesisBlockHeader version cid
@@ -231,6 +238,60 @@ simpleClientSession envIO cid =
 
         -- branchHeaders
 
+        do
+          void $ liftIO $ step "branchHeadersClient: BranchBounds limits exceeded"
+          clientEnv <- liftIO $ do
+            -- ClientM does not have a MonadFail instance for this failable
+            -- pattern match; IO does.
+            BlockHeaderDbsTestClientEnv clientEnv _ _ <- envIO
+            pure clientEnv
+          let query bounds = liftIO
+                $ flip runClientM clientEnv
+                $ branchHeadersClient
+                    version cid Nothing Nothing Nothing Nothing bounds
+          let limit = 32
+          let blockHeaders = testBlockHeaders (ParentHeader gbh0)
+          let maxBlockHeaders = take limit blockHeaders
+          let excessBlockHeaders = take (limit + 1) blockHeaders
+
+          let mkLower :: [BlockHeader] -> HS.HashSet (LowerBound BlockHash)
+              mkLower hs = HS.fromList $ map (LowerBound . key) hs
+          let mkUpper :: [BlockHeader] -> HS.HashSet (UpperBound BlockHash)
+              mkUpper hs = HS.fromList $ map (UpperBound . key) hs
+
+          let emptyLower = mkLower []
+          let badLower = mkLower excessBlockHeaders
+          let goodLower = mkLower maxBlockHeaders
+
+          let emptyUpper = mkUpper []
+          let badUpper = mkUpper excessBlockHeaders
+          let goodUpper = mkUpper maxBlockHeaders
+
+          let badRespCheck :: Int -> ClientError -> Bool
+              badRespCheck s e = isFailureResponse e && clientErrorStatusCode e == Just s
+
+          badLowerResponse <- query (BranchBounds badLower emptyUpper)
+          assertExpectation "branchHeadersClient returned a 400 error code on excess lower"
+            (Expected (Left True))
+            (Actual (first (badRespCheck 400) badLowerResponse))
+
+          badUpperResponse <- query (BranchBounds emptyLower badUpper)
+          assertExpectation "branchHeadersClient returned a 400 error code on excess upper"
+            (Expected (Left True))
+            (Actual (first (badRespCheck 400) badUpperResponse))
+
+          -- This will still fail because a bunch of these keys won't be found,
+          -- but it won't fail the bounds check, which happens first
+          doesntFailBoundsCheck <- query (BranchBounds goodLower goodUpper)
+          assertExpectation "branchHeadersClient returned a 404; bounds were within the limits, still fails key exists check"
+            (Expected (Left True))
+            (Actual (first (badRespCheck 404) doesntFailBoundsCheck))
+
+          doesntFailAtAll <- query (BranchBounds emptyLower emptyUpper)
+          assertExpectation "branchHeadersClient returned a 200; bounds were within the limits, and no keys to check at all"
+            (Expected (Right ()))
+            (Actual (() <$ doesntFailAtAll))
+
         void $ liftIO $ step "branchHeadersClient: get no block headers"
         bhs3 <- branchHeadersClient version cid Nothing Nothing Nothing Nothing
             (BranchBounds mempty mempty)
@@ -320,10 +381,13 @@ simpleClientSession envIO cid =
             void $ liftIO $ step "branchHashesClient: get one block headers with lower and upper bound"
             hs5 <- branchHashesClient version cid Nothing Nothing Nothing Nothing
                 (BranchBounds (HS.singleton (LowerBound $ key lower)) (HS.singleton (UpperBound $ key h)))
+            liftIO $ print hs5
             assertExpectation "branchHashesClient returned wrong number of entries"
                 (Expected i)
                 (Actual $ _pageLimit hs5)
 
+
+
 -- -------------------------------------------------------------------------- --
 -- Paging Tests
 

test/Chainweb/Test/RestAPI/Utils.hs

@@ -9,6 +9,9 @@ module Chainweb.Test.RestAPI.Utils
 
   -- * Utils
 , repeatUntil
+, clientErrorStatusCode
+, isFailureResponse
+, getStatusCode
 
   -- * Pact client DSL
 , PactTestFailure(..)
@@ -53,6 +56,7 @@ import Data.Text (Text)
 import Data.Maybe (fromJust)
 import qualified Data.HashMap.Strict as HM
 import qualified Data.Text as T
+import Network.HTTP.Types.Status (Status(..))
 
 import Rosetta
 
@@ -600,3 +604,19 @@ networkStatus cenv req =
     h _ = Handler $ \case
       NetworkStatusFailure _ -> return True
       _ -> return False
+
+clientErrorStatusCode :: ClientError -> Maybe Int
+clientErrorStatusCode = \case
+  FailureResponse _ resp -> Just $ getStatusCode resp
+  DecodeFailure _ resp -> Just $ getStatusCode resp
+  UnsupportedContentType _ resp -> Just $ getStatusCode resp
+  InvalidContentTypeHeader resp -> Just $ getStatusCode resp
+  ConnectionError _ -> Nothing
+
+isFailureResponse :: ClientError -> Bool
+isFailureResponse = \case
+  FailureResponse {} -> True
+  _ -> False
+
+getStatusCode :: ResponseF a -> Int
+getStatusCode resp = statusCode (responseStatusCode resp)