Fixed admin keys send in case of admin dispute commands #122
Conversation
WalkthroughThis pull request implements new administrative commands in the CLI. It introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Env
participant DisputeHandler
participant MessageSender
CLI->>Env: Retrieve "NSEC_PRIVKEY"
Env-->>CLI: Return admin private key or error
CLI->>DisputeHandler: Dispatch AdmSettle/AdmCancel command with order_id and admin key
DisputeHandler->>MessageSender: Create dispute message (Action: AdminSettle/AdminCancel) and log details
MessageSender-->>DisputeHandler: Acknowledge message delivery
DisputeHandler-->>CLI: Return command result
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/cli.rs (1)
425-446: Consider extracting duplicated admin key retrieval logic.The admin key retrieval and error handling logic is duplicated across multiple command handlers. Consider extracting this into a helper function to improve maintainability.
+ fn get_admin_key() -> Result<Keys> { + match std::env::var("NSEC_PRIVKEY") { + Ok(id_key) => Keys::parse(&id_key), + Err(e) => { + println!("Failed to get mostro admin private key: {}", e); + std::process::exit(1); + } + } + } + Commands::AdmSettle { order_id } => { - let id_key = match std::env::var("NSEC_PRIVKEY") { - Ok(id_key) => Keys::parse(&id_key)?, - Err(e) => { - println!("Failed to get mostro admin private key: {}", e); - std::process::exit(1); - } - }; + let id_key = get_admin_key()?; execute_admin_settle_dispute(order_id, &id_key, &trade_keys, mostro_key, &client) .await?; } Commands::AdmCancel { order_id } => { - let id_key = match std::env::var("NSEC_PRIVKEY") { - Ok(id_key) => Keys::parse(&id_key)?, - Err(e) => { - println!("Failed to get mostro admin private key: {}", e); - std::process::exit(1); - } - }; + let id_key = get_admin_key()?; execute_admin_cancel_dispute(order_id, &id_key, &trade_keys, mostro_key, &client) .await?; }src/cli/take_dispute.rs (1)
40-70: Fix the log message for settle dispute.The log message incorrectly states "Request of take dispute" when it should indicate a settle dispute action.
println!( - "Request of take dispute {} from mostro pubId {}", + "Request of settle dispute {} from mostro pubId {}", dispute_id, mostro_key.clone() );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cli.rs(2 hunks)src/cli/send_dm.rs(0 hunks)src/cli/take_dispute.rs(2 hunks)src/util.rs(1 hunks)
💤 Files with no reviewable changes (1)
- src/cli/send_dm.rs
🔇 Additional comments (5)
src/util.rs (1)
203-204: LGTM! Simple formatting change.The line split for improved readability doesn't affect functionality.
src/cli.rs (2)
36-36: LGTM! Added wildcard import for take_dispute.This import allows direct access to the newly added admin dispute functions.
448-457: Code now correctly uses admin key for dispute operations.The previous implementation might have been using the wrong keys for admin operations. This change ensures admin dispute commands use the admin private key from the environment variable.
src/cli/take_dispute.rs (2)
8-38: LGTM! Admin cancel dispute implementation.The function correctly implements admin cancellation of disputes using the provided admin keys.
93-93: LGTM! Added helpful debug logging.Adding the identity key to the log output helps with debugging and verification.
There was a problem hiding this comment.
when i do admaddsolver, mostro-cli responds:
Sending AdminAddSolver command for order None to mostro pubId 00000018c1ae3147f9010b5d768ec2b121acf1a67f238be8e2bdd7c1f9eef705
Error: Missing order ID
Bye Bye!
Will fix tonite...it's probably the same thing of admin_take_dispute command.
@arkanoider you mean this error?: I can't take a dispute, mostro sends: "cant_do":"invalid_pubkey" What I'm seeing is that mostro-cli is creating a new identity_keys to take the dispute, not using the one I set in the db. but that user is in db with another identity_keys |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cli.rs (1)
36-36: Consider importing only the specific functions needed.While importing all functions from the
take_disputemodule works, importing only the specific functions used would be more precise and maintainable.-use take_dispute::*; +use take_dispute::{execute_admin_settle_dispute, execute_admin_cancel_dispute, execute_take_dispute};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cli.rs(4 hunks)src/cli/get_dm.rs(2 hunks)
🔇 Additional comments (7)
src/cli/get_dm.rs (3)
16-16: LGTM: Added admin parameter to support admin operations.The addition of the
adminboolean parameter allows the function to differentiate between regular user and admin operations, which aligns with the PR objective of fixing admin commands.
20-36: Implementation correctly handles admin vs regular user paths.The conditional logic appropriately handles different retrieval methods based on the user's admin status. For admin users, the function retrieves the admin private key from the environment variable, addressing the issue with admin dispute commands.
65-76: Good addition of Dispute payload handling.The implementation properly handles and displays dispute information, including optional fields like token and additional info. This complements the changes for admin dispute commands.
src/cli.rs (4)
162-170: LGTM: Added GetAdminDm command for admin operations.The new command follows the structure of existing commands and includes appropriate parameters, enabling admin users to retrieve direct messages specifically for dispute handling.
378-382: Implementation correctly passes admin status to execute_get_dm.The function calls properly pass the admin status parameter (false for regular GetDm, true for GetAdminDm), ensuring that the appropriate logic is executed for each command type.
438-459: LGTM: Fixed admin key handling for AdmSettle and AdmCancel commands.The implementation now correctly retrieves the admin private key from the environment variable instead of using identity keys, which addresses the reported issue with admin commands. The error handling for missing keys is also appropriate.
461-470: LGTM: Fixed admin key handling for AdmTakeDispute command.The implementation now correctly retrieves the admin private key from the environment variable, addressing the reported "invalid_pubkey" error mentioned in the PR comments. This ensures that the correct admin key is used for taking disputes.
|
Closing this in favor of #123 |
@grunch , @Catrya
this is the complementary fix for
mostro-clito have disputes working.Summary by CodeRabbit