Add ListAddresses, PutWallet, PutWalletPassphrase to the Servant API definition.

[jonathanknowles]

Issue Number

Issue #118

Overview

• Add ListAddresses operation to the Servant API definition.
• Add PutWallet operation to the Servant API definition.
• Add PutWalletPassphrase operation to the Servant API definition.

[KtorZ]

Arg! We agreed on the scope during the last meeting:

• getWallet
• listWallet
• postWallet
• postTransaction
• listTransaction

I've put the ticket in QA already; let's get these merged, but we can move on to something else now 👍

[KtorZ]

I think I'd even push it further and make it all "automagic" via a Typeable instance 🤔 ?

[KtorZ]

The idea was to have groups matching the groups from the API specification 😅
So I'd suggest to have:

type Wallets =
    DeleteWallet
    :<|> GetWallet
    :<|> ListWallets
    :<|> PostWallet
    :<|> PutWallet
    :<|> PutWalletPassphrase

type Addresses =
    listAddresses

[jonathanknowles]

Interesting! I was under the impression that the listAddresses operation was within the wallets group:

  /wallets/{walletId}/addresses:                                                                                                                                                      
    get:                                                                                                                                                                              
      operationId: listAddresses      

Should we update the specification to move it to another group?

[KtorZ]

Nope, the spec is correct here. Endpoints are grouped by tags, cf:

https://github.com/input-output-hk/cardano-wallet/blob/master/specifications/api/swagger.yaml#L896

https://github.com/input-output-hk/cardano-wallet/blob/master/specifications/api/swagger.yaml#L844

So, even if, conceptually, the Address resource is a sub-resource of the Wallet resource, operations related to addresses are in a separate group to be easier to identify (same apply for transactions).

[jonathanknowles]

Okay! I've moved ListAddresses to a separate group.

[KtorZ]

That, we want in our Primitive.Model the same way we have other metadata about various core types.

[jonathanknowles]

Done.

[KtorZ]

I'd drop the pointfree here and use a do-notation to makes the reading a bit clearer.

[jonathanknowles]

Fair enough :)

[KtorZ]

I really like we have golden tests for all types. It helps controlling the shape of the JSON objects right away 👌

[jonathanknowles]

👍

[KtorZ]

That's the kind of name I imagine @piotr-iohk would use on a wallet yeah 😶

[jonathanknowles]

Buried deep within this name is the character "䖜", which apparently is "the fighting sound made by two tigers" (https://en.wiktionary.org/wiki/%E4%96%9C)

[jonathanknowles]

Arg! We agreed on the scope during the last meeting:
* getWallet
* listWallet
* postWallet
* postTransaction
* listTransaction

I had interpreted the scope to be everything within issue #53, which states:

Extend the API to support all endpoints.

But now I notice that it also states (later on):

The scope of this ticket has been reduced down to the endpoints discussed during the last iteration
planning meeting

In which case I now understand your comment. :)

In that case, perhaps we should update the the scope of issue #53, so that it no longer states "all endpoints"?

[KtorZ]

@jonathanknowles Yes, let's update the scope. I mean, no big deal, we'll to get them done at some point. But I'd integrate endpoints incrementally and start implementing handlers and CLI commands for those we already have, so that we are able to deliver something already.

[jonathanknowles]

start implementing handlers and CLI commands for those we already have, so that we are able to deliver something already.

Okay!

[jonathanknowles]

Yes, let's update the scope.

Done.

[KtorZ]

LGTM.

I've rebased and squashed on top of latest master 👍