fix: restore manual confirm_duplicate compatibility and precision safeguards
Co-authored-by: genichin <4282227+genichin@users.noreply.github.com>
This commit is contained in:
co-authored by
genichin
parent
a71c91f6ef
commit
aa40c7fdff
@@ -21,6 +21,7 @@ logger = logging.getLogger(__name__)
|
|||||||
_LEDGER_NUMERIC_SCALE = 8
|
_LEDGER_NUMERIC_SCALE = 8
|
||||||
_LEDGER_MAX_INTEGER_DIGITS = 12
|
_LEDGER_MAX_INTEGER_DIGITS = 12
|
||||||
_LEDGER_QUANT = Decimal("0.00000001")
|
_LEDGER_QUANT = Decimal("0.00000001")
|
||||||
|
_MAX_SAFE_DOUBLE_FOR_8DP = Decimal("45035996")
|
||||||
|
|
||||||
_SELECT_USER = """
|
_SELECT_USER = """
|
||||||
SELECT
|
SELECT
|
||||||
@@ -2039,6 +2040,21 @@ class PostgresPortfolioRepository:
|
|||||||
raise ValueError(f"{field_name} must be a decimal") from None
|
raise ValueError(f"{field_name} must be a decimal") from None
|
||||||
return cls._validate_ledger_decimal(decimal_value, field_name=field_name, allow_zero=allow_zero)
|
return cls._validate_ledger_decimal(decimal_value, field_name=field_name, allow_zero=allow_zero)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _coerce_portfolio_snapshot_decimal(value: Any, *, field_name: str) -> Decimal:
|
||||||
|
try:
|
||||||
|
decimal_value = Decimal(str(value))
|
||||||
|
except (InvalidOperation, ValueError, TypeError):
|
||||||
|
raise ValueError(f"{field_name} must be a decimal") from None
|
||||||
|
if decimal_value.is_nan() or decimal_value.is_infinite():
|
||||||
|
raise ValueError(f"{field_name} must be a finite decimal")
|
||||||
|
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 record_manual_ledger(
|
def record_manual_ledger(
|
||||||
self,
|
self,
|
||||||
portfolio_id: str,
|
portfolio_id: str,
|
||||||
@@ -2068,15 +2084,20 @@ class PostgresPortfolioRepository:
|
|||||||
if row is None:
|
if row is None:
|
||||||
raise ValueError("포트폴리오를 찾을 수 없습니다")
|
raise ValueError("포트폴리오를 찾을 수 없습니다")
|
||||||
portfolio = self._row_to_portfolio(row)
|
portfolio = self._row_to_portfolio(row)
|
||||||
cash = self._coerce_ledger_decimal(portfolio.cash_balance, field_name="cash_balance", allow_zero=True)
|
cash = self._coerce_portfolio_snapshot_decimal(portfolio.cash_balance, field_name="cash_balance")
|
||||||
qty = self._coerce_ledger_decimal(portfolio.asset_quantity, field_name="asset_quantity", allow_zero=True)
|
qty = self._coerce_portfolio_snapshot_decimal(portfolio.asset_quantity, field_name="asset_quantity")
|
||||||
buffer = self._coerce_ledger_decimal(portfolio.profit_buffer, field_name="profit_buffer", allow_zero=True)
|
buffer = self._coerce_portfolio_snapshot_decimal(portfolio.profit_buffer, field_name="profit_buffer")
|
||||||
asset_price = self._coerce_ledger_decimal(portfolio.price, field_name="asset_price", allow_zero=True)
|
asset_price = self._coerce_portfolio_snapshot_decimal(portfolio.price, field_name="asset_price")
|
||||||
ledger_price = self._coerce_ledger_decimal(price or 0, field_name="price", allow_zero=True)
|
ledger_price = self._coerce_ledger_decimal(price or 0, field_name="price", allow_zero=True)
|
||||||
ledger_quantity = self._coerce_ledger_decimal(quantity or 0, field_name="quantity", allow_zero=True)
|
ledger_quantity = self._coerce_ledger_decimal(quantity or 0, field_name="quantity", allow_zero=True)
|
||||||
fee_f = self._coerce_ledger_decimal(fee or 0, field_name="fee", allow_zero=True)
|
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)
|
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)
|
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(timezone.utc)
|
||||||
if ledger_executed_at.tzinfo is None:
|
if ledger_executed_at.tzinfo is None:
|
||||||
ledger_executed_at = ledger_executed_at.replace(tzinfo=timezone.utc)
|
ledger_executed_at = ledger_executed_at.replace(tzinfo=timezone.utc)
|
||||||
@@ -2163,19 +2184,21 @@ class PostgresPortfolioRepository:
|
|||||||
""",
|
""",
|
||||||
(portfolio_id, target_ledger_entry_id),
|
(portfolio_id, target_ledger_entry_id),
|
||||||
)
|
)
|
||||||
reverse_target = self._row_to_ledger_entry(cur.fetchone())
|
reverse_target_row = cur.fetchone()
|
||||||
|
if reverse_target_row is None:
|
||||||
|
raise ValueError("LEDGER_ENTRY_NOT_FOUND")
|
||||||
cur.execute(
|
cur.execute(
|
||||||
"SELECT 1 FROM portfolio_ledger WHERE portfolio_id = %s AND side = 'reversal' AND target_ledger_entry_id = %s LIMIT 1",
|
"SELECT 1 FROM portfolio_ledger WHERE portfolio_id = %s AND side = 'reversal' AND target_ledger_entry_id = %s LIMIT 1",
|
||||||
(portfolio_id, target_ledger_entry_id),
|
(portfolio_id, target_ledger_entry_id),
|
||||||
)
|
)
|
||||||
if cur.fetchone() is not None:
|
if cur.fetchone() is not None:
|
||||||
raise ValueError("LEDGER_ENTRY_ALREADY_REVERSED")
|
raise ValueError("LEDGER_ENTRY_ALREADY_REVERSED")
|
||||||
target_action = (reverse_target.action or "").upper()
|
target_action = str(reverse_target_row[4] or "").upper()
|
||||||
amount_f = self._coerce_ledger_decimal(reverse_target.price, field_name="amount", allow_zero=True)
|
amount_f = self._coerce_ledger_decimal(reverse_target_row[6], field_name="amount", allow_zero=True)
|
||||||
ledger_price = self._coerce_ledger_decimal(reverse_target.price, field_name="price", allow_zero=True)
|
ledger_price = self._coerce_ledger_decimal(reverse_target_row[6], field_name="price", allow_zero=True)
|
||||||
ledger_quantity = self._coerce_ledger_decimal(reverse_target.quantity, field_name="quantity", allow_zero=True)
|
ledger_quantity = self._coerce_ledger_decimal(reverse_target_row[5], field_name="quantity", allow_zero=True)
|
||||||
fee_f = self._coerce_ledger_decimal(reverse_target.fee_amount, field_name="fee", allow_zero=True)
|
fee_f = self._coerce_ledger_decimal(reverse_target_row[7], field_name="fee", allow_zero=True)
|
||||||
tax_f = self._coerce_ledger_decimal(reverse_target.tax_amount, field_name="tax", allow_zero=True)
|
tax_f = self._coerce_ledger_decimal(reverse_target_row[8], field_name="tax", allow_zero=True)
|
||||||
if target_action == "BUY":
|
if target_action == "BUY":
|
||||||
cash += (ledger_quantity * ledger_price + fee_f + tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
cash += (ledger_quantity * ledger_price + fee_f + tax_f).quantize(_LEDGER_QUANT, rounding=ROUND_HALF_UP)
|
||||||
qty -= ledger_quantity
|
qty -= ledger_quantity
|
||||||
|
|||||||
+4
-4
@@ -177,7 +177,7 @@ class ManualLedgerRequest(BaseModel):
|
|||||||
fee_or_tax_provided = "fee" in provided_fields or "tax" in provided_fields
|
fee_or_tax_provided = "fee" in provided_fields or "tax" in provided_fields
|
||||||
fee_provided = "fee" in provided_fields
|
fee_provided = "fee" in provided_fields
|
||||||
executed_at_provided = "executed_at" in provided_fields
|
executed_at_provided = "executed_at" in provided_fields
|
||||||
confirm_duplicate_provided = "confirm_duplicate" in provided_fields
|
confirm_duplicate_requested = bool(self.confirm_duplicate)
|
||||||
if self.action in amount_actions:
|
if self.action in amount_actions:
|
||||||
if self.amount is None:
|
if self.amount is None:
|
||||||
raise ValueError("amount is required for this action")
|
raise ValueError("amount is required for this action")
|
||||||
@@ -187,7 +187,7 @@ class ManualLedgerRequest(BaseModel):
|
|||||||
or self.target_ledger_entry_id is not None
|
or self.target_ledger_entry_id is not None
|
||||||
or fee_or_tax_provided
|
or fee_or_tax_provided
|
||||||
or executed_at_provided
|
or executed_at_provided
|
||||||
or confirm_duplicate_provided
|
or confirm_duplicate_requested
|
||||||
):
|
):
|
||||||
raise ValueError("unexpected numeric/target field for amount action")
|
raise ValueError("unexpected numeric/target field for amount action")
|
||||||
elif self.action == "DIVIDEND":
|
elif self.action == "DIVIDEND":
|
||||||
@@ -200,7 +200,7 @@ class ManualLedgerRequest(BaseModel):
|
|||||||
elif self.action in trade_actions:
|
elif self.action in trade_actions:
|
||||||
if self.quantity is None or self.price is None:
|
if self.quantity is None or self.price is None:
|
||||||
raise ValueError("quantity and price are required for BUY/SELL")
|
raise ValueError("quantity and price are required for BUY/SELL")
|
||||||
if self.amount is not None or self.target_ledger_entry_id is not None or executed_at_provided or confirm_duplicate_provided:
|
if self.amount is not None or self.target_ledger_entry_id is not None or executed_at_provided or confirm_duplicate_requested:
|
||||||
raise ValueError("unexpected amount/target field for BUY/SELL")
|
raise ValueError("unexpected amount/target field for BUY/SELL")
|
||||||
elif self.action == "REVERSAL":
|
elif self.action == "REVERSAL":
|
||||||
if self.target_ledger_entry_id is None:
|
if self.target_ledger_entry_id is None:
|
||||||
@@ -211,7 +211,7 @@ class ManualLedgerRequest(BaseModel):
|
|||||||
or self.price is not None
|
or self.price is not None
|
||||||
or fee_or_tax_provided
|
or fee_or_tax_provided
|
||||||
or executed_at_provided
|
or executed_at_provided
|
||||||
or confirm_duplicate_provided
|
or confirm_duplicate_requested
|
||||||
):
|
):
|
||||||
raise ValueError("unexpected numeric field for REVERSAL")
|
raise ValueError("unexpected numeric field for REVERSAL")
|
||||||
return self
|
return self
|
||||||
|
|||||||
@@ -312,3 +312,47 @@ def test_dividend_precision_and_reversal_round_trip_on_real_postgres(
|
|||||||
assert cur.fetchone()[0] == "0"
|
assert cur.fetchone()[0] == "0"
|
||||||
|
|
||||||
repo.close()
|
repo.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_existing_high_precision_snapshot_still_allows_legacy_manual_actions(
|
||||||
|
isolated_settings: Settings,
|
||||||
|
) -> None:
|
||||||
|
"""Existing DOUBLE snapshot precision should not block unrelated manual actions."""
|
||||||
|
upgrade_to_head(isolated_settings)
|
||||||
|
repo = _repo(isolated_settings)
|
||||||
|
portfolio_id = _seed_portfolio(repo, "legacy-snapshot-precision")
|
||||||
|
with repo._connect().cursor() as cur:
|
||||||
|
cur.execute(
|
||||||
|
"UPDATE portfolios SET cash_balance = %s WHERE portfolio_id = %s",
|
||||||
|
(0.123456789, portfolio_id),
|
||||||
|
)
|
||||||
|
|
||||||
|
entry = repo.record_manual_ledger(
|
||||||
|
portfolio_id,
|
||||||
|
action="INVEST",
|
||||||
|
amount=Decimal("1.00000000"),
|
||||||
|
note="compat",
|
||||||
|
)
|
||||||
|
assert entry.action == "INVEST"
|
||||||
|
assert entry.price == 1.0
|
||||||
|
repo.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_dividend_large_integer_with_8dp_is_rejected_to_protect_double_snapshot(
|
||||||
|
isolated_settings: Settings,
|
||||||
|
) -> None:
|
||||||
|
"""Inputs valid for NUMERIC(20,8) but unsafe for DOUBLE snapshot are rejected."""
|
||||||
|
upgrade_to_head(isolated_settings)
|
||||||
|
repo = _repo(isolated_settings)
|
||||||
|
portfolio_id = _seed_portfolio(repo, "double-snapshot-guard")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="double snapshot precision"):
|
||||||
|
repo.record_manual_ledger(
|
||||||
|
portfolio_id,
|
||||||
|
action="DIVIDEND",
|
||||||
|
amount=Decimal("1000000000.00000001"),
|
||||||
|
tax=Decimal("0.00000001"),
|
||||||
|
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=timezone.utc),
|
||||||
|
note="too-large-for-double",
|
||||||
|
)
|
||||||
|
repo.close()
|
||||||
|
|||||||
@@ -558,6 +558,37 @@ def test_dividend_duplicate_requires_confirmation() -> None:
|
|||||||
assert confirmed.status_code == 201
|
assert confirmed.status_code == 201
|
||||||
|
|
||||||
|
|
||||||
|
def test_confirm_duplicate_false_is_accepted_for_non_dividend_actions() -> None:
|
||||||
|
repo = _FakeRepo(_portfolio())
|
||||||
|
client = _make_client(repo)
|
||||||
|
|
||||||
|
invest = client.post(
|
||||||
|
f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual",
|
||||||
|
json={"action": "INVEST", "amount": 100, "confirm_duplicate": False},
|
||||||
|
headers=_auth(),
|
||||||
|
)
|
||||||
|
assert invest.status_code == 201
|
||||||
|
|
||||||
|
buy = client.post(
|
||||||
|
f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual",
|
||||||
|
json={"action": "BUY", "quantity": 1, "price": 10, "confirm_duplicate": False},
|
||||||
|
headers=_auth(),
|
||||||
|
)
|
||||||
|
assert buy.status_code == 201
|
||||||
|
|
||||||
|
|
||||||
|
def test_confirm_duplicate_true_is_rejected_for_non_dividend_actions() -> None:
|
||||||
|
repo = _FakeRepo(_portfolio())
|
||||||
|
client = _make_client(repo)
|
||||||
|
|
||||||
|
invest = client.post(
|
||||||
|
f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual",
|
||||||
|
json={"action": "INVEST", "amount": 100, "confirm_duplicate": True},
|
||||||
|
headers=_auth(),
|
||||||
|
)
|
||||||
|
assert invest.status_code == 422
|
||||||
|
|
||||||
|
|
||||||
def test_dividend_reversal_decreases_profit_buffer() -> None:
|
def test_dividend_reversal_decreases_profit_buffer() -> None:
|
||||||
repo = _FakeRepo(_portfolio(buffer=200))
|
repo = _FakeRepo(_portfolio(buffer=200))
|
||||||
client = _make_client(repo)
|
client = _make_client(repo)
|
||||||
|
|||||||
Reference in New Issue
Block a user