diff --git a/CHANGELOG.md b/CHANGELOG.md index 319dcd4069..04be1f6a2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,17 @@ All notable changes to this project will be documented in this file. ### Breaking +- SDK + - `UpdateMulticastGroupRolesCommand.group_pk: Pubkey` becomes `group_pks: Vec` and `CreateSubscribeUserCommand.mgroup_pk: Pubkey` becomes `mgroup_pks: Vec` (non-empty; the first entry is the instruction's primary group). The RFC-26 builders `update_multicast_group_roles` and `create_subscribe_user` gain an `extra_groups: &[Pubkey]` parameter and derive the new `extra_group_count` arg from it. Single-group callers pass a one-element vec / empty slice. `CreateSubscribeUserCommand` measures the built transaction's wire size and rejects a group set that cannot fit under the 1232-byte limit, naming how many groups do fit: the create also carries the device's dz_prefix accounts and an optional feed, so the 16-group role-update chunk does not bound it. (malbeclabs/infra#2114) + ### Changes +- CLI + - `doublezero connect Multicast` with N groups is one transaction in the common case (all groups sharing one publisher/subscriber flag pair fold into the create, skipping the activation wait); `doublezero multicast subscribe|unsubscribe|publish|unpublish`, `doublezero user subscribe`, and the role-strip cleanup in `user delete`/`request-ban` batch their role changes by flag pair, chunked to 16 groups per transaction. Failure reporting in the multicast verbs is per batch: a failed batch lists every group it carried, since none was applied. (malbeclabs/infra#2114) + - `doublezero feed update|delete --force-unsubscribe` strips each user's orphaned groups with one batched role update per user (chunked to 16 groups per transaction) instead of one transaction per group. (malbeclabs/infra#2114) +- Serviceability + - `UpdateMulticastGroupRoles` (58) and `CreateSubscribeUser` (59) accept additional writable MulticastGroup accounts (counted by a new borsh-incremental `extra_group_count: u8` arg), so subscribing a user to N groups is one atomic transaction instead of N: one signature/fee, and a failure rolls back every group. Each batch member is authorized exactly like a single-group call (per-group allowlist checks; in `CreateSubscribeUser`, EdgeSeat extras are coverage-checked against the single passed feed and the seat still ticks once per user per feed, so a seat tick can no longer outlive a partial subscription). Duplicate group accounts in a batch are rejected. Old encodings without the count byte decode as 0, so existing clients are unaffected. Deploy ordering (RFC-1): the program must deploy to all clusters before any client that emits batches — an old program would misread the extra group accounts as the trailing optional accounts. (malbeclabs/infra#2114) + ## [v0.37.0](https://github.com/malbeclabs/doublezero/compare/client/v0.36.0...client/v0.37.0) - 2026-08-21 ### Breaking diff --git a/crates/doublezero-daemon-cli/src/connect.rs b/crates/doublezero-daemon-cli/src/connect.rs index 8906442902..d6e70ee1da 100644 --- a/crates/doublezero-daemon-cli/src/connect.rs +++ b/crates/doublezero-daemon-cli/src/connect.rs @@ -14,7 +14,8 @@ use doublezero_cli_core::CliContext; use doublezero_sdk::{ commands::{ multicastgroup::{ - subscribe::UpdateMulticastGroupRolesCommand, subscribe_feed::SubscribeFeedCommand, + subscribe::{UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION}, + subscribe_feed::SubscribeFeedCommand, unsubscribe_feed::UnsubscribeFeedCommand, }, user::{create::CreateUserCommand, create_subscribe::CreateSubscribeUserCommand}, @@ -1462,19 +1463,23 @@ impl Connect { return Err(eyre::eyre!(err_msg)); } - // Create user with first group (pick from pub_groups first, then sub_groups) - let first_group_pk = all_group_pks - .first() - .ok_or_else(|| eyre::eyre!("At least one multicast group is required"))?; + // Create the user subscribed to every group sharing the first group's + // flag pair in one transaction; other flag pairs follow as batched + // role updates. + let (create_group_pks, (create_publisher, create_subscriber), follow_up_batches) = + plan_group_batches(&all_group_pks, pub_group_pks, sub_group_pks); + if create_group_pks.is_empty() { + eyre::bail!("At least one multicast group is required"); + } let res = ledger.create_subscribe_user(CreateSubscribeUserCommand { user_type: UserType::Multicast, device_pk, cyoa_type: ibrl_user.cyoa_type, client_ip: *client_ip, - mgroup_pk: *first_group_pk, - publisher: pub_group_pks.contains(first_group_pk), - subscriber: sub_group_pks.contains(first_group_pk), + publisher: create_publisher, + subscriber: create_subscriber, + mgroup_pks: create_group_pks, tunnel_endpoint, owner: None, feed_pk: None, @@ -1492,20 +1497,21 @@ impl Connect { } }; - // Wait for user to be activated before subscribing to additional groups - if all_group_pks.len() > 1 { + // Groups with other flag pairs need the user Activated before their + // role updates; the common case (all groups share one flag pair) + // skips the wait entirely. + if !follow_up_batches.is_empty() { self.poll_for_user_activated(ledger, &user_pk, spinner)?; } - - // Subscribe to remaining groups - for group_pk in all_group_pks.iter().skip(1) { - spinner.set_message(format!("Subscribing to group: {group_pk}")); + for (publisher, subscriber, group_pks) in follow_up_batches { + spinner + .set_message(format!("Subscribing to {} more group(s)", group_pks.len())); ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { user_pk, - group_pk: *group_pk, + group_pks, client_ip: *client_ip, - publisher: pub_group_pks.contains(group_pk), - subscriber: sub_group_pks.contains(group_pk), + publisher, + subscriber, device_pk: None, feed_pk: None, })?; @@ -1520,68 +1526,56 @@ impl Connect { self.poll_for_user_activated(ledger, user_pk, spinner)?; } - // Subscribe to any pub groups not already subscribed - for group_pk in pub_group_pks { - if !user.publishers.contains(group_pk) { - spinner.set_message(format!( - "Adding publisher subscription to existing Multicast user: {user_pk}" - )); - - let res = - ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk: *user_pk, - group_pk: *group_pk, - client_ip: *client_ip, - publisher: true, - subscriber: false, - device_pk: None, - feed_pk: None, - }); - - match res { - Ok(_) => { - spinner.set_message("Publisher subscription added"); - } - Err(e) => { - writeln!(out, "❌ Error adding publisher subscription")?; - writeln!(out, "\nError: {e:?}\n")?; - eyre::bail!( - "Error adding publisher subscription to existing user: {e:?}" - ); - } - } + // Add the requested roles, batched by each group's effective + // (publisher, subscriber) flag pair. The instruction sets absolute + // role state, so the desired flags union the request with the roles + // the user already holds — a group in both --publish and --subscribe + // (or already holding the other role) keeps both instead of the last + // write stripping the first. + let mut batches: Vec<(bool, bool, Vec)> = Vec::new(); + for group_pk in all_group_pks.iter() { + let publisher = + pub_group_pks.contains(group_pk) || user.publishers.contains(group_pk); + let subscriber = + sub_group_pks.contains(group_pk) || user.subscribers.contains(group_pk); + // Skip groups whose desired state is already onchain. + if user.publishers.contains(group_pk) == publisher + && user.subscribers.contains(group_pk) == subscriber + { + continue; + } + match batches.iter_mut().find(|(p, s, pks)| { + (*p, *s) == (publisher, subscriber) + && pks.len() < MAX_GROUPS_PER_TRANSACTION + }) { + Some((_, _, pks)) => pks.push(*group_pk), + None => batches.push((publisher, subscriber, vec![*group_pk])), } } + for (publisher, subscriber, group_pks) in batches { + spinner.set_message(format!( + "Adding subscription to existing Multicast user: {user_pk}" + )); + + let res = + ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { + user_pk: *user_pk, + group_pks, + client_ip: *client_ip, + publisher, + subscriber, + device_pk: None, + feed_pk: None, + }); - // Subscribe to any sub groups not already subscribed - for group_pk in sub_group_pks { - if !user.subscribers.contains(group_pk) { - spinner.set_message(format!( - "Adding subscriber subscription to existing Multicast user: {user_pk}" - )); - - let res = - ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk: *user_pk, - group_pk: *group_pk, - client_ip: *client_ip, - publisher: false, - subscriber: true, - device_pk: None, - feed_pk: None, - }); - - match res { - Ok(_) => { - spinner.set_message("Subscriber subscription added"); - } - Err(e) => { - writeln!(out, "❌ Error adding subscriber subscription")?; - writeln!(out, "\nError: {e:?}\n")?; - eyre::bail!( - "Error adding subscriber subscription to existing user: {e:?}" - ); - } + match res { + Ok(_) => { + spinner.set_message("Subscription added"); + } + Err(e) => { + writeln!(out, "❌ Error adding subscription")?; + writeln!(out, "\nError: {e:?}\n")?; + eyre::bail!("Error adding subscription to existing user: {e:?}"); } } } @@ -1607,19 +1601,23 @@ impl Connect { return Err(eyre::eyre!(err_msg)); } - // Create user with first group (pick from pub_groups first, then sub_groups) - let first_group_pk = all_group_pks - .first() - .ok_or_else(|| eyre::eyre!("At least one multicast group is required"))?; + // Create the user subscribed to every group sharing the first group's + // flag pair in one transaction; other flag pairs follow as batched + // role updates. + let (create_group_pks, (create_publisher, create_subscriber), follow_up_batches) = + plan_group_batches(&all_group_pks, pub_group_pks, sub_group_pks); + if create_group_pks.is_empty() { + eyre::bail!("At least one multicast group is required"); + } let res = ledger.create_subscribe_user(CreateSubscribeUserCommand { user_type: UserType::Multicast, device_pk, cyoa_type: UserCYOA::GREOverDIA, client_ip: *client_ip, - mgroup_pk: *first_group_pk, - publisher: pub_group_pks.contains(first_group_pk), - subscriber: sub_group_pks.contains(first_group_pk), + publisher: create_publisher, + subscriber: create_subscriber, + mgroup_pks: create_group_pks, tunnel_endpoint, owner: None, feed_pk: None, @@ -1637,20 +1635,21 @@ impl Connect { } }; - // Wait for user to be activated before subscribing to additional groups - if all_group_pks.len() > 1 { + // Groups with other flag pairs need the user Activated before their + // role updates; the common case (all groups share one flag pair) + // skips the wait entirely. + if !follow_up_batches.is_empty() { self.poll_for_user_activated(ledger, &user_pk, spinner)?; } - - // Subscribe to remaining groups - for group_pk in all_group_pks.iter().skip(1) { - spinner.set_message(format!("Subscribing to group: {group_pk}")); + for (publisher, subscriber, group_pks) in follow_up_batches { + spinner + .set_message(format!("Subscribing to {} more group(s)", group_pks.len())); ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { user_pk, - group_pk: *group_pk, + group_pks, client_ip: *client_ip, - publisher: pub_group_pks.contains(group_pk), - subscriber: sub_group_pks.contains(group_pk), + publisher, + subscriber, device_pk: None, feed_pk: None, })?; @@ -1784,6 +1783,54 @@ impl Connect { } } +/// Upper bound on multicast groups folded into a single CreateSubscribeUser +/// transaction. The instruction already carries the device's dz_prefix blocks, so +/// its account headroom under the 1232-byte transaction size limit is smaller than +/// UpdateMulticastGroupRoles'; 8 groups leave room for several dz_prefix blocks +/// (devices typically advertise one or two). Overflow rides in follow-up update +/// batches, which are chunked to [`MAX_GROUPS_PER_TRANSACTION`]. +const MAX_CREATE_GROUPS: usize = 8; + +/// One follow-up UpdateMulticastGroupRoles batch: (publisher, subscriber, group_pks). +type RoleBatch = (bool, bool, Vec); + +/// Split the deduplicated group list into the batch folded into the +/// CreateSubscribeUser transaction — every group sharing the first group's +/// (publisher, subscriber) flag pair, up to [`MAX_CREATE_GROUPS`] — and the +/// follow-up UpdateMulticastGroupRoles batches for the rest, grouped by flag pair +/// and chunked to [`MAX_GROUPS_PER_TRANSACTION`]. Returns the create batch, its +/// shared flag pair, and the follow-up batches. In the common case (all groups +/// share one flag pair) the follow-up list is empty and connect is a single +/// transaction. +fn plan_group_batches( + all_group_pks: &[Pubkey], + pub_group_pks: &[Pubkey], + sub_group_pks: &[Pubkey], +) -> (Vec, (bool, bool), Vec) { + let flags_of = |pk: &Pubkey| (pub_group_pks.contains(pk), sub_group_pks.contains(pk)); + let Some(first_flags) = all_group_pks.first().map(flags_of) else { + return (Vec::new(), (false, false), Vec::new()); + }; + + let mut create_group_pks = Vec::new(); + let mut follow_ups: Vec = Vec::new(); + for pk in all_group_pks { + let (publisher, subscriber) = flags_of(pk); + if (publisher, subscriber) == first_flags && create_group_pks.len() < MAX_CREATE_GROUPS { + create_group_pks.push(*pk); + continue; + } + // A full batch stops matching, so oversize flag pairs chunk naturally. + match follow_ups.iter_mut().find(|(p, s, pks)| { + (*p, *s) == (publisher, subscriber) && pks.len() < MAX_GROUPS_PER_TRANSACTION + }) { + Some((_, _, pks)) => pks.push(*pk), + None => follow_ups.push((publisher, subscriber, vec![*pk])), + } + } + (create_group_pks, first_flags, follow_ups) +} + fn exclude_ips( users: &HashMap, client_ip: &Ipv4Addr, @@ -2766,7 +2813,7 @@ mod tests { &mut self, pk: Pubkey, user: &User, - mcast_group_pk: Pubkey, + mgroup_pks: Vec, publisher: bool, subscriber: bool, ) { @@ -2775,7 +2822,7 @@ mod tests { device_pk: user.device_pk, cyoa_type: UserCYOA::GREOverDIA, client_ip: user.client_ip, - mgroup_pk: mcast_group_pk, + mgroup_pks: mgroup_pks.clone(), publisher, subscriber, tunnel_endpoint: user.tunnel_endpoint, @@ -2787,10 +2834,10 @@ mod tests { let provisioned = self.provisioned_services.clone(); let mut user = user.clone(); if publisher { - user.publishers.push(mcast_group_pk); + user.publishers.extend(&mgroup_pks); } if subscriber { - user.subscribers.push(mcast_group_pk); + user.subscribers.extend(&mgroup_pks); } self.ledger .expect_create_subscribe_user() @@ -2808,14 +2855,14 @@ mod tests { pub fn expect_update_multicastgroup_roles( &mut self, user_pk: Pubkey, - mcast_group_pk: Pubkey, + group_pks: Vec, client_ip: Ipv4Addr, publisher: bool, subscriber: bool, ) { let expected_command = UpdateMulticastGroupRolesCommand { user_pk, - group_pk: mcast_group_pk, + group_pks, client_ip, publisher, subscriber, @@ -2834,10 +2881,10 @@ mod tests { let mut users = users.lock().unwrap(); if let Some(user) = users.get_mut(&cmd.user_pk) { if cmd.publisher { - user.publishers.push(cmd.group_pk); + user.publishers.extend(&cmd.group_pks); } if cmd.subscriber { - user.subscribers.push(cmd.group_pk); + user.subscribers.extend(&cmd.group_pks); } provisioned .lock() @@ -2998,7 +3045,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &mcast_user, - mcast_group_pk, + vec![mcast_group_pk], true, // publisher false, // subscriber ); @@ -3059,7 +3106,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &mcast_user, - mcast_group_pk, + vec![mcast_group_pk], true, // publisher false, // subscriber ); @@ -3223,7 +3270,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], true, false, ); @@ -3270,11 +3317,11 @@ mod tests { // First group (g1) created via create_subscribe_user as publisher + subscriber. let user_pk = Pubkey::new_unique(); - fixture.expect_create_subscribe_user(user_pk, &user, g1_pk, true, true); + fixture.expect_create_subscribe_user(user_pk, &user, vec![g1_pk], true, true); // Remaining group (g2) added via update_multicastgroup_roles as subscriber-only. fixture.expect_update_multicastgroup_roles( user_pk, - g2_pk, + vec![g2_pk], Ipv4Addr::new(1, 2, 3, 4), false, true, @@ -3305,6 +3352,55 @@ mod tests { }); } + /// When every authorized group shares the same (publisher, subscriber) flag pair, + /// all of them fold into the single CreateSubscribeUser transaction and no + /// follow-up role update is issued. + #[test] + fn test_connect_command_multicast_single_transaction_when_flags_match() { + block_on(async { + let mut fixture = TestFixture::new(); + + let (g1_pk, _) = fixture.add_multicast_group("group-1", "239.0.0.1"); + let (g2_pk, _) = fixture.add_multicast_group("group-2", "239.0.0.2"); + + // Subscribe-only for both groups → one shared flag pair. + { + let mut ap = fixture.accesspass.lock().unwrap(); + ap.mgroup_sub_allowlist = vec![g1_pk, g2_pk]; + } + + let (device1_pk, _device1) = fixture.add_device(DeviceType::Hybrid, 100, true); + let user = fixture.create_user(UserType::Multicast, device1_pk, "1.2.3.4"); + + // Both groups ride in the create, in allowlist order. No + // expect_update_multicastgroup_roles: any such call would panic the mock. + let user_pk = Pubkey::new_unique(); + fixture.expect_create_subscribe_user(user_pk, &user, vec![g1_pk, g2_pk], false, true); + + let command = Connect { + dz_mode: DzMode::Multicast { + mode: None, + multicast_groups: vec![], + pub_groups: vec![], + sub_groups: vec![], + sub_feeds: vec![], + unsub_feeds: vec![], + }, + client_ip: None, + device: None, + verbose: false, + }; + + let (result, output) = run(&fixture, command).await; + assert!( + result.is_ok(), + "single-transaction auto-join must succeed: {:?}", + result.err() + ); + assert!(output.contains("Subscribing to (from AccessPass): group-1, group-2")); + }); + } + /// Auto-join is a no-op success when the AccessPass authorizes no groups: no user /// is created and no subscriptions are issued. #[test] @@ -3364,7 +3460,7 @@ mod tests { // Only g1 survives filtering → single create_subscribe_user as subscriber-only, // no further update calls. let user_pk = Pubkey::new_unique(); - fixture.expect_create_subscribe_user(user_pk, &user, g1_pk, false, true); + fixture.expect_create_subscribe_user(user_pk, &user, vec![g1_pk], false, true); let command = Connect { dz_mode: DzMode::Multicast { @@ -3436,7 +3532,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], true, false, ); @@ -3478,7 +3574,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], false, true, ); @@ -3529,7 +3625,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &mcast_user, - mcast_group_pk, + vec![mcast_group_pk], false, true, ); @@ -3559,8 +3655,56 @@ mod tests { /// Existing multicast user subscribes to a new group with expired access pass. /// Exercises the `(_, Some(mcast))` branch of `find_or_create_user_and_subscribe`, - /// which calls UpdateMulticastGroupRoles (the on-chain processor never had an epoch + /// which calls UpdateMulticastGroupRoles (the onchain processor never had an epoch /// check; this test verifies the CLI gate no longer blocks it either). + /// A group requested in BOTH --publish and --subscribe on an existing Multicast + /// user gets one update with both flags, instead of a publisher add that a later + /// subscriber-only write strips (the instruction sets absolute role state). + #[test] + fn test_connect_existing_user_group_in_both_lists_keeps_both_roles() { + block_on(async { + let mut fixture = TestFixture::new(); + + let (mcast_group_pk, _mcast_group) = + fixture.add_multicast_group("test-group", "239.0.0.1"); + let (device1_pk, _device1) = fixture.add_device(DeviceType::Hybrid, 100, true); + + // Existing multicast user with no roles yet. + let user = fixture.create_user(UserType::Multicast, device1_pk, "1.2.3.4"); + let user_pk = fixture.add_user(&user); + + // Exactly ONE update carrying both roles. + fixture.expect_update_multicastgroup_roles( + user_pk, + vec![mcast_group_pk], + user.client_ip, + true, + true, + ); + + let command = Connect { + dz_mode: DzMode::Multicast { + mode: None, + multicast_groups: vec![], + pub_groups: vec!["test-group".to_string()], + sub_groups: vec!["test-group".to_string()], + sub_feeds: vec![], + unsub_feeds: vec![], + }, + client_ip: Some(user.client_ip.to_string()), + device: None, + verbose: false, + }; + + let (result, _) = run(&fixture, command).await; + assert!( + result.is_ok(), + "both-lists connect on existing user must succeed: {:?}", + result.err() + ); + }); + } + #[test] fn test_connect_command_multicast_add_group_to_existing_user_with_expired_accesspass() { block_on(async { @@ -3581,7 +3725,7 @@ mod tests { // Expect UpdateMulticastGroupRoles for the new group fixture.expect_update_multicastgroup_roles( user_pk, - mcast_group2_pk, + vec![mcast_group2_pk], user.client_ip, false, true, @@ -3669,7 +3813,7 @@ mod tests { // Expect subscribe to second group fixture.expect_update_multicastgroup_roles( user_pk, - mcast_group2_pk, + vec![mcast_group2_pk], user.client_ip, publisher, subscriber, @@ -3745,7 +3889,7 @@ mod tests { // Expect update_multicastgroup_roles call for the new subscriber group fixture.expect_update_multicastgroup_roles( user_pk, - mcast_group2_pk, + vec![mcast_group2_pk], user.client_ip, false, true, @@ -3790,7 +3934,7 @@ mod tests { // Expect update_multicastgroup_roles call for the new publisher group fixture.expect_update_multicastgroup_roles( user_pk, - mcast_group2_pk, + vec![mcast_group2_pk], user.client_ip, true, false, @@ -3892,7 +4036,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], false, true, ); @@ -3949,7 +4093,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], false, true, ); @@ -4144,7 +4288,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], true, // publisher false, // not subscriber ); @@ -4191,7 +4335,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - mcast_group_pk, + vec![mcast_group_pk], false, // not publisher true, // subscriber ); @@ -4398,7 +4542,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &mcast_user, - mcast_group_pk, + vec![mcast_group_pk], false, // publisher true, // subscriber ); @@ -5197,7 +5341,7 @@ mod tests { fixture.expect_create_subscribe_user( Pubkey::new_unique(), &user, - group_pk, + vec![group_pk], false, true, ); diff --git a/crates/doublezero-daemon-cli/src/multicast.rs b/crates/doublezero-daemon-cli/src/multicast.rs index 4051f3acab..5456c3c86b 100644 --- a/crates/doublezero-daemon-cli/src/multicast.rs +++ b/crates/doublezero-daemon-cli/src/multicast.rs @@ -16,7 +16,10 @@ use std::{io::Write, net::Ipv4Addr}; use clap::Args; use doublezero_cli_core::CliContext; use doublezero_sdk::{ - commands::multicastgroup::subscribe::UpdateMulticastGroupRolesCommand, User, UserType, + commands::multicastgroup::subscribe::{ + UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION, + }, + User, UserType, }; use indicatif::ProgressBar; use solana_sdk::pubkey::Pubkey; @@ -100,8 +103,82 @@ fn finish_update(spinner: &ProgressBar, out: &mut W) -> eyre::Result<( Ok(()) } -/// If any per-group calls failed, surface a non-zero exit by returning an error -/// listing the affected codes. Per-group failures are already printed inline. +/// A single batched role update: one atomic transaction applying the same +/// (publisher, subscriber) pair to every group in `groups`. +struct RoleUpdateBatch { + publisher: bool, + subscriber: bool, + groups: Vec<(String, Pubkey)>, +} + +/// Partition `(code, pk, (publisher, subscriber))` triples into batches sharing a +/// flag pair, preserving encounter order. The instruction applies one flag pair to +/// every group it carries, so each distinct pair needs its own transaction (at most +/// two per verb: the carried role is the only variable). Batches are capped at the +/// transaction size limit — a full batch stops matching, so oversize pairs chunk +/// naturally. +fn batch_role_updates(groups: Vec<(String, Pubkey, (bool, bool))>) -> Vec { + let mut batches: Vec = Vec::new(); + for (code, pk, (publisher, subscriber)) in groups { + match batches.iter_mut().find(|b| { + (b.publisher, b.subscriber) == (publisher, subscriber) + && b.groups.len() < MAX_GROUPS_PER_TRANSACTION + }) { + Some(batch) => batch.groups.push((code, pk)), + None => batches.push(RoleUpdateBatch { + publisher, + subscriber, + groups: vec![(code, pk)], + }), + } + } + batches +} + +/// Send each batch as one atomic transaction, printing a per-group `ok_verb` line on +/// success. A failed batch reports every code it carried (nothing in it was applied) +/// and its codes are returned as failures. +fn apply_role_update_batches( + ledger: &L, + out: &mut W, + user_pk: Pubkey, + client_ip: Ipv4Addr, + batches: Vec, + ok_verb: &str, + fail_verb: &str, +) -> eyre::Result> { + let mut failures: Vec = Vec::new(); + for batch in batches { + match ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { + user_pk, + group_pks: batch.groups.iter().map(|(_, pk)| *pk).collect(), + client_ip, + publisher: batch.publisher, + subscriber: batch.subscriber, + device_pk: None, + feed_pk: None, + }) { + Ok(()) => { + for (code, _) in &batch.groups { + writeln!(out, " {ok_verb} {code}")?; + } + } + Err(e) => { + let codes: Vec<&str> = batch.groups.iter().map(|(c, _)| c.as_str()).collect(); + writeln!( + out, + " ❌ failed to {fail_verb} {}: {e}", + codes.join(", ") + )?; + failures.extend(batch.groups.iter().map(|(c, _)| c.clone())); + } + } + } + Ok(failures) +} + +/// If any batched calls failed, surface a non-zero exit by returning an error +/// listing the affected codes. Per-batch failures are already printed inline. fn report_failures(op: &str, failures: &[String]) -> eyre::Result<()> { if failures.is_empty() { return Ok(()); @@ -170,29 +247,24 @@ impl Subscribe { let groups = resolve_groups(ledger, &self.groups)?; spinner.inc(1); - let mut failures: Vec = Vec::new(); + let mut to_apply = Vec::new(); for (code, group_pk) in groups { if user.subscribers.contains(&group_pk) { writeln!(out, " already subscribed to {code} — skipping")?; continue; } let carry_pub = user.publishers.contains(&group_pk); - match ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk, - group_pk, - client_ip, - publisher: carry_pub, - subscriber: true, - device_pk: None, - feed_pk: None, - }) { - Ok(()) => writeln!(out, " subscribed to {code}")?, - Err(e) => { - writeln!(out, " ❌ failed to subscribe to {code}: {e}")?; - failures.push(code); - } - } + to_apply.push((code, group_pk, (carry_pub, true))); } + let failures = apply_role_update_batches( + ledger, + out, + user_pk, + client_ip, + batch_role_updates(to_apply), + "subscribed to", + "subscribe to", + )?; finish_update(&spinner, out)?; report_failures("subscribe", &failures) @@ -225,29 +297,24 @@ impl Unsubscribe { writeln!(out, "{}", warn_idle_tunnel())?; } - let mut failures: Vec = Vec::new(); + let mut to_apply = Vec::new(); for (code, group_pk) in groups { if !user.subscribers.contains(&group_pk) { writeln!(out, " not subscribed to {code} — skipping")?; continue; } let carry_pub = user.publishers.contains(&group_pk); - match ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk, - group_pk, - client_ip, - publisher: carry_pub, - subscriber: false, - device_pk: None, - feed_pk: None, - }) { - Ok(()) => writeln!(out, " unsubscribed from {code}")?, - Err(e) => { - writeln!(out, " ❌ failed to unsubscribe from {code}: {e}")?; - failures.push(code); - } - } + to_apply.push((code, group_pk, (carry_pub, false))); } + let failures = apply_role_update_batches( + ledger, + out, + user_pk, + client_ip, + batch_role_updates(to_apply), + "unsubscribed from", + "unsubscribe from", + )?; finish_update(&spinner, out)?; report_failures("unsubscribe", &failures) @@ -270,29 +337,24 @@ impl Publish { let groups = resolve_groups(ledger, &self.groups)?; spinner.inc(1); - let mut failures: Vec = Vec::new(); + let mut to_apply = Vec::new(); for (code, group_pk) in groups { if user.publishers.contains(&group_pk) { writeln!(out, " already publishing to {code} — skipping")?; continue; } let carry_sub = user.subscribers.contains(&group_pk); - match ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk, - group_pk, - client_ip, - publisher: true, - subscriber: carry_sub, - device_pk: None, - feed_pk: None, - }) { - Ok(()) => writeln!(out, " publishing to {code}")?, - Err(e) => { - writeln!(out, " ❌ failed to publish to {code}: {e}")?; - failures.push(code); - } - } + to_apply.push((code, group_pk, (true, carry_sub))); } + let failures = apply_role_update_batches( + ledger, + out, + user_pk, + client_ip, + batch_role_updates(to_apply), + "publishing to", + "publish to", + )?; finish_update(&spinner, out)?; report_failures("publish", &failures) @@ -335,29 +397,24 @@ impl Unpublish { writeln!(out, "{}", warn_idle_tunnel())?; } - let mut failures: Vec = Vec::new(); + let mut to_apply = Vec::new(); for (code, group_pk) in groups { if !user.publishers.contains(&group_pk) { writeln!(out, " not publishing to {code} — skipping")?; continue; } let carry_sub = user.subscribers.contains(&group_pk); - match ledger.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk, - group_pk, - client_ip, - publisher: false, - subscriber: carry_sub, - device_pk: None, - feed_pk: None, - }) { - Ok(()) => writeln!(out, " unpublished from {code}")?, - Err(e) => { - writeln!(out, " ❌ failed to unpublish from {code}: {e}")?; - failures.push(code); - } - } + to_apply.push((code, group_pk, (false, carry_sub))); } + let failures = apply_role_update_batches( + ledger, + out, + user_pk, + client_ip, + batch_role_updates(to_apply), + "unpublished from", + "unpublish from", + )?; finish_update(&spinner, out)?; report_failures("unpublish", &failures) @@ -546,7 +603,7 @@ mod tests { .expect_update_multicastgroup_roles() .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { cmd.user_pk == user_pk - && cmd.group_pk == g_pk + && cmd.group_pks == vec![g_pk] && cmd.client_ip == ip && cmd.publisher && !cmd.subscriber @@ -645,15 +702,18 @@ mod tests { #[test] fn unsubscribe_continues_after_per_group_failure_and_aggregates_error() { - // g1's onchain call fails; g2's must still be attempted, and the + // g1 and g2 carry different publisher flags, so they land in separate + // batches: g1's batch fails, g2's must still be attempted, and the // command must return an aggregated error naming g1. let ip = Ipv4Addr::new(10, 0, 0, 1); let g1 = Pubkey::new_unique(); let g2 = Pubkey::new_unique(); let user_pk = Pubkey::new_unique(); + // Subscriber of both, publisher of g1 only — unsubscribing g1 carries + // publisher=true and g2 carries publisher=false. let mut users = HashMap::new(); - users.insert(user_pk, user_with_roles(ip, vec![], vec![g1, g2])); + users.insert(user_pk, user_with_roles(ip, vec![g1], vec![g1, g2])); let mut groups = HashMap::new(); groups.insert(g1, make_group("g1")); groups.insert(g2, make_group("g2")); @@ -661,12 +721,16 @@ mod tests { ledger .expect_update_multicastgroup_roles() - .withf(move |cmd: &UpdateMulticastGroupRolesCommand| cmd.group_pk == g1) + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.group_pks == vec![g1] && cmd.publisher + }) .once() .returning(|_| Err(eyre::eyre!("simulated chain failure"))); ledger .expect_update_multicastgroup_roles() - .withf(move |cmd: &UpdateMulticastGroupRolesCommand| cmd.group_pk == g2) + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.group_pks == vec![g2] && !cmd.publisher + }) .once() .returning(|_| Ok(())); @@ -703,7 +767,10 @@ mod tests { ledger .expect_update_multicastgroup_roles() .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { - cmd.user_pk == user_pk && cmd.group_pk == g1 && !cmd.publisher && cmd.subscriber + cmd.user_pk == user_pk + && cmd.group_pks == vec![g1] + && !cmd.publisher + && cmd.subscriber }) .once() .returning(|_| Ok(())); @@ -849,7 +916,10 @@ mod tests { ledger .expect_update_multicastgroup_roles() .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { - cmd.user_pk == user_pk && cmd.group_pk == g_pk && cmd.publisher && cmd.subscriber + cmd.user_pk == user_pk + && cmd.group_pks == vec![g_pk] + && cmd.publisher + && cmd.subscriber }) .once() .returning(|_| Ok(())); @@ -898,6 +968,133 @@ mod tests { ); } + #[test] + fn subscribe_batches_same_flag_groups_into_one_call() { + // Neither group carries a publisher role, so both share the + // (publisher=false, subscriber=true) pair and ride in ONE transaction, + // in the order the codes were passed. + let ip = Ipv4Addr::new(10, 0, 0, 1); + let g1_pk = Pubkey::new_unique(); + let g2_pk = Pubkey::new_unique(); + let user_pk = Pubkey::new_unique(); + + let mut users = HashMap::new(); + users.insert(user_pk, user_with_roles(ip, vec![], vec![])); + let mut groups = HashMap::new(); + groups.insert(g1_pk, make_group("g1")); + groups.insert(g2_pk, make_group("g2")); + let mut ledger = ledger_with_users_and_groups(users, groups); + + ledger + .expect_update_multicastgroup_roles() + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.user_pk == user_pk + && cmd.group_pks == vec![g1_pk, g2_pk] + && !cmd.publisher + && cmd.subscriber + }) + .once() + .returning(|_| Ok(())); + + let daemon = daemon_with_client_ip("10.0.0.1"); + let ctx = cli_context_default_for_tests(); + let mut out = Vec::new(); + let cmd = Subscribe { + groups: vec!["g1".into(), "g2".into()], + }; + block_on(cmd.execute(&ctx, &daemon, &ledger, &mut out)).unwrap(); + + // One transaction, but still one result line per group. + let rendered = String::from_utf8(out).unwrap(); + assert!(rendered.contains("subscribed to g1"), "got: {rendered}"); + assert!(rendered.contains("subscribed to g2"), "got: {rendered}"); + } + + #[test] + fn subscribe_failed_batch_reports_every_code_it_carried() { + // A batch is atomic: when its transaction fails nothing was applied, so + // every code it carried counts as a failure. + let ip = Ipv4Addr::new(10, 0, 0, 1); + let g1_pk = Pubkey::new_unique(); + let g2_pk = Pubkey::new_unique(); + let user_pk = Pubkey::new_unique(); + + let mut users = HashMap::new(); + users.insert(user_pk, user_with_roles(ip, vec![], vec![])); + let mut groups = HashMap::new(); + groups.insert(g1_pk, make_group("g1")); + groups.insert(g2_pk, make_group("g2")); + let mut ledger = ledger_with_users_and_groups(users, groups); + + ledger + .expect_update_multicastgroup_roles() + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.group_pks == vec![g1_pk, g2_pk] + }) + .once() + .returning(|_| Err(eyre::eyre!("simulated chain failure"))); + + let daemon = daemon_with_client_ip("10.0.0.1"); + let ctx = cli_context_default_for_tests(); + let mut out = Vec::new(); + let cmd = Subscribe { + groups: vec!["g1".into(), "g2".into()], + }; + let err = block_on(cmd.execute(&ctx, &daemon, &ledger, &mut out)).unwrap_err(); + + let rendered = String::from_utf8(out).unwrap(); + assert!( + rendered.contains("❌ failed to subscribe to g1, g2"), + "got: {rendered}" + ); + let msg = err.to_string(); + assert!( + msg.contains("subscribe failed for 2 group(s)"), + "got: {msg}" + ); + assert!(msg.contains("g1") && msg.contains("g2"), "got: {msg}"); + } + + #[test] + fn unsubscribe_batches_same_flag_groups_into_one_call() { + // Subscriber-only of both groups, so both carry + // (publisher=false, subscriber=false) and ride in ONE transaction. + let ip = Ipv4Addr::new(10, 0, 0, 1); + let g1_pk = Pubkey::new_unique(); + let g2_pk = Pubkey::new_unique(); + let user_pk = Pubkey::new_unique(); + + let mut users = HashMap::new(); + users.insert(user_pk, user_with_roles(ip, vec![], vec![g1_pk, g2_pk])); + let mut groups = HashMap::new(); + groups.insert(g1_pk, make_group("g1")); + groups.insert(g2_pk, make_group("g2")); + let mut ledger = ledger_with_users_and_groups(users, groups); + + ledger + .expect_update_multicastgroup_roles() + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.user_pk == user_pk + && cmd.group_pks == vec![g1_pk, g2_pk] + && !cmd.publisher + && !cmd.subscriber + }) + .once() + .returning(|_| Ok(())); + + let daemon = daemon_with_client_ip("10.0.0.1"); + let ctx = cli_context_default_for_tests(); + let mut out = Vec::new(); + let cmd = Unsubscribe { + groups: vec!["g1".into(), "g2".into()], + }; + block_on(cmd.execute(&ctx, &daemon, &ledger, &mut out)).unwrap(); + + let rendered = String::from_utf8(out).unwrap(); + assert!(rendered.contains("unsubscribed from g1"), "got: {rendered}"); + assert!(rendered.contains("unsubscribed from g2"), "got: {rendered}"); + } + // --- Publish tests --- #[test] @@ -916,7 +1113,10 @@ mod tests { ledger .expect_update_multicastgroup_roles() .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { - cmd.user_pk == user_pk && cmd.group_pk == g_pk && cmd.publisher && cmd.subscriber + cmd.user_pk == user_pk + && cmd.group_pks == vec![g_pk] + && cmd.publisher + && cmd.subscriber }) .once() .returning(|_| Ok(())); @@ -935,15 +1135,18 @@ mod tests { #[test] fn publish_continues_after_per_group_failure_and_aggregates_error() { - // g1's onchain call fails; g2's must still be attempted, and the + // g1 and g2 carry different subscriber flags, so they land in separate + // batches: g1's batch fails, g2's must still be attempted, and the // command must return an aggregated error naming g1. let ip = Ipv4Addr::new(10, 0, 0, 1); let g1 = Pubkey::new_unique(); let g2 = Pubkey::new_unique(); let user_pk = Pubkey::new_unique(); + // Subscriber of g1 only — publishing g1 carries subscriber=true and g2 + // carries subscriber=false. let mut users = HashMap::new(); - users.insert(user_pk, user_with_roles(ip, vec![], vec![])); + users.insert(user_pk, user_with_roles(ip, vec![], vec![g1])); let mut groups = HashMap::new(); groups.insert(g1, make_group("g1")); groups.insert(g2, make_group("g2")); @@ -951,12 +1154,16 @@ mod tests { ledger .expect_update_multicastgroup_roles() - .withf(move |cmd: &UpdateMulticastGroupRolesCommand| cmd.group_pk == g1) + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.group_pks == vec![g1] && cmd.subscriber + }) .once() .returning(|_| Err(eyre::eyre!("simulated chain failure"))); ledger .expect_update_multicastgroup_roles() - .withf(move |cmd: &UpdateMulticastGroupRolesCommand| cmd.group_pk == g2) + .withf(move |cmd: &UpdateMulticastGroupRolesCommand| { + cmd.group_pks == vec![g2] && !cmd.subscriber + }) .once() .returning(|_| Ok(())); diff --git a/crates/doublezero-serviceability-instruction/src/multicastgroup.rs b/crates/doublezero-serviceability-instruction/src/multicastgroup.rs index 954015fbb2..5f55095037 100644 --- a/crates/doublezero-serviceability-instruction/src/multicastgroup.rs +++ b/crates/doublezero-serviceability-instruction/src/multicastgroup.rs @@ -168,14 +168,21 @@ pub fn delete_multicast_group( ) } -/// `UpdateMulticastGroupRoles` (variant 58) — publisher/subscriber role change. -/// Accounts: `[group, accesspass, user, globalstate, multicast_publisher_block]`. +/// `UpdateMulticastGroupRoles` (variant 58) — publisher/subscriber role change, +/// applied atomically to `group` plus every group in `extra_groups`. +/// Accounts: `[group, accesspass, user, globalstate, multicast_publisher_block, +/// extra_groups...]`. +/// +/// `args.extra_group_count` is DERIVED from `extra_groups.len()`; any +/// caller-supplied value is ignored (the count must stay in lockstep with the +/// emitted group metas, or the processor would misparse the trailing region). pub fn update_multicast_group_roles( program_id: &Pubkey, payer: &Pubkey, group: &Pubkey, accesspass: &Pubkey, user: &Pubkey, + extra_groups: &[Pubkey], mut args: UpdateMulticastGroupRolesArgs, ) -> Instruction { let (globalstate, _) = get_globalstate_pda(program_id); @@ -186,16 +193,20 @@ pub fn update_multicast_group_roles( // a caller-supplied value here can only ever fail. This builder always emits // the `multicast_publisher_block` account, so it forces the flag (as the SDK does). args.use_onchain_allocation = true; + args.extra_group_count = u8::try_from(extra_groups.len()) + .expect("extra_groups cannot exceed the transaction account limit"); + let mut accounts = vec![ + AccountMeta::new(*group, false), + AccountMeta::new(*accesspass, false), + AccountMeta::new(*user, false), + AccountMeta::new(globalstate, false), + AccountMeta::new(multicast_publisher_block, false), + ]; + accounts.extend(extra_groups.iter().map(|g| AccountMeta::new(*g, false))); common::build_with_permission( program_id, DoubleZeroInstruction::UpdateMulticastGroupRoles(args), - vec![ - AccountMeta::new(*group, false), - AccountMeta::new(*accesspass, false), - AccountMeta::new(*user, false), - AccountMeta::new(globalstate, false), - AccountMeta::new(multicast_publisher_block, false), - ], + accounts, payer, ) } @@ -577,8 +588,9 @@ mod tests { subscriber: false, // Left off deliberately: the builder must force it on. use_onchain_allocation: false, + extra_group_count: 0, }; - let ix = update_multicast_group_roles(&pid, &payer, &group, &accesspass, &user, args); + let ix = update_multicast_group_roles(&pid, &payer, &group, &accesspass, &user, &[], args); assert_eq!(ix.data[0], 58); match DoubleZeroInstruction::unpack(&ix.data).unwrap() { DoubleZeroInstruction::UpdateMulticastGroupRoles(a) => { @@ -602,6 +614,59 @@ mod tests { ); } + /// Extra groups are appended after the five fixed accounts, writable, and + /// `extra_group_count` is derived from the slice length (a caller-set value is + /// overwritten). + #[test] + fn test_update_multicast_group_roles_extra_groups() { + let pid = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let group = Pubkey::new_unique(); + let accesspass = Pubkey::new_unique(); + let user = Pubkey::new_unique(); + let extra1 = Pubkey::new_unique(); + let extra2 = Pubkey::new_unique(); + let args = UpdateMulticastGroupRolesArgs { + client_ip: Ipv4Addr::new(192, 168, 1, 1), + publisher: false, + subscriber: true, + use_onchain_allocation: false, + // Wrong on purpose: the builder must derive it from the slice. + extra_group_count: 7, + }; + let ix = update_multicast_group_roles( + &pid, + &payer, + &group, + &accesspass, + &user, + &[extra1, extra2], + args, + ); + match DoubleZeroInstruction::unpack(&ix.data).unwrap() { + DoubleZeroInstruction::UpdateMulticastGroupRoles(a) => { + assert_eq!(a.extra_group_count, 2); + } + other => panic!("unexpected: {other:?}"), + } + let (globalstate, _) = get_globalstate_pda(&pid); + let (mpb, _, _) = get_resource_extension_pda(&pid, ResourceType::MulticastPublisherBlock); + assert_eq!( + ix.accounts, + vec![ + AccountMeta::new(group, false), + AccountMeta::new(accesspass, false), + AccountMeta::new(user, false), + AccountMeta::new(globalstate, false), + AccountMeta::new(mpb, false), + AccountMeta::new(extra1, false), + AccountMeta::new(extra2, false), + AccountMeta::new(payer, true), + AccountMeta::new(system_program::ID, false), + ] + ); + } + #[test] fn test_allowlist_add_and_remove() { let pid = Pubkey::new_unique(); diff --git a/crates/doublezero-serviceability-instruction/src/user.rs b/crates/doublezero-serviceability-instruction/src/user.rs index 269e7dba3f..b904d5948c 100644 --- a/crates/doublezero-serviceability-instruction/src/user.rs +++ b/crates/doublezero-serviceability-instruction/src/user.rs @@ -32,6 +32,7 @@ use solana_program::{ /// multicast_publisher_block (writable) — ResourceType::MulticastPublisherBlock /// device_tunnel_ids (writable) — ResourceType::TunnelIds(device, 0) /// dz_prefix_block[i] (writable) — one per dz_prefix_count +/// extra_mgroup[i] (writable) — one per extra_groups entry /// feed (readonly) — OPTIONAL, appended only when Some /// ``` /// @@ -50,7 +51,8 @@ use solana_program::{ /// `[payer, system]` and the processor peels it by PDA match — so the two never /// collide. This builder is therefore assigned to `common::build_with_permission` /// (permission deferred for now). `dz_prefix_count` is written back into the args -/// so it always matches the number of `dz_prefix_block` accounts produced. +/// so it always matches the number of `dz_prefix_block` accounts produced, and +/// `extra_group_count` is likewise DERIVED from `extra_groups.len()`. #[allow(clippy::too_many_arguments)] pub fn create_subscribe_user( program_id: &Pubkey, @@ -59,6 +61,7 @@ pub fn create_subscribe_user( mgroup: &Pubkey, accesspass: &Pubkey, dz_prefix_count: u8, + extra_groups: &[Pubkey], feed: Option<&Pubkey>, mut args: UserCreateSubscribeArgs, ) -> Instruction { @@ -77,6 +80,8 @@ pub fn create_subscribe_user( args.dz_prefix_count ); args.dz_prefix_count = dz_prefix_count; + args.extra_group_count = u8::try_from(extra_groups.len()) + .expect("extra_groups cannot exceed the transaction account limit"); let (user, _) = get_user_pda(program_id, &args.client_ip, args.user_type); let (globalstate, _) = get_globalstate_pda(program_id); @@ -104,6 +109,10 @@ pub fn create_subscribe_user( accounts.push(AccountMeta::new(dz_prefix, false)); } + // Extra multicast groups (batch subscription), after the dz_prefix blocks and + // before the optional feed — matching the processor's trailing-region layout. + accounts.extend(extra_groups.iter().map(|g| AccountMeta::new(*g, false))); + // Optional trailing Feed account (EdgeSeat metro gate), appended BEFORE // payer/system. A Permission PDA, once activated, is appended AFTER payer/system // (not after the feed) and the processor peels it by PDA match, so the feed and @@ -540,6 +549,7 @@ mod tests { dz_prefix_count: 0, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, } } @@ -560,6 +570,7 @@ mod tests { &mgroup, &accesspass, 1, + &[], None, args.clone(), ); @@ -621,6 +632,7 @@ mod tests { &Pubkey::new_unique(), &Pubkey::new_unique(), 1, + &[], Some(&feed), base_args(client_ip), ); @@ -655,6 +667,7 @@ mod tests { &mgroup, &accesspass, 1, + &[], None, UserCreateSubscribeArgs { ip_proof, @@ -716,6 +729,46 @@ mod tests { assert!(!ix.accounts[11].is_writable); } + /// Extra groups sit between the dz_prefix blocks and the optional feed, writable, + /// and `extra_group_count` is derived from the slice length. + #[test] + fn test_create_subscribe_user_extra_groups_between_dz_prefix_and_feed() { + let pid = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let device = Pubkey::new_unique(); + let feed = Pubkey::new_unique(); + let extra1 = Pubkey::new_unique(); + let extra2 = Pubkey::new_unique(); + let client_ip = Ipv4Addr::new(192, 168, 1, 10); + + let ix = create_subscribe_user( + &pid, + &payer, + &device, + &Pubkey::new_unique(), + &Pubkey::new_unique(), + 1, + &[extra1, extra2], + Some(&feed), + base_args(client_ip), + ); + + match DoubleZeroInstruction::unpack(&ix.data).unwrap() { + DoubleZeroInstruction::CreateSubscribeUser(a) => assert_eq!(a.extra_group_count, 2), + other => panic!("unexpected variant: {other:?}"), + } + + // 8 fixed + 1 dz_prefix + 2 extras + feed + payer + system = 14. + assert_eq!(ix.accounts.len(), 14); + let (dz_prefix0, _, _) = + get_resource_extension_pda(&pid, ResourceType::DzPrefixBlock(device, 0)); + assert_eq!(ix.accounts[8].pubkey, dz_prefix0); + assert_eq!(ix.accounts[9], AccountMeta::new(extra1, false)); + assert_eq!(ix.accounts[10], AccountMeta::new(extra2, false)); + assert_eq!(ix.accounts[11].pubkey, feed); + assert!(!ix.accounts[11].is_writable); + } + fn create_args(client_ip: Ipv4Addr) -> UserCreateArgs { UserCreateArgs { user_type: UserType::IBRLWithAllocatedIP, diff --git a/crates/sentinel/src/dz_ledger_writer.rs b/crates/sentinel/src/dz_ledger_writer.rs index 1db82b6fa1..735b8410a3 100644 --- a/crates/sentinel/src/dz_ledger_writer.rs +++ b/crates/sentinel/src/dz_ledger_writer.rs @@ -140,6 +140,7 @@ pub fn build_create_multicast_publisher_instructions( dz_prefix_count, owner: *owner, ip_proof: None, + extra_group_count: 0, }), create_user_accounts, )?; diff --git a/sdk/geolocation/testdata/fixtures/generate-fixtures/Cargo.lock b/sdk/geolocation/testdata/fixtures/generate-fixtures/Cargo.lock index 17f653d0e4..452447ca67 100644 --- a/sdk/geolocation/testdata/fixtures/generate-fixtures/Cargo.lock +++ b/sdk/geolocation/testdata/fixtures/generate-fixtures/Cargo.lock @@ -374,7 +374,7 @@ dependencies = [ [[package]] name = "doublezero-config" -version = "0.31.0" +version = "0.37.0" dependencies = [ "eyre", "serde", @@ -383,7 +383,7 @@ dependencies = [ [[package]] name = "doublezero-geolocation" -version = "0.31.0" +version = "0.37.0" dependencies = [ "borsh", "borsh-incremental", @@ -398,9 +398,17 @@ dependencies = [ "thiserror", ] +[[package]] +name = "doublezero-ip-proof" +version = "0.37.0" +dependencies = [ + "borsh", + "solana-program", +] + [[package]] name = "doublezero-program-common" -version = "0.31.0" +version = "0.37.0" dependencies = [ "borsh", "byteorder", @@ -412,12 +420,13 @@ dependencies = [ [[package]] name = "doublezero-serviceability" -version = "0.31.0" +version = "0.37.0" dependencies = [ "bitflags", "borsh", "borsh-incremental", "bytemuck", + "doublezero-ip-proof", "doublezero-program-common", "ipnetwork", "solana-program", diff --git a/sdk/serviceability/testdata/fixtures/generate-fixtures/Cargo.lock b/sdk/serviceability/testdata/fixtures/generate-fixtures/Cargo.lock index 1bc2ceb5c0..d28082c623 100644 --- a/sdk/serviceability/testdata/fixtures/generate-fixtures/Cargo.lock +++ b/sdk/serviceability/testdata/fixtures/generate-fixtures/Cargo.lock @@ -323,16 +323,15 @@ dependencies = [ [[package]] name = "doublezero-ip-proof" -version = "0.36.0" +version = "0.37.0" dependencies = [ "borsh", "solana-program", - "thiserror", ] [[package]] name = "doublezero-program-common" -version = "0.36.0" +version = "0.37.0" dependencies = [ "borsh", "byteorder", @@ -344,7 +343,7 @@ dependencies = [ [[package]] name = "doublezero-serviceability" -version = "0.36.0" +version = "0.37.0" dependencies = [ "bitflags", "borsh", @@ -353,7 +352,6 @@ dependencies = [ "doublezero-ip-proof", "doublezero-program-common", "ipnetwork", - "solana-instructions-sysvar", "solana-program", "solana-system-interface 3.2.0", "thiserror", @@ -361,7 +359,7 @@ dependencies = [ [[package]] name = "doublezero-serviceability-instruction" -version = "0.36.0" +version = "0.37.0" dependencies = [ "doublezero-serviceability", "solana-compute-budget-interface", diff --git a/sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs b/sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs index 4559d23973..7c406f0e3d 100644 --- a/sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs +++ b/sdk/serviceability/testdata/fixtures/generate-fixtures/src/main.rs @@ -302,6 +302,7 @@ fn generate_ix_fixtures(dir: &Path) { &b, &c, 1, + &[], Some(&d), UserCreateSubscribeArgs { user_type: UserType::IBRLWithAllocatedIP, @@ -313,6 +314,7 @@ fn generate_ix_fixtures(dir: &Path) { dz_prefix_count: 0, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }, ); write_ix_fixture(dir, "create_subscribe_user", &create_subscribe_user); diff --git a/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.bin b/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.bin index 5492f61b78..47462c2415 100644 Binary files a/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.bin and b/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.bin differ diff --git a/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.json b/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.json index efbbf03725..3c5a7e7ca7 100644 --- a/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.json +++ b/sdk/serviceability/testdata/fixtures/ix_create_subscribe_user.json @@ -1,7 +1,7 @@ { "program_id": "JAQxrJ2WuDF4APfSifurJJ4HzV5Z3FyBuBeSMj7mo9aw", "variant": 59, - "data_hex": "3b01010a0b0c0d0100c0a8010201000000000000000000000000000000000000000000000000000000000000000000", + "data_hex": "3b01010a0b0c0d0100c0a801020100000000000000000000000000000000000000000000000000000000000000000000", "accounts": [ { "pubkey": "2k5Ausiy8hiZqpZragqtTf6L4wmhZv3otUD5xAM7x59g", diff --git a/sdk/telemetry/testdata/fixtures/generate-fixtures/Cargo.lock b/sdk/telemetry/testdata/fixtures/generate-fixtures/Cargo.lock index 84f0287751..4a9a2e4c46 100644 --- a/sdk/telemetry/testdata/fixtures/generate-fixtures/Cargo.lock +++ b/sdk/telemetry/testdata/fixtures/generate-fixtures/Cargo.lock @@ -374,16 +374,24 @@ dependencies = [ [[package]] name = "doublezero-config" -version = "0.31.0" +version = "0.37.0" dependencies = [ "eyre", "serde", "solana-sdk", ] +[[package]] +name = "doublezero-ip-proof" +version = "0.37.0" +dependencies = [ + "borsh", + "solana-program", +] + [[package]] name = "doublezero-program-common" -version = "0.31.0" +version = "0.37.0" dependencies = [ "borsh", "byteorder", @@ -395,12 +403,13 @@ dependencies = [ [[package]] name = "doublezero-serviceability" -version = "0.31.0" +version = "0.37.0" dependencies = [ "bitflags", "borsh", "borsh-incremental", "bytemuck", + "doublezero-ip-proof", "doublezero-program-common", "ipnetwork", "solana-program", @@ -410,7 +419,7 @@ dependencies = [ [[package]] name = "doublezero-telemetry" -version = "0.31.0" +version = "0.37.0" dependencies = [ "borsh", "borsh-incremental", diff --git a/smartcontract/cli/src/feed/delete.rs b/smartcontract/cli/src/feed/delete.rs index 2a524b1107..bd2c0dfda1 100644 --- a/smartcontract/cli/src/feed/delete.rs +++ b/smartcontract/cli/src/feed/delete.rs @@ -133,7 +133,7 @@ mod tests { .expect_update_multicastgroup_roles() .with(predicate::eq(UpdateMulticastGroupRolesCommand { user_pk: f.user_pk, - group_pk: group, + group_pks: vec![group], client_ip: f.client_ip, publisher: false, subscriber: false, diff --git a/smartcontract/cli/src/feed/guard.rs b/smartcontract/cli/src/feed/guard.rs index 38dcbfa44e..566676bf42 100644 --- a/smartcontract/cli/src/feed/guard.rs +++ b/smartcontract/cli/src/feed/guard.rs @@ -15,8 +15,10 @@ use crate::doublezerocommand::CliCommand; use doublezero_sdk::{ commands::{ - accesspass::list::ListAccessPassCommand, device::list::ListDeviceCommand, - feed::list::ListFeedCommand, multicastgroup::subscribe::UpdateMulticastGroupRolesCommand, + accesspass::list::ListAccessPassCommand, + device::list::ListDeviceCommand, + feed::list::ListFeedCommand, + multicastgroup::subscribe::{UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION}, user::list::ListUserCommand, }, Device, Feed, User, @@ -136,32 +138,52 @@ pub fn unsubscribe_orphans( ); } + // Every orphan reaching this point is removal-only (mixed roles bailed + // above), so a user's removals batch into one atomic role update, chunked + // to the transaction limit, instead of one transaction per group. The plan + // is sorted by (user, group), so one linear pass groups it. + let mut batches: Vec<(&Orphan, Vec)> = Vec::new(); for orphan in &orphans { - client - .update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk: orphan.user_pk, - group_pk: orphan.group_pk, - client_ip: orphan.client_ip, - publisher: false, - subscriber: false, - device_pk: None, - feed_pk: None, - }) - .wrap_err_with(|| { - format!( - "failed to unsubscribe user {} from group {}; the feed was left unchanged. \ - If this is an authorization failure, removing another owner's roles needs \ - USER_ADMIN (or foundation membership) on the payer in addition to \ - FEED_AUTHORITY: doublezero permission set --user-payer --add \ - user-admin", - orphan.user_pk, orphan.group_pk - ) - })?; - writeln!( - out, - " unsubscribed user {} from group {}", - orphan.user_pk, orphan.group_pk - )?; + match batches.last_mut() { + Some((first, group_pks)) if first.user_pk == orphan.user_pk => { + group_pks.push(orphan.group_pk) + } + _ => batches.push((orphan, vec![orphan.group_pk])), + } + } + for (orphan, group_pks) in batches { + for chunk in group_pks.chunks(MAX_GROUPS_PER_TRANSACTION) { + let groups = chunk + .iter() + .map(ToString::to_string) + .collect::>() + .join(", "); + client + .update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { + user_pk: orphan.user_pk, + group_pks: chunk.to_vec(), + client_ip: orphan.client_ip, + publisher: false, + subscriber: false, + device_pk: None, + feed_pk: None, + }) + .wrap_err_with(|| { + format!( + "failed to unsubscribe user {} from group(s) {groups}; the feed was \ + left unchanged. If this is an authorization failure, removing another \ + owner's roles needs USER_ADMIN (or foundation membership) on the payer \ + in addition to FEED_AUTHORITY: doublezero permission set --user-payer \ + --add user-admin", + orphan.user_pk + ) + })?; + writeln!( + out, + " unsubscribed user {} from group(s) {groups}", + orphan.user_pk + )?; + } } rounds_done += 1; } diff --git a/smartcontract/cli/src/feed/update.rs b/smartcontract/cli/src/feed/update.rs index 44be20a797..f3645ce48c 100644 --- a/smartcontract/cli/src/feed/update.rs +++ b/smartcontract/cli/src/feed/update.rs @@ -175,7 +175,7 @@ mod tests { .expect_update_multicastgroup_roles() .with(predicate::eq(UpdateMulticastGroupRolesCommand { user_pk: f.user_pk, - group_pk: g2, + group_pks: vec![g2], client_ip: f.client_ip, publisher: false, subscriber: false, @@ -213,6 +213,65 @@ mod tests { .contains(&format!("Signature: {signature}"))); } + /// A user holding several dropped groups is stripped with one batched role update, not one + /// transaction per group. + #[test] + fn test_cli_feed_update_force_unsubscribes_a_user_in_one_batch() { + let mut client = create_test_client(); + client.expect_check_requirements().returning(|_| Ok(())); + + let f = GuardFixture::new(3); + let (g1, g2, g3) = (f.groups[0], f.groups[1], f.groups[2]); + let signature = Signature::new_unique(); + f.expect_get_feed(&mut client, vec![g1, g2, g3]); + f.expect_get_groups(&mut client); + f.expect_scan(&mut client, vec![g2, g3]); + // The mock does not mutate state, so the post-unsubscribe re-scan needs its own snapshot + // with the memberships gone. + f.expect_scan(&mut client, vec![]); + // The plan sorts by (user, group), so the batch carries the dropped groups in pubkey + // order. + let mut dropped = vec![g2, g3]; + dropped.sort(); + client + .expect_update_multicastgroup_roles() + .with(predicate::eq(UpdateMulticastGroupRolesCommand { + user_pk: f.user_pk, + group_pks: dropped, + client_ip: f.client_ip, + publisher: false, + subscriber: false, + device_pk: None, + feed_pk: None, + })) + .times(1) + .returning(|_| Ok(Signature::new_unique())); + client + .expect_update_feed() + .with(predicate::eq(UpdateFeedCommand { + pubkey: f.feed_pk, + name: None, + groups: Some(vec![g1]), + })) + .times(1) + .returning(move |_| Ok(signature)); + + let ctx = cli_context_default_for_tests(); + let mut output = Vec::new(); + let res = block_on( + UpdateFeedCliCommand { + pubkey: Some(f.feed_pk.to_string()), + code: None, + exchange: None, + name: None, + groups: vec![g1.to_string()], + force_unsubscribe: true, + } + .execute(&ctx, &client, &mut output), + ); + assert!(res.is_ok(), "{res:?}"); + } + /// An additive change scans (the guard re-derives the dropped set from its own snapshot) but /// finds nothing dropped, so an existing subscriber needs no flag and no removals happen. #[test] diff --git a/smartcontract/cli/src/user/create_subscribe.rs b/smartcontract/cli/src/user/create_subscribe.rs index 27eea03b75..e6877dd9ac 100644 --- a/smartcontract/cli/src/user/create_subscribe.rs +++ b/smartcontract/cli/src/user/create_subscribe.rs @@ -126,9 +126,9 @@ impl CreateSubscribeUserCliCommand { client_ip: self.client_ip, publisher: publisher_pk.is_some(), subscriber: subscriber_pk.is_some(), - mgroup_pk: publisher_pk + mgroup_pks: vec![publisher_pk .or(subscriber_pk) - .ok_or(eyre::eyre!("Subscriber is required if publisher is not"))?, + .ok_or(eyre::eyre!("Subscriber is required if publisher is not"))?], tunnel_endpoint: Ipv4Addr::UNSPECIFIED, owner: owner_pk, feed_pk, @@ -260,7 +260,7 @@ mod tests { client_ip: [100, 0, 0, 1].into(), publisher: false, subscriber: true, - mgroup_pk: mgroup_pubkey, + mgroup_pks: vec![mgroup_pubkey], tunnel_endpoint: Ipv4Addr::UNSPECIFIED, owner: None, feed_pk: None, @@ -383,7 +383,7 @@ mod tests { client_ip: [100, 0, 0, 1].into(), publisher: false, subscriber: true, - mgroup_pk: mgroup_pubkey, + mgroup_pks: vec![mgroup_pubkey], tunnel_endpoint: Ipv4Addr::UNSPECIFIED, owner: None, feed_pk: Some(feed_pubkey), @@ -519,7 +519,7 @@ mod tests { client_ip: [100, 0, 0, 1].into(), publisher: false, subscriber: true, - mgroup_pk: mgroup_pubkey, + mgroup_pks: vec![mgroup_pubkey], tunnel_endpoint: Ipv4Addr::UNSPECIFIED, owner: None, feed_pk: Some(resolved_feed_pubkey), diff --git a/smartcontract/cli/src/user/subscribe.rs b/smartcontract/cli/src/user/subscribe.rs index 8005c8e91b..a46653ffa5 100644 --- a/smartcontract/cli/src/user/subscribe.rs +++ b/smartcontract/cli/src/user/subscribe.rs @@ -7,9 +7,13 @@ use crate::{ use clap::Args; use doublezero_cli_core::CliContext; use doublezero_sdk::commands::{ - multicastgroup::{get::GetMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand}, + multicastgroup::{ + get::GetMulticastGroupCommand, + subscribe::{UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION}, + }, user::get::GetUserCommand, }; +use solana_sdk::pubkey::Pubkey; use std::io::Write; #[derive(Args, Debug)] @@ -75,16 +79,21 @@ impl SubscribeUserCliCommand { group_pks.push(group_pk); } - // Update roles for each group. An omitted flag preserves the user's + // Update roles for the groups. An omitted flag preserves the user's // current role for that group; the processor sets absolute state // (idempotent add when true, idempotent remove when false), not a // relative toggle. // + // The instruction applies one (publisher, subscriber) pair to every group + // it carries, so groups are batched by their effective flag pair — one + // atomic transaction per pair (at most 4, typically 1). + // // Preserving an already-held role re-asserts it as `true`, which the // processor re-checks against the current onchain allowlist before the // idempotent add/remove. If that allowlist drifted since the user // subscribed, an unrelated role removal can be rejected with NotAllowed. // This is an inherited processor property, not a regression here. + let mut batches: Vec<((bool, bool), Vec)> = Vec::new(); for group_pk in &group_pks { let publisher = self .publisher @@ -92,17 +101,30 @@ impl SubscribeUserCliCommand { let subscriber = self .subscriber .unwrap_or_else(|| user.subscribers.contains(group_pk)); - let signature = - client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { - user_pk, - group_pk: *group_pk, - client_ip: user.client_ip, - publisher, - subscriber, - device_pk: None, - feed_pk: None, - })?; - writeln!(out, "Updated roles for {group_pk}: {signature}")?; + match batches + .iter_mut() + .find(|(flags, _)| *flags == (publisher, subscriber)) + { + Some((_, pks)) => pks.push(*group_pk), + None => batches.push(((publisher, subscriber), vec![*group_pk])), + } + } + for ((publisher, subscriber), batch_pks) in batches { + for chunk in batch_pks.chunks(MAX_GROUPS_PER_TRANSACTION) { + let signature = + client.update_multicastgroup_roles(UpdateMulticastGroupRolesCommand { + user_pk, + group_pks: chunk.to_vec(), + client_ip: user.client_ip, + publisher, + subscriber, + device_pk: None, + feed_pk: None, + })?; + for group_pk in chunk { + writeln!(out, "Updated roles for {group_pk}: {signature}")?; + } + } } if self.wait { @@ -217,7 +239,7 @@ mod tests { .expect_update_multicastgroup_roles() .with(predicate::eq(UpdateMulticastGroupRolesCommand { user_pk: user_pubkey, - group_pk: mgroup_pubkey, + group_pks: vec![mgroup_pubkey], client_ip, publisher: false, subscriber: true, @@ -340,9 +362,21 @@ mod tests { pubkey_or_code: mgroup_pubkey2.to_string(), })) .returning(move |_| Ok((mgroup_pubkey2, mgroup2.clone()))); + // The user holds no roles and both flags are explicit, so both groups share + // the same effective (publisher, subscriber) pair and are batched into a + // single atomic transaction. client .expect_update_multicastgroup_roles() - .times(2) + .with(predicate::eq(UpdateMulticastGroupRolesCommand { + user_pk: user_pubkey, + group_pks: vec![mgroup_pubkey1, mgroup_pubkey2], + client_ip, + publisher: false, + subscriber: true, + device_pk: None, + feed_pk: None, + })) + .times(1) .returning(move |_| Ok(signature)); /*****************************************************************************************************/ @@ -367,6 +401,147 @@ mod tests { ); } + #[test] + fn test_cli_user_subscribe_splits_batches_by_effective_flag_pair() { + let mut client = create_test_client(); + + let (user_pubkey, _bump_seed) = get_user_old_pda(&client.get_program_id(), 1); + let signature1 = Signature::new_unique(); + let signature2 = Signature::new_unique(); + let client_ip = [192, 168, 1, 100].into(); + + let mgroup_pubkey1 = Pubkey::from_str_const("11111115RidqCHAoz6dzmXxGcfWLNzevYqNpaRAUo"); + let mgroup_pubkey2 = Pubkey::from_str_const("11111116EPqoQskEM2Pddp8KTL9JoFhVBkC8GXfRH"); + + // The user publishes to the first group only; neither group is subscribed. + let user = User { + account_type: AccountType::User, + index: 1, + bump_seed: 255, + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + device_pk: Pubkey::new_unique(), + owner: client.get_payer(), + tenant_pk: Pubkey::default(), + client_ip, + dz_ip: client_ip, + tunnel_id: 12345, + tunnel_net: "192.168.1.0/24".parse().unwrap(), + status: doublezero_sdk::UserStatus::Activated, + publishers: vec![mgroup_pubkey1], + subscribers: vec![], + validator_pubkey: Pubkey::default(), + tunnel_endpoint: std::net::Ipv4Addr::UNSPECIFIED, + tunnel_flags: 0, + bgp_status: Default::default(), + last_bgp_up_at: 0, + last_bgp_reported_at: 0, + bgp_rtt_ns: 0, + ..Default::default() + }; + + client + .expect_check_requirements() + .with(predicate::eq(CHECK_ID_JSON | CHECK_BALANCE)) + .returning(|_| Ok(())); + client + .expect_get_user() + .with(predicate::eq(GetUserCommand { + pubkey: user_pubkey, + })) + .returning(move |_| Ok((user_pubkey, user.clone()))); + + let mgroup1 = MulticastGroup { + account_type: AccountType::MulticastGroup, + index: 1, + bump_seed: 255, + tenant_pk: Pubkey::new_unique(), + multicast_ip: [239, 1, 1, 1].into(), + max_bandwidth: 1000, + status: MulticastGroupStatus::Activated, + code: "group1".to_string(), + owner: mgroup_pubkey1, + publisher_count: 1, + subscriber_count: 0, + }; + let mgroup2 = MulticastGroup { + account_type: AccountType::MulticastGroup, + index: 2, + bump_seed: 254, + tenant_pk: Pubkey::new_unique(), + multicast_ip: [239, 1, 1, 2].into(), + max_bandwidth: 1000, + status: MulticastGroupStatus::Activated, + code: "group2".to_string(), + owner: mgroup_pubkey2, + publisher_count: 0, + subscriber_count: 0, + }; + client + .expect_get_multicastgroup() + .with(predicate::eq(GetMulticastGroupCommand { + pubkey_or_code: mgroup_pubkey1.to_string(), + })) + .returning(move |_| Ok((mgroup_pubkey1, mgroup1.clone()))); + client + .expect_get_multicastgroup() + .with(predicate::eq(GetMulticastGroupCommand { + pubkey_or_code: mgroup_pubkey2.to_string(), + })) + .returning(move |_| Ok((mgroup_pubkey2, mgroup2.clone()))); + + // `--publisher` is omitted, so each group preserves its own current + // publisher role: group1 -> (true, true), group2 -> (false, true). The two + // effective flag pairs differ, so the groups cannot share a transaction and + // are split into one batch per pair. + client + .expect_update_multicastgroup_roles() + .with(predicate::eq(UpdateMulticastGroupRolesCommand { + user_pk: user_pubkey, + group_pks: vec![mgroup_pubkey1], + client_ip, + publisher: true, + subscriber: true, + device_pk: None, + feed_pk: None, + })) + .times(1) + .returning(move |_| Ok(signature1)); + client + .expect_update_multicastgroup_roles() + .with(predicate::eq(UpdateMulticastGroupRolesCommand { + user_pk: user_pubkey, + group_pks: vec![mgroup_pubkey2], + client_ip, + publisher: false, + subscriber: true, + device_pk: None, + feed_pk: None, + })) + .times(1) + .returning(move |_| Ok(signature2)); + + let mut output = Vec::new(); + let ctx = cli_context_default_for_tests(); + let res = block_on( + SubscribeUserCliCommand { + user: user_pubkey.to_string(), + groups: vec![mgroup_pubkey1.to_string(), mgroup_pubkey2.to_string()], + publisher: None, + subscriber: Some(true), + wait: false, + } + .execute(&ctx, &client, &mut output), + ); + assert!(res.is_ok()); + let output_str = String::from_utf8(output).unwrap(); + // Each group is reported with the signature of the batch that carried it. + assert_eq!( + output_str, + format!("Updated roles for {mgroup_pubkey1}: {signature1}\nUpdated roles for {mgroup_pubkey2}: {signature2}\n") + ); + } + #[test] fn test_cli_user_unsubscribe_publisher_preserves_subscriber() { let mut client = create_test_client(); @@ -439,7 +614,7 @@ mod tests { .expect_update_multicastgroup_roles() .with(predicate::eq(UpdateMulticastGroupRolesCommand { user_pk: user_pubkey, - group_pk: mgroup_pubkey, + group_pks: vec![mgroup_pubkey], client_ip, publisher: false, subscriber: true, @@ -541,7 +716,7 @@ mod tests { .expect_update_multicastgroup_roles() .with(predicate::eq(UpdateMulticastGroupRolesCommand { user_pk: user_pubkey, - group_pk: mgroup_pubkey, + group_pks: vec![mgroup_pubkey], client_ip, publisher: true, subscriber: false, diff --git a/smartcontract/programs/doublezero-serviceability/src/instructions.rs b/smartcontract/programs/doublezero-serviceability/src/instructions.rs index def2287263..a68995bbaa 100644 --- a/smartcontract/programs/doublezero-serviceability/src/instructions.rs +++ b/smartcontract/programs/doublezero-serviceability/src/instructions.rs @@ -1100,6 +1100,7 @@ mod tests { publisher: false, subscriber: true, use_onchain_allocation: false, + extra_group_count: 0, }), "UpdateMulticastGroupRoles", ); @@ -1114,6 +1115,7 @@ mod tests { dz_prefix_count: 0, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), "CreateSubscribeUser", ); diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs index 0c0f09e9d0..a1bd01bafe 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs @@ -35,14 +35,20 @@ pub struct UpdateMulticastGroupRolesArgs { pub subscriber: bool, #[incremental(default = false)] pub use_onchain_allocation: bool, + /// Number of additional writable MulticastGroup accounts following the five fixed + /// accounts. The role change is applied to the primary group plus every extra + /// group atomically. Old encodings without this byte decode as 0 (single-group + /// behavior). + #[incremental(default = 0)] + pub extra_group_count: u8, } impl fmt::Debug for UpdateMulticastGroupRolesArgs { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "client_ip: {}, publisher: {:?}, subscriber: {:?}, use_onchain_allocation: {:?}", - self.client_ip, self.publisher, self.subscriber, self.use_onchain_allocation + "client_ip: {}, publisher: {:?}, subscriber: {:?}, use_onchain_allocation: {:?}, extra_group_count: {}", + self.client_ip, self.publisher, self.subscriber, self.use_onchain_allocation, self.extra_group_count ) } } @@ -174,12 +180,24 @@ pub fn process_update_multicastgroup_roles( let globalstate = GlobalState::try_from(gs_account)?; let multicast_publisher_block_ext = next_account_info(accounts_iter)?; - // Trailing layout: [payer, system, permission?]. The SDK appends the payer's Permission PDA last - // (via execute_authorized_transaction), and split_trailing_permission identifies it by PDA match - // rather than by position. + // Trailing layout: [mgroup₁..mgroupₙ, payer, system, permission?]. The extra + // multicast groups (batch role change, counted by args.extra_group_count) come + // first; the SDK appends the payer's Permission PDA last (via + // execute_authorized_transaction), and split_trailing_permission identifies it by + // PDA match rather than by position, so the variable-length extras never confuse it. let remaining: Vec<&AccountInfo> = accounts_iter.collect(); - let (payer_account, system_program, _leading, permission_account) = + let (payer_account, system_program, leading, permission_account) = split_trailing_permission(program_id, &remaining)?; + let extra_group_count = value.extra_group_count as usize; + if extra_group_count > leading.len() { + msg!( + "extra_group_count {} exceeds {} supplied accounts", + extra_group_count, + leading.len() + ); + return Err(DoubleZeroError::InvalidArgument.into()); + } + let (extra_group_accounts, _rest) = leading.split_at(extra_group_count); #[cfg(test)] msg!("process_update_multicastgroup_roles({:?})", value); @@ -194,6 +212,26 @@ pub fn process_update_multicastgroup_roles( writable = true, "MulticastGroup" ); + for extra_group_account in extra_group_accounts { + validate_program_account!( + *extra_group_account, + program_id, + writable = true, + "MulticastGroup" + ); + } + // Reject duplicate group accounts. A duplicate aliases the same account data + // twice in the batch loop, making the final counter state depend on write + // ordering — an explicit error is a clearer contract than an accidental no-op. + for (i, group_key) in std::iter::once(mgroup_account.key) + .chain(extra_group_accounts.iter().map(|a| a.key)) + .enumerate() + { + if extra_group_accounts[i..].iter().any(|a| a.key == group_key) { + msg!("duplicate multicast group {} in batch", group_key); + return Err(DoubleZeroError::InvalidArgument.into()); + } + } if accesspass_account.data_is_empty() { return Err(DoubleZeroError::AccessPassNotFound.into()); } @@ -284,23 +322,33 @@ pub fn process_update_multicastgroup_roles( } } - // Every pass type is authorized the same way here: the group must be on the pass's allowlist. - check_mgroup_allowlists( - &accesspass, - mgroup_account.key, - value.publisher, - value.subscriber, - )?; - - let result = update_user_multicastgroup_roles( - mgroup_account, - &mut user, - value.publisher, - value.subscriber, - )?; + // Apply the role change to every group in the batch. Every pass type is + // authorized the same way here: each group must be on the pass's allowlist. Any + // per-group failure aborts the whole instruction, so the batch is atomic. + // Aggregating the transition flag with `|=` is correct for the dz_ip logic + // below: on batch adds only the first add sees empty→non-empty; on batch + // removes only the last removal sees non-empty→empty. + let mut publisher_list_transitioned = false; + for group_account in std::iter::once(mgroup_account).chain(extra_group_accounts.iter().copied()) + { + check_mgroup_allowlists( + &accesspass, + group_account.key, + value.publisher, + value.subscriber, + )?; + let result = update_user_multicastgroup_roles( + group_account, + &mut user, + value.publisher, + value.subscriber, + )?; + publisher_list_transitioned |= result.publisher_list_transitioned; + try_acc_write(&result.mgroup, group_account, payer_account, accounts)?; + } // Allocate dz_ip when gaining first publisher - if result.publisher_list_transitioned + if publisher_list_transitioned && value.publisher && (user.dz_ip == Ipv4Addr::UNSPECIFIED || user.dz_ip == user.client_ip) { @@ -315,7 +363,7 @@ pub fn process_update_multicastgroup_roles( ); user.dz_ip = allocate_ip(multicast_publisher_block_ext, 1)?.ip(); - } else if result.publisher_list_transitioned + } else if publisher_list_transitioned && !value.publisher && user.dz_ip != Ipv4Addr::UNSPECIFIED && user.dz_ip != user.client_ip @@ -337,7 +385,6 @@ pub fn process_update_multicastgroup_roles( user.dz_ip = user.client_ip; } - try_acc_write(&result.mgroup, mgroup_account, payer_account, accounts)?; try_acc_write(&user, user_account, payer_account, accounts)?; Ok(()) diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/user/create_subscribe.rs b/smartcontract/programs/doublezero-serviceability/src/processors/user/create_subscribe.rs index c24f33834d..062a3bb61b 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/user/create_subscribe.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/user/create_subscribe.rs @@ -25,8 +25,9 @@ use super::{ }; use crate::{ processors::{ - feed::enforce_feed_metro_gate, + feed::{check_feed_metro_coverage, enforce_feed_metro_gate}, multicastgroup::subscribe::{check_mgroup_allowlists, update_user_multicastgroup_roles}, + validation::validate_program_account, }, state::accesspass::AccessPassType, }; @@ -55,13 +56,19 @@ pub struct UserCreateSubscribeArgs { /// is `FeatureFlag::RequireIpOwnershipProof`'s call, not the decoder's. #[incremental(default = None)] pub ip_proof: Option, + /// Number of additional writable MulticastGroup accounts following the dz_prefix + /// blocks (before the optional EdgeSeat feed account). The user is subscribed to the + /// primary group plus every extra group atomically, before resource allocation and + /// activation. Old encodings without this byte decode as 0 (single-group behavior). + #[incremental(default = 0)] + pub extra_group_count: u8, } impl fmt::Debug for UserCreateSubscribeArgs { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "user_type: {}, cyoa_type: {}, client_ip: {}, tunnel_endpoint: {}, dz_prefix_count: {}, owner: {}, ip_proof: {}", + "user_type: {}, cyoa_type: {}, client_ip: {}, tunnel_endpoint: {}, dz_prefix_count: {}, owner: {}, ip_proof: {}, extra_group_count: {}", self.user_type, self.cyoa_type, self.client_ip, @@ -69,6 +76,7 @@ impl fmt::Debug for UserCreateSubscribeArgs { self.dz_prefix_count, self.owner, self.ip_proof.is_some(), + self.extra_group_count, ) } } @@ -112,17 +120,47 @@ pub fn process_create_subscribe_user( )? .expect("dz_prefix_count > 0 guarantees Some"); - // Trailing layout after the resource-extension accounts: [feed?, payer, system, permission?]. - // The optional Feed account (EdgeSeat metro gate — the feed covering the device's exchange and - // listing the target multicast group) precedes payer/system; the optional payer Permission PDA - // (appended by the SDK when it exists on-chain, authorizing a USER_ADMIN owner-override inside - // create_user_core) is last. split_trailing_permission identifies the Permission by PDA match - // rather than by position, so Feed and Permission coexist unambiguously — a single positional - // slot cannot, since either may be present or absent independently. + // Trailing layout after the resource-extension accounts: + // [mgroup₁..mgroupₙ, feed?, payer, system, permission?]. The extra multicast groups + // (batch subscription, counted by args.extra_group_count) come first; the optional Feed + // account (EdgeSeat metro gate — the feed covering the device's exchange and listing the + // target multicast groups) follows; the optional payer Permission PDA (appended by the SDK + // when it exists on-chain, authorizing a USER_ADMIN owner-override inside create_user_core) + // is last. split_trailing_permission identifies the Permission by PDA match rather than by + // position, so the variable-length regions never confuse it. let remaining: Vec<&AccountInfo> = accounts_iter.collect(); let (payer_account, system_program, leading, permission_account) = split_trailing_permission(program_id, &remaining)?; - let feed_account = leading.first().copied(); + let extra_group_count = value.extra_group_count as usize; + if extra_group_count > leading.len() { + msg!( + "extra_group_count {} exceeds {} supplied accounts", + extra_group_count, + leading.len() + ); + return Err(DoubleZeroError::InvalidArgument.into()); + } + let (extra_group_accounts, rest) = leading.split_at(extra_group_count); + let feed_account = rest.first().copied(); + + validate_program_account!( + mgroup_account, + program_id, + writable = true, + "MulticastGroup" + ); + // Reject duplicate group accounts. A duplicate aliases the same account data + // twice in the batch loop, making the final counter state depend on write + // ordering — an explicit error is a clearer contract than an accidental no-op. + for (i, group_key) in std::iter::once(mgroup_account.key) + .chain(extra_group_accounts.iter().map(|a| a.key)) + .enumerate() + { + if extra_group_accounts[i..].iter().any(|a| a.key == group_key) { + msg!("duplicate multicast group {} in batch", group_key); + return Err(DoubleZeroError::InvalidArgument.into()); + } + } msg!("process_create_subscribe_user({:?})", value); @@ -186,7 +224,8 @@ pub fn process_create_subscribe_user( value.subscriber && !feed_gated, )?; - // Subscribe user to multicast group + // Subscribe user to the primary multicast group; the feed metro gate above already + // covered it (and ticked the seat) for EdgeSeat passes. let subscribe_result = update_user_multicastgroup_roles( mgroup_account, &mut result.user, @@ -194,6 +233,49 @@ pub fn process_create_subscribe_user( value.subscriber, )?; + // Subscribe the extra groups, each authorized exactly like the primary: the + // publisher allowlist always applies, and the subscriber role comes from the + // allowlist unless the feed metro gate covers it — extra groups on EdgeSeat + // passes get a coverage-only check against the same feed (a feed carries a + // group set; the seat is per-user-per-feed and was ticked once by the gate + // above). Any failure aborts the whole instruction: no user account, + // counters, or seat tick survive a partial batch. + for extra_group_account in extra_group_accounts { + validate_program_account!( + *extra_group_account, + program_id, + writable = true, + "MulticastGroup" + ); + if feed_gated { + check_feed_metro_coverage( + program_id, + &result.accesspass, + &result.device.exchange_pk, + Some(extra_group_account.key), + feed_account, + )?; + } + check_mgroup_allowlists( + &result.accesspass, + extra_group_account.key, + value.publisher, + value.subscriber && !feed_gated, + )?; + let extra_result = update_user_multicastgroup_roles( + extra_group_account, + &mut result.user, + value.publisher, + value.subscriber, + )?; + try_acc_write( + &extra_result.mgroup, + extra_group_account, + payer_account, + accounts, + )?; + } + // Always allocate resources and activate atomically. resource_onchain_helpers::validate_and_allocate_user_resources( program_id, diff --git a/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs b/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs index 7bcb942c5b..c60f5c626c 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs @@ -385,6 +385,7 @@ async fn test_create_subscribe_user_atomic_publisher() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -470,6 +471,7 @@ async fn test_create_subscribe_user_atomic_subscriber() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -643,6 +645,7 @@ async fn test_create_subscribe_user_ignores_tenant_allowlist() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -733,6 +736,7 @@ async fn test_create_subscribe_user_ignores_expired_epoch() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -803,6 +807,7 @@ async fn test_check_access_pass_multicast_stays_activated() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -1128,6 +1133,7 @@ async fn test_create_subscribe_user_foundation_owner_override() { dz_prefix_count: 1, owner: custom_owner, ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -1434,6 +1440,7 @@ async fn test_create_subscribe_user_sentinel_owner_override() { dz_prefix_count: 1, owner: custom_owner, ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -1727,6 +1734,7 @@ async fn test_create_subscribe_user_user_admin_owner_override() { dz_prefix_count: 1, owner: custom_owner, ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2016,6 +2024,7 @@ async fn test_create_subscribe_user_non_foundation_owner_override_rejected() { dz_prefix_count: 0, owner: custom_owner, ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2076,6 +2085,7 @@ async fn test_unsubscribe_pending_user_created_via_create_subscribe() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2111,6 +2121,7 @@ async fn test_unsubscribe_pending_user_created_via_create_subscribe() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -2181,6 +2192,7 @@ async fn test_subscribe_pending_user_succeeds() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2218,6 +2230,7 @@ async fn test_subscribe_pending_user_succeeds() { publisher: true, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -2298,6 +2311,7 @@ async fn test_create_subscribe_user_inactive_mgroup_fails() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2360,6 +2374,7 @@ async fn test_publisher_multicast_publisher_persists_through_disconnect() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2408,6 +2423,7 @@ async fn test_publisher_multicast_publisher_persists_through_disconnect() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -2473,6 +2489,7 @@ async fn test_publisher_disconnect_delete_decrements_publishers_count() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), vec![ AccountMeta::new(user_pubkey, false), @@ -2500,6 +2517,7 @@ async fn test_publisher_disconnect_delete_decrements_publishers_count() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -2558,3 +2576,586 @@ async fn test_publisher_disconnect_delete_decrements_publishers_count() { "subscribers_count must NOT change — user was created as publisher" ); } + +// ============================================================================ +// Batch (extra_group_count) tests +// ============================================================================ + +/// Create a second activated multicast group; optionally add it to the pass's +/// pub+sub allowlists. +async fn create_second_group( + banks_client: &mut BanksClient, + program_id: Pubkey, + payer: &solana_sdk::signature::Keypair, + globalstate_pubkey: Pubkey, + accesspass_pubkey: Pubkey, + user_ip: Ipv4Addr, + allowlist: bool, +) -> Pubkey { + let gs = get_globalstate(banks_client, globalstate_pubkey).await; + let (mgroup2_pubkey, _) = get_multicastgroup_pda(&program_id, gs.account_index + 1); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateMulticastGroup(MulticastGroupCreateArgs { + code: "group2".to_string(), + max_bandwidth: 1000, + owner: payer.pubkey(), + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastGroupBlock).0, + false, + ), + ], + payer, + ) + .await; + + if allowlist { + let recent_blockhash = wait_for_new_blockhash(banks_client).await; + execute_transaction( + banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::AddMulticastGroupPubAllowlist( + AddMulticastGroupPubAllowlistArgs { + client_ip: user_ip, + user_payer: payer.pubkey(), + }, + ), + vec![ + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(payer.pubkey(), false), + ], + payer, + ) + .await; + execute_transaction( + banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::AddMulticastGroupSubAllowlist( + AddMulticastGroupSubAllowlistArgs { + client_ip: user_ip, + user_payer: payer.pubkey(), + }, + ), + vec![ + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(payer.pubkey(), false), + ], + payer, + ) + .await; + } + + mgroup2_pubkey +} + +/// CreateSubscribeUser with an extra group: the user is created Activated with both +/// subscriptions, per-group counters are right, and resources are allocated once. +#[tokio::test] +async fn test_create_subscribe_user_batch_two_groups() { + let client_ip = [100, 0, 0, 41]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + let mgroup2_pubkey = create_second_group( + &mut banks_client, + program_id, + &payer, + globalstate_pubkey, + accesspass_pubkey, + user_ip, + true, + ) + .await; + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + // Extra group after the dz_prefix blocks, before the trailing [payer, system]. + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + AccountMeta::new(mgroup2_pubkey, false), + ], + &payer, + ) + .await + .expect("batch create-subscribe should succeed"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should exist") + .get_user() + .unwrap(); + assert_eq!(user.status, UserStatus::Activated); + assert_eq!(user.subscribers, vec![mgroup_pubkey, mgroup2_pubkey]); + assert!(user.publishers.is_empty()); + assert_ne!(user.tunnel_id, 0, "tunnel resources allocated once"); + + for mgroup_pk in [mgroup_pubkey, mgroup2_pubkey] { + let mgroup = get_account_data(&mut banks_client, mgroup_pk) + .await + .expect("MulticastGroup should exist") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.subscriber_count, 1); + assert_eq!(mgroup.publisher_count, 0); + } +} + +/// A bad extra group (off the allowlist) aborts the whole create: no user account, +/// no counter movement on the good group. +#[tokio::test] +async fn test_create_subscribe_user_batch_atomic_bad_extra_group() { + let client_ip = [100, 0, 0, 42]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + // Second group NOT in the pass's allowlists. + let mgroup2_pubkey = create_second_group( + &mut banks_client, + program_id, + &payer, + globalstate_pubkey, + accesspass_pubkey, + user_ip, + false, + ) + .await; + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + AccountMeta::new(mgroup2_pubkey, false), + ], + &payer, + ) + .await; + match result { + Err(BanksClientError::TransactionError( + solana_sdk::transaction::TransactionError::InstructionError( + 0, + solana_sdk::instruction::InstructionError::Custom(8), // NotAllowed + ), + )) => {} + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), + } + + // Nothing survives: no user account, primary group's counter untouched. + assert!( + get_account_data(&mut banks_client, user_pubkey) + .await + .is_none(), + "user account must not be created on a failed batch" + ); + let mgroup = get_account_data(&mut banks_client, mgroup_pubkey) + .await + .expect("MulticastGroup should exist") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.subscriber_count, 0); +} + +/// An `extra_group_count` larger than the supplied accounts is rejected with +/// InvalidArgument instead of misparsing the trailing region. +#[tokio::test] +async fn test_create_subscribe_user_batch_count_exceeding_accounts_rejected() { + let client_ip = [100, 0, 0, 43]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 3, // no extra accounts supplied + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError( + solana_sdk::transaction::TransactionError::InstructionError( + 0, + solana_sdk::instruction::InstructionError::Custom(65), // InvalidArgument + ), + )) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } +} + +/// The pre-batch encoding (no extra_group_count byte) still decodes and executes as +/// a single-group create-subscribe — wire compatibility for old clients. +#[tokio::test] +async fn test_create_subscribe_user_batch_old_encoding_single_group() { + let client_ip = [100, 0, 0, 44]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + // Serialize the new args and strip the trailing extra_group_count byte to get + // the exact bytes a pre-batching (post-RFC-27) client emits: the payload ends + // at the ip_proof None tag. + let mut data = borsh::to_vec(&DoubleZeroInstruction::CreateSubscribeUser( + UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 0, + }, + )) + .unwrap(); + assert_eq!(data.pop(), Some(0), "last byte must be extra_group_count"); + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let accounts = vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(solana_system_interface::program::ID, false), + ]; + let instruction = + solana_sdk::instruction::Instruction::new_with_bytes(program_id, &data, accounts); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let mut tx = + solana_sdk::transaction::Transaction::new_with_payer(&[instruction], Some(&payer.pubkey())); + tx.try_sign(&[&payer], recent_blockhash).unwrap(); + banks_client + .process_transaction(tx) + .await + .expect("old encoding should execute single-group"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should exist") + .get_user() + .unwrap(); + assert_eq!(user.status, UserStatus::Activated); + assert_eq!(user.subscribers, vec![mgroup_pubkey]); +} + +/// A duplicate group in a batch (primary repeated as an extra) is rejected with +/// InvalidArgument and no user is created. +#[tokio::test] +async fn test_create_subscribe_user_batch_duplicate_group_rejected() { + let client_ip = [100, 0, 0, 45]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + AccountMeta::new(mgroup_pubkey, false), // primary again as the extra + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError( + solana_sdk::transaction::TransactionError::InstructionError( + 0, + solana_sdk::instruction::InstructionError::Custom(65), // InvalidArgument + ), + )) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } + + assert!( + get_account_data(&mut banks_client, user_pubkey) + .await + .is_none(), + "user account must not be created on a rejected batch" + ); + let mgroup = get_account_data(&mut banks_client, mgroup_pubkey) + .await + .expect("MulticastGroup should exist") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.subscriber_count, 0); +} + +/// Two identical extra groups (extra-vs-extra duplicate) are rejected with +/// InvalidArgument, exercising the pairwise branch of the duplicate scan. +#[tokio::test] +async fn test_create_subscribe_user_batch_duplicate_extra_rejected() { + let client_ip = [100, 0, 0, 46]; + let f = setup_create_subscribe_fixture(client_ip).await; + let CreateSubscribeFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + device_pubkey, + accesspass_pubkey, + mgroup_pubkey, + user_ip, + user_tunnel_block, + multicast_publisher_block, + tunnel_ids, + dz_prefix_block, + .. + } = f; + + let mgroup2_pubkey = create_second_group( + &mut banks_client, + program_id, + &payer, + globalstate_pubkey, + accesspass_pubkey, + user_ip, + true, + ) + .await; + + let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 2, + }), + vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(device_pubkey, false), + AccountMeta::new(mgroup_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new(user_tunnel_block, false), + AccountMeta::new(multicast_publisher_block, false), + AccountMeta::new(tunnel_ids, false), + AccountMeta::new(dz_prefix_block, false), + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(mgroup2_pubkey, false), // same extra twice + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError( + solana_sdk::transaction::TransactionError::InstructionError( + 0, + solana_sdk::instruction::InstructionError::Custom(65), // InvalidArgument + ), + )) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } + assert!( + get_account_data(&mut banks_client, user_pubkey) + .await + .is_none(), + "user account must not be created on a rejected batch" + ); +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/feed_metro_gate_test.rs b/smartcontract/programs/doublezero-serviceability/tests/feed_metro_gate_test.rs index 979bd5e17b..cd28d39764 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/feed_metro_gate_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/feed_metro_gate_test.rs @@ -35,7 +35,12 @@ use doublezero_serviceability::{ }, }; use solana_program_test::*; -use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Signer}; +use solana_sdk::{ + instruction::{AccountMeta, InstructionError}, + pubkey::Pubkey, + signature::Signer, + transaction::TransactionError, +}; use std::net::Ipv4Addr; mod test_helpers; @@ -365,6 +370,7 @@ async fn try_subscribe_with_feed( dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), &accounts, &f.payer, @@ -374,6 +380,18 @@ async fn try_subscribe_with_feed( f.banks_client.process_transaction(tx).await } +/// Match the error structurally rather than on its debug text, so a test cannot pass because some +/// other instruction in the transaction failed or because the formatting changed. +fn assert_custom_error(err: &BanksClientError, code: u32) { + match err { + BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(actual), + )) if *actual == code => {} + other => panic!("expected Custom({code}), got {other:?}"), + } +} + #[tokio::test] async fn test_right_metro_joins_group_set() { let mut f = setup_feed_fixture([100, 0, 0, 20]).await; @@ -448,10 +466,7 @@ async fn test_wrong_metro_device_rejected() { let err = try_subscribe_with_feed(&mut f, feed) .await .expect_err("wrong-metro subscribe should be rejected"); - assert!( - format!("{err:?}").contains("Custom(91)"), - "expected MetroMismatch (Custom(91)), got: {err:?}" - ); + assert_custom_error(&err, 91); // MetroMismatch } #[tokio::test] @@ -529,8 +544,175 @@ async fn test_group_not_in_feed_rejected() { let err = try_subscribe_with_feed(&mut f, feed) .await .expect_err("group outside the feed should be rejected"); + assert_custom_error(&err, 94); // GroupNotInFeed +} + +// ============================================================================ +// Batch (extra_group_count) tests +// ============================================================================ + +/// Create a second activated multicast group in the fixture's environment. +async fn create_second_group(f: &mut FeedFixture) -> Pubkey { + let gs = get_globalstate(&mut f.banks_client, f.globalstate_pubkey).await; + let (mgroup2_pubkey, _) = get_multicastgroup_pda(&f.program_id, gs.account_index + 1); + let recent_blockhash = f.banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut f.banks_client, + recent_blockhash, + f.program_id, + DoubleZeroInstruction::CreateMulticastGroup(MulticastGroupCreateArgs { + code: "group2".to_string(), + max_bandwidth: 1000, + owner: f.payer.pubkey(), + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(f.globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&f.program_id, ResourceType::MulticastGroupBlock).0, + false, + ), + ], + &f.payer, + ) + .await; + mgroup2_pubkey +} + +/// Attempt a batch CreateSubscribeUser (subscriber) with one extra group; the extra +/// group account sits between the dz_prefix block and the feed. +async fn try_subscribe_batch_with_feed( + f: &mut FeedFixture, + feed: Pubkey, + extra_group: Pubkey, +) -> Result<(), BanksClientError> { + let recent_blockhash = wait_for_new_blockhash(&mut f.banks_client).await; + let (user_pubkey, _) = get_user_pda(&f.program_id, &f.user_ip, UserType::Multicast); + let accounts = vec![ + AccountMeta::new(user_pubkey, false), + AccountMeta::new(f.device_pubkey, false), + AccountMeta::new(f.mgroup_pubkey, false), + AccountMeta::new(f.accesspass_pubkey, false), + AccountMeta::new(f.globalstate_pubkey, false), + AccountMeta::new(f.user_tunnel_block, false), + AccountMeta::new(f.multicast_publisher_block, false), + AccountMeta::new(f.tunnel_ids, false), + AccountMeta::new(f.dz_prefix_block, false), + AccountMeta::new(extra_group, false), + AccountMeta::new_readonly(feed, false), + ]; + let mut tx = create_transaction_with_extra_accounts( + f.program_id, + &DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs { + user_type: UserType::Multicast, + cyoa_type: UserCYOA::GREOverDIA, + client_ip: f.user_ip, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + dz_prefix_count: 1, + owner: Pubkey::default(), + ip_proof: None, + extra_group_count: 1, + }), + &accounts, + &f.payer, + &[], + ); + tx.try_sign(&[&f.payer], recent_blockhash).unwrap(); + f.banks_client.process_transaction(tx).await +} + +/// A batch EdgeSeat create where every group is in the feed's set succeeds, with the +/// seat ticked exactly once (per-user-per-feed, not per-group). +#[tokio::test] +async fn test_batch_within_feed_group_set_ticks_seat_once() { + let mut f = setup_feed_fixture([100, 0, 0, 24]).await; + let mgroup2 = create_second_group(&mut f).await; + let (exchange, mgroup) = (f.exchange_pubkey, f.mgroup_pubkey); + // Feed serves the device's exchange with BOTH groups. + let feed = create_feed(&mut f, "shreds", exchange, vec![mgroup, mgroup2]).await; + set_pass_feeds( + &mut f, + vec![FeedSeat { + feed_key: feed, + max_users: 2, + max_future_users: 2, + current_users: 0, + anniversary_day: 15, + window_end: TEST_WINDOW_END, + terminates_at: TEST_TERMINATES_AT, + }], + ) + .await; + + try_subscribe_batch_with_feed(&mut f, feed, mgroup2) + .await + .expect("batch subscribe within the feed's group set should succeed"); + + let (user_pubkey, _) = get_user_pda(&f.program_id, &f.user_ip, UserType::Multicast); + let user = get_account_data(&mut f.banks_client, user_pubkey) + .await + .expect("user exists") + .get_user() + .unwrap(); + assert_eq!(user.status, UserStatus::Activated); + assert_eq!(user.subscribers, vec![f.mgroup_pubkey, mgroup2]); + + // One seat tick for the whole batch: the seat is per-user-per-feed. + let pass = get_account_data(&mut f.banks_client, f.accesspass_pubkey) + .await + .unwrap() + .get_accesspass() + .unwrap(); + assert_eq!(pass.feed_seats()[0].current_users, 1); +} + +/// A batch EdgeSeat create with an extra group outside the feed's set is rejected +/// with GroupNotInFeed and rolls back atomically — including the seat tick. +#[tokio::test] +async fn test_batch_extra_group_not_in_feed_rejected_and_seat_not_ticked() { + let mut f = setup_feed_fixture([100, 0, 0, 25]).await; + let mgroup2 = create_second_group(&mut f).await; + let (exchange, mgroup) = (f.exchange_pubkey, f.mgroup_pubkey); + // Feed serves the device's exchange with only the primary group. + let feed = create_feed(&mut f, "shreds", exchange, vec![mgroup]).await; + set_pass_feeds( + &mut f, + vec![FeedSeat { + feed_key: feed, + max_users: 2, + max_future_users: 2, + current_users: 0, + anniversary_day: 15, + window_end: TEST_WINDOW_END, + terminates_at: TEST_TERMINATES_AT, + }], + ) + .await; + + let err = try_subscribe_batch_with_feed(&mut f, feed, mgroup2) + .await + .expect_err("extra group outside the feed's set should be rejected"); + assert_custom_error(&err, 94); // GroupNotInFeed + + // The whole transaction rolled back: no user, no seat consumed. + let (user_pubkey, _) = get_user_pda(&f.program_id, &f.user_ip, UserType::Multicast); assert!( - format!("{err:?}").contains("Custom(94)"), - "expected GroupNotInFeed (Custom(94)), got: {err:?}" + get_account_data(&mut f.banks_client, user_pubkey) + .await + .is_none(), + "user account must not be created on a failed batch" + ); + let pass = get_account_data(&mut f.banks_client, f.accesspass_pubkey) + .await + .unwrap() + .get_accesspass() + .unwrap(); + assert_eq!( + pass.feed_seats()[0].current_users, + 0, + "seat tick must roll back with the failed batch" ); } diff --git a/smartcontract/programs/doublezero-serviceability/tests/feed_subscription_test.rs b/smartcontract/programs/doublezero-serviceability/tests/feed_subscription_test.rs index fc2ef7b188..48f7364c4e 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/feed_subscription_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/feed_subscription_test.rs @@ -359,6 +359,7 @@ async fn try_create_user_at( dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), &accounts, &f.payer, @@ -771,6 +772,7 @@ async fn test_allowlisted_group_joins_without_a_seat() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), &vec![ AccountMeta::new(g[4], false), @@ -811,6 +813,7 @@ async fn test_feed_group_not_joinable_through_the_roles_instruction() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), &vec![ AccountMeta::new(g[1], false), diff --git a/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs b/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs index f3571e1933..7d3f9379ac 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/multicastgroup_subscribe_test.rs @@ -420,6 +420,7 @@ async fn test_subscribe_foundation_admin_payer_differs_from_user_owner() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -474,6 +475,7 @@ async fn test_unsubscribe_foundation_admin_payer_differs_from_user_owner() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -534,6 +536,7 @@ async fn test_unsubscribe_foundation_admin_payer_differs_from_user_owner() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -587,6 +590,7 @@ async fn test_subscribe_unauthorized_payer_rejected() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -641,6 +645,7 @@ async fn test_unsubscribe_user_admin_permission_allowed() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -690,6 +695,7 @@ async fn test_unsubscribe_user_admin_permission_allowed() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -764,6 +770,7 @@ async fn test_subscribe_user_admin_permission_rejected() { publisher: false, subscriber: true, // attempting to ADD a role use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -849,6 +856,7 @@ async fn test_subscribe_access_pass_admin_permission_allowed() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -914,6 +922,7 @@ async fn test_subscribe_onchain_first_publisher_allocates_dz_ip() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -987,6 +996,7 @@ async fn test_subscribe_onchain_subscriber_no_allocation() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -1042,6 +1052,7 @@ async fn test_subscribe_onchain_second_publisher_no_reallocation() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -1072,6 +1083,7 @@ async fn test_subscribe_onchain_second_publisher_no_reallocation() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup2_pubkey, false), @@ -1127,6 +1139,7 @@ async fn test_duplicate_publisher_subscribe_is_noop() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -1164,6 +1177,7 @@ async fn test_duplicate_publisher_subscribe_is_noop() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup1_pubkey, false), @@ -1202,3 +1216,635 @@ async fn test_duplicate_publisher_subscribe_is_noop() { "Should not double-count publisher" ); } + +// ============================================================================ +// Batch (extra_group_count) tests +// ============================================================================ + +/// Batch subscribe to two groups in one transaction: both subscriptions land and +/// each group's subscriber_count is incremented. +#[tokio::test] +async fn test_batch_subscribe_two_groups_one_transaction() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + mgroup2_pubkey, + .. + } = f; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup2_pubkey, false), + ], + &payer, + ) + .await + .expect("batch subscribe to two groups should succeed"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert_eq!(user.subscribers, vec![mgroup1_pubkey, mgroup2_pubkey]); + assert_eq!(user.status, UserStatus::Activated); + + for mgroup_pk in [mgroup1_pubkey, mgroup2_pubkey] { + let mgroup = get_account_data(&mut banks_client, mgroup_pk) + .await + .expect("Unable to get MulticastGroup") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.subscriber_count, 1); + } +} + +/// Batch publisher add from empty allocates dz_ip exactly once; a batch removal of +/// all publisher roles deallocates it (dz_ip reverts to client_ip). +#[tokio::test] +async fn test_batch_publisher_add_and_remove_allocates_dz_ip_once() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + mgroup2_pubkey, + .. + } = f; + + let accounts = vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup2_pubkey, false), + ]; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: true, + subscriber: false, + use_onchain_allocation: true, + extra_group_count: 1, + }), + accounts.clone(), + &payer, + ) + .await + .expect("batch publisher add should succeed"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert_eq!(user.publishers, vec![mgroup1_pubkey, mgroup2_pubkey]); + assert_ne!( + user.dz_ip, + Ipv4Addr::UNSPECIFIED, + "dz_ip should be allocated" + ); + assert_ne!( + user.dz_ip, user.client_ip, + "dz_ip should come from MulticastPublisherBlock, not client_ip" + ); + for mgroup_pk in [mgroup1_pubkey, mgroup2_pubkey] { + let mgroup = get_account_data(&mut banks_client, mgroup_pk) + .await + .expect("Unable to get MulticastGroup") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.publisher_count, 1); + } + + // Batch removal of both publisher roles: exactly one deallocation + // (dz_ip reverts to client_ip on the non-empty -> empty transition). + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: false, + use_onchain_allocation: true, + extra_group_count: 1, + }), + accounts, + &payer, + ) + .await + .expect("batch publisher removal should succeed"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert!(user.publishers.is_empty()); + assert_eq!( + user.dz_ip, user.client_ip, + "dz_ip should be deallocated back to client_ip" + ); + for mgroup_pk in [mgroup1_pubkey, mgroup2_pubkey] { + let mgroup = get_account_data(&mut banks_client, mgroup_pk) + .await + .expect("Unable to get MulticastGroup") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.publisher_count, 0); + } +} + +/// A batch is atomic: one group off the subscriber allowlist fails the whole +/// transaction and no group or user state changes. +#[tokio::test] +async fn test_batch_atomicity_group_off_allowlist_rolls_back() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + .. + } = f; + + // A third activated group, NOT in the pass's allowlists. + let gs = get_globalstate(&mut banks_client, globalstate_pubkey).await; + let (mgroup3_pubkey, _) = get_multicastgroup_pda(&program_id, gs.account_index + 1); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreateMulticastGroup(MulticastGroupCreateArgs { + code: "group3".to_string(), + max_bandwidth: 1000, + owner: payer.pubkey(), + use_onchain_allocation: true, + }), + vec![ + AccountMeta::new(mgroup3_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastGroupBlock).0, + false, + ), + ], + &payer, + ) + .await; + + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup3_pubkey, false), + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(8), // NotAllowed + ))) => {} + _ => panic!("Expected NotAllowed error (Custom(8)), got {:?}", result), + } + + // Nothing applied: the allowlisted group's subscription rolled back with the batch. + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert!( + user.subscribers.is_empty(), + "batch must roll back atomically" + ); + let mgroup1 = get_account_data(&mut banks_client, mgroup1_pubkey) + .await + .expect("Unable to get MulticastGroup") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup1.subscriber_count, 0); +} + +/// An `extra_group_count` larger than the supplied accounts is rejected with +/// InvalidArgument instead of misparsing the trailing region. +#[tokio::test] +async fn test_batch_extra_group_count_exceeding_accounts_rejected() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + .. + } = f; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 2, // no extra accounts supplied + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(65), // InvalidArgument + ))) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } +} + +/// The pre-batch encoding (no extra_group_count byte) still decodes and executes as +/// a single-group role change — wire compatibility for old clients. +#[tokio::test] +async fn test_batch_old_encoding_without_count_byte_single_group() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + .. + } = f; + + // Serialize the new args and strip the trailing extra_group_count byte to get + // the exact bytes an old client emits. + let mut data = borsh::to_vec(&DoubleZeroInstruction::UpdateMulticastGroupRoles( + UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 0, + }, + )) + .unwrap(); + assert_eq!(data.pop(), Some(0), "last byte must be extra_group_count"); + + let accounts = vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(solana_system_interface::program::ID, false), + ]; + let instruction = + solana_sdk::instruction::Instruction::new_with_bytes(program_id, &data, accounts); + let recent_blockhash = wait_for_new_blockhash(&mut banks_client).await; + let mut tx = + solana_sdk::transaction::Transaction::new_with_payer(&[instruction], Some(&payer.pubkey())); + tx.try_sign(&[&payer], recent_blockhash).unwrap(); + banks_client + .process_transaction(tx) + .await + .expect("old encoding should execute single-group"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert_eq!(user.subscribers, vec![mgroup1_pubkey]); +} + +/// A duplicate group in a batch (primary repeated as an extra) is rejected with +/// InvalidArgument: aliased accounts would make counter state depend on write order. +#[tokio::test] +async fn test_batch_duplicate_group_rejected() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + .. + } = f; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup1_pubkey, false), // primary again as the extra + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(65), // InvalidArgument + ))) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert!(user.subscribers.is_empty()); +} + +/// Two identical extra groups (extra-vs-extra duplicate) are rejected with +/// InvalidArgument, exercising the pairwise branch of the duplicate scan. +#[tokio::test] +async fn test_batch_duplicate_extra_group_rejected() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + mgroup2_pubkey, + .. + } = f; + + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + let result = try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 2, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup2_pubkey, false), + AccountMeta::new(mgroup2_pubkey, false), // same extra twice + ], + &payer, + ) + .await; + + match result { + Err(BanksClientError::TransactionError(TransactionError::InstructionError( + 0, + InstructionError::Custom(65), // InvalidArgument + ))) => {} + _ => panic!( + "Expected InvalidArgument error (Custom(65)), got {:?}", + result + ), + } + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert!(user.subscribers.is_empty()); +} + +/// A USER_ADMIN Permission holder can strip roles across a batch: the trailing +/// Permission PDA (after payer/system) coexists with the extra group accounts in +/// the leading region — split_trailing_permission peels it by PDA match. +#[tokio::test] +async fn test_batch_removal_with_trailing_permission_pda() { + let f = setup_fixture().await; + let TestFixture { + mut banks_client, + payer, // foundation + user.owner + program_id, + globalstate_pubkey, + accesspass_pubkey, + user_pubkey, + mgroup1_pubkey, + mgroup2_pubkey, + .. + } = f; + + // Subscribe the user to both groups (as owner, batched) so there are roles to strip. + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup2_pubkey, false), + ], + &payer, + ) + .await + .expect("owner batch subscribe should succeed"); + + // user_admin: not the owner, not in the foundation allowlist, granted USER_ADMIN. + let user_admin = solana_sdk::signature::Keypair::new(); + transfer(&mut banks_client, &payer, &user_admin.pubkey(), 10_000_000).await; + + let (permission_pda, _) = get_permission_pda(&program_id, &user_admin.pubkey()); + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + execute_transaction( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::CreatePermission(PermissionCreateArgs { + user_payer: user_admin.pubkey(), + permissions: permission_flags::USER_ADMIN, + }), + vec![ + AccountMeta::new(permission_pda, false), + AccountMeta::new_readonly(globalstate_pubkey, false), + ], + &payer, + ) + .await; + + // Batch removal signed by user_admin, Permission PDA appended after payer/system. + let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); + try_execute_transaction_with_extra_accounts( + &mut banks_client, + recent_blockhash, + program_id, + DoubleZeroInstruction::UpdateMulticastGroupRoles(UpdateMulticastGroupRolesArgs { + client_ip: [100, 0, 0, 1].into(), + publisher: false, + subscriber: false, + use_onchain_allocation: true, + extra_group_count: 1, + }), + vec![ + AccountMeta::new(mgroup1_pubkey, false), + AccountMeta::new(accesspass_pubkey, false), + AccountMeta::new(user_pubkey, false), + AccountMeta::new(globalstate_pubkey, false), + AccountMeta::new( + get_resource_extension_pda(&program_id, ResourceType::MulticastPublisherBlock).0, + false, + ), + AccountMeta::new(mgroup2_pubkey, false), + ], + &user_admin, + &[AccountMeta::new_readonly(permission_pda, false)], + ) + .await + .expect("USER_ADMIN batch removal with trailing Permission PDA should succeed"); + + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("Unable to get User") + .get_user() + .unwrap(); + assert!( + user.subscribers.is_empty(), + "both roles stripped in one batch" + ); + for mgroup_pk in [mgroup1_pubkey, mgroup2_pubkey] { + let mgroup = get_account_data(&mut banks_client, mgroup_pk) + .await + .expect("Unable to get MulticastGroup") + .get_multicastgroup() + .unwrap(); + assert_eq!(mgroup.subscriber_count, 0); + } +} diff --git a/smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs b/smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs index 75f631c301..56609e31e3 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/rfc26_builders_test.rs @@ -638,6 +638,7 @@ async fn test_builder_create_subscribe_user() { &mgroup_pubkey, &accesspass_pubkey, 1, + &[], None, UserCreateSubscribeArgs { user_type: UserType::Multicast, @@ -649,6 +650,7 @@ async fn test_builder_create_subscribe_user() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }, ); submit(&mut banks_client, &payer, ix) diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_ip_proof_test.rs b/smartcontract/programs/doublezero-serviceability/tests/user_ip_proof_test.rs index a440e3c459..664ef20d9c 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_ip_proof_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_ip_proof_test.rs @@ -368,6 +368,7 @@ impl Fixture { dz_prefix_count: 1, owner, ip_proof: proof, + extra_group_count: 0, }), &accounts, &payer, @@ -687,6 +688,7 @@ async fn test_valid_proof_is_accepted_for_create_subscribe_user() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: Some(proof), + extra_group_count: 0, }), &accounts, &payer, @@ -726,6 +728,7 @@ async fn test_create_subscribe_user_without_proof_is_rejected() { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }), &accounts, &payer, diff --git a/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs b/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs index 375ffa2ed3..f874a26e8f 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/user_onchain_allocation_test.rs @@ -1381,6 +1381,7 @@ async fn test_delete_user_atomic_decrements_multicast_subscribers_count() { publisher: false, subscriber: true, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(multicastgroup_pubkey, false), @@ -1424,6 +1425,7 @@ async fn test_delete_user_atomic_decrements_multicast_subscribers_count() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(multicastgroup_pubkey, false), @@ -1599,6 +1601,7 @@ async fn test_multicast_publisher_block_deallocation_and_reuse() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -1640,6 +1643,7 @@ async fn test_multicast_publisher_block_deallocation_and_reuse() { publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -1779,6 +1783,7 @@ async fn test_multicast_publisher_block_deallocation_and_reuse() { publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -1901,6 +1906,7 @@ async fn test_delete_user_atomic_decrements_subscribers_count_for_non_publisher( publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), @@ -1934,6 +1940,7 @@ async fn test_delete_user_atomic_decrements_subscribers_count_for_non_publisher( publisher: false, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }), vec![ AccountMeta::new(mgroup_pubkey, false), diff --git a/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs b/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs index 3f369f43d2..0c3566731d 100644 --- a/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs +++ b/smartcontract/sdk/rs/src/commands/multicastgroup/subscribe.rs @@ -12,11 +12,22 @@ use doublezero_serviceability::{ state::multicastgroup::MulticastGroupStatus, }; use doublezero_serviceability_instruction::multicastgroup::update_multicast_group_roles; +use eyre::Context; use solana_sdk::{pubkey::Pubkey, signature::Signature}; +/// Upper bound on multicast groups per role-update transaction. Each group adds a +/// 32-byte account key; 16 groups plus the instruction's fixed accounts, the +/// compute-budget prelude, and an optional Permission PDA stay comfortably under +/// the 1232-byte transaction size limit (pinned by `max_group_batch_fits_transaction`). +/// Callers with more groups send one transaction per chunk. +pub const MAX_GROUPS_PER_TRANSACTION: usize = 16; + #[derive(Debug, PartialEq, Clone)] pub struct UpdateMulticastGroupRolesCommand { - pub group_pk: Pubkey, + /// Multicast groups the role change applies to, atomically in one transaction. + /// Must be non-empty; the first entry is the instruction's primary group and the + /// rest ride as extra group accounts. + pub group_pks: Vec, pub client_ip: Ipv4Addr, pub user_pk: Pubkey, pub publisher: bool, @@ -30,21 +41,42 @@ pub struct UpdateMulticastGroupRolesCommand { impl UpdateMulticastGroupRolesCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result { - let (_, mgroup) = GetMulticastGroupCommand { - pubkey_or_code: self.group_pk.to_string(), + // Deduplicate while preserving order: the processor rejects duplicate group + // accounts in a batch, and a repeated group was an idempotent no-op under + // the old per-group loop. + let mut group_pks: Vec = Vec::with_capacity(self.group_pks.len()); + for pk in &self.group_pks { + if !group_pks.contains(pk) { + group_pks.push(*pk); + } } - .execute(client) - .map_err(|_err| eyre::eyre!("MulticastGroup not found"))?; + if group_pks.len() > MAX_GROUPS_PER_TRANSACTION { + eyre::bail!( + "{} multicast groups exceed the {MAX_GROUPS_PER_TRANSACTION}-group transaction limit; send one transaction per chunk", + group_pks.len() + ); + } + let (first_group_pk, extra_group_pks) = group_pks + .split_first() + .ok_or_else(|| eyre::eyre!("At least one multicast group is required"))?; - if mgroup.status != MulticastGroupStatus::Activated { - eyre::bail!("MulticastGroup not active"); + for group_pk in &group_pks { + let (_, mgroup) = GetMulticastGroupCommand { + pubkey_or_code: group_pk.to_string(), + } + .execute(client) + .wrap_err_with(|| format!("MulticastGroup not found ({group_pk})"))?; + + if mgroup.status != MulticastGroupStatus::Activated { + eyre::bail!("MulticastGroup not active ({group_pk})"); + } } let (_, user) = GetUserCommand { pubkey: self.user_pk, } .execute(client) - .map_err(|_err| eyre::eyre!("User not found"))?; + .wrap_err_with(|| format!("User not found ({})", self.user_pk))?; // GetAccessPassCommand prefers a shared dynamic (UNSPECIFIED) pass and falls // back to the exact client-IP pass. @@ -55,11 +87,13 @@ impl UpdateMulticastGroupRolesCommand { .execute(client)? .ok_or_else(|| eyre::eyre!("AccessPass not found"))?; - if self.publisher && !accesspass.mgroup_pub_allowlist.contains(&self.group_pk) { - eyre::bail!("User not allowed to publish multicast group"); - } - if self.subscriber && !accesspass.mgroup_sub_allowlist.contains(&self.group_pk) { - eyre::bail!("User not allowed to subscribe multicast group"); + for group_pk in &group_pks { + if self.publisher && !accesspass.mgroup_pub_allowlist.contains(group_pk) { + eyre::bail!("User not allowed to publish multicast group ({group_pk})"); + } + if self.subscriber && !accesspass.mgroup_sub_allowlist.contains(group_pk) { + eyre::bail!("User not allowed to subscribe multicast group ({group_pk})"); + } } // The EdgeSeat feed metro gate is enforced at connect (CreateSubscribeUser). The optional @@ -70,14 +104,16 @@ impl UpdateMulticastGroupRolesCommand { client.send_transaction(update_multicast_group_roles( &client.get_program_id(), &client.get_payer(), - &self.group_pk, + first_group_pk, &accesspass_pubkey, &self.user_pk, + extra_group_pks, UpdateMulticastGroupRolesArgs { publisher: self.publisher, subscriber: self.subscriber, client_ip: user.client_ip, use_onchain_allocation: true, + extra_group_count: 0, // derived by the builder from extra_group_pks }, )) } @@ -207,11 +243,13 @@ mod tests { &mgroup_pubkey, &accesspass_pubkey, &user_pubkey, + &[], UpdateMulticastGroupRolesArgs { client_ip, publisher: true, subscriber: false, use_onchain_allocation: true, + extra_group_count: 0, }, ); client @@ -220,7 +258,7 @@ mod tests { .returning(|_| Ok(Signature::new_unique())); let res = UpdateMulticastGroupRolesCommand { - group_pk: mgroup_pubkey, + group_pks: vec![mgroup_pubkey], user_pk: user_pubkey, client_ip, publisher: true, @@ -232,4 +270,49 @@ mod tests { assert!(res.is_ok()); } + + /// A max-size batch (MAX_GROUPS_PER_TRANSACTION groups + Permission PDA) must fit + /// the 1232-byte transaction size limit, including the compute-budget prelude the + /// SDK prepends. + #[test] + fn max_group_batch_fits_transaction_size() { + use solana_sdk::{instruction::AccountMeta, message::Message}; + + let program_id = Pubkey::new_unique(); + let payer = Pubkey::new_unique(); + let group_pks: Vec = (0 + ..crate::commands::multicastgroup::subscribe::MAX_GROUPS_PER_TRANSACTION) + .map(|_| Pubkey::new_unique()) + .collect(); + let mut ix = update_multicast_group_roles( + &program_id, + &payer, + &group_pks[0], + &Pubkey::new_unique(), // accesspass + &Pubkey::new_unique(), // user + &group_pks[1..], + UpdateMulticastGroupRolesArgs { + client_ip: std::net::Ipv4Addr::new(1, 2, 3, 4), + publisher: true, + subscriber: true, + use_onchain_allocation: true, + extra_group_count: 0, + }, + ); + // Worst case also carries the payer's Permission PDA. + ix.accounts + .push(AccountMeta::new_readonly(Pubkey::new_unique(), false)); + + let message = Message::new(&[ix], Some(&payer)); + // Wire size: 1-byte signature count + 64 bytes per signature + the message. + let tx_size = + 1 + 64 * message.header.num_required_signatures as usize + message.serialize().len(); + // The SDK's send_transaction prepends two compute-budget instructions + // (one program key + two short instructions), comfortably under this margin. + const COMPUTE_BUDGET_PRELUDE_MARGIN: usize = 100; + assert!( + tx_size + COMPUTE_BUDGET_PRELUDE_MARGIN <= 1232, + "max batch transaction is {tx_size} bytes + {COMPUTE_BUDGET_PRELUDE_MARGIN} margin, over the 1232-byte limit" + ); + } } diff --git a/smartcontract/sdk/rs/src/commands/user/create_subscribe.rs b/smartcontract/sdk/rs/src/commands/user/create_subscribe.rs index 4cc1dd2959..c8655da3de 100644 --- a/smartcontract/sdk/rs/src/commands/user/create_subscribe.rs +++ b/smartcontract/sdk/rs/src/commands/user/create_subscribe.rs @@ -6,8 +6,9 @@ use doublezero_serviceability::{ user::{UserCYOA, UserType}, }, }; -use doublezero_serviceability_instruction::user::create_subscribe_user; -use solana_sdk::{pubkey::Pubkey, signature::Signature}; +use doublezero_serviceability_instruction::{compute_budget_prelude, user::create_subscribe_user}; +use eyre::Context; +use solana_sdk::{message::Message, pubkey::Pubkey, signature::Signature}; use std::net::Ipv4Addr; use crate::{ @@ -18,13 +19,25 @@ use crate::{ DoubleZeroClient, }; +/// Solana's transaction wire-size limit (`solana_packet::PACKET_DATA_SIZE`, not +/// re-exported by the SDK crates this one depends on). +const PACKET_DATA_SIZE: usize = 1232; + +/// Wire-size room reserved for the read-only Permission PDA `build_with_permission` +/// appends at the permission rollout: one more account key plus its index byte. +const PERMISSION_ACCOUNT_RESERVED: usize = 33; + #[derive(Debug, PartialEq, Clone)] pub struct CreateSubscribeUserCommand { pub user_type: UserType, pub device_pk: Pubkey, pub cyoa_type: UserCYOA, pub client_ip: Ipv4Addr, - pub mgroup_pk: Pubkey, + /// Multicast groups the user is subscribed to at creation, atomically in one + /// transaction. Must be non-empty; the first entry is the instruction's primary + /// group and the rest ride as extra group accounts. The publisher/subscriber + /// flags apply to every group. + pub mgroup_pks: Vec, pub publisher: bool, pub subscriber: bool, pub tunnel_endpoint: Ipv4Addr, @@ -39,14 +52,37 @@ pub struct CreateSubscribeUserCommand { impl CreateSubscribeUserCommand { pub fn execute(&self, client: &dyn DoubleZeroClient) -> eyre::Result<(Signature, Pubkey)> { - let (_, mgroup) = GetMulticastGroupCommand { - pubkey_or_code: self.mgroup_pk.to_string(), + // Deduplicate while preserving order: the processor rejects duplicate group + // accounts in a batch. + let mut mgroup_pks: Vec = Vec::with_capacity(self.mgroup_pks.len()); + for pk in &self.mgroup_pks { + if !mgroup_pks.contains(pk) { + mgroup_pks.push(*pk); + } } - .execute(client) - .map_err(|_err| eyre::eyre!("MulticastGroup not found"))?; + // `extra_group_count` is a u8 on the wire, so more extras cannot even be + // encoded; bail before the per-group validation round-trips. The real bound + // is the transaction size, checked below once the account list is known. + if mgroup_pks.len() > usize::from(u8::MAX) + 1 { + eyre::bail!( + "{} multicast groups can never fit one transaction; subscribe the rest via UpdateMulticastGroupRolesCommand", + mgroup_pks.len() + ); + } + let (first_mgroup_pk, extra_mgroup_pks) = mgroup_pks + .split_first() + .ok_or_else(|| eyre::eyre!("At least one multicast group is required"))?; + + for mgroup_pk in &mgroup_pks { + let (_, mgroup) = GetMulticastGroupCommand { + pubkey_or_code: mgroup_pk.to_string(), + } + .execute(client) + .wrap_err_with(|| format!("MulticastGroup not found ({mgroup_pk})"))?; - if mgroup.status != MulticastGroupStatus::Activated { - eyre::bail!("MulticastGroup not active"); + if mgroup.status != MulticastGroupStatus::Activated { + eyre::bail!("MulticastGroup not active ({mgroup_pk})"); + } } // When a custom owner is set, look up the access pass for that owner @@ -68,7 +104,7 @@ impl CreateSubscribeUserCommand { pubkey_or_code: self.device_pk.to_string(), } .execute(client) - .map_err(|_| eyre::eyre!("Device not found"))?; + .wrap_err_with(|| format!("Device not found ({})", self.device_pk))?; let dz_prefix_count = device.dz_prefixes.len(); if dz_prefix_count == 0 { return Err(eyre::eyre!( @@ -88,9 +124,10 @@ impl CreateSubscribeUserCommand { &program_id, &client.get_payer(), &self.device_pk, - &self.mgroup_pk, + first_mgroup_pk, &accesspass_pk, dz_prefix_count_u8, + extra_mgroup_pks, self.feed_pk.as_ref(), UserCreateSubscribeArgs { user_type: self.user_type, @@ -102,9 +139,42 @@ impl CreateSubscribeUserCommand { dz_prefix_count: dz_prefix_count_u8, owner: self.owner.unwrap_or_default(), ip_proof: None, + extra_group_count: 0, // derived by the builder from extra_mgroup_pks }, ); + // Unlike a role update, this transaction also carries the device, one account + // per device dz_prefix, and an optional feed, so no fixed group cap can bound + // it (16 groups fit a role update but overflow a create on a five-prefix + // device with a feed). Measure the wire size of the exact transaction + // send_transaction builds, reserving room for the Permission PDA the builder + // appends at the permission rollout. + let [cu_limit, heap_frame] = compute_budget_prelude(); + let message = Message::new( + &[cu_limit, heap_frame, ix.clone()], + Some(&client.get_payer()), + ); + let tx_size = 1 + + 64 * usize::from(message.header.num_required_signatures) + + message.serialize().len(); + if tx_size + PERMISSION_ACCOUNT_RESERVED > PACKET_DATA_SIZE { + // Every extra group past the fit costs its 32-byte key plus an index byte. + let over = tx_size + PERMISSION_ACCOUNT_RESERVED - PACKET_DATA_SIZE; + eyre::bail!( + "subscribing {} multicast groups at create builds a {tx_size}-byte transaction, \ + over the {PACKET_DATA_SIZE}-byte limit ({dz_prefix_count} dz_prefix account(s){} \ + ride along); at most {} group(s) fit; subscribe the rest via \ + UpdateMulticastGroupRolesCommand after activation", + mgroup_pks.len(), + if self.feed_pk.is_some() { + " and a feed" + } else { + "" + }, + mgroup_pks.len().saturating_sub(over.div_ceil(33)), + ); + } + client.send_transaction(ix).map(|sig| (sig, pda_pubkey)) } } @@ -202,6 +272,7 @@ mod tests { &mgroup_pk, &accesspass_pubkey, 1, + &[], None, UserCreateSubscribeArgs { user_type: UserType::IBRLWithAllocatedIP, @@ -213,6 +284,7 @@ mod tests { dz_prefix_count: 1, owner: Pubkey::default(), ip_proof: None, + extra_group_count: 0, }, ); client @@ -225,7 +297,7 @@ mod tests { device_pk, cyoa_type: UserCYOA::GREOverDIA, client_ip, - mgroup_pk, + mgroup_pks: vec![mgroup_pk], publisher: true, subscriber: false, tunnel_endpoint: Ipv4Addr::UNSPECIFIED, @@ -236,4 +308,145 @@ mod tests { assert!(res.is_ok()); } + + /// Mock the lookups a create with `mgroup_pks` needs: every group Activated, the + /// access pass at the exact-IP PDA, and a device advertising `dz_prefixes`. + fn expect_create_lookups( + client: &mut crate::MockDoubleZeroClient, + mgroup_pks: &[Pubkey], + device_pk: Pubkey, + client_ip: Ipv4Addr, + dz_prefixes: &str, + ) { + let program_id = client.get_program_id(); + let payer = client.get_payer(); + + for mgroup_pk in mgroup_pks.iter().copied() { + let mgroup = MulticastGroup { + status: MulticastGroupStatus::Activated, + ..Default::default() + }; + client + .expect_get() + .with(predicate::eq(mgroup_pk)) + .returning(move |_| Ok(AccountData::MulticastGroup(mgroup.clone()))); + } + + let (accesspass_pubkey, _) = get_accesspass_pda(&program_id, &client_ip, &payer); + let accesspass = AccessPass { + account_type: AccountType::AccessPass, + bump_seed: 0, + accesspass_type: AccessPassType::Prepaid, + client_ip, + user_payer: payer, + last_access_epoch: 0, + connection_count: 0, + status: AccessPassStatus::Requested, + owner: payer, + mgroup_pub_allowlist: vec![], + mgroup_sub_allowlist: vec![], + tenant_allowlist: vec![], + flags: 0, + unicast_user_count: 0, + max_unicast_users: 1, + multicast_user_count: 0, + max_multicast_users: 1, + }; + client + .expect_get() + .with(predicate::eq(accesspass_pubkey)) + .returning(move |_| Ok(AccountData::AccessPass(accesspass.clone()))); + let (dynamic_accesspass_pubkey, _) = + get_accesspass_pda(&program_id, &Ipv4Addr::UNSPECIFIED, &payer); + client + .expect_get() + .with(predicate::eq(dynamic_accesspass_pubkey)) + .returning(|_| Err(eyre::eyre!("account not found"))); + + let device = Device { + account_type: AccountType::Device, + dz_prefixes: dz_prefixes.parse().unwrap(), + ..Default::default() + }; + client + .expect_get() + .with(predicate::eq(device_pk)) + .returning(move |_| Ok(AccountData::Device(device.clone()))); + } + + const FIVE_PREFIXES: &str = "10.0.0.0/24,10.0.1.0/24,10.0.2.0/24,10.0.3.0/24,10.0.4.0/24"; + + /// A batch a role update would accept (16 groups) overflows the create + /// transaction on a five-prefix device with a feed — blocked in the SDK with the + /// measured size, before anything is sent. + #[test] + fn test_commands_user_create_subscribe_rejects_a_batch_over_the_size_limit() { + let mut client = create_test_client(); + let device_pk = Pubkey::new_unique(); + let client_ip = Ipv4Addr::new(192, 168, 1, 10); + let mgroup_pks: Vec = (0..16).map(|_| Pubkey::new_unique()).collect(); + expect_create_lookups( + &mut client, + &mgroup_pks, + device_pk, + client_ip, + FIVE_PREFIXES, + ); + client.expect_send_transaction().times(0); + + let err = CreateSubscribeUserCommand { + user_type: UserType::Multicast, + device_pk, + cyoa_type: UserCYOA::GREOverDIA, + client_ip, + mgroup_pks, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + owner: None, + feed_pk: Some(Pubkey::new_unique()), + } + .execute(&client) + .unwrap_err(); + assert!( + err.to_string().contains("1232-byte limit"), + "unexpected error: {err}" + ); + } + + /// The daemon folds at most eight groups into a create (`MAX_CREATE_GROUPS`); + /// that batch fits even on a five-prefix device with a feed. + #[test] + fn test_commands_user_create_subscribe_max_daemon_fold_fits_the_size_limit() { + let mut client = create_test_client(); + let device_pk = Pubkey::new_unique(); + let client_ip = Ipv4Addr::new(192, 168, 1, 10); + let mgroup_pks: Vec = (0..8).map(|_| Pubkey::new_unique()).collect(); + expect_create_lookups( + &mut client, + &mgroup_pks, + device_pk, + client_ip, + FIVE_PREFIXES, + ); + client + .expect_send_transaction() + .times(1) + .returning(|_| Ok(Signature::new_unique())); + + let res = CreateSubscribeUserCommand { + user_type: UserType::Multicast, + device_pk, + cyoa_type: UserCYOA::GREOverDIA, + client_ip, + mgroup_pks, + publisher: false, + subscriber: true, + tunnel_endpoint: Ipv4Addr::UNSPECIFIED, + owner: None, + feed_pk: Some(Pubkey::new_unique()), + } + .execute(&client); + assert!(res.is_ok(), "{res:?}"); + } } diff --git a/smartcontract/sdk/rs/src/commands/user/delete.rs b/smartcontract/sdk/rs/src/commands/user/delete.rs index 553b6bf673..73199ceac8 100644 --- a/smartcontract/sdk/rs/src/commands/user/delete.rs +++ b/smartcontract/sdk/rs/src/commands/user/delete.rs @@ -5,7 +5,8 @@ use crate::{ accesspass::get::GetAccessPassCommand, device::get::GetDeviceCommand, multicastgroup::{ - list::ListMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand, + list::ListMulticastGroupCommand, + subscribe::{UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION}, }, }, DoubleZeroClient, @@ -42,19 +43,23 @@ impl DeleteUserCommand { .into_iter() .collect(); let multicastgroups = ListMulticastGroupCommand {}.execute(client)?; - for mgroup_pk in &unique_mgroup_pks { - if multicastgroups.contains_key(mgroup_pk) { - UpdateMulticastGroupRolesCommand { - group_pk: *mgroup_pk, - user_pk: self.pubkey, - client_ip: user.client_ip, - publisher: false, - subscriber: false, - device_pk: None, - feed_pk: None, - } - .execute(client)?; + // Strip every remaining multicast role, batched atomically per chunk (one + // transaction each, bounded by the transaction size limit). + let group_pks: Vec = unique_mgroup_pks + .into_iter() + .filter(|pk| multicastgroups.contains_key(pk)) + .collect(); + for chunk in group_pks.chunks(MAX_GROUPS_PER_TRANSACTION) { + UpdateMulticastGroupRolesCommand { + group_pks: chunk.to_vec(), + user_pk: self.pubkey, + client_ip: user.client_ip, + publisher: false, + subscriber: false, + device_pk: None, + feed_pk: None, } + .execute(client)?; } // GetAccessPassCommand prefers a shared dynamic (UNSPECIFIED) pass and falls @@ -280,11 +285,13 @@ mod tests { &mgroup_pubkey, &accesspass_pubkey, &user_pubkey, + &[], UpdateMulticastGroupRolesArgs { publisher: false, subscriber: false, client_ip, use_onchain_allocation: true, + extra_group_count: 0, }, ))) .times(1) @@ -483,11 +490,13 @@ mod tests { &mgroup_pubkey, &accesspass_pubkey, &user_pubkey, + &[], UpdateMulticastGroupRolesArgs { publisher: false, subscriber: false, client_ip, use_onchain_allocation: true, + extra_group_count: 0, }, ))) .times(1) @@ -726,11 +735,13 @@ mod tests { &mgroup_pubkey, &accesspass_pubkey, &user_pubkey, + &[], UpdateMulticastGroupRolesArgs { publisher: false, subscriber: false, client_ip, use_onchain_allocation: true, + extra_group_count: 0, }, ))) .times(1) diff --git a/smartcontract/sdk/rs/src/commands/user/requestban.rs b/smartcontract/sdk/rs/src/commands/user/requestban.rs index 75fb00c598..28c6fecf35 100644 --- a/smartcontract/sdk/rs/src/commands/user/requestban.rs +++ b/smartcontract/sdk/rs/src/commands/user/requestban.rs @@ -4,7 +4,8 @@ use crate::{ commands::{ device::get::GetDeviceCommand, multicastgroup::{ - list::ListMulticastGroupCommand, subscribe::UpdateMulticastGroupRolesCommand, + list::ListMulticastGroupCommand, + subscribe::{UpdateMulticastGroupRolesCommand, MAX_GROUPS_PER_TRANSACTION}, }, }, DoubleZeroClient, @@ -35,19 +36,23 @@ impl RequestBanUserCommand { .into_iter() .collect(); let multicastgroups = ListMulticastGroupCommand {}.execute(client)?; - for mgroup_pk in &unique_mgroup_pks { - if multicastgroups.contains_key(mgroup_pk) { - UpdateMulticastGroupRolesCommand { - group_pk: *mgroup_pk, - user_pk: self.pubkey, - client_ip: user.client_ip, - publisher: false, - subscriber: false, - device_pk: None, - feed_pk: None, - } - .execute(client)?; + // Strip every remaining multicast role, batched atomically per chunk (one + // transaction each, bounded by the transaction size limit). + let group_pks: Vec = unique_mgroup_pks + .into_iter() + .filter(|pk| multicastgroups.contains_key(pk)) + .collect(); + for chunk in group_pks.chunks(MAX_GROUPS_PER_TRANSACTION) { + UpdateMulticastGroupRolesCommand { + group_pks: chunk.to_vec(), + user_pk: self.pubkey, + client_ip: user.client_ip, + publisher: false, + subscriber: false, + device_pk: None, + feed_pk: None, } + .execute(client)?; } let (_, device) = GetDeviceCommand {