Labels / Complexity: bug, financial, database, Backend, reliability · Extremely High — 500
Problem
DepositDBConnector.add_vault_balance in quantara/web_app/db/crud/deposit.py is a non-atomic read-modify-write:
vault = self.get_vault(wallet_id, symbol) # 1. read current amount
...
new_amount = Decimal(vault.amount) + Decimal(amount) # 2. add in Python
db.query(Vault).filter_by(id=vault.id).update(amount=str(new_amount)) # 3. write back
db.commit()
Two concurrent requests for the same (wallet_id, symbol) both read the same vault.amount, each compute old + their_amount, and the second write overwrites the first — a lost update. On a money ledger this silently discards one of the deposits.
A related defect is in the data model and the deposit path: create_vault (quantara/web_app/db/crud/deposit.py) always inserts a new Vault row on every /api/vault/deposit, and the Vault table (quantara/web_app/db/models.py) has no unique constraint on (user_id, symbol). get_vault returns .first(), so once more than one row exists for a user+symbol, add_vault_balance updates an arbitrary row while others accumulate stale balances — the total is never the sum the user actually deposited.
The codebase already contains the correct pattern for exactly this situation: add_extra_deposit_to_position in quantara/web_app/db/crud/position.py uses a PostgreSQL insert(...).on_conflict_do_update(...) with cast(amount, Numeric) + cast(amount, Numeric) to atomically upsert. The vault path does not reuse it.
Root cause
vault = self.get_vault(wallet_id, symbol) # ← read outside the write transaction
new_amount = Decimal(vault.amount) + Decimal(amount) # ← lost-update window
db.query(Vault).filter_by(id=vault.id).update(amount=str(new_amount))
Why this is architecturally hard
- The shortcut —
SELECT ... FOR UPDATE — requires a single transaction that spans the read and the write, but get_vault opens its own Session and closes it before returning, so the contributor must restructure the method into one session/transaction rather than bolting a lock onto the current split.
- The deeper fix is an atomic SQL upsert (
INSERT ... ON CONFLICT (user_id, symbol) DO UPDATE SET amount = amount + excluded.amount), which requires adding the missing unique constraint to the Vault model — an Alembic migration and a data-cleanup decision for any existing duplicate rows.
amount is stored as a String (Vault.amount and Position.amount are String), so arithmetic must go through Decimal/NUMERIC casting inside SQL, exactly as add_extra_deposit_to_position already does — the contributor must reconcile string storage with atomic numeric addition.create_vault and add_vault_balance are two divergent write paths (one inserts, one updates-first-row); unifying them into one upsert is the real fix, and the API endpoints in quantara/web_app/api/vault.py must both route through it.
Proposed design
-- one atomic upsert, mirroring ExtraDeposit's pattern
INSERT INTO vault (user_id, symbol, amount)
VALUES (:user_id, :symbol, :amount)
ON CONFLICT (user_id, symbol)
DO UPDATE SET amount = cast(vault.amount AS numeric) + cast(EXCLUDED.amount AS numeric);
The maintainer decision is whether to add a unique constraint on (user_id, symbol) and migrate existing rows, or to key the upsert by an explicit natural key.
Downstream impact
Adding a unique constraint on (user_id, symbol) requires an Alembic migration (quantara/web_app/alembic/) and reconciliation of any existing duplicate Vault rows. The response shape of /api/vault/deposit and /api/vault/add_balance need not change.
Acceptance criteria
Database
Service
Tests
Out of scope
Do not migrate Position.amount/ExtraDeposit.amount storage in this issue; only fix the Vault balance path.
Getting started
Files in scope: quantara/web_app/db/crud/deposit.py, quantara/web_app/db/models.py, quantara/web_app/api/vault.py, plus an Alembic migration. Verify with:
cd quantara && poetry run pytest web_app/tests -k vault
Good first files to read: quantara/web_app/db/crud/deposit.py, quantara/web_app/db/crud/position.py (the on_conflict_do_update pattern), quantara/web_app/db/models.py.
Labels / Complexity: bug, financial, database, Backend, reliability · Extremely High — 500
Problem
DepositDBConnector.add_vault_balanceinquantara/web_app/db/crud/deposit.pyis a non-atomic read-modify-write:Two concurrent requests for the same
(wallet_id, symbol)both read the samevault.amount, each computeold + their_amount, and the second write overwrites the first — a lost update. On a money ledger this silently discards one of the deposits.A related defect is in the data model and the deposit path:
create_vault(quantara/web_app/db/crud/deposit.py) always inserts a newVaultrow on every/api/vault/deposit, and theVaulttable (quantara/web_app/db/models.py) has no unique constraint on(user_id, symbol).get_vaultreturns.first(), so once more than one row exists for a user+symbol,add_vault_balanceupdates an arbitrary row while others accumulate stale balances — the total is never the sum the user actually deposited.The codebase already contains the correct pattern for exactly this situation:
add_extra_deposit_to_positioninquantara/web_app/db/crud/position.pyuses a PostgreSQLinsert(...).on_conflict_do_update(...)withcast(amount, Numeric) + cast(amount, Numeric)to atomically upsert. The vault path does not reuse it.Root cause
Why this is architecturally hard
SELECT ... FOR UPDATE— requires a single transaction that spans the read and the write, butget_vaultopens its ownSessionand closes it before returning, so the contributor must restructure the method into one session/transaction rather than bolting a lock onto the current split.INSERT ... ON CONFLICT (user_id, symbol) DO UPDATE SET amount = amount + excluded.amount), which requires adding the missing unique constraint to theVaultmodel — an Alembic migration and a data-cleanup decision for any existing duplicate rows.amountis stored as aString(Vault.amountandPosition.amountareString), so arithmetic must go throughDecimal/NUMERICcasting inside SQL, exactly asadd_extra_deposit_to_positionalready does — the contributor must reconcile string storage with atomic numeric addition.create_vaultandadd_vault_balanceare two divergent write paths (one inserts, one updates-first-row); unifying them into one upsert is the real fix, and the API endpoints inquantara/web_app/api/vault.pymust both route through it.Proposed design
The maintainer decision is whether to add a unique constraint on
(user_id, symbol)and migrate existing rows, or to key the upsert by an explicit natural key.Downstream impact
Adding a unique constraint on
(user_id, symbol)requires an Alembic migration (quantara/web_app/alembic/) and reconciliation of any existing duplicateVaultrows. The response shape of/api/vault/depositand/api/vault/add_balanceneed not change.Acceptance criteria
Database
(user_id, symbol)forVault.add_vault_balance(or its replacement) performs the increment atomically in SQL.Service
(wallet_id, symbol)are not lost: N concurrent deposits of X produce a final balance of N*X.create_vaultandadd_vault_balanceconverge on a single upsert path.Tests
cd quantara && poetry run pytest web_app/tests -k vault.Out of scope
Do not migrate
Position.amount/ExtraDeposit.amountstorage in this issue; only fix theVaultbalance path.Getting started
Files in scope:
quantara/web_app/db/crud/deposit.py,quantara/web_app/db/models.py,quantara/web_app/api/vault.py, plus an Alembic migration. Verify with:Good first files to read:
quantara/web_app/db/crud/deposit.py,quantara/web_app/db/crud/position.py(theon_conflict_do_updatepattern),quantara/web_app/db/models.py.