Nazrhom/raw tx #49
Nazrhom/raw tx #49
Conversation
This endpoint must be enabled by the pcEnableTxEndpoint configuration variable. Change-type: minor
Change-type: patch Signed-off-by: Giovanni Garufi <giovanni@mlabs.city>
Change-type: patch Signed-off-by: Giovanni Garufi <giovanni@mlabs.city>
Change-type: patch Signed-off-by: Giovanni Garufi <giovanni@mlabs.city>
samuelWilliams99
left a comment
samuelWilliams99
left a comment
There was a problem hiding this comment.
Very nice! Only a few little notes :)
src/BotPlutusInterface/Types.hs
Outdated
|
|
||
| -- type is a reserved keyword in haskell and can not be used as a field name | ||
| -- when converting this to JSON we drop the _ prefix from each field | ||
| $(deriveJSON defaultOptions {fieldLabelModifier = drop 1} ''RawTx) |
There was a problem hiding this comment.
Can lose the $() for root scope TH :)
src/BotPlutusInterface/Server.hs
Outdated
| rawTxHandler :: PABConfig -> Text -> Handler RawTx | ||
| rawTxHandler config hash = do | ||
| -- Check that endpoint is enabled | ||
| assert (pcEnableTxEndpoint config) |
There was a problem hiding this comment.
We can use record dot here!
| assert (pcEnableTxEndpoint config) | |
| assert config.pcEnableTxEndpoint |
as well as a few other places in this file
| -- can not generate a client for the websocket endpoint. | ||
| txProxy :: | ||
| Proxy | ||
| ( "rawTx" |
There was a problem hiding this comment.
Is it worth maybe type synonym-ing the endpoint part and reusing in both places?
Change-type: patch Signed-off-by: Giovanni Garufi <giovanni@mlabs.city>
| :> Post '[JSON] ContractInstanceId -- Start a new instance. | ||
|
|
||
| type RawTxEndpoint = | ||
| "rawTx" |
There was a problem hiding this comment.
I think we should avoid using camel case in the URL, how about using dash-case instead?
|
|
||
| -- Add/Set .raw extension on path | ||
| let suppliedPath :: FilePath | ||
| suppliedPath = replaceExtension (txFolderPath </> "tx-" <> unpack hash) ".raw" |
There was a problem hiding this comment.
I think it would be a good idea to put the filename conversion into Files
|
|
||
| type RawTxEndpoint = | ||
| "rawTx" | ||
| :> Capture "hash" Text |
There was a problem hiding this comment.
Could we use TxId instead of Text here?
There was a problem hiding this comment.
Unfortunately we can not use TxId directly, this is because when defining an API including Capture “txId” TxId we also need to have FromHttpApiData and ToHttpApiData instances in scope (this is required to parse the data type to/from the URL)
So in order to avoid orphan instances, we would at least have to wrap TxId in a newtype, what do you think about it?
There was a problem hiding this comment.
I feel like orphan captures become pretty common, but also a newtype wouldn't hurt, can just unpack it in the handler pattern match
|
|
||
| -- | This handler will allow to retrieve raw transactions from the pcTxFileDir if pcEnableTxEndpoint is True | ||
| rawTxHandler :: PABConfig -> Text -> Handler RawTx | ||
| rawTxHandler config hash = do |
There was a problem hiding this comment.
Maybe txId instead of hash, to improve readability?
| txHash :: Text | ||
| txHash = "test" | ||
|
|
||
| txFileName :: FilePath |
There was a problem hiding this comment.
Once we have the filename conversion in Files, it could be imported from here
|
@nazrhom sorry, I was late with my review. I had a few comments, could you take a look at them? |
|
Do we want to tackle these in a separate PR? |
No description provided.