[CBR-533] add a new endpoint to calculate the walletid for a given mnemonic #4237
Conversation
|
@cleverca22 Please, add CHANGELOG.md entry under Unreleased Features
Fixes
|
| @@ -3,13 +3,15 @@ | |||
| -- Daedalus client, and aren't useful for wallets, exchanges, and other users. | |||
| module Cardano.Wallet.API.Internal where | |||
|
|
|||
| import Prelude | |||
There was a problem hiding this comment.
Indentation to match import lines below? Also isn't this import already implicit?
There was a problem hiding this comment.
Looks good overall, a few remarks though:
-
When
read_balanceis set, we do a full UTxO scan, can take quite some time, but I suppose it's fine in the context of this endpoint. -
Using
Coinfor the balance is somewhat incorrect, especially when summing them asfoldl' unsafeAddCoin. This is basically a bomb at runtime, yet unlikely to explode. -
There's no tests. Even for internal features, it'd be nice to provide evidences of QA.
-
CHANGELOG needs to be updated
|
@KtorZ without some chain state to fire up the wallet, i don't know how to test this endpoint fully in an automated manner all other review comments should be good |
|
@cleverca22 For the testing, have a look at some integration test scenarios (e.g. https://github.com/input-output-hk/cardano-sl/blob/develop/wallet/test/integration/Test/Integration/Scenario/Wallets.hs#L579-L601) You should be able to do something like: scenario "Can query wallet id and balance using only its mnemonic" $
fixture <- setup $ defaultSetup
& initialCoins .~ [14]
successfulRequest $ Client.deleteWallet
$- (fixture ^. wallet . walletId)
response <- request $ Client.calculateMnemonic
$- (fixture ^. backupPhrase)
$- (Just True)
verify response
[ expectFieldEqual walletId (fixture ^. wallet . walletId)
, expectFieldEqual maybeBalance (Just 14)
]+/- the compilation errors and testing with various scenarios (with the query parameter, without, testing on a wallet with no funds etc ..) |
|
bors r+ |
|
👎 Rejected by too few approved reviews |
|
bors r+ |
4237: [CBR-533] add a new endpoint to calculate the walletid for a given mnemonic r=disassembler a=cleverca22 ## Description <!--- A brief description of this PR and the problem is trying to solve --> ## Linked issue <!--- Put here the relevant issue from YouTrack --> Co-authored-by: Michael Bishop <[email protected]>
Build succeeded |
Description
Linked issue
Type of change
Developer checklist
Testing checklist
QA Steps
Screenshots (if available)
How to merge
Send the message
bors r+to merge this PR. For more information, seedocs/how-to/bors.md.