-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: removed error when inserting a new user in db #406
Conversation
Warning Rate limit exceeded@grunch has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a refactoring of user creation functionality across multiple files. The primary change is in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/admin_add_solver.rs (1)
45-45
: Returned value mismatches the loggingSince
add_new_user
returnsanyhow::Result<()>
rather than a user object, ther
in the success arm will just be()
. If detailed logging of the newly added solver is needed, consider returning aUser
struct or re-querying the database after insertion.src/app.rs (1)
111-111
: No success-path acknowledgmentThe success path is not logged or otherwise acknowledged. Consider adding a log or remark upon successful user creation (e.g.,
info!("User successfully created")
) to confirm that the operation completed as intended.src/db.rs (1)
331-350
: Public key format validation removedWhile the function previously validated the public key format, the new implementation no longer checks it. To maintain consistency throughout the codebase, consider reintroducing the same validation logic here or delegate it to a higher-level layer to ensure correct key formats across all insertion points. Otherwise, invalid keys could be stored in the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/db.rs
(1 hunks)
🔇 Additional comments (2)
src/db.rs (2)
355-361
: LGTM! Improved error handling
The error handling has been enhanced to:
- Include the actual database error message for better debugging
- Add tracing logs for successful user creation
331-334
: Verify public key validation
The public key format validation (32-bytes hex) was removed from this function while other functions like update_user_trade_index
still perform this validation. This inconsistency could lead to data integrity issues.
Let's verify if the validation is handled elsewhere:
✅ Verification successful
Public key validation is handled at conversion level
Based on the code analysis, the public key validation appears to be handled at the type conversion level through PublicKey::from_bech32()
method, which is used when creating new users (as seen in admin_add_solver.rs
). The hex format validation happens during this conversion, making the explicit validation in add_new_user
redundant.
Key findings:
- Public keys are converted using
PublicKey::from_bech32()
which includes validation - The conversion method returns a
Result
type, ensuring invalid keys are caught - The hex format is obtained through the
to_hex()
method after successful validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if User struct validates the public key format
# Search for User struct definition and any validation logic
ast-grep --pattern 'struct User {
$$$
pubkey: $_,
$$$
}'
# Search for any validation functions related to User
rg -A 5 'impl.*User'
Length of output: 87
Script:
#!/bin/bash
# Let's try a different approach to find User-related code
# Find files containing "User" struct definition
rg -l "struct User"
# Search for public key validation patterns
rg -B2 -A2 "pubkey.*hex|hex.*pubkey"
# Search for any validation or constructor methods
rg -B2 -A2 "fn.*validate|new.*User|User::new"
# Look for files that might contain the User model
fd -t f "user" -t f "model"
Length of output: 3230
Script:
#!/bin/bash
# Let's find the User struct definition and its implementation
rg -B5 -A10 "struct User"
# Also check for any validation in PublicKey type since we see it's used
rg -B2 -A5 "impl.*PublicKey|struct PublicKey"
# Look for validation in the from_bech32 method since it's used for public key conversion
rg -B2 -A5 "from_bech32"
Length of output: 706
The query uses PostgreSQL-style positional parameters ($1, $2) while the codebase uses SQLite. This will cause runtime errors as SQLite expects ?1, ?2 style parameters. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
@grunch, @Catrya
this fixes the error when a new user is inserted in db.
Summary by CodeRabbit
New Features
User
object for user creation.Bug Fixes
Refactor