Conversation
Reserve withdrawals before broadcast, journal deposits and transfers transactionally, and migrate legacy balances and addresses with quarantine for unresolved withdrawals. Update the RPC and Telegram adapters and deployment requirements for this accounting model.
Keep uncertain spends locked, require manual transaction matching, recover deposit conflicts and envelope replays, and protect migrations with exclusive database ownership. Preserve exact RPC amounts and normalize legacy null transaction IDs before indexing.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
codeant-ai
Bot
added
the
size:XXL
User descriptionReplace floating-point, separate balance writes with integer groth and MongoDB transactions. Durable deposit, tip, envelope and withdrawal records make replay and recovery idempotent. Two commits separate the accounting/migration foundation from recovery and precision hardening. Withdrawals reserve funds before broadcast, retain uncertain outcomes for manual review, and refund only verified pre-spend rejections. Recovery does not assign transactions from approximate matches. The changes also reconcile missing conflicted deposits, recover zero-balance envelope replays, preserve exact RPC amounts, prevent overlapping bot processes, and normalize legacy null transaction IDs before creating the unique index. RPC/Telegram adapters and deployment requirements are updated accordingly; username-based tips are disabled in favor of reply-based recipient IDs. Validation:
Draft pending live MongoDB replica-set, Firo and Telegram integration testing. Unit tests use an in-memory database fake. Historical balances have not been reconciled against a production wallet. Deployment requires a MongoDB replica set, backup and offline legacy migration. Uncertain withdrawals and stale ownership records after a forced shutdown require operator review, as documented in the README. Overlaps the accounting proposals in #5 and #6; it does not depend on them. #7, #8 and #9 touch the same flows and require adaptation/rebasing before combining. CodeAnt-AI DescriptionMake tipbot accounting precise, transactional, and recoverable What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later. Talking to CodeAnt AIGot a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health. |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| def get_txs_list(self, page_size=1000): | ||
| transactions = [] | ||
| skip = 0 | ||
|
|
||
| # ponytail: full history scan favors correctness; move to a persisted | ||
| # listsinceblock cursor when wallet history makes this measurably slow. | ||
| while True: | ||
| response = self._rpc( | ||
| "listtransactions", | ||
| ["*", page_size, skip], | ||
| request_id=2, | ||
| ) | ||
| if response.get("error"): | ||
| return response | ||
|
|
||
| page = response["result"] | ||
| if not isinstance(page, list): | ||
| raise FiroTransportError("listtransactions returned a non-list result") | ||
| transactions.extend(page) | ||
| if len(page) < page_size: | ||
| response["result"] = transactions | ||
| return response | ||
| skip += len(page) |
There was a problem hiding this comment.
Suggestion: page_size accepts zero or negative values, so an empty page never satisfies the exit condition and the loop repeatedly requests the same offset forever. [logic error]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** api/firo_wallet_api.py
**Line:** 65:87
**Comment:**
*Logic Error: `page_size` accepts zero or negative values, so an empty page never satisfies the exit condition and the loop repeatedly requests the same offset forever.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| self.create_send_tips_image( | ||
| self.user_id, | ||
| "{0:.8f}".format(amount), | ||
| receiver['first_name'], |
There was a problem hiding this comment.
Suggestion: The transfer commits before this lookup; a verified legacy recipient without first_name raises KeyError after receiving funds, so confirmations are never sent. [error handling]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** tipbot.py
**Line:** 2168:2168
**Comment:**
*Error Handling: The transfer commits before this lookup; a verified legacy recipient without `first_name` raises `KeyError` after receiving funds, so confirmations are never sent.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| raise RuntimeError("getsparkdefaultaddress returned invalid data") | ||
| users = self.col_users.find({"Address": {"$in": default_addresses}}) | ||
| for user in users: |
There was a problem hiding this comment.
Suggestion: create_user_wallet() returns one address string, so this loop appends individual characters as separate addresses and corrupts affected users. [type error]
Assessment: 🔴 Critical · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** update_address.py
**Line:** 31:33
**Comment:**
*Type Error: `create_user_wallet()` returns one address string, so this loop appends individual characters as separate addresses and corrupts affected users.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixRequire a positive integer before calling listtransactions so a zero-sized page cannot stall pagination. Cover rejected values without making an RPC request.
Fall back to the recipient Telegram ID when a legacy profile has no display name, so completed transfers still send both confirmations. Cover missing, empty and existing names without changing replay-safe balances.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c3f10ee6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Replace floating-point, separate balance writes with integer groth and MongoDB transactions. Durable deposit, tip, envelope and withdrawal records make replay and recovery idempotent. The commits separate the accounting/migration foundation from recovery and precision hardening.
Withdrawals reserve funds before broadcast, retain uncertain outcomes for manual review, and refund only verified pre-spend rejections. Recovery does not assign transactions from approximate matches. The changes also reconcile missing conflicted deposits, recover zero-balance envelope replays, preserve exact RPC amounts, prevent overlapping bot processes, and normalize legacy null transaction IDs before creating the unique index. RPC/Telegram adapters and deployment requirements are updated accordingly; username-based tips are disabled in favor of reply-based recipient IDs.
Wallet lookup failures now flag individual deposits for review without blocking reconciliation. Withdrawal reviews refresh when evidence changes, deposit address validation runs only for
/deposit, and startup failures exit nonzero.Validation:
python -B -m unittest discover -s tests -q, 58 passed.python -B -W ignore::DeprecationWarning -m unittest discover -s tests -q, 58 passed.py_compilepassed for all three production Python files and four test modules.git diff --check origin/masterand current-basegit merge-treepassed.Live MongoDB replica-set, Firo and Telegram integration testing remains unperformed. Unit tests use an in-memory database fake. Historical balances have not been reconciled against a production wallet.
Deployment requires a MongoDB replica set, backup and offline legacy migration. Uncertain withdrawals and stale ownership records after a forced shutdown require operator review, as documented in the README.
Overlaps the accounting proposals in #5 and #6; it does not depend on them. #7, #8 and #9 touch the same flows and require adaptation/rebasing before combining.