diff --git a/pallets/tables/src/benchmarking.rs b/pallets/tables/src/benchmarking.rs index 6123965d..ef20e5c3 100644 --- a/pallets/tables/src/benchmarking.rs +++ b/pallets/tables/src/benchmarking.rs @@ -663,5 +663,34 @@ mod benchmarks { ); } + #[benchmark] + fn transfer_table_ownership() { + let creator: T::AccountId = whitelisted_caller(); + grant_edit_schema::(creator.clone()); + setup_full_namespace::(creator.clone(), "SCHEMA", TableType::PublicPermissionless); + let table_identifier = TableIdentifier::from_str_unchecked("ONE", "SCHEMA"); + let table_definition = integers_table_definition( + table_identifier.clone(), + TableType::PublicPermissionless, + CommitmentSchemeFlags::all(), + ); + Tables::::create_tables( + RawOrigin::Signed(creator.clone()).into(), + alloc::vec![table_definition].try_into().unwrap(), + ) + .unwrap(); + + let new_owner: T::AccountId = account("new_owner", 0, 0); + + #[extrinsic_call] + transfer_table_ownership( + RawOrigin::Signed(creator), + table_identifier.clone(), + Some(new_owner.clone()), + ); + + assert_eq!(TableOwners::::get(&table_identifier), Some(new_owner)); + } + impl_benchmark_test_suite!(Tables, crate::mock::new_test_ext(), crate::mock::Test); } diff --git a/pallets/tables/src/lib.rs b/pallets/tables/src/lib.rs index 326f9096..83417cc2 100644 --- a/pallets/tables/src/lib.rs +++ b/pallets/tables/src/lib.rs @@ -230,6 +230,16 @@ pub mod pallet { /// The new quorum Settings new_quorum: InsertQuorumSize, }, + + /// A table's ownership has been transferred. + TableOwnershipTransferred { + /// The table whose ownership changed + table: TableIdentifier, + /// The previous owner, if any + old_owner: Option, + /// The new owner, if any + new_owner: Option, + }, } /// Storage map of Column UUIDs by `TableIdentifier` and Version. @@ -908,6 +918,30 @@ pub mod pallet { Self::deposit_event(Event::::SciTableCreated { table: table.ident }); Ok(()) } + + /// Transfer ownership of a table to a new account. + /// + /// # Events + /// Emits `Event::TableOwnershipTransferred`. + /// + /// # Permissions + /// Requires sudo, or the caller must be the current owner of the table. + #[pallet::call_index(14)] + #[pallet::weight(::WeightInfo::transfer_table_ownership())] + pub fn transfer_table_ownership( + origin: OriginFor, + table: TableIdentifier, + new_owner: Option, + ) -> DispatchResult { + let old_owner = Self::ensure_root_or_owner(origin, &table)?; + TableOwners::::set(&table, new_owner.clone()); + Self::deposit_event(Event::::TableOwnershipTransferred { + table, + old_owner, + new_owner, + }); + Ok(()) + } } fn map_uuid_error(error: UpdateUuidError) -> DispatchError { @@ -1333,6 +1367,30 @@ pub mod pallet { Ok(()) } + /// Checks whether the origin is either `Root` or the recorded owner of `table`. + /// + /// Returns: + /// - `Ok(stored_owner)` if the origin is `Root`, where `stored_owner` is the current + /// value in [`TableOwners`] (may be `None` if no owner is recorded), + /// - `Ok(Some(caller))` if the origin is a signed account that matches the stored owner, + /// - `Err(UnsignedTransaction)` if the origin is neither signed nor root, + /// - `Err(InsufficientPermissions)` if the signed account is not the stored owner. + fn ensure_root_or_owner( + origin: OriginFor, + table: &TableIdentifier, + ) -> Result, DispatchError> { + let maybe_owner = TableOwners::::get(table); + match origin.into() { + Ok(RawOrigin::Root) => Ok(maybe_owner), + Ok(RawOrigin::Signed(who)) => { + Ok(Some(maybe_owner.filter(|owner| owner == &who).ok_or( + pallet_permissions::Error::::InsufficientPermissions, + )?)) + } + _ => Err(pallet_permissions::Error::::UnsignedTransaction)?, + } + } + /// Returns the schema for the given table identifier. pub fn table_schema( table_identifier: TableIdentifier, diff --git a/pallets/tables/src/tests.rs b/pallets/tables/src/tests.rs index 450cfa26..30c3e4d8 100644 --- a/pallets/tables/src/tests.rs +++ b/pallets/tables/src/tests.rs @@ -2081,3 +2081,135 @@ fn create_table_with_sci_metadata_fails_for_unpermissioned_user() { ); }); } + +/// Helper: create a community table owned by `user(1)` in the test namespace. +/// Returns the normalised `TableIdentifier`. +fn create_community_table_for_user1() -> TableIdentifier { + let (who, signer) = user(1); + let test_identifier = TableIdentifier::from_str_unchecked_with_preserved_casing( + "VOTES", + "FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT", + ); + let ddl = "CREATE TABLE IF NOT EXISTS FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT.VOTES (TIME_STAMP TIMESTAMP NOT NULL, BLOCK_NUMBER BIGINT NOT NULL, PRIMARY KEY (BLOCK_NUMBER));"; + let create_statement: CreateStatement = + BoundedVec::try_from(ddl.as_bytes().to_vec()).expect("fits"); + let tables: UpdateTableList = BoundedVec::try_from(vec![UpdateTable { + ident: test_identifier.clone(), + create_statement, + table_type: TableType::Community, + commitment: CommitmentCreationCmd::Empty(CommitmentSchemeFlags::default()), + source: Source::Ethereum, + }]) + .expect("fits"); + assert_ok!(pallet_balances::Pallet::::mint_into( + &who, + crate::CREATE_COST * 10, + )); + assert_ok!(Tables::create_tables(signer, tables)); + test_identifier.try_normalize().unwrap() +} + +#[test] +fn transfer_table_ownership_by_owner_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + create_namespace_for_testing("FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT"); + + let (who1, signer1) = user(1); + let (who2, _) = user(2); + let ident = create_community_table_for_user1(); + assert_eq!(TableOwners::::get(&ident), Some(who1.clone())); + + System::reset_events(); + assert_ok!(Tables::transfer_table_ownership( + signer1, + ident.clone(), + Some(who2.clone()), + )); + + assert_eq!(TableOwners::::get(&ident), Some(who2.clone())); + System::assert_has_event(RuntimeEvent::Tables(Event::TableOwnershipTransferred { + table: ident, + old_owner: Some(who1), + new_owner: Some(who2), + })); + }); +} + +#[test] +fn transfer_table_ownership_by_sudo_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + create_namespace_for_testing("FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT"); + + let (who1, _) = user(1); + let (who2, _) = user(2); + let ident = create_community_table_for_user1(); + assert_eq!(TableOwners::::get(&ident), Some(who1.clone())); + + assert_ok!(Tables::transfer_table_ownership( + RuntimeOrigin::root(), + ident.clone(), + Some(who2.clone()), + )); + + assert_eq!(TableOwners::::get(&ident), Some(who2)); + }); +} + +#[test] +fn transfer_table_ownership_fails_for_non_owner() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + create_namespace_for_testing("FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT"); + + let ident = create_community_table_for_user1(); + let (who2, signer2) = user(2); + + assert_err!( + Tables::transfer_table_ownership(signer2, ident, Some(who2)), + pallet_permissions::Error::::InsufficientPermissions, + ); + }); +} + +#[test] +fn transfer_table_ownership_to_none_removes_owner() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + create_namespace_for_testing("FUNNAME_5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT"); + + let (who1, signer1) = user(1); + let ident = create_community_table_for_user1(); + assert_eq!(TableOwners::::get(&ident), Some(who1.clone())); + + System::reset_events(); + assert_ok!(Tables::transfer_table_ownership( + signer1, + ident.clone(), + None, + )); + + assert_eq!(TableOwners::::get(&ident), None); + System::assert_has_event(RuntimeEvent::Tables(Event::TableOwnershipTransferred { + table: ident, + old_owner: Some(who1), + new_owner: None, + })); + }); +} + +#[test] +fn transfer_table_ownership_fails_when_no_owner() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + + let table = TableIdentifier::from_str_unchecked("MY_TABLE", "TEST_NAMESPACE"); + let (who, signer) = user(1); + + assert_err!( + Tables::transfer_table_ownership(signer, table, Some(who)), + pallet_permissions::Error::::InsufficientPermissions, + ); + }); +} diff --git a/pallets/tables/src/weights.rs b/pallets/tables/src/weights.rs index b004e6c8..989244b6 100644 --- a/pallets/tables/src/weights.rs +++ b/pallets/tables/src/weights.rs @@ -47,6 +47,7 @@ pub trait WeightInfo { fn set_block_enforcement() -> Weight; fn set_table_metadata() -> Weight; fn create_table_with_sci_metadata() -> Weight; + fn transfer_table_ownership() -> Weight; } /// Weights for `pallet_tables` using the Substrate node and recommended hardware. @@ -264,6 +265,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(1030_u64)) .saturating_add(T::DbWeight::get().writes(11_u64)) } + fn transfer_table_ownership() -> Weight { + Weight::from_parts(15_000_000, 2656) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } // For backwards compatibility and tests. @@ -480,4 +486,9 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1030_u64)) .saturating_add(RocksDbWeight::get().writes(11_u64)) } + fn transfer_table_ownership() -> Weight { + Weight::from_parts(15_000_000, 2656) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } }