feat: expose wallet API

[jakubkoci]

As a developer, I want to have more granular control of the wallet so that I can decide what flow to run in my business logic according to the state of the wallet. For example, I want to run a different branch (UI screens) of my code if the wallet does not exist without creating the wallet immediately.

I moved the creation of master secret into a create method. It should happen only once. It was unnecessarily called during every open wallet operation.

[jakubkoci]

We need to open the wallet to create a master secret. I think it's a good trade-off to do so in here. The create wallet method should be called only once (or at least fewer times) than the open wallet method. It's a question, if we should close the wallet to avoid user's confusion or assume that the developer will want to call open operation after create and keep it open to save some computer time.

[jakubkoci]

There was a problem when I created a wallet but didn't call open then the shutdown method didn't delete the wallet.

[codecov-commenter]

Codecov Report

Merging #566 (f566b81) into main (4e7b9cf) will increase coverage by 0.16%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##             main     #566      +/-   ##
==========================================
+ Coverage   86.01%   86.17%   +0.16%     
==========================================
  Files         267      267              
  Lines        5840     5846       +6     
  Branches      875      877       +2     
==========================================
+ Hits         5023     5038      +15     
+ Misses        814      805       -9     
  Partials        3        3              

Impacted Files	Coverage Δ
packages/core/src/wallet/IndyWallet.ts	73.38% <91.66%> (+4.96%)	⬆️
packages/core/src/agent/Agent.ts	97.97% <100.00%> (ø)
...ages/core/src/wallet/error/WalletDuplicateError.ts	100.00% <0.00%> (+50.00%)	⬆️
...ges/core/src/wallet/error/WalletInvalidKeyError.ts	100.00% <0.00%> (+50.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e7b9cf...f566b81. Read the comment docs.

[TimoGlastra]

Nice! I do think exposing the wallet api may expose some lower level methods that we don't necessarily want to make public, but I'm good with this change 👍

[TimoGlastra]

The close method also checks whether the wallet handle is present, do you want this as an extra safeguard?

[jakubkoci]

Good point. But this one is tricky.

As I'm thinking about it now, I don't think it's a good idea to just silently do nothing if the developer calls close and walletHandle is not set, because it can lead to situations when the developer accidentally sets agent.wallet.walletHandle = undefined but wallet will be still opened. It should actually throw an error when you try to call close directly and walletHandle is not set. TypeScript doesn't allow me to call only await this.indy.closeWallet(this.walletHandle), it says:

Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
  Type 'undefined' is not assignable to type 'number'.ts(2345)

Therefore, I should validate this.walletHandle and eventually throw an error before calling await this.indy.closeWallet(this.walletHandle).

However, if I'm deleting a wallet that has not been opened yet, I don't want to call close on it because that could throw an error I described above. So, although I don't like this double-check (each time for a different reason), I think it's a more robust solution.

[TimoGlastra]

isCreated feels a bit misleading. I would assume this means whether the wallet was created in the current session, but if you call open it will also be set.

Maybe we can call it something like isProvisioned. According to google the term provision means:

the action of providing or supplying something for use.

Which is exactly what having the wallet config means: it has the needed info to use the wallet

[jakubkoci]

Agree. I also didn't like it so much.

[TimoGlastra]

I think we should throw an error here, or update the config parameter to include that it will only delete if the wallet is provisioned.

Or maybe remove the option? It is mostly used for tests, but now that the wallet api is public we can just use that right?

[jakubkoci]

I kind of like that shutdown method, also deleting a wallet if you need it. But you're right. It does not make sense in the "real" world outside the test. We need to delete the wallet manually, which is probably worth the effort.

[TimoGlastra]

BTW, this is a breaking change now, so make sure to add the BREAKING CHANGE line to the squashed commit message