[CO-334] Implement remaining V0 endpoints to V1 #3197

parsonsmatt · 2018-07-05T17:46:22Z

Description

This topic branch tracks the implementation of the last few V0 endpoints in V1, under a special daedalus namespace.

Linked issue

https://iohk.myjetbrains.com/youtrack/issue/CO-334

Type of change

🐞 Bug fix (non-breaking change which fixes an issue)
🛠 New feature (non-breaking change which adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)
🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
🔨 New or improved tests for existing code
⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

I have read the style guide document, and my code follows the code style of this project.
If my code deals with exceptions, it follows the guidelines.
I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.

Testing checklist

I have added tests to cover my changes.
All new and existing tests passed.

QA Steps

Screenshots (if available)

KtorZ · 2018-07-06T09:43:34Z

wallet-new/src/Cardano/Wallet/API/V1/Internal.hs

+-- The redemption datatypes differ only by the backup phrase. Otherwise the
+-- wallet ID and seed are both present. Potentially we can unify these into the
+-- same endpoint. Seems like a query parameter like `paper_vend_passphrase=...`
+-- would do nicely.


I agree. I see no point for two endpoints here. I'd rather unify them and have a switch kind of parameter to target the right behavior. Perhaps it's not even needed and we can, at parsing-time, identify which structure is being passed and pick the right behavior after that.

Note that, I am not sure this redeem should be in the internal part of the API. We probably want to have them in the public-facing part?

KtorZ · 2018-07-06T09:43:42Z

wallet-new/src/Cardano/Wallet/API/V1/Internal.hs

+--     -> m CTx
+-- data CPaperVendWalletRedeem = CPaperVendWalletRedeem
+--     { pvWalletId     :: !CAccountId
+--     , pvSeed         :: !Text -- TODO: newtype!


Yes please!

KtorZ · 2018-07-06T09:46:15Z

Note (wasn't maybe really clear from the requirements). What we use to call development API and this new internal API are the same thing.
"Development" just conveys a wrong idea of what it is. Therefore, we have to unify this with:

https://github.com/input-output-hk/cardano-sl/blob/develop/wallet-new/src/Cardano/Wallet/API/Development.hs

adinapoli-iohk · 2018-07-06T10:19:07Z

What we use to call development API and this new internal API are the same thing.

I agree in principle. The way I see it, there are two subtle faces of the same coin, as the original spirit of that Development endpoints was to capture that there were endpoints what frontend folks were using only for the sake of testing or troubleshooting the wallet, the endpoint to dump the full WalletStorage being the perfect example.

I think that in principle though unifying everything into only one category make sense, although I'm not sure how this would work in practice: for example (if I recall correctly) we do not even expose certain dev endpoints if the --wallet-debug option is not set.

KtorZ · 2018-07-06T11:03:37Z

@adinapoli-iohk Yes, and this has still the same meaning / behavior although we will also expose the documentation for these endpoints somewhere. These update apply and update postpone things aren't especially used in testing but they fall in the category of frontend-specific endpoints that are irrelevant for exchanges.

KtorZ · 2018-07-25T15:25:39Z

Finalized 'Internal' API

Remove the old 'Development' modules and group them together with the new 'Internal'
Remove unused endpoints ('dump-wallet-state' and 'fail')
Review module organization and put 'Internal' one level up, at the same level than V0 and V1, rather than being under V1
Remove the 'Development' server and always expose 'Internal' endpoints. Note that the 'reset-wallet-state' will only work in debug mode (flag '--debug') with the CLI

KtorZ · 2018-07-25T15:28:58Z

Note that I've also re-organized commits to get a nice log-graph and easily follow the development tasks here.

* e37c5d7 - (HEAD -> Squad1/CO-334/implement-v0-endpoints, origin/Squad1/CO-334/implement-v0-endpoints) [CO-334] Finalize 'Internal' API (3 minutes ago) <KtorZ>
*   5ff10f7 - Merge branch 'CO-335' into Squad1/CO-334/implement-v0-endpoints (3 hours ago) <KtorZ>
|\  
| * c54ed1d - (CO-335) [CO-335] Remove dead code (3 hours ago) <parsonsmatt>
| * 09ff641 - [CO-335] Implement legacy functionality (3 hours ago) <parsonsmatt>
|/  
*   7bfe0c5 - Merge branch 'CO-336' into Squad1/CO-334/implement-v0-endpoints (3 hours ago) <KtorZ>
|\  
| * 8a6c6d7 - (CO-336) [CO-336] Fix tests (3 hours ago) <parsonsmatt>
| * b8cdd38 - [CO-336] Remove mnemonic from the wallet's API types; rely on semantic newtypes instead. (3 hours ago) <parsonsmatt>
| * 4ab6b5c - [CO-336] Replace Mnemonic with a newtype (3 hours ago) <parsonsmatt>
| * 7581a31 - [CO-336] Remove redundant pragma (3 hours ago) <parsonsmatt>
| * f28c2ab - [CO-336] Newtype the redemption mnemonic (3 hours ago) <parsonsmatt>
| * ee82474 - [CO-336] Implement type error for conversions (3 hours ago) <parsonsmatt>
| * d8086ee - [CO-336] Prevent zero-coin payment test failure (3 hours ago) <parsonsmatt>
| * 51a7b77d - [CO-336] Fix bad Swagger spec (3 hours ago) <parsonsmatt>
| * ad42200 - [CO-336] Move redemption endpoints under Accounts (3 hours ago) <parsonsmatt>
| * a33dd44 - [CO-336] Finish implementing handler (3 hours ago) <parsonsmatt>
| * f0cec96 - [CO-336] Sketch redemption endpoints (3 hours ago) <parsonsmatt>
|/  
* ec54841 - [CO-334] Build success (3 hours ago) <parsonsmatt>
* 0cbb110 - [CO-334] Rename to internal (3 hours ago) <parsonsmatt>
* b721404 - [CO-334] Draft 'Seed' and 'WalletRedemption' types (3 hours ago) <parsonsmatt>
* 72727ef - [CO-334] Add comments with request/response types (3 hours ago) <parsonsmatt>
* ba2f30b - [CO-334] Stub API endpoints (3 hours ago) <parsonsmatt>
*   7da7cd8 - (origin/develop, origin/HEAD, develop) Merge pull request #3287 from input-output-hk/erikd/CDEC-461 (14 hours ago) <Erik de Castro Lopo>

parsonsmatt · 2018-07-25T15:37:57Z

Nice job with the commit history @KtorZ :) I'll get this merged once CI passes.

Previously, we were going to keep the redemption endpoints in an internal section. We decided that they fit naturally into the accounts endpoints, so we moved them there instead.

…c newtypes instead.

KtorZ · 2018-07-26T09:06:02Z

CI was green, but something was merged to develop conflicting in generate-swagger-file/Main.hs ... Rebased, fixed, aaaaaaaaand going for another 2-hour trip :)

- Remove the old 'Development' modules and group them together with the new 'Internal' - Remove unused endpoints ('dump-wallet-state' and 'fail') - Review module organization and put 'Internal' one level up, at the same level than V0 and V1, rather than being under V1 - Remove the 'Development' server and always expose 'Internal' endpoints. Note that the 'reset-wallet-state' will only work in debug mode (flag '--debug') with the CLI

parsonsmatt · 2018-07-26T19:33:18Z

Everything is green, no conflicts, GOTTA GO FAST

…-v0-endpoints [CO-334] Implement remaining V0 endpoints to V1

…hk/Squad1/CO-334/implement-v0-endpoints [CO-334] Implement remaining V0 endpoints to V1

parsonsmatt requested review from adinapoli-iohk and denisshevchenko as code owners July 5, 2018 17:46

parsonsmatt requested review from KtorZ and removed request for denisshevchenko and adinapoli-iohk July 5, 2018 21:33

KtorZ suggested changes Jul 6, 2018

View reviewed changes

KtorZ self-assigned this Jul 6, 2018

parsonsmatt requested review from akegalj and erikd as code owners July 19, 2018 22:29

KtorZ force-pushed the Squad1/CO-334/implement-v0-endpoints branch from 02a9a80 to 5ff10f7 Compare July 25, 2018 12:44

KtorZ approved these changes Jul 25, 2018

View reviewed changes

KtorZ force-pushed the Squad1/CO-334/implement-v0-endpoints branch from e37c5d7 to 86b4f5d Compare July 25, 2018 16:08

parsonsmatt added 12 commits July 26, 2018 11:04

[CO-334] Stub API endpoints

cc40e11

[CO-334] Add comments with request/response types

3d7881c

[CO-334] Draft 'Seed' and 'WalletRedemption' types

bc174b9

[CO-336] Sketch redemption endpoints

3c96961

[CO-334] Rename to internal

248d2d9

[CO-336] Finish implementing handler

d24b22d

[CO-334] Build success

a0254b6

[CO-336] Move redemption endpoints under Accounts

cd0e5d7

Previously, we were going to keep the redemption endpoints in an internal section. We decided that they fit naturally into the accounts endpoints, so we moved them there instead.

[CO-336] Fix bad Swagger spec

869e481

[CO-336] Prevent zero-coin payment test failure

afd21db

[CO-336] Implement type error for conversions

6218fdb

[CO-336] Newtype the redemption mnemonic

f1df250

parsonsmatt and others added 8 commits July 26, 2018 11:04

[CO-336] Remove redundant pragma

2d9c720

[CO-336] Replace Mnemonic with a newtype

0c7e82a

[CO-336] Remove mnemonic from the wallet's API types; rely on semanti…

9afceb8

…c newtypes instead.

[CO-336] Fix tests

0b61b19

[CO-335] Implement legacy functionality

47fa08c

Merge branch 'CO-336' into Squad1/CO-334/implement-v0-endpoints

dd115c3

[CO-335] Remove dead code

4ff8025

Merge branch 'CO-335' into Squad1/CO-334/implement-v0-endpoints

39e7d2a

KtorZ force-pushed the Squad1/CO-334/implement-v0-endpoints branch from 86b4f5d to 03fb291 Compare July 26, 2018 09:05

KtorZ force-pushed the Squad1/CO-334/implement-v0-endpoints branch from 03fb291 to 538f44b Compare July 26, 2018 12:12

parsonsmatt merged commit 1c42e4c into develop Jul 26, 2018

KtorZ deleted the Squad1/CO-334/implement-v0-endpoints branch August 16, 2018 14:53

KtorZ pushed a commit that referenced this pull request Nov 9, 2018

Merge pull request #3197 from input-output-hk/Squad1/CO-334/implement…

273b0fc

…-v0-endpoints [CO-334] Implement remaining V0 endpoints to V1

KtorZ pushed a commit to input-output-hk/cardano-wallet-legacy that referenced this pull request Nov 9, 2018

Merge pull request input-output-hk/cardano-sl#3197 from input-output-…

0894e6d

…hk/Squad1/CO-334/implement-v0-endpoints [CO-334] Implement remaining V0 endpoints to V1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CO-334] Implement remaining V0 endpoints to V1 #3197

[CO-334] Implement remaining V0 endpoints to V1 #3197

parsonsmatt commented Jul 5, 2018

KtorZ Jul 6, 2018

KtorZ Jul 6, 2018

KtorZ Jul 6, 2018

KtorZ commented Jul 6, 2018

adinapoli-iohk commented Jul 6, 2018

KtorZ commented Jul 6, 2018

KtorZ commented Jul 25, 2018 •

edited

Loading

KtorZ commented Jul 25, 2018

parsonsmatt commented Jul 25, 2018

KtorZ commented Jul 26, 2018

parsonsmatt commented Jul 26, 2018

[CO-334] Implement remaining V0 endpoints to V1 #3197

[CO-334] Implement remaining V0 endpoints to V1 #3197

Conversation

parsonsmatt commented Jul 5, 2018

Description

Linked issue

Type of change

Developer checklist

Testing checklist

QA Steps

Screenshots (if available)

KtorZ Jul 6, 2018

Choose a reason for hiding this comment

KtorZ Jul 6, 2018

Choose a reason for hiding this comment

KtorZ Jul 6, 2018

Choose a reason for hiding this comment

KtorZ commented Jul 6, 2018

adinapoli-iohk commented Jul 6, 2018

KtorZ commented Jul 6, 2018

KtorZ commented Jul 25, 2018 • edited Loading

KtorZ commented Jul 25, 2018

parsonsmatt commented Jul 25, 2018

KtorZ commented Jul 26, 2018

parsonsmatt commented Jul 26, 2018

KtorZ commented Jul 25, 2018 •

edited

Loading