search

cardano-foundation / cardano-wallet

HTTP server & command-line for managing UTxOs and HD wallets in Cardano.
Apache License 2.0
758 stars 213 forks source link

[ADP-3335] Change `Cardano.Wallet.Deposit.IO.DB` to depend on `sqlite-simple` #4526

Closed HeinrichApfelmus closed 2 months ago

HeinrichApfelmus commented 3 months ago

This pull request changes Cardano.Wallet.Deposit.IO.DB to use sqlite-simple instead of persistent.

The reason for this change is that withDBHandleInMemory throws an exception when called twice in some circumstances.

It appears that this bug is located in a dependency. Rumor has it that there might be a bug in persistent where SQL statements are not finalized. It seems likely that the SQL statement which is not finalized is the BEGIN TRANSACTION statement that persistent reuses in each call to runSqlConn. I did not investigate this further, as I want to move away from persistent anyway, and using sqlite-simple fixes the issue.

Comment

This pull request prepares the implementation of a mock environment for the Deposit Wallet. In turn, this enables execution of user scenarios.

Issue Number

ADP-3335

HeinrichApfelmus commented 2 months ago

What about adding now tests like: (a) open-close-open-close... OK (b) open-open -> error (c) close-close -> error just to be sure this basic db operations work as expected? Make sense to do it now?

Since only with…-style functions are exported, only case (a) is available for testing. If anything, I would add concurrency to the tests. However, I would like to postpone testing these functions for now.