Finalize CLI implementation for endpoints in scope #189
Conversation
app/cli/Main.hs
Outdated
| res <- runClientM (listAddresses (ApiT wid) (Just $ ApiT Unused)) (mkClientEnv manager' (BaseUrl Http "localhost" 8080 "")) | ||
| case res of | ||
| Left err -> putStrLn $ "Error: " ++ show err | ||
| Right address -> print address |
There was a problem hiding this comment.
Please refer to #96 about the scope of the CLI commands for now :s ... This endpoint isn't in scope, nor implemented in the API at the moment.
app/cli/Main.hs
Outdated
| @@ -128,6 +135,11 @@ exec args | |||
| | args `isPresent` command "address" && args `isPresent` command "list" = do | |||
| wid <- args `parseArg` longOption "wallet-id" | |||
| print (wid :: WalletId) | |||
| manager' <- newManager defaultManagerSettings | |||
| res <- runClientM (listAddresses (ApiT wid) (Just $ ApiT Unused)) (mkClientEnv manager' (BaseUrl Http "localhost" 8080 "")) | |||
There was a problem hiding this comment.
We ought to create the connection manager before and pass it as argument I believe. In particular, the port address can be set via a CLI arg although, we don't have access to it at this stage 🤔 ...
I see two options:
- Allow the port to be passed directly from the CLI for commands to (with default)
- Do some network discovery and try to find a corresponding wallet backend service
The former sounds more reasonable in the time-frame we have to finalize this.
There was a problem hiding this comment.
yes - I was about to opt for the former
app/cli/Main.hs
Outdated
| @@ -130,7 +136,11 @@ exec args | |||
| print (wid :: WalletId) | |||
|
|
|||
| | args `isPresent` command "wallet" && args `isPresent` command "list" = do | |||
| return () | |||
| walletPort <- args `parseArg` longOption "port" | |||
| res <- runClientM listWallets (mkClientEnv manager (BaseUrl Http "localhost" walletPort "")) | |||
There was a problem hiding this comment.
what do you think about this approach vs the one we have in httpBridge https://github.com/input-output-hk/cardano-wallet/blob/master/src/Cardano/Wallet/Network/HttpBridge.hs#L167 where we are wrapping client calls to a record and having a smart constructor in main which will construct this record to talk to the backend (we could call it WalletClient or similar).
With this one we are passing Manager to the exec where with the HttpBridge like we would defined WalletClient, defined the api calls in there and then create an instance of this in main and pass it to exec.
akegalj
force-pushed
the
akegalj/96/list-wallet-cli
branch
from
05ab1a2
to
20e147d
Compare
app/cli/Main.hs
Outdated
| let walletData = | ||
| WalletPostData | ||
| (Just $ ApiT poolGap) | ||
| (ApiMnemonicT (mnemonic, ["todo"])) |
There was a problem hiding this comment.
@KtorZ
I don't have access to raw mnemonic sentance for ApiMnemonicT. Would we prefer to:
-
Add
ToMnemonicclass similar toFromMnemonicthat would convertPassphraseback to[Text]. This would involve implementingToMnemonictypeclass and few instances in https://github.com/input-output-hk/cardano-wallet/blob/f54e90fcba9c60e780c0b4c710f335f8149c158d/src/Cardano/Wallet/Primitive/AddressDerivation.hs#L267 . -
Modify
getRequiredSensitiveValueandgetOptionalSensitiveValueto return tupleIO (a, Text)instead of currentIO a
There was a problem hiding this comment.
Passphrase back to [Text]
This isn't possible 😶 ... We have to go for 2), although, returning a is not really necessary here. We may simply try to parse the mnemonic, and in case of success, return the raw version as a [Text]
There was a problem hiding this comment.
This isn't possible
hm - I had a quick look at the FromMnemonic instance https://github.com/input-output-hk/cardano-wallet/blob/f54e90fcba9c60e780c0b4c710f335f8149c158d/src/Cardano/Wallet/Primitive/AddressDerivation.hs#L322 and my intuition was: well we just have to do entropyToBytes $ mnemonicToEntropy m in reverse - so something like toMnemonic . toEntropy. But maybe there is hidden complexity that I have overlooked.
There was a problem hiding this comment.
To go from mnemonic words to entropy, there are actually some hash going on. So it's not reversible :/
akegalj
force-pushed
the
akegalj/96/list-wallet-cli
branch
from
a1cbe94
to
607e715
Compare
KtorZ
force-pushed
the
akegalj/96/list-wallet-cli
branch
from
607e715
to
6499efa
Compare
KtorZ
changed the title
Issue Number
#96
Overview
Comments
Playing out with types still to see what will play well. Will compare how we work with similar use case in HttpBridge and with our old v1 version