Skip to content
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

feat(new-RPC): Add get_wallet RPC for Retrieving Active Wallet #2198

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions mm2src/mm2_main/src/lp_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use http::StatusCode;
use mm2_core::mm_ctx::MmArc;
use mm2_err_handle::prelude::*;
use serde::de::DeserializeOwned;
use serde::Serialize;
use serde_json::{self as json};

cfg_wasm32! {
Expand Down Expand Up @@ -499,3 +500,87 @@ pub async fn get_mnemonic_rpc(ctx: MmArc, req: GetMnemonicRequest) -> MmResult<G
},
}
}

/// `GetWalletError` is an enum representing possible errors that might occur during the `get_wallet` RPC method.
///
/// It includes variants for situations where the wallet name is not initialized or there is an internal error.
#[derive(Debug, Display, Serialize, SerializeErrorType)]
#[serde(tag = "error_type", content = "error_data")]
pub enum GetWalletError {
/// The wallet name has not been initialized in the context.
#[display(fmt = "Wallet name is not initialized")]
WalletNameNotInitialized,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should remove empty line

/// An internal error occurred while processing the request.
#[display(fmt = "Internal error: {}", _0)]
Internal(String),
}

impl HttpStatusCode for GetWalletError {
fn status_code(&self) -> StatusCode {
match self {
GetWalletError::WalletNameNotInitialized => StatusCode::BAD_REQUEST,
GetWalletError::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR,
}
}
}

/// `GetWalletRequest` is a struct representing the parameters for the `get_wallet` RPC method.
///
/// Currently, it does not contain any fields but serves as a placeholder for future extensibility.
/// It ensures compatibility with future versions that might introduce new fields.
#[derive(Debug, Deserialize, Default)]
#[serde(default)]
pub struct GetWalletRequest {
// Currently empty, but may hold future parameters.
}

/// `GetWalletResponse` is a struct representing the response from the `get_wallet` RPC method.
///
/// It currently includes the name of the active wallet. This struct is designed to be extendable in the future
/// to include more wallet properties such as balance, state, or configurations.
#[derive(Debug, Serialize)]
pub struct GetWalletResponse {
/// The name of the currently active wallet.
pub wallet_name: String,
}

/// Retrieves the active wallet name from the context.
///
/// # Parameters
///
/// - `ctx`: The context.
/// - `req`: The request structure (optional).
///
/// # Returns
///
/// A `Result` type containing:
///
/// * `Ok(GetWalletResponse)` - The wallet name.
/// * `MmError<GetWalletError>` - Error indicating the wallet name is not initialized.
Comment on lines +550 to +560
Copy link
Member

@laruh laruh Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no need to add params and return sections in doc comment for get_wallet_rpc function.

we prefer to add doc comms right above the params and return types: GetWalletRequest, GetWalletResponse, GetWalletError

so you can remove Parameters and Returns and Examples :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess @CharlVS was following the get_mnemonic_rpc example, I was trying different things back then when I implemented it. I wanted also to have request/response examples in docs e.g.

/// `GetMnemonicRequest` is a struct representing a request to get a mnemonic.
///
/// It contains a single field, `mnemonic_format`, which is an instance of the `MnemonicFormat` enum.
/// The `#[serde(flatten)]` attribute is used so that the fields of the `MnemonicFormat` enum are included
/// directly in the `GetMnemonicRequest` when it is deserialized, rather than nested under a
/// `mnemonic_format` field.
///
/// # Examples
///
/// For a `GetMnemonicRequest` where the `MnemonicFormat` is `Encrypted`, the JSON representation would be:
/// ```json
/// {
/// "format": "encrypted"
/// }
/// ```
///
/// For a `GetMnemonicRequest` where the `MnemonicFormat` is `PlainText` with a password of "password123", the JSON representation would be:
/// ```json
/// {
/// "format": "plaintext",
/// "password": "password123"
/// }
/// ```
#[derive(Debug, Deserialize)]
pub struct GetMnemonicRequest {
#[serde(flatten)]
pub mnemonic_format: MnemonicFormat,
}
But I guess API docs will be moved from docs repo to KDF and we won't need this. Thanks for trying to keep docs consistent @laruh we need to have a consistent way of writing docs across all modules. I am working on this implementation in a different branch anyway and will be opening another PR this week, just wanted to clarify this point here :)

///
/// # Examples
///
/// ```rust
/// let ctx = MmArc::new(MmCtx::default());
/// let req = GetWalletRequest {}; // This is optional
/// let result = get_wallet_rpc(ctx, req).await;
/// match result {
/// Ok(response) => println!("Wallet Name: {:?}", response.wallet_name),
/// Err(e) => println!("Error: {:?}", e),
/// }
/// ```
pub async fn get_wallet_rpc(
ctx: MmArc,
req: Option<GetWalletRequest>, // Allowing `None` for when `params` is absent
) -> MmResult<GetWalletResponse, GetWalletError> {
// If request is None (params missing), use the default value
let _req = req.unwrap_or_default();
Comment on lines +573 to +578
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can leave param like this, no need to define new variable if you dont use it.

pub async fn get_wallet_rpc(
    ctx: MmArc,
    _req: Option<GetWalletRequest>, // Just pass it through
) -> MmResult<GetWalletResponse, GetWalletError> {
    let wallet_name = ctx
        .wallet_name
        .clone_or(None)
        .ok_or(GetWalletError::WalletNameNotInitialized)?;

    Ok(GetWalletResponse { wallet_name })
}


let wallet_name = ctx
.wallet_name
.clone_or(None)
.ok_or(GetWalletError::WalletNameNotInitialized)?;

Ok(GetWalletResponse { wallet_name })
}
2 changes: 2 additions & 0 deletions mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::lp_ordermatch::{best_orders_rpc_v2, orderbook_rpc_v2, start_simple_ma
stop_simple_market_maker_bot};
use crate::lp_swap::swap_v2_rpcs::{active_swaps_rpc, my_recent_swaps_rpc, my_swap_status_rpc};
use crate::lp_wallet::get_mnemonic_rpc;
use crate::lp_wallet::get_wallet_rpc;
use crate::rpc::rate_limiter::{process_rate_limit, RateLimitContext};
use crate::{lp_stats::{add_node_to_version_stat, remove_node_from_version_stat, start_version_stat_collection,
stop_version_stat_collection, update_version_stat_collection},
Expand Down Expand Up @@ -190,6 +191,7 @@ async fn dispatcher_v2(request: MmRpcRequest, ctx: MmArc) -> DispatcherResult<Re
"get_raw_transaction" => handle_mmrpc(ctx, request, get_raw_transaction).await,
"get_shared_db_id" => handle_mmrpc(ctx, request, get_shared_db_id).await,
"get_staking_infos" => handle_mmrpc(ctx, request, get_staking_infos).await,
"get_wallet" => handle_mmrpc(ctx, request, get_wallet_rpc).await,
Copy link
Member

@laruh laruh Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the purpose is to get wallet info, I suggest to use a clearer RPC name, eg get_wallet_info.

Description: Add get_wallet RPC Method for Retrieving Active Wallet Information

same for GetWalletError and GetWalletRequest types. You can have GetWalletInfoError and GetWalletInfoReq

"max_maker_vol" => handle_mmrpc(ctx, request, max_maker_vol).await,
"my_recent_swaps" => handle_mmrpc(ctx, request, my_recent_swaps_rpc).await,
"my_swap_status" => handle_mmrpc(ctx, request, my_swap_status_rpc).await,
Expand Down
Loading