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

Pact API upgrade #208

Merged
merged 10 commits into from
May 24, 2019
Merged
Show file tree
Hide file tree
Changes from 6 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
2 changes: 1 addition & 1 deletion cabal.project
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ package aeson
source-repository-package
type: git
location: https://github.com/kadena-io/pact.git
tag: d9a35f6edee1a99cd6f864f68fc3b3b44ce0b4cd
tag: 9456a750f67b62d48bed5ef2e60fbc7e9d2a4396

source-repository-package
type: git
Expand Down
4 changes: 2 additions & 2 deletions default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ in
pact = dontCheck ( addBuildDepend (self.callCabal2nix "pact" (pkgs.fetchFromGitHub {
owner = "kadena-io";
repo = "pact";
rev = "d9a35f6edee1a99cd6f864f68fc3b3b44ce0b4cd";
sha256 = "0cxy0azlg9p7zljpxv4fc4hcdizhv48q8cgpwh4557zcii1vbqhk";
rev = "9456a750f67b62d48bed5ef2e60fbc7e9d2a4396";
sha256 = "039fmgb5d4p524vagmkmmp3gajmvrly7r3iwy9mcdg6j7a6vqj9w";
}) {}) pkgs.z3);

streaming = callHackageDirect {
Expand Down
14 changes: 7 additions & 7 deletions src/Chainweb/BlockHeader/Genesis/Testnet00Payload.hs

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions src/Chainweb/BlockHeader/Genesis/Testnet01Payload.hs

Large diffs are not rendered by default.

45 changes: 24 additions & 21 deletions src/Chainweb/Pact/PactService.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ module Chainweb.Pact.PactService
, mkPureState
, serviceRequests
, createCoinContract
, toHashedLogTxOutput
, initialPayloadState
, transactionsFromPayload
, restoreCheckpointer
Expand Down Expand Up @@ -196,39 +195,42 @@ serviceRequests memPoolAccess reqQ = do
LocalMsg LocalReq{..} -> do
r <- try $ execLocal _localRequest
case r of
Left e -> liftIO $ putMVar _localResultVar $ Left e
Right r' -> liftIO $ putMVar _localResultVar r'
Left (SomeException e) -> liftIO $ putMVar _localResultVar $ toPactInternalError e
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume Left can only be of type SomeExecption ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unusual one, where we don't think any exceptions will fire but "just in case". Also, the SomeException specializes try otherwise it wouldn't compile.

Right r' -> liftIO $ putMVar _localResultVar $ Right r'
go
NewBlockMsg NewBlockReq {..} -> do
txs <- try $ execNewBlock memPoolAccess _newBlockHeader _newMiner
case txs of
Left (SomeException e) -> do
logError (show e)
liftIO $ putMVar _newResultVar $ Left $ PactInternalError $ T.pack $ show e
liftIO $ putMVar _newResultVar $ toPactInternalError e
Right r -> liftIO $ putMVar _newResultVar $ Right r
go
ValidateBlockMsg ValidateBlockReq {..} -> do
txs <- try $ execValidateBlock False _valBlockHeader _valPayloadData
case txs of
Left (SomeException e) -> do
logError (show e)
liftIO $ putMVar _valResultVar $ Left $ PactInternalError $ T.pack $ show e
liftIO $ putMVar _valResultVar $ toPactInternalError e
Right r ->
liftIO $ putMVar _valResultVar $ validateHashes r _valBlockHeader
go
toPactInternalError e = Left $ PactInternalError $ T.pack $ show e


toTransactionBytes :: P.Command ByteString -> Transaction
toTransactionBytes cwTrans =
let plBytes = encodeToByteString cwTrans
in Transaction { _transactionBytes = plBytes }

toOutputBytes :: FullLogTxOutput -> TransactionOutput
toOutputBytes flOut =
let hashedLogOut = toHashedLogTxOutput flOut
outBytes = A.encode hashedLogOut

toOutputBytes :: HashCommandResult -> TransactionOutput
toOutputBytes cr =
let outBytes = A.encode cr
in TransactionOutput { _transactionOutputBytes = toS outBytes }



toPayloadWithOutputs :: MinerInfo -> Transactions -> PayloadWithOutputs
toPayloadWithOutputs mi ts =
let oldSeq = Seq.fromList $ V.toList $ _transactionPairs ts
Expand All @@ -244,6 +246,7 @@ toPayloadWithOutputs mi ts =
plData = payloadData blockTrans blockPL
in payloadWithOutputs plData cb transOuts


validateHashes :: PayloadWithOutputs -> BlockHeader -> Either PactException PayloadWithOutputs
validateHashes pwo bHeader =
let newHash = _payloadWithOutputsPayloadHash pwo
Expand Down Expand Up @@ -308,7 +311,7 @@ execNewGenesisBlock miner newTrans = do


execLocal :: ChainwebTransaction ->
PactServiceM (Either SomeException (P.CommandSuccess A.Value))
PactServiceM HashCommandResult
execLocal cmd = do

bh <- use psStateValidated >>= \v -> case v of
Expand All @@ -330,7 +333,7 @@ execLocal cmd = do

discardCheckpointer

return (fmap (\(P.CommandSuccess t) -> P.CommandSuccess (A.toJSON t)) r)
return $! toHashCommandResult r



Expand Down Expand Up @@ -407,28 +410,25 @@ runCoinbase
:: Maybe BlockHash
-> Env'
-> MinerInfo
-> PactServiceM FullLogTxOutput
-> PactServiceM HashCommandResult
runCoinbase Nothing _ _ = return noCoinbase
runCoinbase (Just parentHash) (Env' dbEnv) mi@MinerInfo{..} = do
runCoinbase (Just _parentHash) (Env' dbEnv) mi@MinerInfo{..} = do
psEnv <- ask

let reward = 42.0 -- TODO. Not dispatching on chainweb version yet as E's PR will have PublicData
pd = _psPublicData psEnv
logger = _cpeLogger . _psCheckpointEnv $ psEnv

(result, txLogs) <- liftIO $ applyCoinbase logger dbEnv mi reward pd

let output = A.object [ "result" A..= P._crResult result, "parentHash" A..= parentHash]
toHashCommandResult <$> liftIO (applyCoinbase logger dbEnv mi reward pd)

pure $! FullLogTxOutput output txLogs

-- | Apply multiple Pact commands, incrementing the transaction Id for each
applyPactCmds
:: Bool
-> Env'
-> Vector (P.Command PayloadWithText)
-> MinerInfo
-> PactServiceM (Vector FullLogTxOutput)
-> PactServiceM (Vector HashCommandResult)
applyPactCmds isGenesis env' cmds miner = V.mapM f cmds
where
f cmd = applyPactCmd isGenesis env' cmd miner
Expand All @@ -439,7 +439,7 @@ applyPactCmd
-> Env'
-> P.Command PayloadWithText
-> MinerInfo
-> PactServiceM FullLogTxOutput
-> PactServiceM HashCommandResult
applyPactCmd isGenesis (Env' dbEnv) cmdIn miner = do
psEnv <- ask
let logger = _cpeLogger . _psCheckpointEnv $ psEnv
Expand All @@ -449,11 +449,14 @@ applyPactCmd isGenesis (Env' dbEnv) cmdIn miner = do

-- cvt from Command PayloadWithTexts to Command ((Payload PublicMeta ParsedCode)
let cmd = payloadObj <$> cmdIn
(result, txLogs) <- liftIO $! if isGenesis
result <- liftIO $! if isGenesis
then applyGenesisCmd logger dbEnv pd spv cmd
else applyCmd logger dbEnv miner gasModel pd spv cmd

pure $! FullLogTxOutput (P._crResult result) txLogs
pure $! toHashCommandResult result

toHashCommandResult :: P.CommandResult [P.TxLog A.Value] -> HashCommandResult
toHashCommandResult = over (P.crLogs . _Just) (P.pactHash . encodeToByteString)

Copy link
Contributor

Choose a reason for hiding this comment

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

I doesn't seem that LamdaCase or TypeApplication are needed extension anymore

updateState :: PactDbState -> PactServiceM ()
updateState = assign psStateDb
Expand Down
14 changes: 2 additions & 12 deletions src/Chainweb/Pact/RestAPI.hs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ module Chainweb.Pact.RestAPI where

------------------------------------------------------------------------------
import Control.Monad.Identity
import Data.Aeson.Types
import Data.Text (Text)
import Pact.Types.API
import Pact.Types.Command
import Pact.Server.API as API
import Servant
------------------------------------------------------------------------------
import Chainweb.ChainId
Expand All @@ -23,14 +20,7 @@ import Chainweb.Version
-- -------------------------------------------------------------------------- --
-- @GET /chainweb/<ApiVersion>/<ChainwebVersion>/chain/<ChainId>/pact/@

type ApiV1API = SendApi :<|> PollApi :<|> ListenApi :<|> LocalApi

type SendApi = "send" :> ReqBody '[JSON] SubmitBatch :> Post '[JSON] RequestKeys
type PollApi = "poll" :> ReqBody '[JSON] Poll :> Post '[JSON] PollResponses
type ListenApi = "listen" :> ReqBody '[JSON] ListenerRequest :> Post '[JSON] ApiResult
type LocalApi = "local" :> ReqBody '[JSON] (Command Text) :> Post '[JSON] (CommandSuccess Value)

type PactApi_ = "pact" :> ApiV1API
type PactApi_ = "pact" :> API.ApiV1API -- TODO unify with Pact versioning
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should really unify these. Pact api is api/v1/send etc. Chainweb is [chainweb stuff]/pact/send. Pact docs refer to the former. Do we fully conform, ie [chainweb stuff]/pact/api/v1/send? Or do we update the pact docs to start with v1/send and say the prefix can vary? Or, I guess we could modify pact -s to be pact/v1/send and then chainweb is strictly a prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on discussion with @larskuhtz , going with fully specified url, since the "Pact API" is cross platform while chainweb might hang other services off of pact/ (ie SPV queries etc). Thus [chainweb stuff]/pact/api/v1/send.


type PactApi (v :: ChainwebVersionT) (c :: ChainIdT)
= 'ChainwebEndpoint v :> ChainEndpoint c :> Reassoc PactApi_
Expand Down
27 changes: 12 additions & 15 deletions src/Chainweb/Pact/RestAPI/Orphans.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,31 @@

module Chainweb.Pact.RestAPI.Orphans () where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we move these to Pact? Are they meaningful there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also have a couple of Aeson instances of GasPrice in Chainweb.Oprhans that I assume needs to be moved to Pact


import Data.Aeson
import Data.Swagger
import Data.Text (Text)
import Pact.Types.API
import Pact.Types.Command

instance ToSchema (CommandSuccess a) where
declareNamedSchema _ = return $ NamedSchema (Just "CommandSuccess") mempty
instance ToSchema (CommandResult a) where
declareNamedSchema = simpleNS "CommandResult"

instance ToSchema (Command a) where
declareNamedSchema _ = return $ NamedSchema (Just "Command") mempty

instance ToSchema Value where
declareNamedSchema _ = return $ NamedSchema (Just "Value") mempty

instance ToSchema ApiResult where
declareNamedSchema _ = return $ NamedSchema (Just "ApiResult") mempty
declareNamedSchema = simpleNS "Command"

instance ToSchema ListenerRequest where
declareNamedSchema _ = return $ NamedSchema (Just "ListenerRequest") mempty
declareNamedSchema = simpleNS "ListenerRequest"

instance ToSchema PollResponses where
declareNamedSchema _ = return $ NamedSchema (Just "PollResponses") mempty
declareNamedSchema = simpleNS "PollResponses"

instance ToSchema Poll where
declareNamedSchema _ = return $ NamedSchema (Just "Poll") mempty
declareNamedSchema = simpleNS "Poll"

instance ToSchema RequestKeys where
declareNamedSchema _ = return $ NamedSchema (Just "RequestKeys") mempty
declareNamedSchema = simpleNS "RequestKeys"

instance ToSchema SubmitBatch where
declareNamedSchema _ = return $ NamedSchema (Just "SubmitBatch") mempty
declareNamedSchema = simpleNS "SubmitBatch"

simpleNS :: Monad m => Text -> p -> m NamedSchema
simpleNS n _ = return $ NamedSchema (Just n) mempty
29 changes: 14 additions & 15 deletions src/Chainweb/Pact/RestAPI/Server.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE AllowAmbiguousTypes #-}
{-# LANGUAGE BangPatterns #-}
{-# LANGUAGE DataKinds #-}
Expand Down Expand Up @@ -42,6 +43,7 @@ import qualified GHC.Event as Ev
import qualified Pact.Types.Hash as H
import Pact.Types.API
import Pact.Types.Command
import Pact.Types.Hash
import Prelude hiding (init, lookup)
import Servant

Expand Down Expand Up @@ -154,25 +156,26 @@ listenHandler
-> ChainResources logger
-> TransactionBloomCache
-> ListenerRequest
-> Handler ApiResult
-> Handler (CommandResult Hash)
listenHandler cutR cid chain bloomCache (ListenerRequest key) =
liftIO $ handleTimeout runListen
liftIO (handleTimeout runListen) >>= handleResult
where
nullResponse = ApiResult (object []) Nothing Nothing
handleResult (Right r) = return r
handleResult _ = throwError $ err400 { errBody = "Timeout" }

runListen timedOut = go Nothing
where
go !prevCut = do
m <- waitForNewCut prevCut
case m of
Nothing -> return nullResponse -- timeout
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gregorycollins I think I have the semantics right here, please confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

Timeout is an expected condition here, it shouldn't return 400. Instead the CommandResult should be populated with a timeout code if possible. I guess it doesn't matter as long as the web client code does the right thing with respect to retry. If you do use an HTTP error code, properly it should be 500-series because 400-series responses are meant to indicate some problem with the request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should it return then? 500 "Timeout"? cloudflare does a "522 Timeout" https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#Cloudflare

Copy link
Contributor

Choose a reason for hiding this comment

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

If the timeout is expected. What about 204 No Content or 202 Accepted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like 204

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OTOH, maybe we should just do 200 with { "status": "timeout" } body, just to avoid finicky header code docs. One thing I didn't do with this update is go for the "literate error responses", ie use errorResponse or whatever to make 402s etc have a parseable body with { "status": "failure", "code": "402", "message": ... } as discussed, so that JS frontends can simply parse a body.

Nothing -> return $ Left () -- timeout
(Just cut) -> poll cut

poll cut = do
hm <- internalPoll cutR cid chain bloomCache cut [key]
if HashMap.null hm
then go (Just cut)
else return $! snd $ head $ HashMap.toList hm
else return $! Right $ snd $ head $ HashMap.toList hm

waitForNewCut lastCut = atomically $ do
-- TODO: we should compute greatest common ancestor here to bound the
Expand Down Expand Up @@ -206,7 +209,7 @@ localHandler
-> ChainId
-> ChainResources logger
-> Command Text
-> Handler (CommandSuccess Value)
-> Handler (CommandResult Hash)
localHandler _ _ cr cmd = do
cmd' <- case validateCommand cmd of
Right c -> return c
Expand All @@ -229,11 +232,11 @@ internalPoll
-> TransactionBloomCache
-> Cut
-> [RequestKey]
-> IO (HashMap RequestKey ApiResult)
-> IO (HashMap RequestKey (CommandResult Hash))
internalPoll cutR cid chain bloomCache cut requestKeys =
toHashMap <$> mapM lookup requestKeys
where
lookup :: RequestKey -> IO (Maybe (RequestKey, ApiResult))
lookup :: RequestKey -> IO (Maybe (RequestKey, CommandResult Hash))
lookup key =
fmap (key,) <$> lookupRequestKey cid cut cutR chain bloomCache key
toHashMap = HashMap.fromList . catMaybes
Expand All @@ -247,7 +250,7 @@ lookupRequestKey
-> ChainResources logger
-> TransactionBloomCache
-> RequestKey
-> IO (Maybe ApiResult)
-> IO (Maybe (CommandResult Hash))
lookupRequestKey cid cut cutResources chain bloomCache key = do
-- get leaf block header for our chain from current best cut
chainLeaf <- lookupCutM cid cut
Expand All @@ -273,7 +276,7 @@ lookupRequestKeyInBlock
-> RequestKey -- ^ key to search
-> BlockHeight -- ^ lowest block to search
-> BlockHeader -- ^ search starts here
-> MaybeT IO ApiResult
-> MaybeT IO (CommandResult Hash)
lookupRequestKeyInBlock cutR chain bloomCache key minHeight = go
where
keyHash :: H.Hash
Expand All @@ -298,12 +301,8 @@ lookupRequestKeyInBlock cutR chain bloomCache key minHeight = go
case find matchingHash txs of
(Just (_cmd, (TransactionOutput output))) -> do

-- this will be a HashedTxLogOutput containing a Value of
-- of `CommandSuccess` or `CommandFailure`.
-- The metadata could be used to track request time, chain metadata etc.

val <- MaybeT $ return $ decodeStrict output
return $! ApiResult val Nothing Nothing
return $! val

Nothing -> lookupParent blockHeader

Expand Down
18 changes: 10 additions & 8 deletions src/Chainweb/Pact/SPV.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# LANGUAGE RecordWildCards #-}
Copy link
Member

Choose a reason for hiding this comment

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

No wildcards are used in this file

Copy link
Contributor

Choose a reason for hiding this comment

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

This didn't need to be added

{-# LANGUAGE GADTs #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedStrings #-}
Expand Down Expand Up @@ -92,16 +93,17 @@ pactSPV cdbv l = SPVSupport $ \s o -> readMVar cdbv >>= go s o

extractOutputs :: TransactionOutput -> IO (Either Text (Object Name))
extractOutputs (TransactionOutput t) =
case decodeStrict t of
case decodeStrict t :: Maybe HashCommandResult of
Nothing -> internalError $
"unable to decode spv transaction output"
Just (HashedLogTxOutput u _) ->
case fromJSON u of
Error e -> spvError' e
Success (CommandSuccess (TObject o _)) -> return $ Right o
Success o -> do
logLog l "ERROR" $ show o
internalError "associated pact transaction outputs have wrong format"
Just (CommandResult _ _ (PactResult (Right pv)) _ _ _ _) -> case fromPactValue pv of
(TObject o _) -> return $ Right o
o -> do
logLog l "ERROR" $ show o
spvError' "associated pact transaction outputs have wrong format"
Just o -> do
logLog l "ERROR" $ show o
spvError' "associated pact transaction outputs have wrong format"
Copy link
Member

Choose a reason for hiding this comment

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

We should differentiate these two error cases so we know that one is a decode error vs. one being a 'wrong result format'



-- | Look up pact tx hash at some block height in the
Expand Down
8 changes: 3 additions & 5 deletions src/Chainweb/Pact/Service/BlockValidation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ import Chainweb.Pact.Service.PactQueue
import Chainweb.Pact.Service.Types
import Chainweb.Pact.Types
import Chainweb.Payload
import Pact.Types.Command
import Data.Aeson (Value)
import Control.Exception
import Chainweb.Transaction


newBlock :: MinerInfo -> BlockHeader -> TQueue RequestMsg -> IO (MVar (Either PactException PayloadWithOutputs))
newBlock :: MinerInfo -> BlockHeader -> TQueue RequestMsg ->
IO (MVar (Either PactException PayloadWithOutputs))
newBlock mi bHeader reqQ = do
resultVar <- newEmptyMVar :: IO (MVar (Either PactException PayloadWithOutputs))
let msg = NewBlockMsg NewBlockReq
Expand All @@ -55,7 +53,7 @@ validateBlock bHeader plData reqQ = do
addRequest reqQ msg
return resultVar

local :: ChainwebTransaction -> TQueue RequestMsg -> IO (MVar (Either SomeException (CommandSuccess Value)))
local :: ChainwebTransaction -> TQueue RequestMsg -> IO (MVar (Either PactException HashCommandResult))
local ct reqQ = do
resultVar <- newEmptyMVar
let msg = LocalMsg LocalReq
Expand Down
Loading