feat(new-RPC): Add get_wallet RPC for Retrieving Active Wallet#2198
feat(new-RPC): Add get_wallet RPC for Retrieving Active Wallet#2198CharlVS wants to merge 1 commit intoGLEECBTC:devfrom
get_wallet RPC for Retrieving Active Wallet#2198Conversation
- Implemented the `get_wallet` function to retrieve the active wallet from the context (Currently only includes wallet_name). - Introduced a `GetWalletRequest` struct for future extensibility with request parameters. - Updated the dispatcher to handle the `get_wallet` RPC method correctly, including parameter handling.
CharlVS
changed the title
get_wallet RPC Method for Retrieving Active Walletget_wallet RPC for Retrieving Active Wallet
laruh
left a comment
laruh
left a comment
There was a problem hiding this comment.
Thanks, for the PR. did a brief review
| /// # 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. |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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.
https://github.com/KomodoPlatform/komodo-defi-framework/blob/7723b50fcb533ebe12b621eb862b8a958113696b/mm2src/mm2_main/src/lp_wallet.rs#L330-L357 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 :)
| /// The wallet name has not been initialized in the context. | ||
| #[display(fmt = "Wallet name is not initialized")] | ||
| WalletNameNotInitialized, | ||
|
|
| 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(); |
There was a problem hiding this comment.
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 })
}
| "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, |
There was a problem hiding this comment.
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
|
Superseded by #2202 |
3 participants
Description: Add
get_walletRPC Method for Retrieving Active Wallet InformationThis PR introduces the
get_walletRPC method, which allows clients to retrieve information about the currently active wallet. While the initial implementation returns the wallet name, the design is extensible to support additional wallet properties in future updates.Key Features:
get_wallet_rpcMethod: Retrieves the active wallet's name and can be expanded to return more wallet-specific data.GetWalletRequeststruct is currently empty but prepared for future request parameters.GetWalletResponsestruct currently includes the wallet name but is designed to be expanded for future needs.