Skip to content

Updating the dev - #51

Merged
xbuddhi merged 23 commits into
devfrom
main
Aug 22, 2025
Merged

Updating the dev#51
xbuddhi merged 23 commits into
devfrom
main

Conversation

@xbuddhi

@xbuddhi xbuddhi commented Aug 22, 2025

Copy link
Copy Markdown

PR Type

Enhancement


Description

  • Add savings pots feature for organizing Bitcoin funds

  • Implement atomic pot transfers with database transactions

  • Update balance display to show main, pot, and total balances

  • Add comprehensive pot management commands and validation


Diagram Walkthrough

flowchart LR
  A["User Balance"] --> B["Create Pot"]
  B --> C["Transfer to Pot"]
  C --> D["Pot Balance"]
  D --> E["Withdraw from Pot"]
  E --> A
  F["Balance Display"] --> G["Main + Pot + Total"]
Loading

File Walkthrough

Relevant files
Enhancement
types.go
Define SavingsPot database model                                                 

internal/lnbits/types.go

  • Add SavingsPot struct with GORM annotations
  • Include balance validation and foreign key relationships
  • Add auto-timestamps for creation and updates
+10/-0   
balance.go
Enhance balance display with pot information                         

internal/telegram/balance.go

  • Replace API balance fetch with database balance
  • Add pot balance calculation and display
  • Show main, pot, and total balances with USD/LKR values
  • Implement conditional pot information display
+33/-9   
handler.go
Register pot management command handlers                                 

internal/telegram/handler.go

  • Register five new pot management endpoints
  • Add handlers for create, list, add, withdraw, delete operations
  • Apply standard interceptors for authentication and logging
+80/-0   
pots.go
Implement comprehensive pot management system                       

internal/telegram/pots.go

  • Implement complete pot management system
  • Add atomic database transactions for transfers
  • Include validation for pot names and limits
  • Provide CRUD operations with error handling
+366/-0 
Configuration changes
database.go
Add pot model to database migration                                           

internal/telegram/database.go

  • Add SavingsPot model to database auto-migration
  • Ensure pot table creation during initialization
+4/-0     
Documentation
en.toml
Add pot feature translations and help text                             

translations/en.toml

  • Add pot commands to help documentation
  • Update balance message format for main/pot/total display
  • Include detailed help text for all pot operations
+43/-1   

helloscoopa and others added 23 commits July 21, 2025 16:54
Merge pull request #38 from CeyLabs/main
fix: Enhance Telegram ID validation and update username handling in A…
fix: Refactor GetSatPrice to include LKR exchange rate calculation an…
fix: Add error handling for HTTP request in GetUSDToLKRRate function
fix: Refactor inline receive/send handling to support LKR amounts
fix: Update inline send result formatting to include LKR representation
feat: Add user balance API endpoint with wallet-based HMAC security
@coderabbitai

coderabbitai Bot commented Aug 22, 2025

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch main

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@qodo-code-review

Copy link
Copy Markdown

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unit Mismatch

Balance switched from API call to database user.Wallet.Balance / 1000 with a comment stating msat-to-sat conversion, but other pot operations appear to use sat units. Verify wallet balance storage units and ensure consistent conversions across balance display and pot transfer logic to avoid off-by-1000 errors.

// Use database balance (in msat) to reflect pot transfers
balance := user.Wallet.Balance / 1000

log.Infof("[/balance] %s's balance: %s\n", usrStr, utils.FormatSats(balance))
GORM Update Paths

Atomic balance updates reference lnbits.User fields wallet_balance and nested userWallet.Wallet.Balance, which may not match the actual schema. Confirm that wallet_balance is a real column and that the loaded model includes the wallet/balance fields; otherwise updates and checks may silently fail.

return bot.DB.Users.Transaction(func(tx *gorm.DB) error {
	// Get current user balance (within transaction)
	var userWallet lnbits.User
	if err := tx.Where("id = ?", user.ID).First(&userWallet).Error; err != nil {
		return fmt.Errorf("failed to get user: %w", err)
	}

	balance := userWallet.Wallet.Balance
	// Check if sufficient funds
	if balance < amount {
		return fmt.Errorf("insufficient balance. Available: %d sats, Requested: %d sats", balance, amount)
	}

	// Verify the pot exists
	var pot lnbits.SavingsPot
	if err := tx.Where("user_id = ? AND name = ?", user.ID, strings.TrimSpace(potName)).First(&pot).Error; err != nil {
		if err == gorm.ErrRecordNotFound {
			return fmt.Errorf("pot '%s' not found", potName)
		}
		return err
	}

	// Atomically deduct from user balance
	result := tx.Model(&lnbits.User{}).Where("id = ? AND wallet_balance >= ?", user.ID, amount).
		UpdateColumn("wallet_balance", gorm.Expr("wallet_balance - ?", amount))
	if result.Error != nil {
		return fmt.Errorf("failed to update user balance: %w", result.Error)
	}
	if result.RowsAffected == 0 {
		return fmt.Errorf("insufficient balance or user not found")
	}

	// Atomically add to pot balance
	if err := tx.Model(&lnbits.SavingsPot{}).Where("user_id = ? AND name = ?", user.ID, strings.TrimSpace(potName)).
		UpdateColumn("balance", gorm.Expr("balance + ?", amount)).Error; err != nil {
		return fmt.Errorf("failed to update pot balance: %w", err)
SQL Aggregation Check

GetUserTotalPotBalance uses Select("COALESCE(SUM(balance), 0)") with Scan(&totalBalance); without an alias some drivers may fail to scan into a scalar. Consider Select("COALESCE(SUM(balance), 0) AS total").Pluck("total", &totalBalance) or Row().Scan(&totalBalance).

func (bot *TipBot) GetUserTotalPotBalance(user *lnbits.User) (int64, error) {
	var totalBalance int64
	err := bot.DB.Users.Model(&lnbits.SavingsPot{}).
		Where("user_id = ?", user.ID).
		Select("COALESCE(SUM(balance), 0)").
		Scan(&totalBalance).Error
	return totalBalance, err
}

@xbuddhi xbuddhi changed the title up Updating the dev Aug 22, 2025
@xbuddhi
xbuddhi merged commit 9adf6e6 into dev Aug 22, 2025
2 checks passed
@qodo-code-review

Copy link
Copy Markdown

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Update correct balance column and units

The User model in this code does not show a wallet_balance column; balance
appears nested under user.Wallet.Balance and historically in msat. Updating a
non-existent or wrong unit column will silently fail or corrupt balances. Update
the correct wallet table/column and ensure unit consistency (sats vs msats)
before arithmetic.

internal/telegram/pots.go [121-122]

-result := tx.Model(&lnbits.User{}).Where("id = ? AND wallet_balance >= ?", user.ID, amount).
-	UpdateColumn("wallet_balance", gorm.Expr("wallet_balance - ?", amount))
+// Assuming wallet balance is stored in msats on Wallet table and linked by wallet ID
+// Convert sats -> msats for storage arithmetic
+msats := amount * 1000
+result := tx.Model(&lnbits.Wallet{}).
+    Where("id = ? AND balance >= ?", user.Wallet.ID, msats).
+    UpdateColumn("balance", gorm.Expr("balance - ?", msats))
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical inconsistency in balance handling, where user.Wallet.Balance (in msats) is checked while an update is performed on a different wallet_balance column (in sats), which could lead to balance corruption.

High
Refresh wallet before reading balance

This assumes user.Wallet is fully preloaded; if it's nil or stale, division may
panic or show outdated balances. Fetch the latest wallet from the database (and
handle nil safely) to ensure accurate and safe balance reads.

internal/telegram/balance.go [39-40]

-// Use database balance (in msat) to reflect pot transfers
-balance := user.Wallet.Balance / 1000
+// Safely fetch latest wallet balance (msats) from DB and convert to sats
+var freshUser lnbits.User
+if err := bot.DB.Users.Preload("Wallet").Where("id = ?", user.ID).First(&freshUser).Error; err != nil || freshUser.Wallet == nil {
+    log.Errorf("[/balance] failed to load wallet for %s: %v", usrStr, err)
+    bot.trySendMessage(ctx.Sender(), Translate(ctx, "balanceErrorMessage"))
+    return ctx, err
+}
+balance := freshUser.Wallet.Balance / 1000
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the user's balance might be stale and proposes fetching it from the database to ensure the displayed balance is accurate after pot transfers, which is a valid improvement for correctness.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants