fix: enforce snapshot-consistent precision checks for manual ledger
Co-authored-by: genichin <4282227+genichin@users.noreply.github.com>
This commit is contained in:
co-authored by
genichin
parent
aa40c7fdff
commit
b74284e626
@@ -7,21 +7,39 @@ import json
|
||||
import logging
|
||||
import uuid
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime, timezone
|
||||
from decimal import Decimal, InvalidOperation, ROUND_HALF_UP
|
||||
from datetime import UTC, datetime
|
||||
from decimal import ROUND_HALF_UP, Decimal, InvalidOperation
|
||||
from typing import Any
|
||||
|
||||
import psycopg
|
||||
from psycopg import sql
|
||||
|
||||
from app.domain.errors import DaemonInactiveError, DaemonInUseError, DaemonNotFoundError, StrategyInactiveError, StrategyNotFoundError
|
||||
from app.domain.models import Asset, AuthToken, BrokerCredential, Daemon, LedgerEntry, Portfolio, PortfolioComment, PortfolioTag, StrategySet, Tag, TokenPrincipal, User
|
||||
from app.domain.errors import (
|
||||
DaemonInactiveError,
|
||||
DaemonInUseError,
|
||||
DaemonNotFoundError,
|
||||
StrategyInactiveError,
|
||||
StrategyNotFoundError,
|
||||
)
|
||||
from app.domain.models import (
|
||||
Asset,
|
||||
AuthToken,
|
||||
BrokerCredential,
|
||||
Daemon,
|
||||
LedgerEntry,
|
||||
Portfolio,
|
||||
PortfolioComment,
|
||||
PortfolioTag,
|
||||
StrategySet,
|
||||
Tag,
|
||||
TokenPrincipal,
|
||||
User,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
_LEDGER_NUMERIC_SCALE = 8
|
||||
_LEDGER_MAX_INTEGER_DIGITS = 12
|
||||
_LEDGER_QUANT = Decimal("0.00000001")
|
||||
_MAX_SAFE_DOUBLE_FOR_8DP = Decimal("45035996")
|
||||
|
||||
_SELECT_USER = """
|
||||
SELECT
|
||||
@@ -1523,7 +1541,7 @@ class PostgresPortfolioRepository:
|
||||
note: str = "",
|
||||
) -> None:
|
||||
self.ensure_schema()
|
||||
trade_time = datetime.now(timezone.utc)
|
||||
trade_time = datetime.now(UTC)
|
||||
conn = self._connect()
|
||||
with conn.transaction(), conn.cursor() as cur:
|
||||
cur.execute(
|
||||
@@ -1602,9 +1620,9 @@ class PostgresPortfolioRepository:
|
||||
self,
|
||||
portfolio_id: str,
|
||||
*,
|
||||
date: "datetime | None" = None,
|
||||
from_: "datetime | None" = None,
|
||||
to_: "datetime | None" = None,
|
||||
date: datetime | None = None,
|
||||
from_: datetime | None = None,
|
||||
to_: datetime | None = None,
|
||||
) -> list[LedgerEntry]:
|
||||
"""Return ledger entries for portfolio_id filtered by date range (REQ-039).
|
||||
|
||||
@@ -2051,9 +2069,18 @@ class PostgresPortfolioRepository:
|
||||
return decimal_value
|
||||
|
||||
@staticmethod
|
||||
def _ensure_safe_double_snapshot_precision(value: Decimal, *, field_name: str) -> None:
|
||||
if value.copy_abs() > _MAX_SAFE_DOUBLE_FOR_8DP:
|
||||
raise ValueError(f"{field_name} exceeds double snapshot precision at 8 decimal places")
|
||||
def _to_snapshot_float_and_ledger_decimal(value: Decimal, *, field_name: str) -> tuple[float, Decimal]:
|
||||
if value.is_nan() or value.is_infinite():
|
||||
raise ValueError(f"{field_name} must be a finite decimal")
|
||||
try:
|
||||
as_float = float(value)
|
||||
except (OverflowError, ValueError):
|
||||
raise ValueError(f"{field_name} cannot be represented in DOUBLE PRECISION snapshot") from None
|
||||
expected_ledger = value.quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
||||
actual_ledger = Decimal(str(as_float)).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
||||
if actual_ledger != expected_ledger:
|
||||
raise ValueError(f"{field_name} cannot be represented in DOUBLE PRECISION snapshot")
|
||||
return as_float, actual_ledger
|
||||
|
||||
def record_manual_ledger(
|
||||
self,
|
||||
@@ -2063,8 +2090,8 @@ class PostgresPortfolioRepository:
|
||||
amount: Decimal | None = None,
|
||||
quantity: Decimal | None = None,
|
||||
price: Decimal | None = None,
|
||||
fee: Decimal = Decimal("0"),
|
||||
tax: Decimal = Decimal("0"),
|
||||
fee: Decimal = Decimal(0),
|
||||
tax: Decimal = Decimal(0),
|
||||
executed_at: datetime | None = None,
|
||||
target_ledger_entry_id: int | None = None,
|
||||
note: str = "",
|
||||
@@ -2093,15 +2120,9 @@ class PostgresPortfolioRepository:
|
||||
fee_f = self._coerce_ledger_decimal(fee or 0, field_name="fee", allow_zero=True)
|
||||
tax_f = self._coerce_ledger_decimal(tax or 0, field_name="tax", allow_zero=True)
|
||||
amount_f = self._coerce_ledger_decimal(amount or 0, field_name="amount", allow_zero=True)
|
||||
self._ensure_safe_double_snapshot_precision(amount_f, field_name="amount")
|
||||
self._ensure_safe_double_snapshot_precision(ledger_price, field_name="price")
|
||||
self._ensure_safe_double_snapshot_precision(ledger_quantity, field_name="quantity")
|
||||
self._ensure_safe_double_snapshot_precision(fee_f, field_name="fee")
|
||||
self._ensure_safe_double_snapshot_precision(tax_f, field_name="tax")
|
||||
ledger_executed_at = executed_at or datetime.now(timezone.utc)
|
||||
ledger_executed_at = executed_at or datetime.now(UTC)
|
||||
if ledger_executed_at.tzinfo is None:
|
||||
ledger_executed_at = ledger_executed_at.replace(tzinfo=timezone.utc)
|
||||
reverse_target: LedgerEntry | None = None
|
||||
ledger_executed_at = ledger_executed_at.replace(tzinfo=UTC)
|
||||
|
||||
if normalized == "INVEST":
|
||||
buffer += amount_f
|
||||
@@ -2118,12 +2139,7 @@ class PostgresPortfolioRepository:
|
||||
cash -= amount_f
|
||||
buffer += amount_f
|
||||
ledger_price = amount_f
|
||||
elif normalized == "WITHDRAW":
|
||||
if buffer < amount_f:
|
||||
raise ValueError("INSUFFICIENT_BUFFER")
|
||||
buffer -= amount_f
|
||||
ledger_price = amount_f
|
||||
elif normalized == "PROFIT":
|
||||
elif normalized == "WITHDRAW" or normalized == "PROFIT":
|
||||
if buffer < amount_f:
|
||||
raise ValueError("INSUFFICIENT_BUFFER")
|
||||
buffer -= amount_f
|
||||
@@ -2234,7 +2250,25 @@ class PostgresPortfolioRepository:
|
||||
if buffer < 0:
|
||||
raise ValueError("INSUFFICIENT_BUFFER")
|
||||
|
||||
total_value = (cash + qty * asset_price + buffer).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
||||
snapshot_cash_float, snapshot_cash_ledger = self._to_snapshot_float_and_ledger_decimal(
|
||||
cash,
|
||||
field_name="cash_balance_after",
|
||||
)
|
||||
snapshot_qty_float, snapshot_qty_ledger = self._to_snapshot_float_and_ledger_decimal(
|
||||
qty,
|
||||
field_name="asset_quantity_after",
|
||||
)
|
||||
snapshot_price_float, _ = self._to_snapshot_float_and_ledger_decimal(
|
||||
asset_price,
|
||||
field_name="asset_price",
|
||||
)
|
||||
snapshot_buffer_float, snapshot_buffer_ledger = self._to_snapshot_float_and_ledger_decimal(
|
||||
buffer,
|
||||
field_name="profit_buffer_after",
|
||||
)
|
||||
total_value = (
|
||||
snapshot_cash_ledger + snapshot_qty_ledger * Decimal(str(snapshot_price_float)) + snapshot_buffer_ledger
|
||||
).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
||||
cur.execute(
|
||||
"""
|
||||
UPDATE portfolios
|
||||
@@ -2246,7 +2280,7 @@ class PostgresPortfolioRepository:
|
||||
last_trade_at = NOW()
|
||||
WHERE portfolio_id = %s
|
||||
""",
|
||||
(float(cash), float(qty), float(asset_price), float(buffer), portfolio_id),
|
||||
(snapshot_cash_float, snapshot_qty_float, snapshot_price_float, snapshot_buffer_float, portfolio_id),
|
||||
)
|
||||
cur.execute(
|
||||
"""
|
||||
@@ -2268,10 +2302,10 @@ class PostgresPortfolioRepository:
|
||||
ledger_quantity,
|
||||
fee_f,
|
||||
tax_f,
|
||||
cash,
|
||||
qty,
|
||||
snapshot_cash_ledger,
|
||||
snapshot_qty_ledger,
|
||||
total_value,
|
||||
buffer,
|
||||
snapshot_buffer_ledger,
|
||||
"manual",
|
||||
note,
|
||||
ledger_executed_at,
|
||||
@@ -2711,7 +2745,8 @@ class PostgresPortfolioRepository:
|
||||
raise ValueError(f"asset을 찾을 수 없습니다: {asset_id}")
|
||||
|
||||
def rotate_service_token(self, name: str) -> tuple[str, AuthToken]:
|
||||
from app.core.security import generate_token, hash_token, token_prefix as _token_prefix
|
||||
from app.core.security import generate_token, hash_token
|
||||
from app.core.security import token_prefix as _token_prefix
|
||||
|
||||
self.ensure_schema()
|
||||
with self._connect().cursor() as cur:
|
||||
@@ -3047,10 +3082,9 @@ def database_exists(config: PortfolioDBConfig, *, maintenance_db: str = "postgre
|
||||
connect_timeout=config.connect_timeout_sec,
|
||||
application_name=config.application_name,
|
||||
autocommit=True,
|
||||
) as conn:
|
||||
with conn.cursor() as cur:
|
||||
cur.execute("SELECT 1 FROM pg_database WHERE datname = %s", (config.dbname,))
|
||||
return cur.fetchone() is not None
|
||||
) as conn, conn.cursor() as cur:
|
||||
cur.execute("SELECT 1 FROM pg_database WHERE datname = %s", (config.dbname,))
|
||||
return cur.fetchone() is not None
|
||||
|
||||
|
||||
def create_database_if_missing(config: PortfolioDBConfig, *, maintenance_db: str = "postgres") -> bool:
|
||||
|
||||
+6
-7
@@ -8,7 +8,6 @@ from typing import Any, Literal
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET/POST/PATCH /v1/tags (REQ-004)
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -114,8 +113,8 @@ class LedgerTradeRequest(BaseModel):
|
||||
side: Literal["buy", "sell"]
|
||||
quantity: Decimal = Field(..., gt=0)
|
||||
price: Decimal = Field(..., gt=0)
|
||||
fee_amount: Decimal = Field(default=Decimal("0"), ge=0)
|
||||
tax_amount: Decimal = Field(default=Decimal("0"), ge=0)
|
||||
fee_amount: Decimal = Field(default=Decimal(0), ge=0)
|
||||
tax_amount: Decimal = Field(default=Decimal(0), ge=0)
|
||||
executed_at: datetime
|
||||
|
||||
|
||||
@@ -160,8 +159,8 @@ class ManualLedgerRequest(BaseModel):
|
||||
amount: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8)
|
||||
quantity: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8)
|
||||
price: Decimal | None = Field(default=None, gt=0, max_digits=20, decimal_places=8)
|
||||
fee: Decimal = Field(default=Decimal("0"), ge=0, max_digits=20, decimal_places=8)
|
||||
tax: Decimal = Field(default=Decimal("0"), ge=0, max_digits=20, decimal_places=8)
|
||||
fee: Decimal = Field(default=Decimal(0), ge=0, max_digits=20, decimal_places=8)
|
||||
tax: Decimal = Field(default=Decimal(0), ge=0, max_digits=20, decimal_places=8)
|
||||
executed_at: date | None = None
|
||||
target_ledger_entry_id: int | None = None
|
||||
note: str = ""
|
||||
@@ -170,7 +169,7 @@ class ManualLedgerRequest(BaseModel):
|
||||
confirmation_evidence: str | None = None
|
||||
|
||||
@model_validator(mode="after")
|
||||
def validate_action_fields(self) -> "ManualLedgerRequest":
|
||||
def validate_action_fields(self) -> ManualLedgerRequest:
|
||||
amount_actions = {"INVEST", "BUFFER_OUT", "BUFFER_IN", "WITHDRAW", "PROFIT"}
|
||||
trade_actions = {"BUY", "SELL"}
|
||||
provided_fields = self.model_fields_set
|
||||
@@ -418,7 +417,7 @@ class StrategyUpdateRequest(BaseModel):
|
||||
return normalized
|
||||
|
||||
@model_validator(mode="after")
|
||||
def require_change(self) -> "StrategyUpdateRequest":
|
||||
def require_change(self) -> StrategyUpdateRequest:
|
||||
if not self.model_fields_set:
|
||||
raise ValueError("at least one field is required")
|
||||
return self
|
||||
|
||||
@@ -9,7 +9,7 @@ or production application database.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from datetime import datetime, timezone
|
||||
from datetime import UTC, datetime
|
||||
from decimal import Decimal
|
||||
from urllib.parse import unquote, urlparse
|
||||
from uuid import uuid4
|
||||
@@ -106,10 +106,9 @@ def _current_revision(settings: Settings) -> str:
|
||||
user=settings.active_db_user,
|
||||
password=settings.active_db_password,
|
||||
sslmode=settings.active_db_sslmode,
|
||||
) as conn:
|
||||
with conn.cursor() as cur:
|
||||
cur.execute("SELECT version_num FROM alembic_version")
|
||||
return str(cur.fetchone()[0])
|
||||
) as conn, conn.cursor() as cur:
|
||||
cur.execute("SELECT version_num FROM alembic_version")
|
||||
return str(cur.fetchone()[0])
|
||||
|
||||
|
||||
def _metadata_columns(settings: Settings) -> dict[str, tuple[str, str, str | None]]:
|
||||
@@ -120,18 +119,17 @@ def _metadata_columns(settings: Settings) -> dict[str, tuple[str, str, str | Non
|
||||
user=settings.active_db_user,
|
||||
password=settings.active_db_password,
|
||||
sslmode=settings.active_db_sslmode,
|
||||
) as conn:
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(
|
||||
"""
|
||||
) as conn, conn.cursor() as cur:
|
||||
cur.execute(
|
||||
"""
|
||||
SELECT column_name, data_type, is_nullable, column_default
|
||||
FROM information_schema.columns
|
||||
WHERE table_schema = 'public' AND table_name = 'portfolio_ledger'
|
||||
AND column_name IN ('source', 'target_ledger_entry_id',
|
||||
'confirmation_evidence', 'actor_user_id')
|
||||
"""
|
||||
)
|
||||
return {row[0]: (row[1], row[2], row[3]) for row in cur.fetchall()}
|
||||
)
|
||||
return {row[0]: (row[1], row[2], row[3]) for row in cur.fetchall()}
|
||||
|
||||
|
||||
def _seed_portfolio(repo: PostgresPortfolioRepository, suffix: str) -> str:
|
||||
@@ -169,7 +167,7 @@ def test_upgrade_from_pre_0028_preserves_business_data_and_backfills_source(
|
||||
command.upgrade(get_alembic_config(isolated_settings), _PRE_BAT025)
|
||||
repo = _repo(isolated_settings)
|
||||
portfolio_id = _seed_portfolio(repo, "preserve")
|
||||
recorded_at = datetime(2026, 7, 15, tzinfo=timezone.utc)
|
||||
recorded_at = datetime(2026, 7, 15, tzinfo=UTC)
|
||||
with repo._connect().cursor() as cur:
|
||||
cur.execute(
|
||||
"""
|
||||
@@ -249,7 +247,7 @@ def test_broker_and_manual_ledger_writes_regress_after_real_upgrade(
|
||||
side="buy",
|
||||
quantity=2,
|
||||
price=100,
|
||||
executed_at=datetime.now(timezone.utc),
|
||||
executed_at=datetime.now(UTC),
|
||||
)
|
||||
manual = repo.record_manual_ledger(
|
||||
portfolio_id,
|
||||
@@ -287,7 +285,7 @@ def test_dividend_precision_and_reversal_round_trip_on_real_postgres(
|
||||
action="DIVIDEND",
|
||||
amount=Decimal("0.00000001"),
|
||||
tax=Decimal("0.00000001"),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=timezone.utc),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=UTC),
|
||||
note="precision-test",
|
||||
)
|
||||
|
||||
@@ -352,7 +350,60 @@ def test_dividend_large_integer_with_8dp_is_rejected_to_protect_double_snapshot(
|
||||
action="DIVIDEND",
|
||||
amount=Decimal("1000000000.00000001"),
|
||||
tax=Decimal("0.00000001"),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=timezone.utc),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=UTC),
|
||||
note="too-large-for-double",
|
||||
)
|
||||
repo.close()
|
||||
|
||||
|
||||
def test_large_existing_buffer_plus_min_dividend_is_rejected_before_snapshot_divergence(
|
||||
isolated_settings: Settings,
|
||||
) -> None:
|
||||
"""Reject writes that would diverge between NUMERIC ledger and DOUBLE snapshot."""
|
||||
upgrade_to_head(isolated_settings)
|
||||
repo = _repo(isolated_settings)
|
||||
portfolio_id = _seed_portfolio(repo, "large-buffer-min-dividend")
|
||||
with repo._connect().cursor() as cur:
|
||||
cur.execute("UPDATE portfolios SET profit_buffer = %s WHERE portfolio_id = %s", (1_000_000_000.0, portfolio_id))
|
||||
|
||||
with pytest.raises(ValueError, match="DOUBLE PRECISION snapshot"):
|
||||
repo.record_manual_ledger(
|
||||
portfolio_id,
|
||||
action="DIVIDEND",
|
||||
amount=Decimal("0.00000001"),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=UTC),
|
||||
note="should-reject",
|
||||
)
|
||||
|
||||
with repo._connect().cursor() as cur:
|
||||
cur.execute("SELECT COUNT(*) FROM portfolio_ledger WHERE portfolio_id = %s AND side = 'dividend'", (portfolio_id,))
|
||||
assert cur.fetchone()[0] == 0
|
||||
repo.close()
|
||||
|
||||
|
||||
def test_large_invest_and_large_dividend_tax_metadata_are_accepted_when_snapshot_is_safe(
|
||||
isolated_settings: Settings,
|
||||
) -> None:
|
||||
"""No hidden per-field cap for values that do not cause snapshot divergence."""
|
||||
upgrade_to_head(isolated_settings)
|
||||
repo = _repo(isolated_settings)
|
||||
portfolio_id = _seed_portfolio(repo, "large-invest-large-tax")
|
||||
|
||||
invest = repo.record_manual_ledger(
|
||||
portfolio_id,
|
||||
action="INVEST",
|
||||
amount=Decimal("50000000.00000000"),
|
||||
note="large-invest",
|
||||
)
|
||||
assert invest.price == 50000000.0
|
||||
|
||||
dividend = repo.record_manual_ledger(
|
||||
portfolio_id,
|
||||
action="DIVIDEND",
|
||||
amount=Decimal("1.00000000"),
|
||||
tax=Decimal("50000000.00000000"),
|
||||
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=UTC),
|
||||
note="large-tax-metadata",
|
||||
)
|
||||
assert dividend.tax_amount == 50000000.0
|
||||
repo.close()
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from contextlib import ExitStack
|
||||
from datetime import datetime, timezone
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
@@ -195,7 +195,7 @@ class _FakeRepo:
|
||||
fee_amount=fee,
|
||||
tax_amount=tax,
|
||||
executed_at=executed_at,
|
||||
recorded_at=datetime.now(timezone.utc),
|
||||
recorded_at=datetime.now(UTC),
|
||||
cash_balance_after=self.portfolio.cash_balance,
|
||||
asset_quantity_after=self.portfolio.asset_quantity,
|
||||
profit_buffer_after=self.portfolio.profit_buffer,
|
||||
@@ -674,7 +674,7 @@ def test_unified_ledger_paginates_mixed_sources_and_hides_side() -> None:
|
||||
repo.entries.append(
|
||||
LedgerEntry(
|
||||
ledger_entry_id=99, portfolio_id=_PORTFOLIO_ID, side="buy", quantity=2, price=10, source="broker",
|
||||
recorded_at=datetime.now(timezone.utc), cash_balance_after=900, asset_quantity_after=12,
|
||||
recorded_at=datetime.now(UTC), cash_balance_after=900, asset_quantity_after=12,
|
||||
)
|
||||
)
|
||||
first = client.get(f"/v1/portfolios/{_PORTFOLIO_ID}/ledger?limit=1", headers=_auth())
|
||||
|
||||
Reference in New Issue
Block a user