diff --git a/common/client-libs/validator-client/src/nyxd/contract_traits/node_families_signing_client.rs b/common/client-libs/validator-client/src/nyxd/contract_traits/node_families_signing_client.rs index 34690c8be5..422b38e335 100644 --- a/common/client-libs/validator-client/src/nyxd/contract_traits/node_families_signing_client.rs +++ b/common/client-libs/validator-client/src/nyxd/contract_traits/node_families_signing_client.rs @@ -54,6 +54,27 @@ pub trait NodeFamiliesSigningClient { .await } + /// Update the name and/or description of the caller's family. Each + /// argument follows `None = keep` / `Some(_) = replace` semantics; a + /// call with both `None` is a server-side no-op. + async fn update_family( + &self, + updated_name: Option, + updated_description: Option, + fee: Option, + ) -> Result { + self.execute_node_families_contract( + fee, + NodeFamiliesExecuteMsg::UpdateFamily { + updated_name, + updated_description, + }, + "NodeFamiliesContract::UpdateFamily".to_string(), + vec![], + ) + .await + } + async fn disband_family(&self, fee: Option) -> Result { self.execute_node_families_contract( fee, @@ -224,6 +245,12 @@ mod tests { NodeFamiliesExecuteMsg::CreateFamily { name, description } => client .create_family(name, description, None, vec![]) .ignore(), + NodeFamiliesExecuteMsg::UpdateFamily { + updated_name, + updated_description, + } => client + .update_family(updated_name, updated_description, None) + .ignore(), NodeFamiliesExecuteMsg::DisbandFamily {} => client.disband_family(None).ignore(), NodeFamiliesExecuteMsg::InviteToFamily { node_id, diff --git a/common/cosmwasm-smart-contracts/node-families-contract/src/constants.rs b/common/cosmwasm-smart-contracts/node-families-contract/src/constants.rs index 372b2b9d9e..5ef0f48c74 100644 --- a/common/cosmwasm-smart-contracts/node-families-contract/src/constants.rs +++ b/common/cosmwasm-smart-contracts/node-families-contract/src/constants.rs @@ -101,4 +101,14 @@ pub mod events { pub const NODE_UNBOND_CLEANUP_EVENT_NAME: &str = "family_node_unbond_cleanup"; pub const NODE_UNBOND_CLEANUP_EVENT_NODE_ID: &str = "node_id"; + + pub const FAMILY_UPDATE_EVENT_NAME: &str = "family_update"; + pub const FAMILY_UPDATE_EVENT_FAMILY_ID: &str = "family_id"; + pub const FAMILY_UPDATE_EVENT_OWNER_ADDRESS: &str = "owner_address"; + /// Attribute carrying the new family name. Only emitted when the + /// `UpdateFamily` message carried `updated_name = Some(_)`. + pub const FAMILY_UPDATE_EVENT_UPDATED_NAME: &str = "updated_name"; + /// Attribute carrying the new family description. Only emitted when the + /// `UpdateFamily` message carried `updated_description = Some(_)`. + pub const FAMILY_UPDATE_EVENT_UPDATED_DESCRIPTION: &str = "updated_description"; } diff --git a/common/cosmwasm-smart-contracts/node-families-contract/src/msg.rs b/common/cosmwasm-smart-contracts/node-families-contract/src/msg.rs index b5a509edcf..a542a0b707 100644 --- a/common/cosmwasm-smart-contracts/node-families-contract/src/msg.rs +++ b/common/cosmwasm-smart-contracts/node-families-contract/src/msg.rs @@ -38,6 +38,16 @@ pub enum ExecuteMsg { /// `create_family_fee` must be attached as funds. CreateFamily { name: String, description: String }, + /// Update the name and/or description of the family owned by the message + /// sender. Each field is independently optional: `None` leaves the + /// existing value unchanged, `Some(_)` replaces it. Updated values are + /// validated against the same length / normalisation / global-uniqueness + /// rules as [`Self::CreateFamily`]. + UpdateFamily { + updated_name: Option, + updated_description: Option, + }, + /// Disband the family owned by the message sender. The family must have /// no current members; any still-pending invitations are revoked. DisbandFamily {}, diff --git a/contracts/node-families/schema/node-families.json b/contracts/node-families/schema/node-families.json index b9165c7546..9a731548ba 100644 --- a/contracts/node-families/schema/node-families.json +++ b/contracts/node-families/schema/node-families.json @@ -135,6 +135,34 @@ }, "additionalProperties": false }, + { + "description": "Update the name and/or description of the family owned by the message sender. Each field is independently optional: `None` leaves the existing value unchanged, `Some(_)` replaces it. Updated values are validated against the same length / normalisation / global-uniqueness rules as [`Self::CreateFamily`].", + "type": "object", + "required": [ + "update_family" + ], + "properties": { + "update_family": { + "type": "object", + "properties": { + "updated_description": { + "type": [ + "string", + "null" + ] + }, + "updated_name": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }, { "description": "Disband the family owned by the message sender. The family must have no current members; any still-pending invitations are revoked.", "type": "object", diff --git a/contracts/node-families/schema/raw/execute.json b/contracts/node-families/schema/raw/execute.json index 7b06d071f0..729559190c 100644 --- a/contracts/node-families/schema/raw/execute.json +++ b/contracts/node-families/schema/raw/execute.json @@ -51,6 +51,34 @@ }, "additionalProperties": false }, + { + "description": "Update the name and/or description of the family owned by the message sender. Each field is independently optional: `None` leaves the existing value unchanged, `Some(_)` replaces it. Updated values are validated against the same length / normalisation / global-uniqueness rules as [`Self::CreateFamily`].", + "type": "object", + "required": [ + "update_family" + ], + "properties": { + "update_family": { + "type": "object", + "properties": { + "updated_description": { + "type": [ + "string", + "null" + ] + }, + "updated_name": { + "type": [ + "string", + "null" + ] + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }, { "description": "Disband the family owned by the message sender. The family must have no current members; any still-pending invitations are revoked.", "type": "object", diff --git a/contracts/node-families/src/contract.rs b/contracts/node-families/src/contract.rs index 64d37f6df8..54918514ca 100644 --- a/contracts/node-families/src/contract.rs +++ b/contracts/node-families/src/contract.rs @@ -16,7 +16,7 @@ use crate::storage::NodeFamiliesStorage; use crate::transactions::{ try_accept_family_invitation, try_create_family, try_disband_family, try_handle_node_unbonding, try_invite_to_family, try_kick_from_family, try_leave_family, try_reject_family_invitation, - try_revoke_family_invitation, try_update_config, + try_revoke_family_invitation, try_update_config, try_update_family, }; use cosmwasm_std::{ entry_point, to_json_binary, Binary, Deps, DepsMut, Env, MessageInfo, Response, @@ -69,6 +69,10 @@ pub fn execute( ExecuteMsg::CreateFamily { name, description } => { try_create_family(deps, env, info, name, description) } + ExecuteMsg::UpdateFamily { + updated_name, + updated_description, + } => try_update_family(deps, env, info, updated_name, updated_description), ExecuteMsg::DisbandFamily {} => try_disband_family(deps, env, info), ExecuteMsg::InviteToFamily { node_id, diff --git a/contracts/node-families/src/helpers.rs b/contracts/node-families/src/helpers.rs index 03ba312779..4f62720bff 100644 --- a/contracts/node-families/src/helpers.rs +++ b/contracts/node-families/src/helpers.rs @@ -4,7 +4,7 @@ use crate::storage::NodeFamiliesStorage; use cosmwasm_std::{Addr, Deps}; use nym_mixnet_contract_common::{MixnetContractQuerier, NodeId}; -use nym_node_families_contract_common::NodeFamiliesContractError; +use nym_node_families_contract_common::{Config, NodeFamiliesContractError, NodeFamilyId}; /// Normalise a family name into the canonical form used as the unique-index key. /// @@ -18,6 +18,80 @@ pub fn normalise_family_name(name: &str) -> String { .collect() } +/// A new `(display, normalised)` family name pair that has passed +/// length and non-empty-after-normalisation validation. Constructed by +/// [`validate_family_name`]; consumed by storage write paths that update +/// the family's `name` + `normalised_name` columns together. +pub(crate) struct NewFamilyName { + pub(crate) name: String, + pub(crate) normalised_name: String, +} + +/// Validate a candidate family name against [`Config::family_name_length_limit`] +/// and the non-empty-after-normalisation rule, returning the paired display + +/// normalised forms. The caller is responsible for the uniqueness check +/// against existing families (see [`ensure_normalised_name_unique`]). +pub(crate) fn validate_family_name( + name: String, + config: &Config, +) -> Result { + if name.len() > config.family_name_length_limit { + return Err(NodeFamiliesContractError::FamilyNameTooLong { + length: name.len(), + limit: config.family_name_length_limit, + }); + } + let normalised_name = normalise_family_name(&name); + if normalised_name.is_empty() { + return Err(NodeFamiliesContractError::EmptyFamilyName); + } + Ok(NewFamilyName { + name, + normalised_name, + }) +} + +/// Validate a candidate family description against +/// [`Config::family_description_length_limit`]. +pub(crate) fn validate_family_description( + description: &str, + config: &Config, +) -> Result<(), NodeFamiliesContractError> { + if description.len() > config.family_description_length_limit { + return Err(NodeFamiliesContractError::FamilyDescriptionTooLong { + length: description.len(), + limit: config.family_description_length_limit, + }); + } + Ok(()) +} + +/// Ensure no family other than `excluded_id` has `normalised_name` as its +/// normalised name. `excluded_id = None` is used on the create path (any +/// match is a collision); `excluded_id = Some(family.id)` on the update +/// path lets a family keep its current normalised key. +pub(crate) fn ensure_normalised_name_unique( + storage: &NodeFamiliesStorage, + deps: Deps, + normalised_name: &str, + excluded_id: Option, +) -> Result<(), NodeFamiliesContractError> { + if let Some((_, existing)) = storage + .families + .idx + .normalised_name + .item(deps.storage, normalised_name.to_owned())? + { + if Some(existing.id) != excluded_id { + return Err(NodeFamiliesContractError::FamilyNameAlreadyTaken { + name: normalised_name.to_owned(), + family_id: existing.id, + }); + } + } + Ok(()) +} + /// Ensure no node controlled by `address` is currently a member of any family. pub(crate) fn ensure_address_holds_no_family_membership( storage: &NodeFamiliesStorage, diff --git a/contracts/node-families/src/storage/mod.rs b/contracts/node-families/src/storage/mod.rs index 62ae93c82d..371631bac9 100644 --- a/contracts/node-families/src/storage/mod.rs +++ b/contracts/node-families/src/storage/mod.rs @@ -4,6 +4,7 @@ // storage will be used in subsequent PRs/tickets #![allow(dead_code)] +use crate::helpers::NewFamilyName; use crate::storage::storage_indexes::{ FamilyMembersIndex, NodeFamiliesIndex, NodeFamilyInvitationIndex, PastFamilyInvitationsIndex, PastFamilyMembersIndex, @@ -221,22 +222,20 @@ impl NodeFamiliesStorage<'_> { /// invariants via unique indexes on `owner` and `normalised_name` as a /// defence-in-depth check, so this call will fail if either is already /// taken — but the caller must not rely on it for the membership check. - #[allow(clippy::too_many_arguments)] pub(crate) fn register_new_family( &self, store: &mut dyn Storage, env: &Env, fee: Coin, owner: Addr, - name: String, - normalised_name: String, + name: NewFamilyName, description: String, ) -> Result { let id = self.next_family_id(store)?; let family = NodeFamily { id, - name, - normalised_name, + name: name.name, + normalised_name: name.normalised_name, description, owner, paid_fee: fee, @@ -247,6 +246,38 @@ impl NodeFamiliesStorage<'_> { Ok(family) } + /// Apply name and/or description updates to an existing family, leaving + /// every other field (id, owner, members, paid_fee, created_at) + /// untouched. Each argument follows `None = keep` / `Some = replace` + /// semantics. + /// + /// No validation is performed here — the caller (transaction handler) + /// owns the length / non-empty / global-uniqueness checks before invoking. + /// Errors with [`FamilyNotFound`] if `family_id` does not exist. + /// + /// [`FamilyNotFound`]: NodeFamiliesContractError::FamilyNotFound + pub(crate) fn update_family_details( + &self, + store: &mut dyn Storage, + family_id: NodeFamilyId, + updated_name: Option, + updated_description: Option, + ) -> Result { + let mut family = self + .families + .may_load(store, family_id)? + .ok_or(NodeFamiliesContractError::FamilyNotFound { family_id })?; + if let Some(new_name) = updated_name { + family.name = new_name.name; + family.normalised_name = new_name.normalised_name; + } + if let Some(description) = updated_description { + family.description = description; + } + self.families.save(store, family.id, &family)?; + Ok(family) + } + /// Persist a new pending invitation for `node_id` to join `family_id`. /// /// `expires_at` is taken as a unix-seconds absolute deadline (the caller @@ -703,6 +734,7 @@ impl NodeFamiliesStorage<'_> { #[cfg(test)] mod tests { use super::*; + use crate::helpers::NewFamilyName; use crate::testing::{init_contract_tester, NodeFamiliesContractTesterExt}; use nym_contracts_common_testing::ContractOpts; @@ -780,8 +812,10 @@ mod tests { &env, fee, owner.clone(), - "Fam!".into(), - "fam".into(), + NewFamilyName { + name: "Fam!".into(), + normalised_name: "fam".into(), + }, "desc".into(), ) .unwrap(); @@ -823,8 +857,10 @@ mod tests { &env, fee.clone(), alice, - "Shared".into(), - "shared".into(), + NewFamilyName { + name: "Shared".into(), + normalised_name: "shared".into(), + }, "".into(), ) .unwrap(); @@ -836,8 +872,10 @@ mod tests { &env, fee, bob, - "$$shared$$".into(), - "shared".into(), + NewFamilyName { + name: "$$shared$$".into(), + normalised_name: "shared".into(), + }, "".into(), ); assert!(res.is_err()); @@ -859,8 +897,10 @@ mod tests { &env, fee, alice, - "second".into(), - "second".into(), + NewFamilyName { + name: "second".into(), + normalised_name: "second".into(), + }, "".into(), ); assert!(res.is_err()); @@ -1272,8 +1312,10 @@ mod tests { &env, fee, alice, - "2".into(), - "2".into(), + NewFamilyName { + name: "2".into(), + normalised_name: "2".into(), + }, "".into(), ) .unwrap(); diff --git a/contracts/node-families/src/testing.rs b/contracts/node-families/src/testing.rs index f76bbacf26..d68c357d45 100644 --- a/contracts/node-families/src/testing.rs +++ b/contracts/node-families/src/testing.rs @@ -6,7 +6,7 @@ #![allow(clippy::expect_used)] use crate::contract::{execute, instantiate, migrate, query}; -use crate::helpers::normalise_family_name; +use crate::helpers::{normalise_family_name, NewFamilyName}; use crate::storage::NodeFamiliesStorage; use cosmwasm_std::{coin, Addr, Coin, Storage}; use mixnet_contract::testable_mixnet_contract::{EmbeddedMixnetContractExt, MixnetContract}; @@ -126,7 +126,7 @@ pub trait NodeFamiliesContractTesterExt: } fn make_named_family(&mut self, owner: &Addr, name: &str) -> NodeFamily { - let normalised = normalise_family_name(name); + let normalised_name = normalise_family_name(name); let env = self.env(); let fee = self.family_fee(); NodeFamiliesStorage::new() @@ -135,8 +135,10 @@ pub trait NodeFamiliesContractTesterExt: &env, fee, owner.clone(), - name.to_string(), - normalised, + NewFamilyName { + name: name.to_string(), + normalised_name, + }, "dummy".to_string(), ) .unwrap() diff --git a/contracts/node-families/src/transactions.rs b/contracts/node-families/src/transactions.rs index 0237dcf3ab..e18f84b3d1 100644 --- a/contracts/node-families/src/transactions.rs +++ b/contracts/node-families/src/transactions.rs @@ -5,7 +5,8 @@ use crate::helpers::{ ensure_address_holds_no_family_membership, ensure_has_bonded_node, ensure_node_is_bonded, - ensure_node_not_in_family, normalise_family_name, + ensure_node_not_in_family, ensure_normalised_name_unique, validate_family_description, + validate_family_name, }; use crate::storage::NodeFamiliesStorage; use cosmwasm_std::{BankMsg, DepsMut, Env, Event, MessageInfo, Response}; @@ -60,25 +61,9 @@ pub(crate) fn try_create_family( }); } - // validate family name - if name.len() > config.family_name_length_limit { - return Err(NodeFamiliesContractError::FamilyNameTooLong { - length: name.len(), - limit: config.family_name_length_limit, - }); - } - let normalised = normalise_family_name(&name); - if normalised.is_empty() { - return Err(NodeFamiliesContractError::EmptyFamilyName); - } - - // validate family description - if description.len() > config.family_description_length_limit { - return Err(NodeFamiliesContractError::FamilyDescriptionTooLong { - length: description.len(), - limit: config.family_description_length_limit, - }); - } + // validate name + description (shared with try_update_family) + let validated_name = validate_family_name(name, &config)?; + validate_family_description(&description, &config)?; // check if the sender already owns a family if let Some(existing) = storage.may_get_owned_family(deps.storage, &info.sender)? { @@ -89,17 +74,12 @@ pub(crate) fn try_create_family( } // explicitly verify duplicate family name for a better error message - if let Some((_, existing)) = storage - .families - .idx - .normalised_name - .item(deps.storage, normalised.clone())? - { - return Err(NodeFamiliesContractError::FamilyNameAlreadyTaken { - name: normalised, - family_id: existing.id, - }); - } + ensure_normalised_name_unique( + &storage, + deps.as_ref(), + &validated_name.normalised_name, + None, + )?; // check whether this owner has a bonded node which belongs to a family ensure_address_holds_no_family_membership(&storage, deps.as_ref(), &info.sender)?; @@ -109,8 +89,7 @@ pub(crate) fn try_create_family( &env, config.create_family_fee, info.sender, - name, - normalised, + validated_name, description, )?; @@ -129,6 +108,75 @@ pub(crate) fn try_create_family( )) } +/// Update the name and/or description of the family owned by `info.sender`. +/// Each field is independently optional: `None` keeps the existing value, +/// `Some(_)` replaces it. Updated values are validated against the same +/// length / normalisation / global-uniqueness rules as [`try_create_family`]. +/// A fully-empty call (both arguments `None`) is silently accepted as a +/// no-op — no state change, no event. +pub(crate) fn try_update_family( + deps: DepsMut, + _env: Env, + info: MessageInfo, + updated_name: Option, + updated_description: Option, +) -> Result { + // Short-circuit on the no-op case so we don't load config / storage. + if updated_name.is_none() && updated_description.is_none() { + return Ok(Response::default()); + } + + let storage = NodeFamiliesStorage::new(); + let config = storage.config.load(deps.storage)?; + + // Owner gate — same unique-index lookup used by disband / invite / kick. + let family = storage.must_get_owned_family(deps.storage, &info.sender)?; + + // Validate name + description (shared with try_create_family). + let validated_name = updated_name + .map(|name| validate_family_name(name, &config)) + .transpose()?; + if let Some(ref new) = validated_name { + ensure_normalised_name_unique( + &storage, + deps.as_ref(), + &new.normalised_name, + Some(family.id), + )?; + } + if let Some(ref description) = updated_description { + validate_family_description(description, &config)?; + } + + let name_was_updated = validated_name.is_some(); + let description_was_updated = updated_description.is_some(); + + let updated = storage.update_family_details( + deps.storage, + family.id, + validated_name, + updated_description, + )?; + + let mut event = Event::new(events::FAMILY_UPDATE_EVENT_NAME) + .add_attribute( + events::FAMILY_UPDATE_EVENT_FAMILY_ID, + updated.id.to_string(), + ) + .add_attribute(events::FAMILY_UPDATE_EVENT_OWNER_ADDRESS, &updated.owner); + if name_was_updated { + event = event.add_attribute(events::FAMILY_UPDATE_EVENT_UPDATED_NAME, &updated.name); + } + if description_was_updated { + event = event.add_attribute( + events::FAMILY_UPDATE_EVENT_UPDATED_DESCRIPTION, + &updated.description, + ); + } + + Ok(Response::new().add_event(event)) +} + /// Disband the family owned by `info.sender` and refund the original /// creation fee. /// @@ -844,6 +892,329 @@ mod tests { } } + mod update_family { + use super::*; + use crate::testing::NodeFamiliesContractTesterExt; + use nym_node_families_contract_common::constants::events; + + #[test] + fn happy_path_updates_name_only() -> anyhow::Result<()> { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let original = tester.make_named_family(&alice, "Original"); + let env = tester.env(); + let info = message_info(&alice, &[]); + + try_update_family( + tester.deps_mut(), + env, + info, + Some("Renamed".to_string()), + None, + )?; + + let storage = NodeFamiliesStorage::new(); + let updated = storage.families.load(tester.deps().storage, original.id)?; + + assert_eq!(updated.name, "Renamed"); + assert_eq!(updated.normalised_name, "renamed"); + // every other field is preserved + assert_eq!(updated.description, original.description); + assert_eq!(updated.id, original.id); + assert_eq!(updated.owner, original.owner); + assert_eq!(updated.paid_fee, original.paid_fee); + assert_eq!(updated.members, original.members); + assert_eq!(updated.created_at, original.created_at); + + Ok(()) + } + + #[test] + fn happy_path_updates_description_only() -> anyhow::Result<()> { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let original = tester.make_named_family(&alice, "Family"); + let env = tester.env(); + let info = message_info(&alice, &[]); + + try_update_family( + tester.deps_mut(), + env, + info, + None, + Some("new description".to_string()), + )?; + + let storage = NodeFamiliesStorage::new(); + let updated = storage.families.load(tester.deps().storage, original.id)?; + + assert_eq!(updated.name, original.name); + assert_eq!(updated.normalised_name, original.normalised_name); + assert_eq!(updated.description, "new description"); + + Ok(()) + } + + #[test] + fn happy_path_updates_both_fields() -> anyhow::Result<()> { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let original = tester.make_named_family(&alice, "Original"); + let env = tester.env(); + let info = message_info(&alice, &[]); + + try_update_family( + tester.deps_mut(), + env, + info, + Some("Renamed".to_string()), + Some("new description".to_string()), + )?; + + let storage = NodeFamiliesStorage::new(); + let updated = storage.families.load(tester.deps().storage, original.id)?; + + assert_eq!(updated.name, "Renamed"); + assert_eq!(updated.normalised_name, "renamed"); + assert_eq!(updated.description, "new description"); + + Ok(()) + } + + #[test] + fn noop_when_both_fields_are_none() -> anyhow::Result<()> { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let original = tester.make_named_family(&alice, "Original"); + let env = tester.env(); + let info = message_info(&alice, &[]); + + let res = try_update_family(tester.deps_mut(), env, info, None, None)?; + + // no event, no messages + assert!(res.events.is_empty()); + assert!(res.messages.is_empty()); + + // family record untouched + let storage = NodeFamiliesStorage::new(); + let unchanged = storage.families.load(tester.deps().storage, original.id)?; + assert_eq!(unchanged, original); + + Ok(()) + } + + #[test] + fn noop_short_circuits_before_the_owner_lookup() -> anyhow::Result<()> { + // sender owns no family at all; the no-op short-circuit must run + // before the ownership check or this would error with + // SenderDoesntOwnAFamily. + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + let res = try_update_family(tester.deps_mut(), env, info, None, None)?; + assert!(res.events.is_empty()); + assert!(res.messages.is_empty()); + + Ok(()) + } + + #[test] + fn rejects_when_sender_owns_no_family() { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + let err = try_update_family( + tester.deps_mut(), + env, + info, + Some("Newname".to_string()), + None, + ) + .unwrap_err(); + + assert_eq!( + err, + NodeFamiliesContractError::SenderDoesntOwnAFamily { address: alice } + ); + } + + #[test] + fn rejects_name_too_long() { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + tester.make_named_family(&alice, "Original"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + let limit = NodeFamiliesStorage::new() + .config + .load(tester.deps().storage) + .unwrap() + .family_name_length_limit; + let too_long = "x".repeat(limit + 1); + + let err = try_update_family(tester.deps_mut(), env, info, Some(too_long.clone()), None) + .unwrap_err(); + + assert_eq!( + err, + NodeFamiliesContractError::FamilyNameTooLong { + length: too_long.len(), + limit, + } + ); + } + + #[test] + fn rejects_name_normalising_to_empty() { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + tester.make_named_family(&alice, "Original"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + let err = try_update_family( + tester.deps_mut(), + env, + info, + Some("!!!---".to_string()), + None, + ) + .unwrap_err(); + + assert_eq!(err, NodeFamiliesContractError::EmptyFamilyName); + } + + #[test] + fn rejects_name_already_taken_by_another_family() { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let bob = tester.addr_make("bob"); + tester.make_named_family(&alice, "Alice family"); + let bobs_family = tester.make_named_family(&bob, "Bob family"); + + let info = message_info(&alice, &[]); + let env = tester.env(); + let err = try_update_family( + tester.deps_mut(), + env, + info, + Some("Bob Family".to_string()), + None, + ) + .unwrap_err(); + + assert_eq!( + err, + NodeFamiliesContractError::FamilyNameAlreadyTaken { + name: "bobfamily".to_string(), + family_id: bobs_family.id, + } + ); + } + + #[test] + fn case_only_rename_keeping_normalised_form_is_allowed() -> anyhow::Result<()> { + // a rename whose normalised form matches the family's current + // normalised key must succeed - it's a no-collision change against + // the family's own row, which the uniqueness pre-check skips. + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + let original = tester.make_named_family(&alice, "MyFamily"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + try_update_family( + tester.deps_mut(), + env, + info, + Some("MYFAMILY".to_string()), + None, + )?; + + let updated = NodeFamiliesStorage::new() + .families + .load(tester.deps().storage, original.id)?; + assert_eq!(updated.name, "MYFAMILY"); + assert_eq!(updated.normalised_name, "myfamily"); + + Ok(()) + } + + #[test] + fn rejects_description_too_long() { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + tester.make_named_family(&alice, "Original"); + let info = message_info(&alice, &[]); + let env = tester.env(); + + let limit = NodeFamiliesStorage::new() + .config + .load(tester.deps().storage) + .unwrap() + .family_description_length_limit; + let too_long = "x".repeat(limit + 1); + + let err = try_update_family(tester.deps_mut(), env, info, None, Some(too_long.clone())) + .unwrap_err(); + + assert_eq!( + err, + NodeFamiliesContractError::FamilyDescriptionTooLong { + length: too_long.len(), + limit, + } + ); + } + + #[test] + fn event_only_carries_attributes_for_changed_fields() -> anyhow::Result<()> { + let mut tester = init_contract_tester(); + let alice = tester.addr_make("alice"); + tester.make_named_family(&alice, "Original"); + let env = tester.env(); + + // name-only update: no `updated_description` attribute + let info = message_info(&alice, &[]); + let res = try_update_family( + tester.deps_mut(), + env.clone(), + info, + Some("Renamed".to_string()), + None, + )?; + assert_eq!(res.events.len(), 1); + let event = &res.events[0]; + assert_eq!(event.ty, events::FAMILY_UPDATE_EVENT_NAME); + let keys: Vec<&str> = event.attributes.iter().map(|a| a.key.as_str()).collect(); + assert!(keys.contains(&events::FAMILY_UPDATE_EVENT_FAMILY_ID)); + assert!(keys.contains(&events::FAMILY_UPDATE_EVENT_OWNER_ADDRESS)); + assert!(keys.contains(&events::FAMILY_UPDATE_EVENT_UPDATED_NAME)); + assert!(!keys.contains(&events::FAMILY_UPDATE_EVENT_UPDATED_DESCRIPTION)); + + // description-only update: no `updated_name` attribute + let info = message_info(&alice, &[]); + let res = try_update_family( + tester.deps_mut(), + env, + info, + None, + Some("new desc".to_string()), + )?; + let event = &res.events[0]; + let keys: Vec<&str> = event.attributes.iter().map(|a| a.key.as_str()).collect(); + assert!(!keys.contains(&events::FAMILY_UPDATE_EVENT_UPDATED_NAME)); + assert!(keys.contains(&events::FAMILY_UPDATE_EVENT_UPDATED_DESCRIPTION)); + + Ok(()) + } + } + mod disband_family { use super::*; use crate::testing::NodeFamiliesContractTesterExt; diff --git a/openspec/specs/node-families-contract/spec.md b/openspec/specs/node-families-contract/spec.md index 8446cdcf97..1d7a0ff674 100644 --- a/openspec/specs/node-families-contract/spec.md +++ b/openspec/specs/node-families-contract/spec.md @@ -113,6 +113,52 @@ A given owner address SHALL own at most one family at any time, enforced by the - **WHEN** `CreateFamily` is sent with a `description` whose byte length exceeds `Config::family_description_length_limit` - **THEN** the call fails with `FamilyDescriptionTooLong { length, limit }` +### Requirement: A family's name and/or description can be updated by its owner + +`ExecuteMsg::UpdateFamily { updated_name: Option, updated_description: Option }` SHALL allow the owner of an existing family to replace either or both fields independently. Each argument follows `None = keep` / `Some(_) = replace` semantics; a call carrying `None` for both fields SHALL be a silent no-op (empty `Response`, no event, no state change) and SHALL short-circuit BEFORE the owner-ownership check (so a sender that owns no family can also no-op without error). Otherwise the handler MUST look up the sender's family via the `owner` unique index and fail with `SenderDoesntOwnAFamily { address }` if none exists. Updated values MUST be validated against the same rules as `CreateFamily`: a `Some(name)` MUST satisfy `Config::family_name_length_limit` (else `FamilyNameTooLong { length, limit }`) and MUST normalise to a non-empty string (else `EmptyFamilyName`); a `Some(description)` MUST satisfy `Config::family_description_length_limit` (else `FamilyDescriptionTooLong { length, limit }`). The new normalised name MUST be globally unique against OTHER families (else `FamilyNameAlreadyTaken { name, family_id }`); a name change whose normalised form matches the family's own existing `normalised_name` (a case-only or punctuation-only rename) MUST be allowed. The handler MUST preserve `id`, `owner`, `paid_fee`, `members`, and `created_at`. On a state-changing success the response SHALL include a `family_update` event with `family_id` and `owner_address` attributes plus conditional `updated_name` / `updated_description` attributes for each field actually changed by the call. + +#### Scenario: Owner updates the family name only +- **WHEN** the family owner sends `UpdateFamily { updated_name: Some(n), updated_description: None }` with a valid `n` +- **THEN** the persisted family's `name` and `normalised_name` are replaced with `n` and its normalised form, while `description`, `id`, `owner`, `paid_fee`, `members`, and `created_at` are unchanged +- **AND** the response carries a `family_update` event with `family_id`, `owner_address`, and `updated_name = n` (no `updated_description` attribute) + +#### Scenario: Owner updates the description only +- **WHEN** the family owner sends `UpdateFamily { updated_name: None, updated_description: Some(d) }` with a valid `d` +- **THEN** the persisted family's `description` is replaced with `d` and every other field is unchanged +- **AND** the response carries a `family_update` event with `family_id`, `owner_address`, and `updated_description = d` (no `updated_name` attribute) + +#### Scenario: Owner updates both fields +- **WHEN** the family owner sends `UpdateFamily { updated_name: Some(n), updated_description: Some(d) }` with both values valid +- **THEN** both `name`/`normalised_name` and `description` are replaced, and the event carries both `updated_name` and `updated_description` attributes + +#### Scenario: A fully-empty call is a silent no-op for any sender +- **WHEN** any sender (whether or not they own a family) sends `UpdateFamily { updated_name: None, updated_description: None }` +- **THEN** the response is `Response::default()` (no events, no messages) and storage is unchanged + +#### Scenario: Sender that owns no family is rejected when at least one field is set +- **WHEN** an address that does not own any family sends `UpdateFamily` with at least one `Some(_)` field +- **THEN** the call fails with `SenderDoesntOwnAFamily { address }` + +#### Scenario: Overlong name is rejected +- **WHEN** `UpdateFamily { updated_name: Some(n), .. }` is sent with `n.len() > Config::family_name_length_limit` +- **THEN** the call fails with `FamilyNameTooLong { length, limit }` and storage is unchanged + +#### Scenario: Name normalising to empty is rejected +- **WHEN** `UpdateFamily { updated_name: Some(n), .. }` is sent with an `n` whose normalisation yields the empty string +- **THEN** the call fails with `EmptyFamilyName` and storage is unchanged + +#### Scenario: Renaming to another family's normalised name is rejected +- **WHEN** family `A`'s owner sends `UpdateFamily { updated_name: Some(n), .. }` whose normalisation equals a different family `B`'s `normalised_name` +- **THEN** the call fails with `FamilyNameAlreadyTaken { name, family_id: B.id }` and storage is unchanged + +#### Scenario: Case-only or punctuation-only rename is allowed +- **WHEN** the owner sends `UpdateFamily { updated_name: Some(n), .. }` whose normalisation equals the family's own existing `normalised_name` +- **THEN** the display `name` is replaced with `n`, the `normalised_name` is unchanged, and the call succeeds + +#### Scenario: Overlong description is rejected +- **WHEN** `UpdateFamily { updated_description: Some(d), .. }` is sent with `d.len() > Config::family_description_length_limit` +- **THEN** the call fails with `FamilyDescriptionTooLong { length, limit }` and storage is unchanged + ### Requirement: Family ids are monotonic, never recycled, and start at 1 The contract SHALL assign family ids sequentially starting at `1`. The id counter SHALL be persisted as an `Item` and incremented on each successful family creation. Disbanding a family MUST NOT free or recycle its id. `0` SHALL be reserved as a "no family" sentinel and never assigned. @@ -406,11 +452,12 @@ Every constant exported under `nym_node_families_contract_common::constants::sto ### Requirement: Emitted events form a stable public surface -Every event name and attribute key exported under `nym_node_families_contract_common::constants::events` SHALL be treated as part of the public contract surface. Each successful **state-mutating user-facing execute path** (every variant of `ExecuteMsg` except `UpdateConfig`, which is an admin handler that returns `Response::default()` without an event) SHALL emit exactly one event whose name and attribute keys come from these constants. Renaming an event name constant, renaming an attribute-key constant, or changing the set of attributes a given event carries SHALL be treated as breaking changes. Adding new constants for new events / attributes is non-breaking. +Every event name and attribute key exported under `nym_node_families_contract_common::constants::events` SHALL be treated as part of the public contract surface. Each successful **state-mutating user-facing execute path** (every variant of `ExecuteMsg` except `UpdateConfig`, which is an admin handler that returns `Response::default()` without an event, and the no-op `UpdateFamily { updated_name: None, updated_description: None }` short-circuit, which also returns `Response::default()` because nothing changed) SHALL emit exactly one event whose name and attribute keys come from these constants. Renaming an event name constant, renaming an attribute-key constant, or changing the set of attributes a given event always carries SHALL be treated as breaking changes. Adding new constants for new events / attributes is non-breaking, as is making a previously-always-emitted attribute conditional only by extending the variant set that triggers it (see `family_update` below). At the time of this spec the constant surface comprises (refer to `constants.rs` for the authoritative list): - `family_creation` — attributes: `family_name`, `owner_address`, `family_id`, `paid_fee` +- `family_update` — required attributes: `family_id`, `owner_address`. Conditional attributes: `updated_name` (emitted only when the call carried `updated_name = Some(_)`), `updated_description` (emitted only when the call carried `updated_description = Some(_)`). At least one of the two conditional attributes is always present, because the no-op no-name-no-description case short-circuits to `Response::default()` without emitting any event at all. - `family_disband` — attributes: `family_id`, `owner_address`, `refunded_fee` - `family_invitation` — attributes: `family_id`, `node_id`, `expires_at` - `family_invitation_revoked` — attributes: `family_id`, `node_id` @@ -430,3 +477,7 @@ At the time of this spec the constant surface comprises (refer to `constants.rs` - **WHEN** the admin sends `ExecuteMsg::UpdateConfig` and the call succeeds - **THEN** the response is `Response::default()` and carries no event (this is intentional — `UpdateConfig` is administrative metadata, not a tracked state transition) +#### Scenario: No-op UpdateFamily is the second exception — no event is emitted +- **WHEN** any sender sends `ExecuteMsg::UpdateFamily { updated_name: None, updated_description: None }` and the call succeeds (the no-op short-circuit) +- **THEN** the response is `Response::default()` and carries no event — because nothing in storage changed, there is nothing for indexers to observe +