Skip to content

Commit 57c1b54

Browse files
author
Vyacheslav-Tomashevskiy
committed
fix(lock_ledger): do not credit balances for bridge-owned locks
release_lock() unconditionally credits balances because it is the counterpart of create_lock(), which debits. That pairing does not hold for bridge-produced rows: create_lock() has no live callers, and every real lock_ledger row is written directly by bridge_api.create_bridge_transfer(), which does its own hard debit and tracks the refund on bridge_transfers.source_debited -- a flag release_lock() neither reads nor clears. Releasing a bridge-owned lock therefore reverses a debit the bridge still considers outstanding: - POST /api/lock/release (released_by="admin", bypasses the unlock timer by design) un-does the hard debit of a pending deposit; if the deposit then completes, the source keeps the RTC on both chains. - The bridge's own void path refunds again on top of it (100 -> 200 RTC). - POST /api/lock/auto-release does the same with no operator action once the 7-day lock expires while the deposit is still pending; get_pending_unlocks() does not filter by lock_type. The restored balance is also immediately spendable: available_balance .encumbered_i64() only counts deposits with source_debited = 0 OR NULL, and release_lock() leaves source_debited = 1, so it is invisible to the encumbrance check. Fix: fail closed in release_lock() for locks carrying a bridge_transfer_id, and skip them in the auto-release worker (an expired bridge deposit is resolved by voiding, not by releasing, so it is a normal state rather than a per-run error). Locks created through create_lock() keep their debit/credit symmetry. Both lock_ledger and bridge_api register under the same HAVE_BRIDGE flag (rustchain_v2_integrated_v2.2.1_rip200.py:1462-1465), so the producer and these routes are always live together. Tests: 4 of the 5 new tests fail on main; the fifth guards against over-reach by asserting create_lock()/release_lock() still balance. Existing tests/test_bridge_lock_ledger.py (71) stays green -- it only exercises the symmetric create_lock()/release_lock() pair and so never fed a bridge-produced lock into release_lock().
1 parent 96660f0 commit 57c1b54

2 files changed

Lines changed: 183 additions & 2 deletions

File tree

node/lock_ledger.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,22 +242,41 @@ def release_lock(
242242

243243
# Find the lock
244244
row = cursor.execute("""
245-
SELECT id, miner_id, amount_i64, lock_type, status, unlock_at
245+
SELECT id, miner_id, amount_i64, lock_type, status, unlock_at,
246+
bridge_transfer_id
246247
FROM lock_ledger
247248
WHERE id = ?
248249
""", (lock_id,)).fetchone()
249250

250251
if not row:
251252
return False, {"error": "Lock not found"}
252253

253-
lid, miner_id, amount_i64, lock_type, status, unlock_at = row
254+
lid, miner_id, amount_i64, lock_type, status, unlock_at, bridge_transfer_id = row
254255

255256
if status != "locked":
256257
return False, {
257258
"error": f"Lock already {status}",
258259
"hint": "Only locked entries can be released"
259260
}
260261

262+
# Bridge-owned locks settle through bridge_api, not here. This function
263+
# credits `balances` because it is the counterpart of create_lock(), which
264+
# debits. bridge_api.create_bridge_transfer() does its own hard debit and
265+
# writes the lock row directly, tracking the refund on
266+
# bridge_transfers.source_debited — a flag this function neither reads nor
267+
# clears. Crediting such a lock therefore reverses a debit the bridge still
268+
# considers outstanding, and the bridge's own void path refunds it again.
269+
if bridge_transfer_id is not None:
270+
return False, {
271+
"error": "Lock is owned by a bridge transfer",
272+
"lock_id": lock_id,
273+
"bridge_transfer_id": bridge_transfer_id,
274+
"hint": (
275+
"Settle via the bridge: complete the transfer, or void it to "
276+
"refund the source. Releasing here would double-credit."
277+
)
278+
}
279+
261280
# Check if unlock time has passed (unless admin override)
262281
if now < unlock_at and released_by != "admin":
263282
return False, {
@@ -641,9 +660,18 @@ def auto_release_expired_locks(
641660

642661
released_count = 0
643662
total_amount = 0
663+
skipped_bridge_owned = 0
644664
errors = []
645665

646666
for lock in expired:
667+
# Bridge-owned locks are settled by bridge_api (complete or void); this
668+
# worker must not touch their balances. Skip rather than call through to
669+
# release_lock() and log an error per lock on every run — an expired
670+
# bridge deposit is a normal state that an operator resolves by voiding.
671+
if lock.bridge_transfer_id is not None:
672+
skipped_bridge_owned += 1
673+
continue
674+
647675
success, result = release_lock(
648676
db_conn,
649677
lock.id,
@@ -663,6 +691,7 @@ def auto_release_expired_locks(
663691
return {
664692
"released_count": released_count,
665693
"total_amount_rtc": total_amount / LOCK_UNIT,
694+
"skipped_bridge_owned": skipped_bridge_owned,
666695
"errors": errors,
667696
"processed_at": now
668697
}
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
"""
2+
Regression tests: lock_ledger must not credit balances for bridge-owned locks.
3+
4+
bridge_api.create_bridge_transfer() hard-debits the source, writes the
5+
lock_ledger row directly (it never calls lock_ledger.create_lock()), and tracks
6+
the refund itself on bridge_transfers.source_debited. lock_ledger.release_lock()
7+
credits unconditionally because it is the counterpart of create_lock(), which
8+
debits — but that pairing does not hold for bridge-produced rows, so releasing
9+
one reverses a debit the bridge still considers outstanding.
10+
11+
Every test here fails on main:
12+
- admin release of a pending deposit un-does the hard debit
13+
- the routine auto-release worker does the same once the lock expires
14+
- release followed by the bridge's own void refunds twice (100 -> 200 RTC)
15+
"""
16+
17+
import os
18+
import sqlite3
19+
import sys
20+
import time
21+
22+
import pytest
23+
24+
NODE_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "node")
25+
if NODE_DIR not in sys.path:
26+
sys.path.insert(0, NODE_DIR)
27+
28+
import bridge_api # noqa: E402
29+
import lock_ledger # noqa: E402
30+
31+
UNIT = bridge_api.BRIDGE_UNIT
32+
33+
34+
@pytest.fixture
35+
def conn(tmp_path):
36+
db = sqlite3.connect(str(tmp_path / "bridge.db"))
37+
cur = db.cursor()
38+
cur.execute(
39+
"CREATE TABLE IF NOT EXISTS balances ("
40+
" miner_id TEXT PRIMARY KEY,"
41+
" amount_i64 INTEGER NOT NULL DEFAULT 0"
42+
")"
43+
)
44+
bridge_api.init_bridge_schema(cur)
45+
lock_ledger.init_lock_ledger_schema(cur)
46+
cur.execute("INSERT INTO balances VALUES (?, ?)", ("alice", 100 * UNIT))
47+
db.commit()
48+
yield db
49+
db.close()
50+
51+
52+
def _balance(db, miner_id="alice"):
53+
row = db.execute(
54+
"SELECT amount_i64 FROM balances WHERE miner_id = ?", (miner_id,)
55+
).fetchone()
56+
return row[0] if row else 0
57+
58+
59+
def _open_deposit(db, amount_rtc=100.0):
60+
"""Open a real deposit: hard-debits alice and writes the bridge-owned lock."""
61+
request = bridge_api.BridgeTransferRequest(
62+
direction="deposit",
63+
source_chain="rustchain",
64+
dest_chain="solana",
65+
source_address="alice",
66+
dest_address="SoLaNaAddr",
67+
amount_rtc=amount_rtc,
68+
bridge_type="lock_mint",
69+
memo=None,
70+
)
71+
ok, _ = bridge_api.create_bridge_transfer(db, request)
72+
assert ok, "fixture: deposit should open"
73+
tx_hash, lock_id = db.execute(
74+
"SELECT t.tx_hash, l.id FROM bridge_transfers t"
75+
" JOIN lock_ledger l ON l.bridge_transfer_id = t.id"
76+
).fetchone()
77+
assert _balance(db) == 0, "fixture: deposit hard-debits the source"
78+
return tx_hash, lock_id
79+
80+
81+
def test_admin_release_does_not_credit_bridge_deposit(conn):
82+
"""Admin release of a bridge lock must not undo the bridge's hard debit."""
83+
_tx_hash, lock_id = _open_deposit(conn)
84+
85+
# released_by="admin" bypasses the unlock timer by design.
86+
ok, result = lock_ledger.release_lock(conn, lock_id, released_by="admin")
87+
88+
assert not ok, "bridge-owned locks must not be released via lock_ledger"
89+
assert "bridge" in result["error"].lower()
90+
assert _balance(conn) == 0, "hard debit must stand while the deposit is pending"
91+
92+
93+
def test_release_then_complete_does_not_mint(conn):
94+
"""The deposit completes on Solana; alice must not also keep her RTC here."""
95+
tx_hash, lock_id = _open_deposit(conn)
96+
97+
lock_ledger.release_lock(conn, lock_id, released_by="admin")
98+
ok, result = bridge_api.update_external_confirmation(conn, tx_hash, "solana_tx_abc", 12)
99+
assert ok and result["status"] == "completed"
100+
101+
# She holds 100 RTC on Solana. Any RustChain balance here is minted supply.
102+
assert _balance(conn) == 0, "completed deposit must not leave a RustChain balance"
103+
104+
105+
def test_release_then_void_refunds_once(conn):
106+
"""Void refunds the source exactly once, not on top of a lock release."""
107+
tx_hash, lock_id = _open_deposit(conn)
108+
109+
lock_ledger.release_lock(conn, lock_id, released_by="admin")
110+
ok, _ = bridge_api.void_bridge_transfer(conn, tx_hash, reason="stuck", voided_by="admin")
111+
assert ok
112+
113+
assert _balance(conn) == 100 * UNIT, "void must refund exactly once (not 200 RTC)"
114+
115+
116+
def test_auto_release_worker_skips_bridge_owned_locks(conn):
117+
"""The routine worker must not credit an expired-but-pending deposit."""
118+
_tx_hash, _lock_id = _open_deposit(conn)
119+
120+
# The 7-day lock expires while the deposit is still pending.
121+
conn.execute("UPDATE lock_ledger SET unlock_at = ?", (int(time.time()) - 1,))
122+
conn.commit()
123+
124+
result = lock_ledger.auto_release_expired_locks(conn)
125+
126+
assert result["released_count"] == 0
127+
assert result["skipped_bridge_owned"] == 1
128+
assert result["errors"] == [], "skipping is a normal state, not a per-run error"
129+
assert _balance(conn) == 0, "worker must not undo the bridge's hard debit"
130+
status, source_debited = conn.execute(
131+
"SELECT status, source_debited FROM bridge_transfers"
132+
).fetchone()
133+
assert (status, source_debited) == ("pending", 1)
134+
135+
136+
def test_standalone_lock_release_still_credits(conn):
137+
"""Guard against over-reach: non-bridge locks keep debit/credit symmetry."""
138+
unlock_at = int(time.time()) + 3600
139+
ok, created = lock_ledger.create_lock(
140+
conn,
141+
miner_id="alice",
142+
amount_i64=40 * UNIT,
143+
lock_type="bridge_deposit",
144+
unlock_at=unlock_at,
145+
)
146+
assert ok, created
147+
assert _balance(conn) == 60 * UNIT, "create_lock debits"
148+
149+
ok, _ = lock_ledger.release_lock(conn, created["lock_id"], released_by="admin")
150+
151+
assert ok, "locks created here are still ours to release"
152+
assert _balance(conn) == 100 * UNIT, "release_lock credits its own locks back"

0 commit comments

Comments
 (0)