From a55506a8ac9691dbc59a07f8fd719cfa74d09a8d Mon Sep 17 00:00:00 2001 From: Steve Shaw Date: Fri, 10 Apr 2026 23:50:14 +0000 Subject: [PATCH] serviceability: allow pending users to subscribe to multicast groups the SubscribeMulticastGroup instruction rejected Pending users, but CreateSubscribeUser only takes one mgroup account. this meant callers could create a user and subscribe to the first group, but subsequent SubscribeMulticastGroup calls for additional groups failed with InvalidStatus (0x7) because the user was still Pending. the inner subscribe_user_to_multicastgroup function already handles Pending users fine (CreateSubscribeUser proves this every time it runs). the status gate only existed on the standalone instruction, creating an inconsistency. relax the check to also allow Pending status. also fixes a flaky TestE2E_UserBGPStatus disconnect step where pkill -9 can tear down the container before the exec session completes. --- CHANGELOG.md | 3 ++ e2e/user_bgp_status_test.go | 7 ++- .../processors/multicastgroup/subscribe.rs | 8 +-- .../tests/create_subscribe_user_test.rs | 53 +++++++++++-------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8684d087c..9c248370b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ All notable changes to this project will be documented in this file. ### Changes +- Smartcontract + - Allow `SubscribeMulticastGroup` for users in `Pending` status so that `CreateSubscribeUser` can be followed by additional subscribe calls before the activator runs ([#3521](https://github.com/malbeclabs/doublezero/pull/3521)) + ## [v0.17.0](https://github.com/malbeclabs/doublezero/compare/client/v0.16.0...client/v0.17.0) - 2026-04-10 ### Breaking diff --git a/e2e/user_bgp_status_test.go b/e2e/user_bgp_status_test.go index d383281bf..18b378bb1 100644 --- a/e2e/user_bgp_status_test.go +++ b/e2e/user_bgp_status_test.go @@ -181,8 +181,11 @@ func TestE2E_UserBGPStatus(t *testing.T) { // the user account onchain, leaving no record to check BGP status on. // With an ungraceful kill the user stays activated onchain, giving the // BGP status submitter a chance to detect the dropped session and submit Down. - _, err := client.Exec(t.Context(), []string{"bash", "-c", "pkill -9 doublezerod || true"}) - require.NoError(t, err) + // + // Ignore the error: killing doublezerod (PID 1) can tear down the + // container, which terminates the exec session with exit 137 before + // the "|| true" runs. + client.Exec(t.Context(), []string{"bash", "-c", "pkill -9 doublezerod || true"}) //nolint:errcheck }) { t.FailNow() } diff --git a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs index 2fd17c7b5..77ae39301 100644 --- a/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs +++ b/smartcontract/programs/doublezero-serviceability/src/processors/multicastgroup/subscribe.rs @@ -201,13 +201,15 @@ pub fn process_subscribe_multicastgroup( // Parse and validate user let mut user: User = User::try_from(user_account)?; - // Allow pure-unsubscribe (both false) for any status so that users - // created atomically via CreateSubscribeUser can be cleaned up before - // activation. Subscribe operations still require Activated/Updating. + // Allow subscribe for Pending users so that CreateSubscribeUser (which + // only takes one mgroup) can be followed by additional SubscribeMulticastGroup + // calls before the activator runs. Also allow pure-unsubscribe (both false) + // for any status so cleanup works before activation. let is_unsubscribe_only = !value.publisher && !value.subscriber; if !is_unsubscribe_only && user.status != UserStatus::Activated && user.status != UserStatus::Updating + && user.status != UserStatus::Pending { msg!("UserStatus: {:?}", user.status); return Err(DoubleZeroError::InvalidStatus.into()); 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 c18cab5ec..17da27f48 100644 --- a/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs +++ b/smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs @@ -51,12 +51,7 @@ use doublezero_serviceability::{ }, }; use solana_program_test::*; -use solana_sdk::{ - instruction::{AccountMeta, InstructionError}, - pubkey::Pubkey, - signature::Signer, - transaction::TransactionError, -}; +use solana_sdk::{instruction::AccountMeta, pubkey::Pubkey, signature::Signer}; use std::net::Ipv4Addr; mod test_helpers; @@ -2635,9 +2630,12 @@ async fn test_unsubscribe_pending_user_created_via_create_subscribe() { assert_eq!(mgroup.publisher_count, 0); } -/// Subscribing (publisher: true) a Pending user must still be rejected. +/// Subscribing a Pending user must succeed so that CreateSubscribeUser (which +/// only takes one mgroup) can be followed by additional SubscribeMulticastGroup +/// calls before the activator runs. This mirrors the shred oracle flow where a +/// user is subscribed to multiple multicast groups at creation time. #[tokio::test] -async fn test_subscribe_pending_user_still_rejected() { +async fn test_subscribe_pending_user_succeeds() { let client_ip = [100, 0, 0, 98]; let f = setup_create_subscribe_fixture(client_ip).await; let CreateSubscribeFixture { @@ -2655,7 +2653,7 @@ async fn test_subscribe_pending_user_still_rejected() { let (user_pubkey, _) = get_user_pda(&program_id, &user_ip, UserType::Multicast); let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); - // Create user via legacy path — user is Pending. + // Create user via legacy path — user is Pending with publisher subscription. execute_transaction( &mut banks_client, recent_blockhash, @@ -2681,15 +2679,25 @@ async fn test_subscribe_pending_user_still_rejected() { ) .await; - // Attempting to subscribe (add) a Pending user should still fail. + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should exist") + .get_user() + .unwrap(); + assert_eq!(user.status, UserStatus::Pending); + assert_eq!(user.publishers, vec![mgroup_pubkey]); + + // Subscribe the Pending user as subscriber to the same group. + // Note: publisher must remain true to keep the existing subscription + // (false means "unsubscribe from publisher"). let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap(); - let result = try_execute_transaction( + try_execute_transaction( &mut banks_client, recent_blockhash, program_id, DoubleZeroInstruction::SubscribeMulticastGroup(MulticastGroupSubscribeArgs { client_ip: user_ip, - publisher: false, + publisher: true, subscriber: true, use_onchain_allocation: false, }), @@ -2700,16 +2708,19 @@ async fn test_subscribe_pending_user_still_rejected() { ], &payer, ) - .await; + .await + .expect("Subscribe should succeed for Pending user"); - assert!( - result.is_err(), - "Subscribe should still be rejected for Pending user" - ); - let err = result.unwrap_err(); + let user = get_account_data(&mut banks_client, user_pubkey) + .await + .expect("User should exist") + .get_user() + .unwrap(); assert_eq!( - err.unwrap(), - TransactionError::InstructionError(0, InstructionError::Custom(7)), - "Should return InvalidStatus (0x7)" + user.status, + UserStatus::Pending, + "User should remain Pending" ); + assert_eq!(user.publishers, vec![mgroup_pubkey]); + assert_eq!(user.subscribers, vec![mgroup_pubkey]); }