EkuboProtocol / EkuboProtocol/wallet

Remove the Windows SQLCipher stack-overflow workarounds once libsqlite3-sys vendors SQLCipher >= 4.16

Open
#113 0 comments 0 reactions 0 assignees View on GitHub
dependencies rust
Dominant language
Rust
Stars
4
Forks
2
Avg merge
13h 29m
Merged PRs (30d)
33

Description

## Summary

Both Windows workarounds in `PolicyStore` exist for a SQLCipher bug that is now
fixed upstream. We cannot pick the fix up yet: it reaches us only through the
SQLCipher amalgamation vendored inside `libsqlite3-sys`, and the newest
published crate still vendors a version that predates the fix.

This issue records the finding, the exact unblock condition, and the complete
scope of the eventual change so it can be done in one pass when the crate
catches up.

## The bug we worked around

SQLCipher's log sink allocated through `sqlite3_malloc`, which
`cipher_memory_security` wraps with `VirtualLock`. On Windows, when
`VirtualLock` failed under the process working-set quota, SQLCipher logged the
failure at WARN level — and that log allocated again, failing again, recursing
until the thread stack was exhausted.

We shipped two independent mitigations for it:

1. **`PRAGMA cipher_log_level = NONE`**, issued before any allocation-heavy
work in `PolicyStore::open` (`crates/ekubo-wallet-core/src/policy_store.rs`,
~line 296). Making the logger return before it allocates breaks the cycle
while memory locking stays enabled.
2. **Statement-by-statement schema creation.** `create_current_schema` and
`run_transaction` (same file, ~lines 1661 and 2144) deliberately issue one
statement per `execute_batch` call, because preparing the equivalent
multi-statement string against the bundled SQLCipher overflows the stack on
Windows MSVC while the identical statements executed individually succeed.

History: `ecd9ba1`, `0200413`, `2d795f7`.

## Upstream status — fixed

| Release | Date | Relevance |
| --- | --- | --- |
| SQLCipher 4.16.0 | 2026-05-12 | "Removes redundant logging of `mlock` and `VirtualLock` failures at WARN level" — this alone removes the recursion trigger |
| SQLCipher 4.18.0 | 2026-08-18 | "Avoids allocating memory on Windows during log writes which could cause a crash on Windows under non-default log settings with `PRAGMA cipher_memory_security = ON`" — the definitive fix |

Our upstream report, https://github.com/sqlcipher/sqlcipher/pull/602, was closed
unmerged on 2026-08-18: the maintainer confirmed the diagnosis but preferred
eliminating dynamic allocation in the log path over our thread-local reentrancy
guard. The fork it was raised from has been deleted; the PR discussion remains
readable on `sqlcipher/sqlcipher`.

## Why we cannot act yet

We consume SQLCipher only as the amalgamation vendored in `libsqlite3-sys`, via
the `bundled-sqlcipher-vendored-openssl` feature.

- `Cargo.lock` already pins `rusqlite` 0.40.2 and `libsqlite3-sys` 0.38.2 — the
newest published versions of both. **There is no version bump available.**
- `libsqlite3-sys` 0.38.2 (published 2026-08-08) vendors **SQLCipher 4.14.0**
(SQLite 3.51.3). Verified against a fresh crates.io download rather than a
local registry cache:

```sh
curl -sL https://static.crates.io/crates/libsqlite3-sys/libsqlite3-sys-0.38.2.crate | tar xz
grep -n '#define CIPHER_VERSION_NUMBER' libsqlite3-sys-0.38.2/sqlcipher/sqlite3.c
# 109519:#define CIPHER_VERSION_NUMBER 4.14.0
```

- rusqlite `master` bumped the vendored amalgamation to 4.17.0 in
https://github.com/rusqlite/rusqlite/pull/1870 (merged 2026-07-14), which
landed after 0.38.2 was cut and is therefore unreleased.

So the next `libsqlite3-sys` release should carry ≥ 4.17.0 and clear this on its
own. No fork, patch, or code change on our side can bring the fix forward today.

## Unblock condition

On each new `libsqlite3-sys` release, re-run the `grep` above against the new
version. When `CIPHER_VERSION_NUMBER` is **≥ 4.16.0**, this is ready to fix.

## Scope of the fix

In `crates/ekubo-wallet-core/src/policy_store.rs`:

- [ ] Bump the `rusqlite` requirement in `crates/ekubo-wallet-core/Cargo.toml`
to the release carrying the updated amalgamation, and confirm the vendored
version in the new `Cargo.lock`.
- [ ] Remove the `cipher_log_level = NONE` pragma from `PolicyStore::open`.
Restoring default logging is the point — it is diagnostic signal we
currently suppress on every platform to work around one platform's bug.
- [ ] Reconsider `create_current_schema` / `run_transaction`. Note this one is
a judgement call, not an automatic revert: one statement per
`execute_batch` is also simply clearer, and it gives per-statement error
context (`"schema statement failed"`) that a single batch string cannot.
Per CLAUDE.md, judge it on maintainability rather than line count — it is
defensible to keep the loop and delete only the Windows rationale from its
doc comment.
- [ ] Fix the two dangling references to `docs/windows-sqlcipher-overflow.md`
(~lines 296 and 1663). **That file does not exist** — it was removed in
`2d795f7` while the comments citing it stayed. Whoever picks this up
should either restore a short doc or, more likely, inline the remaining
rationale and drop the citation.

## Explicitly out of scope

The 64 MiB `RUST_MIN_STACK` in `.cargo/config.toml` and the `opt-level = 1`
override on `[profile.dev]` in `Cargo.toml` are **not** related to this bug.
They exist for unoptimized MSVC frames in the deeply generic `alloy`, `rmcp`,
and `schemars` call chains overflowing Windows test-thread stacks. Leave both
alone; a SQLCipher bump does not justify touching them.

## Do not work around this with a git pin

A `[patch.crates-io]` pin at rusqlite `master` would fetch 4.17.0 early, but it
swaps a registry dependency for a git one. That changes what the OSV gate
scans (`scripts/generate-osv-lockfiles.py` and the `--lockfile` arguments in
CLAUDE.md's gate) and what `contrib/generate-third-party-licenses.py` reads for
`THIRD_PARTY_LICENSES.md`. That is a supply-chain policy change for a bug we
already mitigate correctly in code, so it is not worth it — wait for the
release.

## Verification when done

Beyond the usual full gate, this needs a real Windows run, since neither
mitigation's failure mode reproduces on macOS or Linux: confirm `PolicyStore`
opens, creates its schema, and passes `PRAGMA cipher_integrity_check` on
Windows MSVC with `cipher_memory_security = ON` and default log level.

Contributor guide

No contributing guide indexed for this repository

Research direction

Wait for a libsqlite3-sys release carrying SQLCipher >= 4.16.0, then verify CIPHER_VERSION_NUMBER in the downloaded crate and inspect crates/ekubo-wallet-core/Cargo.toml, Cargo.lock, and crates/ekubo-wallet-core/src/policy_store.rs. Remove or revise the documented workarounds and dangling references as appropriate, then run the full gate and verify PolicyStore schema creation and cipher_integrity_check on Windows MSVC with memory security enabled.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sqlite
Domain
build-system, database
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.