Add 24-hour red envelope expiry with automatic refund#7
Add 24-hour red envelope expiry with automatic refund#7
Conversation
- Use atomic find_one_and_update with $gte guards for tips, withdrawals, and red envelope creation to prevent spending more than available balance - Use $inc instead of $set for all balance mutations to prevent race conditions between concurrent operations - Use Decimal arithmetic for all FIRO calculations to prevent floating point precision errors (stored as float in MongoDB for backward compatibility with existing database) - Fix withdrawal fee logic: atomic deduct full amount, rollback exact amount on RPC failure - Fix catch_envelope to atomically deduct from envelope remains with $gte guard, preventing over-distribution - Fix update_balance withdrawal handler to properly handle locked amount reduction without leaving stale locked values - Store MongoClient as self.client for transaction session access - Replace print() with structured logging throughout - Replace deprecated insert() calls with insert_one() https://claude.ai/code/session_01K7FrkbpYpqJAoJfcx67osm
Replace float storage with BSON Decimal128 via a custom pymongo codec that transparently converts between Python Decimal and Decimal128. This eliminates floating-point drift from accumulated $inc operations. The codec is registered on the database connection so all collections automatically serialize Decimal values as Decimal128 and deserialize them back. The to_decimal() helper handles reading both legacy float values (pre-migration) and Decimal128 values (post-migration). Add migrate_to_decimal128.py script to batch-convert existing float Balance/Locked fields to Decimal128. Supports dry-run mode by default and logs every change. The bot works correctly both before and after migration (graceful degradation). https://claude.ai/code/session_01K7FrkbpYpqJAoJfcx67osm
Red envelopes now expire after 24 hours. Unclaimed funds are automatically returned to the creator's balance with a DM notification. The group message button is cleaned up on expiry. - Add expire_envelopes() background task running every 60 seconds - Add expiry guard in catch_envelope() to reject catches on expired envelopes - Add "expired" field to envelope documents for explicit state tracking - Atomic find_one_and_update prevents double-refund races https://claude.ai/code/session_01K7FrkbpYpqJAoJfcx67osm
Ensures envelopes that expired while the bot was offline are immediately processed before the polling loop begins, closing the 60-second gap before the first scheduled run. https://claude.ai/code/session_01K7FrkbpYpqJAoJfcx67osm
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale remains value causes over-refund on expiry
- Changed to ReturnDocument.BEFORE and read remains from the atomically returned document, ensuring the refund uses the actual value at update time rather than a stale cursor value.
Or push these changes by commenting:
@cursor push 090ad7b211
Preview (090ad7b211)
diff --git a/tipbot.py b/tipbot.py
--- a/tipbot.py
+++ b/tipbot.py
@@ -543,13 +543,8 @@
})
for envelope in expired:
- remains = to_decimal(envelope['remains'])
- if remains <= 0:
- continue
-
- store_remains = decimal_to_store(remains)
-
# Atomically zero out the envelope remains and mark as expired
+ # Use BEFORE to get the actual remains at update time (avoids race with catch_envelope)
updated = self.col_envelopes.find_one_and_update(
{
"_id": envelope['_id'],
@@ -563,24 +558,31 @@
"expired_at": int(datetime.datetime.now().timestamp())
}
},
- return_document=ReturnDocument.AFTER
+ return_document=ReturnDocument.BEFORE
)
if not updated:
continue
+ # Read the actual remains from the document at update time
+ remains = to_decimal(updated['remains'])
+ if remains <= 0:
+ continue
+
+ store_remains = decimal_to_store(remains)
+
# Credit the creator
self.col_users.update_one(
- {"_id": envelope['creator_id']},
+ {"_id": updated['creator_id']},
{"$inc": {"Balance": store_remains}}
)
logger.info("Envelope %s expired: refunded %s FIRO to user %s",
- envelope['_id'], remains, envelope['creator_id'])
+ updated['_id'], remains, updated['creator_id'])
# Notify the creator
try:
self.bot.send_message(
- envelope['creator_id'],
+ updated['creator_id'],
"<b>Your red envelope expired.</b>\n"
"<b>%s FIRO</b> unclaimed funds have been returned to your balance." %
"{0:.8f}".format(remains),
@@ -588,11 +590,11 @@
)
except Exception as exc:
logger.error("Failed to notify envelope creator %s: %s",
- envelope['creator_id'], exc)
+ updated['creator_id'], exc)
# Clean up the group message button
try:
- self.bot.delete_message(envelope['group_id'], envelope['msg_id'])
+ self.bot.delete_message(updated['group_id'], updated['msg_id'])
except Exception:
pass| # Credit the creator | ||
| self.col_users.update_one( | ||
| {"_id": envelope['creator_id']}, | ||
| {"$inc": {"Balance": store_remains}} |
There was a problem hiding this comment.
Stale remains value causes over-refund on expiry
High Severity
The expire_envelopes method reads remains from the initial find() cursor and uses that value to credit the creator. Since expire_envelopes runs in a background thread while catch_envelope runs in the main thread, a catch can reduce remains between the cursor read and the find_one_and_update. The atomic update zeroes out the current (lower) remains, but the refund uses the stale (higher) value — creating FIRO out of thin air. Using ReturnDocument.BEFORE instead of ReturnDocument.AFTER and reading remains from the returned document would give the actual balance at update time.



Summary
Test plan
Note
High Risk
Changes core balance-handling logic (deposits, withdrawals, tips, envelopes) and introduces a DB type migration from floats to
Decimal128, which could impact monetary correctness if any edge cases or partial migrations occur.Overview
Implements 24-hour red envelope expiry: envelopes older than
ENVELOPE_EXPIRY_SECONDSare markedexpired, remaining funds are atomically zeroed and refunded to the creator, the creator is DM’d, and the original group message is deleted; cleanup runs on startup and every 60s.Migrates monetary handling to precise decimals: adds
migrate_to_decimal128.pyfor one-time conversion ofusers.Balance/Locked,envelopes.amount/remains, andtip_logs.amountto BSONDecimal128, and updatestipbot.pyto useDecimal+ a MongoDB codec so reads/writes are quantized to 8dp.Hardens balance mutations by switching tips, deposits, withdrawals, envelopes, and expiry refunds to
$inc/find_one_and_updatewith$gteguards to reduce race conditions and double-processing; also replaces ad-hocprint/tracebackwith structured logging.Written by Cursor Bugbot for commit 6f60ea3. This will update automatically on new commits. Configure here.