Skip to content

feat(wallet): add accounts.rename call #1583

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.

Sign up for GitHub

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

Merged
sdbondi merged 1 commit into from
Sep 24, 2025
Merged

feat(wallet): add accounts.rename call #1583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions applications/tari_walletd/src/handlers/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ use tari_wallet_daemon_client::{
AccountsGetBalancesResponse,
AccountsListRequest,
AccountsListResponse,
AccountsRenameRequest,
AccountsRenameResponse,
AccountsTransferRequest,
AccountsTransferResponse,
BalanceEntry,
Expand Down Expand Up @@ -213,6 +215,19 @@ pub async fn handle_set_default(
Ok(AccountSetDefaultResponse {})
}

pub async fn handle_rename(
context: &HandlerContext,
token: Option<&Bearer>,
req: AccountsRenameRequest,
) -> Result<AccountsRenameResponse, anyhow::Error> {
context.check_auth(token, &[JrpcPermission::Admin])?;
let sdk = context.wallet_sdk();
let account = get_account(&req.account, &sdk.accounts_api())?;
sdk.accounts_api()
.rename_account(account.component_address(), &req.new_name)?;
Ok(AccountsRenameResponse {})
}
Comment on lines +218 to +229

@coderabbitai coderabbitai Bot Sep 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Map duplicate-name error to a client error (consistency with create).

rename_account may return AccountNameAlreadyExists. Like handle_create, map it to invalid_request instead of propagating a generic error.

Apply this diff:

-    sdk.accounts_api()
-        .rename_account(account.component_address(), &req.new_name)?;
+    sdk.accounts_api()
+        .rename_account(account.component_address(), &req.new_name)
+        .map_err(|e| {
+            if e.is_name_exists_error() {
+                invalid_request(e)
+            } else {
+                general_error(e)
+            }
+        })?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub async fn handle_rename(
context: &HandlerContext,
token: Option<&Bearer>,
req: AccountsRenameRequest,
) -> Result<AccountsRenameResponse, anyhow::Error> {
context.check_auth(token, &[JrpcPermission::Admin])?;
let sdk = context.wallet_sdk();
let account = get_account(&req.account, &sdk.accounts_api())?;
sdk.accounts_api()
.rename_account(account.component_address(), &req.new_name)?;
Ok(AccountsRenameResponse {})
}
pub async fn handle_rename(
context: &HandlerContext,
token: Option<&Bearer>,
req: AccountsRenameRequest,
) -> Result<AccountsRenameResponse, anyhow::Error> {
context.check_auth(token, &[JrpcPermission::Admin])?;
let sdk = context.wallet_sdk();
let account = get_account(&req.account, &sdk.accounts_api())?;
sdk.accounts_api()
.rename_account(account.component_address(), &req.new_name)
.map_err(|e| {
if e.is_name_exists_error() {
invalid_request(e)
} else {
general_error(e)
}
})?;
Ok(AccountsRenameResponse {})
}
🤖 Prompt for AI Agents 
In applications/tari_walletd/src/handlers/accounts.rs around lines 218 to 229,
the call to sdk.accounts_api().rename_account can return
AccountNameAlreadyExists but is currently propagated as a generic error; catch
the specific AccountNameAlreadyExists error and convert it into the same
client-facing invalid_request error used by handle_create (preserving a clear
message like "account name already exists"), returning that client error instead
of propagating the raw error; keep other errors propagated as before.


pub async fn handle_list(
context: &HandlerContext,
token: Option<&Bearer>,
Expand Down
1 change: 1 addition & 0 deletions applications/tari_walletd/src/jrpc_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ async fn handler(
},
"stealth_transfer" => call_handler(context, value, token, accounts::handle_stealth_transfer).await,
"set_default" => call_handler(context, value, token, accounts::handle_set_default).await,
"rename" => call_handler(context, value, token, accounts::handle_rename).await,

@coderabbitai coderabbitai Bot Sep 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Route wiring for accounts.rename looks correct; verify validation/uniqueness.
Handler enforces Admin. Please confirm SDK/storage rejects empty or duplicate names and surfaces meaningful errors to JSON-RPC.

🏁 Script executed:

#!/bin/bash
# Locate rename_account implementation and callers for validation/uniqueness checks
rg -n -C3 --type=rust '\brename_account\s*\('
rg -n -C3 --type=rust 'struct\s+AccountsRenameRequest|enum\s+AccountsRenameRequest'
rg -n -C3 --type=rust 'UNIQUE|unique|accounts_get_by_name'

Length of output: 35011

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== crates/wallet/sdk/src/apis/accounts.rs ==="
sed -n '1,420p' crates/wallet/sdk/src/apis/accounts.rs || true
echo
echo "=== applications/tari_walletd/src/handlers/accounts.rs ==="
sed -n '1,360p' applications/tari_walletd/src/handlers/accounts.rs || true
echo
echo "=== crates/wallet/sdk/src/storage.rs ==="
sed -n '1,260p' crates/wallet/sdk/src/storage.rs || true
echo
echo "=== crates/wallet/storage_sqlite/src/reader.rs (accounts_get_by_name) ==="
sed -n '380,460p' crates/wallet/storage_sqlite/src/reader.rs || true
echo
echo "=== crates/wallet/storage_sqlite/src/writer.rs (accounts_update) ==="
sed -n '1,320p' crates/wallet/storage_sqlite/src/writer.rs || true
echo
echo "=== crates/wallet/sdk/src/apis/errors.rs (if exists) & AccountsApiError in accounts.rs ==="
rg -n "enum\s+AccountsApiError" || true
rg -n "AccountsApiError" crates/wallet/sdk/src -n -S || true

Length of output: 56713

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Search for accounts_update definitions/calls ==="
rg -n --type=rust '\bfn\s+accounts_update\b' || true
rg -n --type=rust '\baccounts_update\s*\(' || true

echo
echo "=== Show surrounding code for writer::accounts_update (if found) ==="
FILES=$(rg -n --type=rust '\bfn\s+accounts_update\b' -g 'crates/**' -g 'applications/**' -g '*/src/**' -S --no-line-number || true)
if [ -n "$FILES" ]; then
  for f in $(echo "$FILES" | cut -d: -f1 | sort -u); do
    echo "----- $f -----"
    sed -n '1,240p' "$f" || true
    echo
  done
else
  echo "No explicit fn accounts_update found; searching for trait method implementation 'accounts_update' in WalletStoreWriter impls..."
  rg -n --type=rust 'accounts_update' -S || true
fi

echo
echo "=== Search for AccountUpdate struct/type ==="
rg -n --type=rust 'struct\s+AccountUpdate\b' || true
rg -n --type=rust 'pub struct\s+AccountUpdate\b' || true
rg -n --type=rust '\benum\s+AccountUpdate\b' || true
rg -n --type=rust '\bAccountUpdate\b' -S || true

echo
echo "=== Show models file(s) that define Account/AccountUpdate ==="
rg -n --type=rust 'struct\s+Account\b' -S || true
rg -n --type=rust 'mod models' -S || true
rg -n --hidden --no-ignore -g 'crates/**/src/**' 'AccountUpdate' -n -S || true

Length of output: 21275

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== AccountUpdate struct ==="
sed -n '1,240p' crates/wallet/sdk/src/models/account.rs || true
echo
echo "=== storage_sqlite writer.accounts_update implementation ==="
sed -n '500,660p' crates/wallet/storage_sqlite/src/writer.rs || true
echo
echo "=== storage_sqlite models/account.rs ==="
sed -n '1,240p' crates/wallet/storage_sqlite/src/models/account.rs || true
echo
echo "=== Diesel schema for accounts (schema.rs) ==="
rg -n --hidden -S "table!.*accounts" || true
sed -n '1,240p' crates/wallet/storage_sqlite/src/schema.rs || true
echo
echo "=== storage_sqlite tests for accounts ==="
sed -n '1,240p' crates/wallet/storage_sqlite/tests/accounts.rs || true

Length of output: 16863

Reject empty/duplicate account names on rename (server-side)

rename_account (crates/wallet/sdk/src/apis/accounts.rs) calls accounts_update directly and does not validate new_name or check for existing names; the storage writer accepts empty/duplicate names. Add checks in rename_account to reject new_name.trim().is_empty() and to call tx.accounts_get_by_name(name).optional()? and return AccountsApiError::AccountNameAlreadyExists on duplicates. Update applications/tari_walletd/src/handlers/accounts.rs::handle_rename to map that error to an invalid_request (same mapping as create_account) so JSON‑RPC returns a clear client error.

🤖 Prompt for AI Agents 
In applications/tari_walletd/src/jrpc_server.rs around line 146 (affects rename
flow), the server currently allows empty or duplicate account names on rename;
update the SDK method crates/wallet/sdk/src/apis/accounts.rs::rename_account to
validate new_name by rejecting new_name.trim().is_empty(), and before calling
accounts_update call tx.accounts_get_by_name(name).optional()? to detect
existing names and return AccountsApiError::AccountNameAlreadyExists on
duplicates; then update
applications/tari_walletd/src/handlers/accounts.rs::handle_rename to map
AccountsApiError::AccountNameAlreadyExists to an invalid_request JSON‑RPC error
the same way create_account does so the client receives a clear validation
error.

"create_free_test_coins" => {
call_handler(context, value, token, accounts::handle_create_free_test_coins).await
},
Expand Down
2 changes: 1 addition & 1 deletion bindings/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@tari-project/typescript-bindings",
"version": "1.17.0",
"version": "1.17.1",
"description": "TypeScript types synchronized to the Tari Ootle Rust codebase",
"homepage": "https://github.com/tari-project/tari-ootle#readme",
"bugs": {
Expand Down
4 changes: 4 additions & 0 deletions bindings/src/types/wallet-daemon-client/AccountsRenameRequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { ComponentAddressOrName } from "./ComponentAddressOrName";

export type AccountsRenameRequest = { account: ComponentAddressOrName; new_name: string };
3 changes: 3 additions & 0 deletions bindings/src/types/wallet-daemon-client/AccountsRenameResponse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.

export type AccountsRenameResponse = Record<string, never>;
2 changes: 2 additions & 0 deletions bindings/src/wallet-daemon-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export * from "./types/wallet-daemon-client/TransactionSubmitManifestRequest";
export * from "./types/wallet-daemon-client/ConfidentialTransferResponse";
export * from "./types/wallet-daemon-client/TransactionGetResponse";
export * from "./types/wallet-daemon-client/SubstatesGetResponse";
export * from "./types/wallet-daemon-client/AccountsRenameRequest";
export * from "./types/wallet-daemon-client/AuthRevokeTokenRequest";
export * from "./types/wallet-daemon-client/ProofsGenerateRequest";
export * from "./types/wallet-daemon-client/MintFaucetNftResponse";
Expand Down Expand Up @@ -95,6 +96,7 @@ export * from "./types/wallet-daemon-client/TemplatesGetResponse";
export * from "./types/wallet-daemon-client/AccountsGetBalancesRequest";
export * from "./types/wallet-daemon-client/AccountsCreateFreeTestCoinsRequest";
export * from "./types/wallet-daemon-client/AccountGetByKeyIndexRequest";
export * from "./types/wallet-daemon-client/AccountsRenameResponse";
export * from "./types/wallet-daemon-client/KeysCreateResponse";
export * from "./types/wallet-daemon-client/AccountsCreateOrGetResponse";
export * from "./types/wallet-daemon-client/AccountInfo";
Expand Down
2 changes: 1 addition & 1 deletion clients/javascript/wallet_daemon_client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@tari-project/wallet_jrpc_client",
"version": "1.9.0",
"version": "1.9.1",
"description": "Tari wallet JSON-RPC client library",
"homepage": "https://github.com/tari-project/tari-ootle#readme",
"bugs": {
Expand Down
6 changes: 5 additions & 1 deletion clients/javascript/wallet_daemon_client/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
AccountsGetBalancesRequest,
AccountsGetBalancesResponse,
AccountsListRequest,
AccountsListResponse,
AccountsListResponse, AccountsRenameRequest, AccountsRenameResponse,
AccountsTransferRequest,
AccountsTransferResponse,
AuthGetAllJwtRequest,
Expand Down Expand Up @@ -169,6 +169,10 @@ export class WalletDaemonClient {
return this.__invokeRpc("accounts.create", params);
}

public accountsRename(params: AccountsRenameRequest): Promise<AccountsRenameResponse> {
return this.__invokeRpc("accounts.rename", params);
}

public accountsClaimBurn(params: ClaimBurnRequest): Promise<ClaimBurnResponse> {
return this.__invokeRpc("accounts.claim_burn", params);
}
Expand Down
11 changes: 11 additions & 0 deletions clients/wallet_daemon_client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ use crate::{
AccountsGetBalancesResponse,
AccountsListRequest,
AccountsListResponse,
AccountsRenameRequest,
AccountsRenameResponse,
AuthGetAllJwtRequest,
AuthGetAllJwtResponse,
AuthRevokeTokenRequest,
Expand Down Expand Up @@ -318,6 +320,15 @@ impl WalletDaemonClient {
.await
}

pub async fn accounts_rename(
&mut self,
account: ComponentAddressOrName,
new_name: String,
) -> Result<AccountsRenameResponse, WalletDaemonClientError> {
self.send_request("accounts.rename", &AccountsRenameRequest { account, new_name })
.await
}

pub async fn accounts_transfer<T: Borrow<AccountsTransferRequest>>(
&mut self,
req: T,
Expand Down
12 changes: 12 additions & 0 deletions clients/wallet_daemon_client/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,18 @@ pub struct AccountSetDefaultRequest {
#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export, export_to = "wallet-daemon-client/"))]
pub struct AccountSetDefaultResponse {}

#[derive(Debug, Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export, export_to = "wallet-daemon-client/"))]
pub struct AccountsRenameRequest {
#[serde(deserialize_with = "string_or_struct")]
pub account: ComponentAddressOrName,
pub new_name: String,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export, export_to = "wallet-daemon-client/"))]
pub struct AccountsRenameResponse {}

#[derive(Debug, Clone, Deserialize, Serialize)]
#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export, export_to = "wallet-daemon-client/"))]
pub struct AccountsTransferRequest {
Expand Down
12 changes: 11 additions & 1 deletion crates/wallet/sdk/src/apis/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<'a, TStore: WalletStore, TNetworkInterface> AccountsApi<'a, TStore, TNetwor
pub fn update_account(
&self,
account_address: &ComponentAddress,
update: AccountUpdate,
update: AccountUpdate<'_>,
) -> Result<(), AccountsApiError> {
self.store.with_write_tx(|tx| {
tx.accounts_update(account_address, update)?;
Expand Down Expand Up @@ -282,6 +282,16 @@ impl<'a, TStore: WalletStore, TNetworkInterface> AccountsApi<'a, TStore, TNetwor
Ok(exists)
}

pub fn rename_account(&self, account_addr: &ComponentAddress, new_name: &str) -> Result<(), AccountsApiError> {
let mut tx = self.store.create_write_tx()?;
tx.accounts_update(account_addr, AccountUpdate {
name: Some(new_name),
..Default::default()
})?;
tx.commit()?;
Ok(())
}

Comment on lines +285 to +294

@coderabbitai coderabbitai Bot Sep 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent duplicate account names and validate input on rename.

Currently, rename does not check for an existing account with the same name. This can lead to duplicates or a DB constraint violation if a unique index exists. Also trim the new name and allow no-op when unchanged.

Apply this diff:

 pub fn rename_account(&self, account_addr: &ComponentAddress, new_name: &str) -> Result<(), AccountsApiError> {
-        let mut tx = self.store.create_write_tx()?;
-        tx.accounts_update(account_addr, AccountUpdate {
-            name: Some(new_name),
-            ..Default::default()
-        })?;
-        tx.commit()?;
-        Ok(())
+        let new_name = new_name.trim();
+        if new_name.is_empty() {
+            // Mirror other API patterns by surfacing a store error for bad input without adding a new variant
+            return Err(WalletStorageError::bad_query("accounts_rename", "new_name cannot be empty").into());
+        }
+
+        let mut tx = self.store.create_write_tx()?;
+        if let Some(existing) = tx.accounts_get_by_name(new_name).optional()? {
+            // Allow renaming to the same name on the same account, otherwise reject
+            if existing.component_address != *account_addr {
+                return Err(AccountsApiError::AccountNameAlreadyExists { name: new_name.to_string() });
+            }
+        }
+
+        tx.accounts_update(
+            account_addr,
+            AccountUpdate {
+                name: Some(new_name),
+                ..Default::default()
+            },
+        )?;
+        tx.commit()?;
+        Ok(())
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn rename_account(&self, account_addr: &ComponentAddress, new_name: &str) -> Result<(), AccountsApiError> {
let mut tx = self.store.create_write_tx()?;
tx.accounts_update(account_addr, AccountUpdate {
name: Some(new_name),
..Default::default()
})?;
tx.commit()?;
Ok(())
}
pub fn rename_account(&self, account_addr: &ComponentAddress, new_name: &str) -> Result<(), AccountsApiError> {
let new_name = new_name.trim();
if new_name.is_empty() {
// Mirror other API patterns by surfacing a store error for bad input without adding a new variant
return Err(WalletStorageError::bad_query("accounts_rename", "new_name cannot be empty").into());
}
let mut tx = self.store.create_write_tx()?;
if let Some(existing) = tx.accounts_get_by_name(new_name).optional()? {
// Allow renaming to the same name on the same account, otherwise reject
if existing.component_address != *account_addr {
return Err(AccountsApiError::AccountNameAlreadyExists { name: new_name.to_string() });
}
}
tx.accounts_update(
account_addr,
AccountUpdate {
name: Some(new_name),
..Default::default()
},
)?;
tx.commit()?;
Ok(())
}
🤖 Prompt for AI Agents 
In crates/wallet/sdk/src/apis/accounts.rs around lines 285 to 294, update
rename_account to trim the provided new_name, reject empty names after trimming,
treat renaming to the same trimmed name as a no-op (return Ok), and check the
store for any other account already using that trimmed name before applying the
update; if a duplicate exists return an appropriate AccountsApiError (e.g.,
DuplicateName or Conflict) instead of attempting the update. Ensure validation
happens before creating the write transaction and keep existing
transaction/commit logic for the actual update when all checks pass.

pub fn set_default_account(&self, account_addr: &ComponentAddress) -> Result<(), AccountsApiError> {
let mut tx = self.store.create_write_tx()?;
tx.accounts_set_default(account_addr)?;
Expand Down
4 changes: 2 additions & 2 deletions crates/wallet/sdk/src/models/account.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub struct NewAccountData {
}

#[derive(Debug, Clone, Default)]
pub struct AccountUpdate {
pub name: Option<String>,
pub struct AccountUpdate<'a> {
pub name: Option<&'a str>,
pub is_account_on_chain: Option<bool>,
}
2 changes: 1 addition & 1 deletion crates/wallet/sdk/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ pub trait WalletStoreWriter {
fn accounts_update(
&mut self,
account_addr: &ComponentAddress,
update: AccountUpdate,
update: AccountUpdate<'_>,
) -> Result<(), WalletStorageError>;

fn accounts_add_stealth_resource(
Expand Down
6 changes: 5 additions & 1 deletion crates/wallet/storage_sqlite/src/writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,11 @@ impl WalletStoreWriter for WriteTransaction<'_> {
Ok(())
}

fn accounts_update(&mut self, address: &ComponentAddress, update: AccountUpdate) -> Result<(), WalletStorageError> {
fn accounts_update(
&mut self,
address: &ComponentAddress,
update: AccountUpdate<'_>,
) -> Result<(), WalletStorageError> {
use crate::schema::accounts;
let AccountUpdate {
name,
Expand Down
2 changes: 1 addition & 1 deletion crates/wallet/storage_sqlite/tests/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn update_account() {
let mut tx = db.create_write_tx().unwrap();
tx.accounts_insert(Some("test"), &address, 0, false, false).unwrap();
tx.accounts_update(&address, AccountUpdate {
name: Some("foo".to_string()),
name: Some("foo"),
..Default::default()
})
.unwrap();
Expand Down
Loading