search

kadena-io / pact

The Pact Smart Contract Language
https://docs.kadena.io/build/pact
BSD 3-Clause "New" or "Revised" License
578 stars 98 forks source link

Rationalize Pact API #451

Closed sirlensalot closed 5 years ago

sirlensalot commented 5 years ago

Research issue for finalizing Pact API across Chainweb, pact -s, kadena (scalableBFT).

Proposals

Related/required are

Follow on Chainweb issues include using the exported Servant API (currently copy-pasted), supporting Yield and SPV, and otherwise supporting this upgrade.

Servant API

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] CommandResult
type LocalApi = "local"
  :> ReqBody '[JSON] (Command Text)
  :> Post '[JSON] CommandResult

newtype PollResponses = PollResponses (HM.HashMap RequestKey CommandResult)
sirlensalot commented 5 years ago

Chainweb Servant API:

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

type SendApi = "send" 
  :> ReqBody '[JSON] Pact.Types.API.SubmitBatch 
  :> Post '[JSON] Pact.Types.Api.RequestKeys
type PollApi = "poll" 
  :> ReqBody '[JSON] Pact.Types.API.Poll 
  :> Post '[JSON] Pact.Types.API.PollResponses
type ListenApi = "listen" 
  :> ReqBody '[JSON] Pact.Types.API.ListenerRequest 
  :> Post '[JSON] Pact.Types.API.ApiResult
type LocalApi = "local" 
  :> ReqBody '[JSON] (Pact.Types.Command.Command Text) 
  :> Post '[JSON] (Pact.Types.Command.CommandSuccess Value)
sirlensalot commented 5 years ago

Pact server API:

type ApiV1API =
  (    "send" :> ReqBody '[JSON] SubmitBatch :>
    Post '[JSON] RequestKeys
  :<|> "poll" :> ReqBody '[JSON] Poll :>
    Post '[JSON] PollResponses
  :<|> "listen" :> ReqBody '[JSON] ListenerRequest :>
    Post '[JSON] ApiResult
  :<|> "local" :> ReqBody '[JSON] (Command Text) :>
    Post '[JSON] (CommandSuccess Value)
  )

type PactServerAPI =
       "api" :> "v1" :> ApiV1API
  :<|> "verify" :> ReqBody '[JSON] Analyze.Request :> Post '[JSON] Analyze.Response
  :<|> "version" :> Get '[PlainText] Text
sirlensalot commented 5 years ago

Kadena API:

api :: Api ()
api = route [
       ("send",sendPublicBatch)
      ,("poll",poll)
      ,("listen",registerListener)
      ,("local",sendLocal)
      ,("private",sendPrivateBatch)
      ,("config", sendClusterChange)
      ]

-- note all handlers catch and log exceptions;
-- functions like `readJSON` use `die` 

sendLocal = ... do
  -- 'Command Text' request
  (cmd :: Pact.Command BS.ByteString) <- fmap encodeUtf8 <$> readJSON
  ...
  -- read `Value` from MVar; this is a _crResult from CommandResult
  writeResponse $ ApiSuccess $ r 

sendPublicBatch = ... do
      SubmitBatch cmds <- readJSON
      ...
      queueRpcs rpcs 
      ... -> writeResponse $ ApiSuccess rks

poll = ... do
    (Poll rks) <- readJSON
    ...
    writeResponse $ pollResultToReponse possiblyIncompleteResults
        ... -> ApiSuccess $ PollResponses $ scrToAr <$> m

registerListener = do
    ...
    (ListenerRequest rk) <- readJSON
    ...
    return $ ApiSuccess $ scrToAr scr
    ...
    -- also is the only endpoint to actually throw a caught exception
   throw e

sendPrivateBatch = ... do
    ...
    SubmitBatch cmds <- readJSON
    ...
    queueRpcs rpcs

sendClusterChange = ... do
    SubmitCC ccCmds <- readJSON
    ...
        queueRpcs [finalRpc]

queueRpcs :: [(RequestKey,CMDWire)] -> Api ()
queueRpcs rpcs = do
  -- rks :: 'RequestKeys'
  writeResponse $ ApiSuccess rks

-- including to show how ApiResult is populated
scrToAr :: CommandResult -> ApiResult
scrToAr cr = case cr of
  SmartContractResult{..} ->
    ApiResult (toJSON (Pact._crResult _scrResult)) (Pact._crTxId _scrResult) metaData'
  ConsensusChangeResult{..} ->
    ApiResult (toJSON _concrResult) tidFromLid metaData'
  PrivateCommandResult{..} ->
    ApiResult (handlePR _pcrResult) tidFromLid metaData'
  where metaData' = Just $ toJSON $ _crLatMetrics $ cr
        tidFromLid = Just $ Pact.TxId $ fromIntegral $ _crLogIndex $ cr
        handlePR (PrivateFailure e) = toJSON $ "ERROR: " ++ show e
        handlePR PrivatePrivate = String "Private message"
        handlePR (PrivateSuccess pr) = toJSON (Pact._crResult pr)

die :: String -> Api t
die res = do
  _ <- getResponse -- chuck what we've done so far
  setJSON
  log res
  writeLBS $ encode $ (ApiFailure ("Kadena.HTTP.ApiServer" ++ res) :: ApiResponse ())
  finishWith =<< getResponse

ApiResponse

ApiSuccess is ApiResponse in Pact 2.4.1: https://github.com/kadena-io/pact/blob/8f4216d274749507158c5e23be8c4b32a75191a6/src-ghc/Pact/Types/API.hs#L43

data ApiResponse a =
  ApiSuccess
    { _apiResponse :: !a} |
  ApiFailure
  { _apiError :: !String}

Note that Pact as of 2.6.1 still used ApiResponse, so the lack of this in other APIs must have been introduced in changes leading up to testnet.

vaibhavsagar commented 5 years ago

ApiResponse was removed in https://github.com/kadena-io/pact/pull/368.

sirlensalot commented 5 years ago

send

Request: SubmitBatch.

newtype SubmitBatch = SubmitBatch { _sbCmds :: [Command Text] }

Response:

sirlensalot commented 5 years ago

poll

Request:Poll

newtype Poll = Poll { _pRequestKeys :: [RequestKey] }

Response:

PollResponse:

newtype PollResponses = PollResponses (HM.HashMap RequestKey ApiResult)

ApiResult:

data ApiResult = ApiResult {
  _arResult :: !Value,
  _arTxId :: !(Maybe TxId),
  _arMetaData :: !(Maybe Value)
  } deriving (Eq,Show,Generic)

ApiResult encoding: kadena:

pact -s:

Chainweb:

sirlensalot commented 5 years ago

CommandResult, CommandSuccess, CommandError, Receipts

data CommandResult = CommandResult
  { _crReqKey :: RequestKey
  , _crTxId :: Maybe TxId
  , _crResult :: Value
  , _crGas :: Gas  -- not in Kadena
  } deriving (Eq,Show)

Kadena: uses pact 2.4.1 jsonResult: 

jsonResult :: ToJSON a => ExecutionMode -> RequestKey -> a -> CommandResult
jsonResult ex cmd a = CommandResult cmd (exToTx ex) (toJSON a)

a (i.e., _crResult) is CommandSuccess or CommandFailure.

pact -s: same

chainweb: same. Also, _crResult is included of HashedTxLogOutput, ie the receipt, as the "output" to the SPV system/payload header. API might consider supplying the Full output. Receipts definitely need more that just _crResult.

data FullLogTxOutput = FullLogTxOutput
    { _flCommandResult :: A.Value -- note this is just _crResult not a full CommandResult
    , _flTxLogs :: [TxLog A.Value]
    } deriving (Show, Eq)

data HashedLogTxOutput = HashedLogTxOutput
    { _hlCommandResult :: Value -- see note above about _crResult
    , _hlTxLogHash :: Hash
    } deriving (Eq, Show)

CommandSuccess, CommandError:

data CommandError = CommandError {
      _ceMsg :: String
    , _ceDetail :: Maybe String
}
instance ToJSON CommandError where
    toJSON (CommandError m d) =
        object $ [ "status" .= ("failure" :: String)
                 , "error" .= m ] ++
        maybe [] ((:[]) . ("detail" .=)) d

data CommandSuccess a = CommandSuccess {
      _csData :: a
    }
instance (ToJSON a) => ToJSON (CommandSuccess a) where
    toJSON (CommandSuccess a) =
        object [ "status" .= ("success" :: String)
                , "data" .= a ]

CommandSuccess:

kadena: populated with last erOutput from EvalResult ([Term name]) pact -s: same chainweb: same

Proposal for CommandResult changes

Make CommandResult uniform/ubiquitous over the API

Naming: consider PactResult or ExecResult, also consider ...Response

Changes:

EDIT: something like

CommandResult l = CommandResult {
{ _crReqKey :: RequestKey
  , _crTxId :: Maybe TxId
  , _crResult :: Either PactError PactValue
  , _crGas :: Gas  
  , _crLogs :: l -- this would be [TxLog Value] in pact -s
  , _crContinuation :: Maybe PactExec
  , _crMeta :: Maybe Value }
sirlensalot commented 5 years ago

local

Request: Command Text

Response: kadena: ApiSuccess with _crResult populated as above (CommandSuccess/CommandError) pact -s: CommandSuccess as above chainweb: CommandSuccess as above

sirlensalot commented 5 years ago

listen

Request: ListenerRequest

newtype ListenerRequest = ListenerRequest RequestKey

Response: kadena: ApiResult same as poll pact -s: ApiResult same as poll chainweb: ApiResult same as poll

sirlensalot commented 5 years ago

Draft of goals in unification:

sirlensalot commented 5 years ago

Continuations

Pact continuations are handled with send (or private) within the payload ContMsg. Responses vary significantly based on whether a private pact is executed (in kadena), a public pact (in pact -s and chainweb) and, in the future, SPV (chainweb but all platforms ideally).

Request: Command Text with ContMsg payload. kadena/private/pact 2.4.1:

data ContMsg = ContMsg
  { _cmTxId :: !TxId -- "pact id"
  , _cmStep :: !Int -- step idx
  , _cmRollback :: !Bool -- rollback flag
  , _cmResume :: !(Maybe Value) -- `yield` value made from pact `object`
} deriving (Eq,Show,Generic)

pact -s, chainweb:

data ContMsg = ContMsg
  { _cmPactId :: !PactId -- now a proper datatype
  , _cmStep :: !Int -- step idx
  , _cmRollback :: !Bool -- rollback flag
  , _cmData :: !Value -- payload message data (note that kadena lacks this)
  } deriving (Eq,Show,Generic)

Response: kadena/private: CommandSuccess output of pact erResult, plus automatic dispatch of subsequent continuation request with yield value. pact -s, chainweb: CommandSuccess output of pact erResult.

Internal state: kadena/private: Tracking of pacts by pact ID with Pact datatype. Note that yield/resume is not tracked.

data Pact = Pact
  { _pTxId :: TxId -- pact ID
  , _pContinuation :: Term Name
  , _pSigs :: S.Set PublicKey -- note trust model implied here as resumes are automatic
  , _pStepCount :: Int
  }

pact -s/chainweb: Tracking of pacts by pact ID directly using PactExec result from pact interpreter. Yield/resume is tracked; continuation is strict App of args.

-- | Result of evaluation of a 'defpact'.
data PactExec = PactExec
  { -- | Count of steps in pact (discovered when code is executed)
    _peStepCount :: Int
    -- | Yield value if invoked
  , _peYield :: !(Maybe (Term Name))
    -- | Whether step was executed (in private cases, it can be skipped)
  , _peExecuted :: Bool
    -- | Step that was executed or skipped
  , _peStep :: Int
    -- | Pact id. On a new pact invocation, is copied from tx id.
  , _pePactId :: PactId
    -- | Strict (in arguments) application of pact, for future step invocations.
  , _peContinuation :: PactContinuation
  } deriving (Eq,Show)

newtype PactContinuation = PactContinuation (App (Term Ref))

SPV requirements

For SPV, yield results and endorsements (see #443 and related) will need to become part of the receipt; most transparent way to do this is encode all of this in the API result; in general all of the data loss for API results is not desirable.

Private vs Public/SPV

Conclusion: Private continuations should be sent via a different endpoint and JSON type, which in turn will encode the Yield value into data. Since this is intended to be a machine-generated request this will avoid confusion about ContMsg as a JSON type for user use.

EDIT on second thought this is unsatisfying.

Three kinds of pacts, and implications for resume, user data, and argument rehydration:

Simple user-driven stepped execution

SPV Cross-chain execution (also user-driven)

Private automated serial execution

ContMsg Proposal

Yield will be created/specified per #443 and in CommandResult above. Continuation as (Name,[TermOutput]) is specified in CommandResult above.

Proposed ContMsg:

data ContMsg = ContMsg
  { _cmPactId :: !PactId -- now a proper datatype
  , _cmStep :: !Int -- step idx
  , _cmRollback :: !Bool -- rollback flag
  , _cmData :: !(Object Name) -- or whatever the TermOutput safe version of this is 
  , _cmProof :: !(Maybe ByteString) -- or some specific proof object?
  } deriving (Eq,Show,Generic)

NB private data:

Private messages will simply overload _cmData with Yield data. One day endorsements could make sense as Petersen commitments or something, and target would indicate entity. Maximum future-proofing would suggest Private uses Yield and _cmData be encoded as an ADT like:

data ContData = ContUserData !(Object Name) | ContPrivYield !Yield

But again we would probably like to see this come in another endpoint, implying that the API-visible type should not bother with this.

sirlensalot commented 5 years ago

Command Text/Command (Payload ParsedCode m)

Note that the full specification/transformation is:

data Command a = Command
  { _cmdPayload :: !a
  , _cmdSigs :: ![UserSig]
  , _cmdHash :: !Hash
  }

data Payload m c = Payload
  { _pPayload :: !(PactRPC c)
  , _pNonce :: !Text
  , _pMeta :: !m
  } 

data PactRPC c =
    Exec (ExecMsg c) |
    Continuation ContMsg

data ExecMsg c = ExecMsg
  { _pmCode :: c
  , _pmData :: Value
  }

data ContMsg = ContMsg
  { _cmPactId :: !PactId
  , _cmStep :: !Int
  , _cmRollback :: !Bool
  , _cmData :: !Value
  }

data ParsedCode = ParsedCode
  { _pcCode :: !Text
  , _pcExps :: ![Exp Parsed]
  }

Metadata types:


newtype PrivateMeta = PrivateMeta { _pmAddress :: Maybe Address }

data PublicMeta = PublicMeta
  { _pmChainId :: Text
  , _pmSender :: Text
  , _pmGasLimit :: ParsedInteger
  , _pmGasPrice :: ParsedDecimal
  }
sirlensalot commented 5 years ago

Always Return 200 debate vs std HTTP error codes

Argument for error codes: https://medium.com/@sunitparekh/guidelines-on-json-responses-for-restful-services-1ba7c0c015d

Here's an argument for both:

for example the foursquare API provides HTTP status return codes and then also sends those codes in the response body. http://www.startupcto.com/backend-tech/building-an-api-best-practices#handling-errors

My $.02 is that "you have to handle 500 etc anyway", so error codes are a thing, and it does clean up the API. The practice above of including a nice JSON is good anyway, so we can try to use modifyResponse to do this.

LindaOrtega commented 5 years ago

@slpopejoy

change _crResult to Either e TermOutput, e being a string or exception object

Would e then be a type parameter of CommandResult or should it just be String? And TermOutput becomes PactOutput?

sirlensalot commented 5 years ago

Would e then be a type parameter of CommandResult or should it just be String? And TermOutput becomes PactOutput?

@LindaOrtega it should probably be PactError, and PactValue as the output type.

mercadoa commented 5 years ago

➤ anagha@kadena.io commented:

Question - do we need Stuart to review PR as well?

mercadoa commented 5 years ago

➤ anagha@kadena.io commented:

Not all of this is required for May 30th

LindaOrtega commented 5 years ago

@slpopejoy for the ContMsg proposal:

data ContMsg = ContMsg
  { _cmPactId :: !PactId -- now a proper datatype
  , _cmStep :: !Int -- step idx
  , _cmRollback :: !Bool -- rollback flag
  , _cmData :: !(Object Name) -- or whatever the TermOutput safe version of this is 
  , _cmProof :: !(Maybe ByteString) -- or some specific proof object?
  } deriving (Eq,Show,Generic)

Should _cmData be of type !(ObjectMap PactValue), which is the same data type as the Yield field in PactExec? Moreover, should the data section of ExecMsg also be of this type?

Moreover, should I create a proof object for _cmProof or is ByteString/Text fine here?

sirlensalot commented 5 years ago

@LindaOrtega

sirlensalot commented 5 years ago

~I wonder if we should enforce in Pact 3.0 that user data is always an object though, and break ExecMsg too. In which case it would be ObjectMap PactValue for both~ EDIT nevermind, this would break how read-integer and other functions work

emilypi commented 5 years ago

_cmData is user data and Object Name is currently a little ambiguous. However, discrimination will be easier when #480 is in, since continuation yield/resume data will be a Yield object with a potential endorsement.

sirlensalot commented 5 years ago

@emilypi the question for you is the _cmProof format, I answered re _cmData above https://github.com/kadena-io/pact/issues/451#issuecomment-494471018

emilypi commented 5 years ago

_cmProof would be easiest to work with as a Maybe Hash, since that's the end result of an Endorsement if it's present. If we want to represent that as a bytestring, that's fine, but we should make a note that it's just the bytestring associated with a Hash. Hash would be better.

LindaOrtega commented 5 years ago

@emilypi @slpopejoy based on both of your inputs, ContMsg should look as follows correct?

data ContMsg = ContMsg
  { _cmPactId :: !PactId
  , _cmStep :: !Int
  , _cmRollback :: !Bool
  , _cmData :: !Value
  , _cmProof :: !(Maybe Hash)
  } deriving (Eq,Show,Generic)

Edit: 05/21/2019 Changes per offline conversation

data ContMsg = ContMsg
  { _cmPactId :: !PactId
  , _cmStep :: !Int
  , _cmRollback :: !Bool
  , _cmData :: !Value
  , _cmProof :: !(Maybe ByteString)
  } deriving (Eq,Show,Generic)
LindaOrtega commented 5 years ago

Use modifyResponse to have "lucid errors", ie HTTP error codes but also a full JSON { "status": "failure", "code": 402, "Bad Input: you got your chocolate in my peanut butter" }

@slpopejoy I have some questions related to this proposal:

  1. The Servant API already outputs http error codes (i.e. 404 if you try to ping a non-existent endpoint) via ServantError. Is it that we want to convert PactErrors into 400 (or some other http code) using something like throwErr inside the Servant Handler code (see this Chainweb code for an example).

  2. Is the intent of this proposal to make ServantError (See definition)'s errBody field be of type Value (i.e. JSON)?

  3. Do we still want to reintroduce ApiResponse per this comment?

mercadoa commented 5 years ago

➤ Linda@kadena.io commented:

3/4 tasks have been merged in.

mercadoa commented 5 years ago

➤ anagha@kadena.io commented:

Moved the 4th task to its own separate issue (#P515).

Will wait for Linda to get back and close this issue

LindaOrtega commented 5 years ago

Closed via https://github.com/kadena-io/pact/pull/505 and https://github.com/kadena-io/pact/pull/504. 4th task moved to its own issue: https://github.com/kadena-io/pact/issues/515