input-output-hk / cardano-wallet-legacy

Official Wallet Backend & API for Cardano-SL
https://input-output-hk.github.io/cardano-wallet/
MIT License
21 stars 12 forks source link

[BIP44-239] Prepare to extend prefiltering to EOS wallets for ApplyBlock #321

Closed uroboros closed 5 years ago

uroboros commented 5 years ago

https://github.com/input-output-hk/cardano-wallet/issues/239

#

Overview

Comments

This PR adds prefiltering for Eo wallets during ApplyBlock. This means that we can now create new Eo wallets and track them during ApplyBlock.

NOTE: at this stage we cannot yet create new txs for Eo wallets. This will be in a subsequent commit.

QA

We should test that we can create an Eo wallet, and if there are funds transferred to that wallet (using cardano=cli, say) we should see the funds appear in Daedelus.

We should also test Eo tracking in the presence of an HdRnd wallet! (In this case we have to merge prefilter results for the different wallet types, hence it needs special testing.

KtorZ commented 5 years ago

I believe the invariant error was right, and was there to enforce that after prefiltering a singleton block, we get back a singleton result. Note that using a NonEmpty list doesn't quite solve the issue because we are now swallowing the failing pattern match because of:

     nonEmpty <$> prefilterBlocks_ bs foreigns hdRnds 

If, given a non empty list of blocks, prefilterBlocks was to return an empty list (or, as the original invariant indicated, a different number of blocks), then this is now appears as if there were no credentials at all and we would discard this result, without even knowing that something has happened.

I think the real question here is why do we use a version of prefilterBlocks that relies on a list of blocks, where we do know that we are only passing around a single block.

https://github.com/input-output-hk/cardano-wallet/blob/develop/src/Cardano/Wallet/Kernel/BListener.hs#L210

I agree there's no need for this invariant, the fix however isn't going in the right direction. Rather, make prefilterBlock singular and have it take a single block and then later, in

https://github.com/input-output-hk/cardano-wallet/blob/develop/src/Cardano/Wallet/Kernel/BListener.hs#L312

fallback to a forM ormapM of that function -- that's what the current implementation does anyway.

Moreover, this new function of yours prefilterBlocks_ does a fair job at focusing only on the prefiltering part, leaving to the caller the freedom of getting the corresponding credentials (and not doing this in prefilterBlocks itself). It's nice to keep function doing one-and-one thing only.

PS: stylish-haskell isn't happy :/