feat: implement UpdateFamily for the node families contract (#6834)

This commit is contained in:
Jędrzej Stuczyński
2026-05-28 09:12:32 +01:00
committed by GitHub
parent e7057f3932
commit 86021937df
11 changed files with 702 additions and 55 deletions
@@ -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",
@@ -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",
+5 -1
View File
@@ -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,
+75 -1
View File
@@ -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<NewFamilyName, NodeFamiliesContractError> {
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<NodeFamilyId>,
) -> 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,
+57 -15
View File
@@ -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<NodeFamily, NodeFamiliesContractError> {
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<NewFamilyName>,
updated_description: Option<String>,
) -> Result<NodeFamily, NodeFamiliesContractError> {
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();
+6 -4
View File
@@ -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()
+404 -33
View File
@@ -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<String>,
updated_description: Option<String>,
) -> Result<Response, NodeFamiliesContractError> {
// 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;