Skip to content

Commit b0fe8cf

Browse files
martinsander00martinsander00
committed
address comments
1 parent 3b74c90 commit b0fe8cf

10 files changed

Lines changed: 94 additions & 139 deletions

File tree

‎activator/src/process/multicastgroup.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,14 +406,13 @@ mod tests {
406406
.with(predicate::eq(pubkey))
407407
.returning(move |_| Ok(AccountData::MulticastGroup(mgroup_for_get.clone())));
408408

409-
// Stateless mode: use_onchain_deallocation=true, close_index=true
409+
// Stateless mode: use_onchain_deallocation=true
410410
client
411411
.expect_execute_transaction()
412412
.with(
413413
predicate::eq(DoubleZeroInstruction::DeactivateMulticastGroup(
414414
MulticastGroupDeactivateArgs {
415415
use_onchain_deallocation: true,
416-
close_index: true,
417416
},
418417
)),
419418
predicate::always(),

‎smartcontract/programs/doublezero-serviceability/src/instructions.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,15 +1011,13 @@ mod tests {
10111011
test_instruction(
10121012
DoubleZeroInstruction::DeleteMulticastGroup(MulticastGroupDeleteArgs {
10131013
use_onchain_deallocation: false,
1014-
close_index: false,
10151014
}),
10161015
"DeleteMulticastGroup",
10171016
);
10181017

10191018
test_instruction(
10201019
DoubleZeroInstruction::DeactivateMulticastGroup(MulticastGroupDeactivateArgs {
10211020
use_onchain_deallocation: false,
1022-
close_index: false,
10231021
}),
10241022
"DeactivateMulticastGroup",
10251023
);

‎smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/closeaccount.rs‎

Lines changed: 35 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{
22
error::DoubleZeroError,
33
pda::{get_index_pda, get_resource_extension_pda},
4-
processors::resource::deallocate_ip,
4+
processors::{resource::deallocate_ip, validation::validate_program_account},
55
resource::ResourceType,
66
seeds::SEED_MULTICAST_GROUP,
77
serializer::try_acc_close,
@@ -25,17 +25,14 @@ pub struct MulticastGroupDeactivateArgs {
2525
/// When false, legacy behavior is used (no deallocation).
2626
#[incremental(default = false)]
2727
pub use_onchain_deallocation: bool,
28-
/// When true, close the associated Index account alongside the multicast group.
29-
#[incremental(default = false)]
30-
pub close_index: bool,
3128
}
3229

3330
impl fmt::Debug for MulticastGroupDeactivateArgs {
3431
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
3532
write!(
3633
f,
37-
"use_onchain_deallocation: {}, close_index: {}",
38-
self.use_onchain_deallocation, self.close_index
34+
"use_onchain_deallocation: {}",
35+
self.use_onchain_deallocation
3936
)
4037
}
4138
}
@@ -51,26 +48,18 @@ pub fn process_closeaccount_multicastgroup(
5148
let owner_account = next_account_info(accounts_iter)?;
5249
let globalstate_account = next_account_info(accounts_iter)?;
5350

54-
// Optional accounts (before payer/system):
55-
// Account layout WITH deallocation + index:
51+
// Account layout WITH deallocation:
5652
// [multicastgroup, owner, globalstate, multicast_group_block, index, payer, system]
57-
// Account layout WITHOUT deallocation, with index:
53+
// Account layout WITHOUT deallocation:
5854
// [multicastgroup, owner, globalstate, index, payer, system]
59-
// Legacy (no deallocation, no index):
60-
// [multicastgroup, owner, globalstate, payer, system]
6155
let resource_extension_account = if value.use_onchain_deallocation {
6256
let multicast_group_block_ext = next_account_info(accounts_iter)?;
6357
Some(multicast_group_block_ext)
6458
} else {
6559
None
6660
};
6761

68-
let index_account = if value.close_index {
69-
Some(next_account_info(accounts_iter)?)
70-
} else {
71-
None
72-
};
73-
62+
let index_account = next_account_info(accounts_iter)?;
7463
let payer_account = next_account_info(accounts_iter)?;
7564
let system_program = next_account_info(accounts_iter)?;
7665

@@ -80,25 +69,24 @@ pub fn process_closeaccount_multicastgroup(
8069
// Check if the payer is a signer
8170
assert!(payer_account.is_signer, "Payer must be a signer");
8271

83-
// Check the owner of the accounts
84-
assert_eq!(
85-
multicastgroup_account.owner, program_id,
86-
"Invalid PDA Account Owner"
72+
// Validate accounts
73+
validate_program_account!(
74+
multicastgroup_account,
75+
program_id,
76+
writable = true,
77+
"MulticastGroup"
8778
);
88-
assert_eq!(
89-
globalstate_account.owner, program_id,
90-
"Invalid GlobalState Account Owner"
79+
validate_program_account!(
80+
globalstate_account,
81+
program_id,
82+
writable = false,
83+
"GlobalState"
9184
);
9285
assert_eq!(
9386
*system_program.unsigned_key(),
9487
solana_system_interface::program::ID,
9588
"Invalid System Program Account Owner"
9689
);
97-
// Check if the account is writable
98-
assert!(
99-
multicastgroup_account.is_writable,
100-
"PDA Account is not writable"
101-
);
10290

10391
let globalstate = GlobalState::try_from(globalstate_account)?;
10492
if globalstate.activator_authority_pk != *payer_account.key {
@@ -119,25 +107,14 @@ pub fn process_closeaccount_multicastgroup(
119107
// Deallocate multicast_ip from ResourceExtension if account provided
120108
// Deallocation is idempotent - safe to call even if resource wasn't allocated
121109
if let Some(multicast_group_block_ext) = resource_extension_account {
122-
// Validate multicast_group_block_ext (MulticastGroupBlock - global)
123-
assert_eq!(
124-
multicast_group_block_ext.owner, program_id,
125-
"Invalid ResourceExtension Account Owner for MulticastGroupBlock"
126-
);
127-
assert!(
128-
multicast_group_block_ext.is_writable,
129-
"ResourceExtension Account for MulticastGroupBlock is not writable"
130-
);
131-
assert!(
132-
!multicast_group_block_ext.data_is_empty(),
133-
"ResourceExtension Account for MulticastGroupBlock is empty"
134-
);
135-
136-
let (expected_multicast_group_pda, _, _) =
110+
let (expected_pda, _, _) =
137111
get_resource_extension_pda(program_id, ResourceType::MulticastGroupBlock);
138-
assert_eq!(
139-
multicast_group_block_ext.key, &expected_multicast_group_pda,
140-
"Invalid ResourceExtension PDA for MulticastGroupBlock"
112+
validate_program_account!(
113+
multicast_group_block_ext,
114+
program_id,
115+
writable = true,
116+
pda = &expected_pda,
117+
"MulticastGroupBlock"
141118
);
142119

143120
// Deallocate multicast_ip from global MulticastGroupBlock
@@ -149,22 +126,25 @@ pub fn process_closeaccount_multicastgroup(
149126

150127
try_acc_close(multicastgroup_account, owner_account)?;
151128

152-
// Close the Index account if provided
153-
if let Some(index_acc) = index_account {
154-
assert_eq!(index_acc.owner, program_id, "Invalid Index Account Owner");
155-
assert!(index_acc.is_writable, "Index Account is not writable");
156-
129+
// Close the Index account (skip for pre-migration accounts using Pubkey::default())
130+
if *index_account.key != Pubkey::default() {
157131
let (expected_index_pda, _) =
158132
get_index_pda(program_id, SEED_MULTICAST_GROUP, &multicastgroup.code);
159-
assert_eq!(index_acc.key, &expected_index_pda, "Invalid Index Pubkey");
133+
validate_program_account!(
134+
index_account,
135+
program_id,
136+
writable = true,
137+
pda = &expected_index_pda,
138+
"Index"
139+
);
160140

161-
let index = Index::try_from(index_acc)?;
141+
let index = Index::try_from(index_account)?;
162142
assert_eq!(
163143
index.pk, *multicastgroup_account.key,
164144
"Index does not point to this MulticastGroup"
165145
);
166146

167-
try_acc_close(index_acc, payer_account)?;
147+
try_acc_close(index_account, payer_account)?;
168148
}
169149

170150
#[cfg(test)]

‎smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/create.rs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,18 @@ pub fn process_create_multicastgroup(
8484
validate_account_code(&value.code).map_err(|_| DoubleZeroError::InvalidAccountCode)?;
8585
let lowercase_code = code.to_ascii_lowercase();
8686

87-
// Check the owner of the accounts
88-
assert_eq!(
89-
globalstate_account.owner, program_id,
90-
"Invalid GlobalState Account Owner"
87+
// Validate accounts
88+
validate_program_account!(
89+
globalstate_account,
90+
program_id,
91+
writable = true,
92+
"GlobalState"
9193
);
9294
assert_eq!(
9395
*system_program.unsigned_key(),
9496
solana_system_interface::program::ID,
9597
"Invalid System Program Account Owner"
9698
);
97-
// Check if the account is writable
9899
assert!(mgroup_account.is_writable, "PDA Account is not writable");
99100

100101
// Parse the global state account & check if the payer is in the allowlist
@@ -183,6 +184,8 @@ pub fn process_create_multicastgroup(
183184
let index = Index {
184185
account_type: AccountType::Index,
185186
pk: *mgroup_account.key,
187+
entity_account_type: AccountType::MulticastGroup,
188+
key: multicastgroup.code.clone(),
186189
bump_seed: index_bump_seed,
187190
};
188191

‎smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/delete.rs‎

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,14 @@ pub struct MulticastGroupDeleteArgs {
3030
/// Requires ResourceExtension accounts and owner account.
3131
#[incremental(default = false)]
3232
pub use_onchain_deallocation: bool,
33-
/// When true, close the associated Index account alongside the multicast group.
34-
#[incremental(default = false)]
35-
pub close_index: bool,
3633
}
3734

3835
impl fmt::Debug for MulticastGroupDeleteArgs {
3936
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4037
write!(
4138
f,
42-
"use_onchain_deallocation: {}, close_index: {}",
43-
self.use_onchain_deallocation, self.close_index
39+
"use_onchain_deallocation: {}",
40+
self.use_onchain_deallocation
4441
)
4542
}
4643
}
@@ -55,13 +52,10 @@ pub fn process_delete_multicastgroup(
5552
let multicastgroup_account = next_account_info(accounts_iter)?;
5653
let globalstate_account = next_account_info(accounts_iter)?;
5754

58-
// Optional: additional accounts for atomic deallocation
59-
// Account layout WITH deallocation + index:
55+
// Account layout WITH deallocation:
6056
// [mgroup, globalstate, multicast_group_block, owner, index, payer, system]
61-
// Account layout WITHOUT deallocation, with index:
57+
// Account layout WITHOUT deallocation:
6258
// [mgroup, globalstate, index, payer, system]
63-
// Legacy (no deallocation, no index):
64-
// [mgroup, globalstate, payer, system]
6559
let deallocation_accounts = if value.use_onchain_deallocation {
6660
let multicast_group_block_ext = next_account_info(accounts_iter)?;
6761
let owner_account = next_account_info(accounts_iter)?;
@@ -70,12 +64,7 @@ pub fn process_delete_multicastgroup(
7064
None
7165
};
7266

73-
let index_account = if value.close_index {
74-
Some(next_account_info(accounts_iter)?)
75-
} else {
76-
None
77-
};
78-
67+
let index_account = next_account_info(accounts_iter)?;
7968
let payer_account = next_account_info(accounts_iter)?;
8069
let system_program = next_account_info(accounts_iter)?;
8170

@@ -85,24 +74,24 @@ pub fn process_delete_multicastgroup(
8574
// Check if the payer is a signer
8675
assert!(payer_account.is_signer, "Payer must be a signer");
8776

88-
// Check the owner of the accounts
89-
assert_eq!(
90-
multicastgroup_account.owner, program_id,
91-
"Invalid PDA Account Owner"
77+
// Validate accounts
78+
validate_program_account!(
79+
multicastgroup_account,
80+
program_id,
81+
writable = true,
82+
"MulticastGroup"
9283
);
93-
assert_eq!(
94-
globalstate_account.owner, program_id,
95-
"Invalid GlobalState Account Owner"
84+
validate_program_account!(
85+
globalstate_account,
86+
program_id,
87+
writable = false,
88+
"GlobalState"
9689
);
9790
assert_eq!(
9891
*system_program.unsigned_key(),
9992
solana_system_interface::program::ID,
10093
"Invalid System Program Account Owner"
10194
);
102-
assert!(
103-
multicastgroup_account.is_writable,
104-
"PDA Account is not writable"
105-
);
10695

10796
// Parse the global state account & check if the payer is in the allowlist
10897
let globalstate = GlobalState::try_from(globalstate_account)?;
@@ -172,24 +161,26 @@ pub fn process_delete_multicastgroup(
172161
msg!("Deleted: {:?}", multicastgroup_account);
173162
}
174163

175-
// Close the Index account if provided
176-
if let Some(index_acc) = index_account {
177-
assert_eq!(index_acc.owner, program_id, "Invalid Index Account Owner");
178-
assert!(index_acc.is_writable, "Index Account is not writable");
179-
180-
// Verify the Index PDA matches
164+
// Close the Index account (skip for pre-migration accounts using Pubkey::default())
165+
if *index_account.key != Pubkey::default() {
181166
let (expected_index_pda, _) =
182167
get_index_pda(program_id, SEED_MULTICAST_GROUP, &multicastgroup_code);
183-
assert_eq!(index_acc.key, &expected_index_pda, "Invalid Index Pubkey");
168+
validate_program_account!(
169+
index_account,
170+
program_id,
171+
writable = true,
172+
pda = &expected_index_pda,
173+
"Index"
174+
);
184175

185176
// Verify it's an Index account pointing to this multicast group
186-
let index = Index::try_from(index_acc)?;
177+
let index = Index::try_from(index_account)?;
187178
assert_eq!(
188179
index.pk, *multicastgroup_account.key,
189180
"Index does not point to this MulticastGroup"
190181
);
191182

192-
try_acc_close(index_acc, payer_account)?;
183+
try_acc_close(index_account, payer_account)?;
193184
}
194185

195186
Ok(())

0 commit comments

Comments
 (0)