feat: store signer state in sqlite

[hstove]

This PR adds a new mod at stacks-signer/src/signerdb.rs with functions to interact with a SQLite database for state management.

This is built on top of #4287 because that PR refactors a lot of the signer file structure.

At the moment, this replaces a HashMap that was used for storing block information.

• closes [Nakamoto] stacks-signer needs track processed blocks responses, block requests, and block sign requests #4242

[codecov]

Codecov Report

Attention: Patch coverage is 71.19741% with 89 lines in your changes are missing coverage. Please review.

Project coverage is 66.86%. Comparing base (bf5a833) to head (b4274f3).
Report is 35 commits behind head on next.

Additional details and impacted files

@@             Coverage Diff             @@
##             next    #4348       +/-   ##
===========================================
- Coverage   83.17%   66.86%   -16.32%     
===========================================
  Files         451      452        +1     
  Lines      324497   326005     +1508     
  Branches      318      323        +5     
===========================================
- Hits       269916   217990    -51926     
- Misses      54573   108007    +53434     
  Partials        8        8               

Files	Coverage Δ
libsigner/src/runloop.rs	72.79% <ø> (-0.20%)	⬇️
libstackerdb/src/libstackerdb.rs	82.51% <ø> (-9.80%)	⬇️
stacks-signer/src/client/mod.rs	95.44% <100.00%> (-3.71%)	⬇️
stacks-signer/src/config.rs	86.54% <100.00%> (+2.65%)	⬆️
stacks-signer/src/runloop.rs	81.75% <100.00%> (-9.43%)	⬇️
stackslib/src/net/api/poststackerdbchunk.rs	82.15% <92.00%> (-0.02%)	⬇️
stacks-signer/src/client/stackerdb.rs	90.08% <53.33%> (-1.91%)	⬇️
stacks-signer/src/signer.rs	73.76% <75.29%> (-0.29%)	⬇️
stacks-signer/src/signerdb.rs	63.12% <63.12%> (ø)

... and 270 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

[saralab]

Reviews can progress , hold off merge until casting DKG vote PR is merged

[hstove]

@jcnelson FYI I've rebased off of Jacinta's PR and removed all of the r2d2 stuff. I'll admit that I couldn't figure out maintaining a connection, so this PR just opens a new connection for every read/write. The issue stems from the Signer trait in libsigner requiring the signer_loop to have the Sync trait.

I follow your idea of instantiating a struct inside of a thread, but I haven't found a way of doing that without getting the same compiler error. I would still need some struct to have something like signer_db: Optional<SignerDb>, but that doesn't fix the fact that it breaks the Sync trait. It's not clear if I would need to use something like a Mutex or a channel. I apologize for hitting the boundaries of my current Rust knowledge - any more advice would be helpful!

[jferrant]

Sure, that's a good point. I'd prefer then to also have the SignerDB::in_memory() constructor to make it explicit for users who don't know the ":memory:" special file name. I just learned about it looking at your code. I'll do that unless you object.

Actually I won't since the tests are using temporary database files so the in_memory() constructor would just be dead code.

Could also add some tests for in memory rather than using files :P

[jferrant]

You need to update the construction of the signer file to include the database name. i.e. add db_path = ":memory:". See build_signer_config_tomls

[jferrant]

Whenever I try to run the integration tests I get complaints about too many open files. I thought maybe you would need to reduce the number of signers, but I am surprised that having 10 open files is too much (it runs with 10 signers)

E.g. test that failed for me
BITCOIND_TEST=1 RUST_LOG=debug STACKS_LOG_DEBUG=1 cargo test --package stacks-node --bin stacks-node -- tests::signer::stackerdb_dkg --nocapture --ignored

[netrome]

You need to update the construction of the signer file to include the database name. i.e. add db_path = ":memory:". See build_signer_config_tomls

Ah I see. I will update this. This feels like the type of error that shouldn't pass compilation or unit tests. I might refactor that function in the process if I spot any simple enhancements.

[netrome]

Whenever I try to run the integration tests I get complaints about too many open files. I thought maybe you would need to reduce the number of signers, but I am surprised that having 10 open files is too much (it runs with 10 signers)

E.g. test that failed for me BITCOIND_TEST=1 RUST_LOG=debug STACKS_LOG_DEBUG=1 cargo test --package stacks-node --bin stacks-node -- tests::signer::stackerdb_dkg --nocapture --ignored

Interesting. I'll try running it and see if I encounter the same behavior.

[xoloki]

Whenever I try to run the integration tests I get complaints about too many open files. I thought maybe you would need to reduce the number of signers, but I am surprised that having 10 open files is too much (it runs with 10 signers)

E.g. test that failed for me BITCOIND_TEST=1 RUST_LOG=debug STACKS_LOG_DEBUG=1 cargo test --package stacks-node --bin stacks-node -- tests::signer::stackerdb_dkg --nocapture --ignored

Remember that each signer has multiple connections (event queues, web server, rpc client, etc). If we're adding sqlite to the mix we might be hitting some limits.

What platform are you seeing the errors on? Mac or Linux? Version?

[netrome]

Whenever I try to run the integration tests I get complaints about too many open files. I thought maybe you would need to reduce the number of signers, but I am surprised that having 10 open files is too much (it runs with 10 signers)
E.g. test that failed for me BITCOIND_TEST=1 RUST_LOG=debug STACKS_LOG_DEBUG=1 cargo test --package stacks-node --bin stacks-node -- tests::signer::stackerdb_dkg --nocapture --ignored

Interesting. I'll try running it and see if I encounter the same behavior.

I have tried multiple times now and I'm not managing to reproduce any error so far.

[netrome]

You need to update the construction of the signer file to include the database name. i.e. add db_path = ":memory:". See build_signer_config_tomls

Done ✅

[netrome]

Could also add some tests for in memory rather than using files :P

Sure. I added another test now. It feels a bit outside our domain to do this since we have no specific code path for the in-memory database in our code base, but why not? Let's be defensive.