mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-10-05 12:28:22 +00:00
fix: give the Routstr top-up threshold an explicit unit
The auto top-up field has always been labelled "When credits are below (Sats)", but the backend compared it as `balance >= threshold * 1000` against a balance get_balance() had already converted to sats. Its sibling topup_amount_limit is plain sats - it goes straight to send_token(amount, "sat", ...). So one settings blob carried two units and the UI promised the one it did not use: an operator asking to top up below 1000 sats got one below 1,000,000. topup_threshold_sats says what it means and is compared as written. A legacy topup_threshold keeps the thousandfold it has always been compared with, since reinterpreting stored values as sats would drop the trigger point by a factor of 1000 on upgrade and leave a peer to run dry. A one-off warning per provider names the sats value to migrate to. The settings form now edits topup_threshold_sats, seeding it from the legacy value times 1000 so the number shown is the number in force. Editing therefore starts from the truth rather than silently moving the trigger on the next save, and saving drops the legacy key so the stored blob carries one unit. Validation accepts either key and rejects a blob carrying neither. The PPQ path keeps its own topup_threshold, which is USD and unaffected.
This commit is contained in:
@@ -200,15 +200,48 @@ def validate_ppq_auto_topup_settings(settings: dict | None) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
_legacy_threshold_hinted: set[int] = set()
|
||||
|
||||
|
||||
def _routstr_threshold_sats(row: UpstreamProviderRow, settings: dict) -> float:
|
||||
"""Balance, in sats, below which a top-up fires.
|
||||
|
||||
``topup_threshold_sats`` is used as written. A legacy ``topup_threshold``
|
||||
keeps the thousandfold it has always been compared with — reinterpreting it
|
||||
as sats would drop an operator's trigger point by a factor of 1000 on
|
||||
upgrade and leave the peer to run dry. The hint names the value to migrate
|
||||
to, once per provider rather than once per scheduler tick.
|
||||
"""
|
||||
explicit = settings.get("topup_threshold_sats")
|
||||
if explicit is not None:
|
||||
return float(typing.cast(int | float, explicit))
|
||||
|
||||
threshold_sats = float(typing.cast(int | float, settings["topup_threshold"])) * 1000
|
||||
if row.id is not None and row.id not in _legacy_threshold_hinted:
|
||||
_legacy_threshold_hinted.add(row.id)
|
||||
logger.warning(
|
||||
"Routstr auto top-up uses the legacy unitless threshold; set "
|
||||
"topup_threshold_sats to state the unit",
|
||||
extra={"provider_id": row.id, "threshold_sats": threshold_sats},
|
||||
)
|
||||
return threshold_sats
|
||||
|
||||
|
||||
def validate_routstr_auto_topup_settings(settings: dict | None) -> str | None:
|
||||
"""Return why enabled Routstr auto top-up settings are invalid, if anything."""
|
||||
if not settings or not settings.get("auto_topup"):
|
||||
return None
|
||||
|
||||
threshold = settings.get("topup_threshold")
|
||||
threshold_sats = settings.get("topup_threshold_sats")
|
||||
legacy_threshold = settings.get("topup_threshold")
|
||||
amount = settings.get("topup_amount_limit")
|
||||
mint_url = settings.get("topup_mint_url")
|
||||
if _invalid_topup_number(threshold):
|
||||
if threshold_sats is not None:
|
||||
if _invalid_topup_number(threshold_sats):
|
||||
return "Routstr auto top-up threshold must be a positive number of sats"
|
||||
elif legacy_threshold is None:
|
||||
return "Routstr auto top-up requires a threshold"
|
||||
elif _invalid_topup_number(legacy_threshold):
|
||||
return "Routstr auto top-up threshold must be a positive number"
|
||||
if _invalid_topup_number(amount, integer=True):
|
||||
return "Routstr auto top-up amount must be a positive whole number"
|
||||
@@ -270,7 +303,7 @@ async def _check_and_topup(row: UpstreamProviderRow) -> None:
|
||||
)
|
||||
return
|
||||
|
||||
threshold = float(settings["topup_threshold"])
|
||||
threshold_sats = _routstr_threshold_sats(row, settings)
|
||||
amount = int(settings["topup_amount_limit"])
|
||||
mint_url = str(settings["topup_mint_url"])
|
||||
|
||||
@@ -293,7 +326,7 @@ async def _check_and_topup(row: UpstreamProviderRow) -> None:
|
||||
)
|
||||
return
|
||||
|
||||
if balance >= threshold * 1000:
|
||||
if balance >= threshold_sats:
|
||||
return
|
||||
|
||||
spent_24h_sats = await _routstr_spent_last_24h_sats()
|
||||
@@ -323,7 +356,7 @@ async def _check_and_topup(row: UpstreamProviderRow) -> None:
|
||||
extra={
|
||||
"provider_id": row.id,
|
||||
"balance": balance,
|
||||
"threshold": threshold,
|
||||
"threshold_sats": threshold_sats,
|
||||
"topup_amount": amount,
|
||||
"mint_url": mint_url,
|
||||
},
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
"""The Routstr top-up threshold has to state its unit.
|
||||
|
||||
``topup_amount_limit`` was already plain sats — it goes straight to
|
||||
``send_token(amount, "sat", ...)`` and is added to the balance for logging — but
|
||||
its sibling ``topup_threshold`` was compared as ``balance >= threshold * 1000``.
|
||||
Two keys from one settings blob, two different units, and nothing naming either.
|
||||
An operator asking for "top up below 1000 sats" got one below 1,000,000.
|
||||
|
||||
``topup_threshold_sats`` says what it means. Legacy values keep their effective
|
||||
trigger point: reinterpreting them as sats would silently drop the trigger a
|
||||
thousandfold and let a provider run dry.
|
||||
"""
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from routstr.core.db import UpstreamProviderRow, create_session
|
||||
from routstr.upstream import auto_topup as auto_topup_module
|
||||
from routstr.upstream.auto_topup import (
|
||||
_check_and_topup,
|
||||
validate_routstr_auto_topup_settings,
|
||||
)
|
||||
|
||||
from .test_routstr_auto_topup_claim import _patch_wallet, _peer, _sent_tokens
|
||||
|
||||
# No module-level asyncio mark: the settings cases are sync, and the suite
|
||||
# already runs asyncio in auto mode.
|
||||
|
||||
TOPUP_SATS = 50
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _forget_legacy_hints() -> Any:
|
||||
# The hint fires once per provider for the life of the process. Reach it
|
||||
# through the module: a sibling test reloads auto_topup, which rebinds the
|
||||
# set, so a name imported here would go on clearing the old one.
|
||||
auto_topup_module._legacy_threshold_hinted.clear()
|
||||
yield
|
||||
auto_topup_module._legacy_threshold_hinted.clear()
|
||||
|
||||
|
||||
async def _seed(**topup_settings: Any) -> UpstreamProviderRow:
|
||||
row = UpstreamProviderRow(
|
||||
id=1,
|
||||
slug="peer-1",
|
||||
provider_type="routstr",
|
||||
base_url="https://peer.test",
|
||||
api_key="secret",
|
||||
enabled=True,
|
||||
provider_settings=json.dumps(
|
||||
{
|
||||
"auto_topup": True,
|
||||
"topup_amount_limit": TOPUP_SATS,
|
||||
"topup_mint_url": "https://mint.test",
|
||||
**topup_settings,
|
||||
}
|
||||
),
|
||||
)
|
||||
async with create_session() as session:
|
||||
session.add(row)
|
||||
await session.commit()
|
||||
await session.refresh(row)
|
||||
return row
|
||||
|
||||
|
||||
async def _topped_up(row: UpstreamProviderRow, balance: float) -> bool:
|
||||
peer = _peer(balance)
|
||||
with _patch_wallet(auto_topup_module, peer, "cashu-token-1"):
|
||||
await _check_and_topup(row)
|
||||
return bool(await _sent_tokens())
|
||||
|
||||
|
||||
async def test_explicit_sats_threshold_is_compared_against_a_sats_balance(
|
||||
patched_db_engine: Any,
|
||||
) -> None:
|
||||
row = await _seed(topup_threshold_sats=1000)
|
||||
assert await _topped_up(row, 1500.0) is False
|
||||
|
||||
|
||||
async def test_explicit_sats_threshold_tops_up_below_the_stated_amount(
|
||||
patched_db_engine: Any,
|
||||
) -> None:
|
||||
row = await _seed(topup_threshold_sats=1000)
|
||||
assert await _topped_up(row, 500.0) is True
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("balance", "expected"),
|
||||
[(1500.0, False), (500.0, True)],
|
||||
)
|
||||
async def test_legacy_threshold_keeps_its_effective_trigger_point(
|
||||
patched_db_engine: Any, balance: float, expected: bool
|
||||
) -> None:
|
||||
# 1 x 1000 == the 1000 sats the old comparison actually used. Reading it as
|
||||
# 1 sat instead would leave the peer to run dry.
|
||||
row = await _seed(topup_threshold=1)
|
||||
assert await _topped_up(row, balance) is expected
|
||||
|
||||
|
||||
async def test_explicit_sats_threshold_overrides_the_legacy_key(
|
||||
patched_db_engine: Any,
|
||||
) -> None:
|
||||
row = await _seed(topup_threshold=1, topup_threshold_sats=100)
|
||||
assert await _topped_up(row, 500.0) is False
|
||||
|
||||
|
||||
async def test_legacy_threshold_reports_the_sats_value_to_migrate_to(
|
||||
patched_db_engine: Any,
|
||||
) -> None:
|
||||
row = await _seed(topup_threshold=1)
|
||||
# The app logger does not propagate to root, so caplog would see nothing.
|
||||
with patch.object(auto_topup_module, "logger") as log:
|
||||
await _topped_up(row, 5000.0)
|
||||
hints = [
|
||||
c for c in log.warning.call_args_list if "topup_threshold_sats" in c.args[0]
|
||||
]
|
||||
assert len(hints) == 1
|
||||
assert hints[0].kwargs["extra"]["threshold_sats"] == 1000.0
|
||||
|
||||
# The scheduler runs every minute; the hint must not run with it.
|
||||
await _topped_up(row, 5000.0)
|
||||
assert (
|
||||
sum("topup_threshold_sats" in c.args[0] for c in log.warning.call_args_list)
|
||||
== 1
|
||||
)
|
||||
|
||||
|
||||
def test_settings_accept_the_sats_threshold_on_its_own() -> None:
|
||||
assert (
|
||||
validate_routstr_auto_topup_settings(
|
||||
{
|
||||
"auto_topup": True,
|
||||
"topup_threshold_sats": 1000,
|
||||
"topup_amount_limit": TOPUP_SATS,
|
||||
"topup_mint_url": "https://mint.test",
|
||||
}
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", [0, -1, True, float("inf"), "1000", None])
|
||||
def test_settings_reject_an_unusable_sats_threshold(value: object) -> None:
|
||||
assert validate_routstr_auto_topup_settings(
|
||||
{
|
||||
"auto_topup": True,
|
||||
"topup_threshold_sats": value,
|
||||
"topup_amount_limit": TOPUP_SATS,
|
||||
"topup_mint_url": "https://mint.test",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def test_settings_require_one_of_the_threshold_keys() -> None:
|
||||
assert validate_routstr_auto_topup_settings(
|
||||
{
|
||||
"auto_topup": True,
|
||||
"topup_amount_limit": TOPUP_SATS,
|
||||
"topup_mint_url": "https://mint.test",
|
||||
}
|
||||
)
|
||||
@@ -14,12 +14,32 @@ import { Switch } from '@/components/ui/switch';
|
||||
interface ProviderSettings {
|
||||
topup_mint_url?: string;
|
||||
auto_topup?: boolean;
|
||||
topup_threshold_sats?: number;
|
||||
/** Legacy, read-only here: the backend compares it as `x 1000` sats. */
|
||||
topup_threshold?: number;
|
||||
topup_amount_limit?: number;
|
||||
refund_on_expiry?: boolean;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sats a legacy provider actually tops up below.
|
||||
*
|
||||
* This field has always been labelled Sats, but the backend compared
|
||||
* `topup_threshold` as `balance >= threshold * 1000`, so the number shown here
|
||||
* was never the number in force. Show the figure the backend uses, so editing
|
||||
* starts from the truth instead of silently moving the trigger a thousandfold
|
||||
* on the next save.
|
||||
*/
|
||||
function thresholdSats(settings: ProviderSettings): number | undefined {
|
||||
if (settings.topup_threshold_sats !== undefined) {
|
||||
return settings.topup_threshold_sats;
|
||||
}
|
||||
return settings.topup_threshold !== undefined
|
||||
? settings.topup_threshold * 1000
|
||||
: undefined;
|
||||
}
|
||||
|
||||
interface RoutstrNodeSettingsProps {
|
||||
settings: ProviderSettings;
|
||||
onSettingsChange: (settings: ProviderSettings) => void;
|
||||
@@ -92,19 +112,23 @@ export function RoutstrNodeSettings({
|
||||
<div className='border-primary/20 grid gap-4 border-l-2 pt-2 pl-4'>
|
||||
<div className='grid gap-2'>
|
||||
<Label
|
||||
htmlFor={`${prefix}topup_threshold`}
|
||||
htmlFor={`${prefix}topup_threshold_sats`}
|
||||
className='text-xs font-medium'
|
||||
>
|
||||
When credits are below (Sats)
|
||||
</Label>
|
||||
<Input
|
||||
id={`${prefix}topup_threshold`}
|
||||
id={`${prefix}topup_threshold_sats`}
|
||||
type='number'
|
||||
className='h-9'
|
||||
placeholder='e.g. 1000'
|
||||
value={settings.topup_threshold || ''}
|
||||
value={thresholdSats(settings) ?? ''}
|
||||
onChange={(e) =>
|
||||
update({ topup_threshold: parseInt(e.target.value) })
|
||||
update({
|
||||
topup_threshold_sats: parseInt(e.target.value),
|
||||
// Drop the legacy key so the saved blob carries one unit.
|
||||
topup_threshold: undefined,
|
||||
})
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user