Skip to content

Commit fb9d996

Browse files
smartcontract: rename subscribe processor for clarity (#3499)
Resolves: #3498 ## Summary - Rename `process_subscribe_multicastgroup` → `process_update_multicastgroup_subscription` and `subscribe_user_to_multicastgroup` → `update_user_multicastgroup_subscription` to reflect that these functions handle both subscribe and unsubscribe - Simplify the status validation guard to use a positive `is_subscribe` check instead of double-negated `!is_unsubscribe_only`, making the unsubscribe bypass for pending users easier to reason about ## Lines of Code | Section | Added | Removed | |---------|-------|---------| | serviceability processor | +7 | -11 | | serviceability entrypoint | +2 | -2 | | create_subscribe processor | +2 | -2 | | tests | +2 | -2 | ## Testing Verification - Existing `multicastgroup_subscribe_test`, `create_subscribe_user_test`, and `user_onchain_allocation_test` suites cover all subscribe/unsubscribe paths including the pending user regression added in #3494
1 parent 9583416 commit fb9d996

14 files changed

Lines changed: 152 additions & 148 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
1515
- Smartcontract
1616
- Allow `SubscribeMulticastGroup` for users in `Pending` status so that `CreateSubscribeUser` can be followed by additional subscribe calls before the activator runs ([#3521](https://github.com/malbeclabs/doublezero/pull/3521))
1717
- Add optional `owner` field to `UpdateMulticastGroup` instruction, allowing foundation members to reassign ownership of a multicast group ([#3527](https://github.com/malbeclabs/doublezero/pull/3527))
18+
- Rename `SubscribeMulticastGroup` instruction variant to `UpdateMulticastGroupRoles` and rename associated processor functions, args struct, and SDK command to use "roles" terminology, clarifying they manage publisher/subscriber roles rather than just subscriptions
1819
- Geolocation
1920
- Add optional result destination to `GeolocationUser` so LocationOffsets can be sent to an alternate endpoint instead of the target IP; supports both IP and domain destinations (e.g., `185.199.108.1:9000` or `results.example.com:9000`); includes `SetResultDestination` onchain instruction, CLI `user set-result-destination` command, and Go SDK deserialization (backwards-compatible with existing accounts)
2021
- CLI

client/doublezero/src/command/connect.rs

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use doublezero_sdk::{
1515
accesspass::get::GetAccessPassCommand,
1616
device::{get::GetDeviceCommand, list::ListDeviceCommand},
1717
multicastgroup::{
18-
list::ListMulticastGroupCommand, subscribe::SubscribeMulticastGroupCommand,
18+
list::ListMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand,
1919
},
2020
tenant::get::GetTenantCommand,
2121
user::{
@@ -621,7 +621,7 @@ impl ProvisioningCliCommand {
621621
// Subscribe to remaining groups
622622
for group_pk in all_group_pks.iter().skip(1) {
623623
spinner.println(format!(" Subscribing to group: {group_pk}"));
624-
client.subscribe_multicastgroup(SubscribeMulticastGroupCommand {
624+
client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand {
625625
user_pk,
626626
group_pk: *group_pk,
627627
client_ip: *client_ip,
@@ -647,13 +647,14 @@ impl ProvisioningCliCommand {
647647
user_pk
648648
));
649649

650-
let res = client.subscribe_multicastgroup(SubscribeMulticastGroupCommand {
651-
user_pk: *user_pk,
652-
group_pk: *group_pk,
653-
client_ip: *client_ip,
654-
publisher: true,
655-
subscriber: false,
656-
});
650+
let res =
651+
client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand {
652+
user_pk: *user_pk,
653+
group_pk: *group_pk,
654+
client_ip: *client_ip,
655+
publisher: true,
656+
subscriber: false,
657+
});
657658

658659
match res {
659660
Ok(_) => {
@@ -676,13 +677,14 @@ impl ProvisioningCliCommand {
676677
user_pk
677678
));
678679

679-
let res = client.subscribe_multicastgroup(SubscribeMulticastGroupCommand {
680-
user_pk: *user_pk,
681-
group_pk: *group_pk,
682-
client_ip: *client_ip,
683-
publisher: false,
684-
subscriber: true,
685-
});
680+
let res =
681+
client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand {
682+
user_pk: *user_pk,
683+
group_pk: *group_pk,
684+
client_ip: *client_ip,
685+
publisher: false,
686+
subscriber: true,
687+
});
686688

687689
match res {
688690
Ok(_) => {
@@ -757,7 +759,7 @@ impl ProvisioningCliCommand {
757759
// Subscribe to remaining groups
758760
for group_pk in all_group_pks.iter().skip(1) {
759761
spinner.println(format!(" Subscribing to group: {group_pk}"));
760-
client.subscribe_multicastgroup(SubscribeMulticastGroupCommand {
762+
client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand {
761763
user_pk,
762764
group_pk: *group_pk,
763765
client_ip: *client_ip,
@@ -1518,15 +1520,15 @@ mod tests {
15181520
}
15191521

15201522
#[allow(dead_code)]
1521-
pub fn expect_subscribe_multicastgroup(
1523+
pub fn expect_update_multicastgroup_roles(
15221524
&mut self,
15231525
user_pk: Pubkey,
15241526
mcast_group_pk: Pubkey,
15251527
client_ip: Ipv4Addr,
15261528
publisher: bool,
15271529
subscriber: bool,
15281530
) {
1529-
let expected_command = SubscribeMulticastGroupCommand {
1531+
let expected_command = UpdateMulticastGroupRolesCommand {
15301532
user_pk,
15311533
group_pk: mcast_group_pk,
15321534
client_ip,
@@ -1537,7 +1539,7 @@ mod tests {
15371539
let users = self.users.clone();
15381540
let provisioned = self.provisioned_services.clone();
15391541
self.client
1540-
.expect_subscribe_multicastgroup()
1542+
.expect_update_multicastgroup_roles()
15411543
.times(1)
15421544
.with(predicate::eq(expected_command))
15431545
.returning_st(move |cmd| {
@@ -1887,7 +1889,7 @@ mod tests {
18871889
let user_pk = fixture.add_user(&user);
18881890

18891891
// Expect subscribe to second group
1890-
fixture.expect_subscribe_multicastgroup(
1892+
fixture.expect_update_multicastgroup_roles(
18911893
user_pk,
18921894
mcast_group2_pk,
18931895
user.client_ip,
@@ -1963,8 +1965,8 @@ mod tests {
19631965
user.publishers.push(mcast_group_pk);
19641966
let user_pk = fixture.add_user(&user);
19651967

1966-
// Expect subscribe_multicastgroup call for the new subscriber group
1967-
fixture.expect_subscribe_multicastgroup(
1968+
// Expect update_multicastgroup_roles call for the new subscriber group
1969+
fixture.expect_update_multicastgroup_roles(
19681970
user_pk,
19691971
mcast_group2_pk,
19701972
user.client_ip,
@@ -2005,8 +2007,8 @@ mod tests {
20052007
user.subscribers.push(mcast_group_pk);
20062008
let user_pk = fixture.add_user(&user);
20072009

2008-
// Expect subscribe_multicastgroup call for the new publisher group
2009-
fixture.expect_subscribe_multicastgroup(
2010+
// Expect update_multicastgroup_roles call for the new publisher group
2011+
fixture.expect_update_multicastgroup_roles(
20102012
user_pk,
20112013
mcast_group2_pk,
20122014
user.client_ip,

smartcontract/cli/src/doublezerocommand.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ use doublezero_sdk::{
7878
get::GetMulticastGroupCommand,
7979
list::ListMulticastGroupCommand,
8080
reject::RejectMulticastGroupCommand,
81-
subscribe::SubscribeMulticastGroupCommand,
81+
subscribe::UpdateMulticastGroupRolesCommand,
8282
update::UpdateMulticastGroupCommand,
8383
},
8484
permission::{
@@ -298,9 +298,9 @@ pub trait CliCommand {
298298
&self,
299299
cmd: DeactivateMulticastGroupCommand,
300300
) -> eyre::Result<Signature>;
301-
fn subscribe_multicastgroup(
301+
fn update_multicastgroup_roles(
302302
&self,
303-
cmd: SubscribeMulticastGroupCommand,
303+
cmd: UpdateMulticastGroupRolesCommand,
304304
) -> eyre::Result<Signature>;
305305
fn add_multicastgroup_pub_allowlist(
306306
&self,
@@ -737,9 +737,9 @@ impl CliCommand for CliCommandImpl<'_> {
737737
) -> eyre::Result<Signature> {
738738
cmd.execute(self.client)
739739
}
740-
fn subscribe_multicastgroup(
740+
fn update_multicastgroup_roles(
741741
&self,
742-
cmd: SubscribeMulticastGroupCommand,
742+
cmd: UpdateMulticastGroupRolesCommand,
743743
) -> eyre::Result<Signature> {
744744
cmd.execute(self.client)
745745
}

smartcontract/cli/src/user/subscribe.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
};
88
use clap::Args;
99
use doublezero_sdk::commands::{
10-
multicastgroup::{get::GetMulticastGroupCommand, subscribe::SubscribeMulticastGroupCommand},
10+
multicastgroup::{get::GetMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand},
1111
user::get::GetUserCommand,
1212
};
1313
use std::io::Write;
@@ -59,13 +59,14 @@ impl SubscribeUserCliCommand {
5959

6060
// Subscribe to each group
6161
for group_pk in &group_pks {
62-
let signature = client.subscribe_multicastgroup(SubscribeMulticastGroupCommand {
63-
user_pk,
64-
group_pk: *group_pk,
65-
client_ip: user.client_ip,
66-
publisher: self.publisher,
67-
subscriber: self.subscriber,
68-
})?;
62+
let signature =
63+
client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand {
64+
user_pk,
65+
group_pk: *group_pk,
66+
client_ip: user.client_ip,
67+
publisher: self.publisher,
68+
subscriber: self.subscriber,
69+
})?;
6970
writeln!(out, "Subscribed to {group_pk}: {signature}")?;
7071
}
7172

@@ -93,7 +94,7 @@ mod tests {
9394
use doublezero_sdk::{
9495
commands::{
9596
multicastgroup::{
96-
get::GetMulticastGroupCommand, subscribe::SubscribeMulticastGroupCommand,
97+
get::GetMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand,
9798
},
9899
user::get::GetUserCommand,
99100
},
@@ -172,8 +173,8 @@ mod tests {
172173
}))
173174
.returning(move |_| Ok((mgroup_pubkey, mgroup.clone())));
174175
client
175-
.expect_subscribe_multicastgroup()
176-
.with(predicate::eq(SubscribeMulticastGroupCommand {
176+
.expect_update_multicastgroup_roles()
177+
.with(predicate::eq(UpdateMulticastGroupRolesCommand {
177178
user_pk: user_pubkey,
178179
group_pk: mgroup_pubkey,
179180
client_ip,
@@ -292,7 +293,7 @@ mod tests {
292293
}))
293294
.returning(move |_| Ok((mgroup_pubkey2, mgroup2.clone())));
294295
client
295-
.expect_subscribe_multicastgroup()
296+
.expect_update_multicastgroup_roles()
296297
.times(2)
297298
.returning(move |_| Ok(signature));
298299

smartcontract/programs/doublezero-serviceability/src/entrypoint.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ use crate::{
7777
delete::process_delete_multicastgroup,
7878
reactivate::process_reactivate_multicastgroup,
7979
reject::process_reject_multicastgroup,
80-
subscribe::process_subscribe_multicastgroup,
80+
subscribe::process_update_multicastgroup_roles,
8181
suspend::process_suspend_multicastgroup,
8282
update::process_update_multicastgroup,
8383
},
@@ -293,8 +293,8 @@ pub fn process_instruction(
293293
DoubleZeroInstruction::RemoveMulticastGroupSubAllowlist(value) => {
294294
process_remove_multicast_sub_allowlist(program_id, accounts, &value)?
295295
}
296-
DoubleZeroInstruction::SubscribeMulticastGroup(value) => {
297-
process_subscribe_multicastgroup(program_id, accounts, &value)?
296+
DoubleZeroInstruction::UpdateMulticastGroupRoles(value) => {
297+
process_update_multicastgroup_roles(program_id, accounts, &value)?
298298
}
299299
DoubleZeroInstruction::CreateSubscribeUser(value) => {
300300
process_create_subscribe_user(program_id, accounts, &value)?

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ use crate::processors::{
6363
delete::MulticastGroupDeleteArgs,
6464
reactivate::MulticastGroupReactivateArgs,
6565
reject::MulticastGroupRejectArgs,
66-
subscribe::MulticastGroupSubscribeArgs,
66+
subscribe::UpdateMulticastGroupRolesArgs,
6767
suspend::MulticastGroupSuspendArgs,
6868
update::MulticastGroupUpdateArgs,
6969
},
@@ -162,8 +162,8 @@ pub enum DoubleZeroInstruction {
162162
AddMulticastGroupSubAllowlist(AddMulticastGroupSubAllowlistArgs), // variant 56
163163
RemoveMulticastGroupSubAllowlist(RemoveMulticastGroupSubAllowlistArgs), // variant 57
164164

165-
SubscribeMulticastGroup(MulticastGroupSubscribeArgs), // variant 58
166-
CreateSubscribeUser(UserCreateSubscribeArgs), // variant 59
165+
UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs), // variant 58
166+
CreateSubscribeUser(UserCreateSubscribeArgs), // variant 59
167167

168168
CreateContributor(ContributorCreateArgs), // variant 60
169169
UpdateContributor(ContributorUpdateArgs), // variant 61
@@ -304,7 +304,7 @@ impl DoubleZeroInstruction {
304304
55 => Ok(Self::RemoveMulticastGroupPubAllowlist(RemoveMulticastGroupPubAllowlistArgs::try_from(rest).unwrap())),
305305
56 => Ok(Self::AddMulticastGroupSubAllowlist(AddMulticastGroupSubAllowlistArgs::try_from(rest).unwrap())),
306306
57 => Ok(Self::RemoveMulticastGroupSubAllowlist(RemoveMulticastGroupSubAllowlistArgs::try_from(rest).unwrap())),
307-
58 => Ok(Self::SubscribeMulticastGroup(MulticastGroupSubscribeArgs::try_from(rest).unwrap())),
307+
58 => Ok(Self::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs::try_from(rest).unwrap())),
308308
59 => Ok(Self::CreateSubscribeUser(UserCreateSubscribeArgs::try_from(rest).unwrap())),
309309

310310
60 => Ok(Self::CreateContributor(ContributorCreateArgs::try_from(rest).unwrap())),
@@ -438,8 +438,8 @@ impl DoubleZeroInstruction {
438438
"RemoveMulticastGroupSubAllowlist".to_string()
439439
} // variant 57
440440

441-
Self::SubscribeMulticastGroup(_) => "SubscribeMulticastGroup".to_string(), // variant 58
442-
Self::CreateSubscribeUser(_) => "CreateSubscribeUser".to_string(), // variant 59
441+
Self::UpdateMulticastGroupRoles(_) => "UpdateMulticastGroupRoles".to_string(), // variant 58
442+
Self::CreateSubscribeUser(_) => "CreateSubscribeUser".to_string(), // variant 59
443443

444444
Self::CreateContributor(_) => "CreateContributor".to_string(), // variant 60
445445
Self::UpdateContributor(_) => "UpdateContributor".to_string(), // variant 61
@@ -564,7 +564,7 @@ impl DoubleZeroInstruction {
564564
Self::DeleteMulticastGroup(args) => format!("{args:?}"), // variant 51
565565
Self::UpdateMulticastGroup(args) => format!("{args:?}"), // variant 52
566566
Self::DeactivateMulticastGroup(args) => format!("{args:?}"), // variant 53
567-
Self::SubscribeMulticastGroup(args) => format!("{args:?}"), // variant 54
567+
Self::UpdateMulticastGroupRoles(args) => format!("{args:?}"), // variant 54
568568
Self::AddMulticastGroupPubAllowlist(args) => format!("{args:?}"), // variant 55
569569
Self::RemoveMulticastGroupPubAllowlist(args) => format!("{args:?}"), // variant 56
570570
Self::AddMulticastGroupSubAllowlist(args) => format!("{args:?}"), // variant 57
@@ -1060,13 +1060,13 @@ mod tests {
10601060
"RemoveMulticastGroupSubAllowlist",
10611061
);
10621062
test_instruction(
1063-
DoubleZeroInstruction::SubscribeMulticastGroup(MulticastGroupSubscribeArgs {
1063+
DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs {
10641064
client_ip: [1, 2, 3, 4].into(),
10651065
publisher: false,
10661066
subscriber: true,
10671067
use_onchain_allocation: false,
10681068
}),
1069-
"SubscribeMulticastGroup",
1069+
"UpdateMulticastGroupRoles",
10701070
);
10711071
test_instruction(
10721072
DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs {

smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use solana_program::{
2727
};
2828
use std::{fmt, net::Ipv4Addr};
2929
#[derive(BorshSerialize, BorshDeserializeIncremental, PartialEq, Clone)]
30-
pub struct MulticastGroupSubscribeArgs {
30+
pub struct UpdateMulticastGroupRolesArgs {
3131
#[incremental(default = Ipv4Addr::UNSPECIFIED)]
3232
pub client_ip: Ipv4Addr,
3333
pub publisher: bool,
@@ -36,7 +36,7 @@ pub struct MulticastGroupSubscribeArgs {
3636
pub use_onchain_allocation: bool,
3737
}
3838

39-
impl fmt::Debug for MulticastGroupSubscribeArgs {
39+
impl fmt::Debug for UpdateMulticastGroupRolesArgs {
4040
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4141
write!(
4242
f,
@@ -54,13 +54,13 @@ pub struct SubscribeUserResult {
5454
pub publisher_list_transitioned: bool,
5555
}
5656

57-
/// Toggle a user's multicast group subscription.
57+
/// Toggle a user's multicast group roles.
5858
///
5959
/// Handles both create-time subscription (user lists start empty, only adds)
6060
/// and post-activation subscription changes (add/remove toggle). The caller is
6161
/// responsible for setting `user.status = Updating` when
6262
/// `publisher_list_transitioned` is true and the user is already activated.
63-
pub fn subscribe_user_to_multicastgroup(
63+
pub fn update_user_multicastgroup_roles(
6464
mgroup_account: &AccountInfo,
6565
accesspass: &AccessPass,
6666
user: &mut User,
@@ -130,10 +130,10 @@ pub fn subscribe_user_to_multicastgroup(
130130
})
131131
}
132132

133-
pub fn process_subscribe_multicastgroup(
133+
pub fn process_update_multicastgroup_roles(
134134
program_id: &Pubkey,
135135
accounts: &[AccountInfo],
136-
value: &MulticastGroupSubscribeArgs,
136+
value: &UpdateMulticastGroupRolesArgs,
137137
) -> ProgramResult {
138138
let num_accounts = accounts.len();
139139
let accounts_iter = &mut accounts.iter();
@@ -171,7 +171,7 @@ pub fn process_subscribe_multicastgroup(
171171
let system_program = next_account_info(accounts_iter)?;
172172

173173
#[cfg(test)]
174-
msg!("process_subscribe_multicastgroup({:?})", value);
174+
msg!("process_update_multicastgroup_roles({:?})", value);
175175

176176
// Check if the payer is a signer
177177
assert!(payer_account.is_signer, "Payer must be a signer");
@@ -201,12 +201,12 @@ pub fn process_subscribe_multicastgroup(
201201

202202
// Parse and validate user
203203
let mut user: User = User::try_from(user_account)?;
204-
// Allow subscribe for Pending users so that CreateSubscribeUser (which
205-
// only takes one mgroup) can be followed by additional SubscribeMulticastGroup
206-
// calls before the activator runs. Also allow pure-unsubscribe (both false)
207-
// for any status so cleanup works before activation.
208-
let is_unsubscribe_only = !value.publisher && !value.subscriber;
209-
if !is_unsubscribe_only
204+
// Removing all roles is allowed for any status so that users
205+
// created via CreateSubscribeUser can be cleaned up before activation.
206+
// Adding roles is also allowed for Pending users so that CreateSubscribeUser
207+
// (which only takes one mgroup) can be followed by additional calls.
208+
let has_role = value.publisher || value.subscriber;
209+
if has_role
210210
&& user.status != UserStatus::Activated
211211
&& user.status != UserStatus::Updating
212212
&& user.status != UserStatus::Pending
@@ -254,7 +254,7 @@ pub fn process_subscribe_multicastgroup(
254254
}
255255
}
256256

257-
let result = subscribe_user_to_multicastgroup(
257+
let result = update_user_multicastgroup_roles(
258258
mgroup_account,
259259
&accesspass,
260260
&mut user,

0 commit comments

Comments
 (0)