serviceability: tell an IP conflict apart from an already-subscribed user

CHANGELOG.md

@@ -6,6 +6,9 @@ All notable changes to this project will be documented in this file.
 
 ### Breaking
 
+- Serviceability
+  - User creation no longer answers two unrelated refusals with the same error. `CreateUser` and `CreateSubscribeUser` both returned `AccountAlreadyInitialized` for conditions that share neither cause nor remedy: a user already existing at the requested client IP with a **different** device, owner, type or tenant, versus a `CreateSubscribeUser` that matched an existing user **exactly**. The first is what two devices claiming one IP looks like — the User PDA is derived from `(client_ip, user_type)` with no device dimension, so the second device derives the first one's account — and the caller has to pick another IP. The second means the subscription is already in place and there is nothing to do. A client seeing one code could not tell which it had, so it could not say which. They are now `UserExistsWithDifferentAttributes` (`Custom(105)`) and `SubscribeUserAlreadyExists` (`Custom(106)`); both variants are appended to the end of `DoubleZeroError`, so no existing code shifts. Anything matching `AccountAlreadyInitialized` on either path needs updating. (malbeclabs/infra#2171, #4159)
+
 ### Changes
 
 - CLI

crates/sentinel/src/dz_ledger_writer.rs

@@ -91,7 +91,7 @@ pub fn build_create_multicast_publisher_instructions(
     // When dz_prefix_count > 0 we append the ResourceExtension accounts so the contract takes
     // the atomic create+allocate+activate path. Without those accounts the user is created
     // Pending and never reaches Activated, so the next poll cycle would re-attempt creation
-    // and trip AccountAlreadyInitialized.
+    // and trip SubscribeUserAlreadyExists.
     let (user_pda, _) = get_user_pda(program_id, &user.client_ip, SvcUserType::Multicast);
     let mut create_user_accounts = vec![
         AccountMeta::new(user_pda, false),

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

@@ -216,6 +216,12 @@ pub enum DoubleZeroError {
     UserFeedLimitExceeded, // variant 103
     #[error("An EdgeSeat feed seat is only held by a Multicast user")]
     EdgeSeatIsMulticastOnly, // variant 104
+    #[error(
+        "A user already exists at this client IP with a different device, owner, type or tenant"
+    )]
+    UserExistsWithDifferentAttributes, // variant 105
+    #[error("This user is already subscribed to the requested multicast group")]
+    SubscribeUserAlreadyExists, // variant 106
 }
 
 impl From<DoubleZeroError> for ProgramError {
@@ -326,6 +332,8 @@ impl From<DoubleZeroError> for ProgramError {
             DoubleZeroError::UserDeviceMismatch => ProgramError::Custom(102),
             DoubleZeroError::UserFeedLimitExceeded => ProgramError::Custom(103),
             DoubleZeroError::EdgeSeatIsMulticastOnly => ProgramError::Custom(104),
+            DoubleZeroError::UserExistsWithDifferentAttributes => ProgramError::Custom(105),
+            DoubleZeroError::SubscribeUserAlreadyExists => ProgramError::Custom(106),
         }
     }
 }
@@ -437,6 +445,8 @@ impl From<u32> for DoubleZeroError {
             102 => DoubleZeroError::UserDeviceMismatch,
             103 => DoubleZeroError::UserFeedLimitExceeded,
             104 => DoubleZeroError::EdgeSeatIsMulticastOnly,
+            105 => DoubleZeroError::UserExistsWithDifferentAttributes,
+            106 => DoubleZeroError::SubscribeUserAlreadyExists,
             _ => DoubleZeroError::Custom(e),
         }
     }

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

@@ -55,6 +55,14 @@ pub fn get_user_old_pda(program_id: &Pubkey, index: u128) -> (Pubkey, u8) {
     Pubkey::find_program_address(&[SEED_PREFIX, SEED_USER, &index.to_le_bytes()], program_id)
 }
 
+/// The seeds carry the client IP and user type but no device, so a `(ip, user_type)` pair
+/// addresses exactly one User account network-wide. Two devices asking for the same client IP
+/// therefore derive the same address, and the second one fails in `create_user_core` with
+/// `UserExistsWithDifferentAttributes` rather than getting its own account.
+///
+/// That IP uniqueness is a property of this derivation, not a check somewhere: adding a device
+/// dimension here would give each device its own account and silently drop the guarantee, and it
+/// would orphan every live User, so it is not a change to make casually.
 pub fn get_user_pda(program_id: &Pubkey, ip: &Ipv4Addr, user_type: UserType) -> (Pubkey, u8) {
     Pubkey::find_program_address(
         &[SEED_PREFIX, SEED_USER, &ip.octets(), &[user_type as u8]],

smartcontract/programs/doublezero-serviceability/src/processors/user/create_core.rs

@@ -58,8 +58,8 @@ pub struct CreateUserCoreResult {
 /// device validation, max users checks, epoch check) and sets up the initial User struct.
 ///
 /// Returns `Ok(None)` when the user already exists and matches the requested owner, device,
-/// user type, and tenant; a mismatch errors with `AccountAlreadyInitialized`, and a banned user
-/// with `InvalidStatus`.
+/// user type, and tenant; a mismatch errors with `UserExistsWithDifferentAttributes`, and a
+/// banned user with `InvalidStatus`.
 ///
 /// Callers are responsible for:
 /// - Parsing the required resource extension accounts
@@ -224,7 +224,12 @@ pub fn create_user_core(
                 user_type,
                 requested_tenant
             );
-            return Err(ProgramError::AccountAlreadyInitialized);
+            // The User PDA is derived from the client IP, so the common way to reach this is two
+            // devices claiming one IP: the second request derives the first device's account and
+            // mismatches on device_pk. Distinct from the exact-duplicate case in
+            // create_subscribe, which used to share this error code and left a client unable to
+            // tell "that IP belongs to another device" from "you are already subscribed".
+            return Err(DoubleZeroError::UserExistsWithDifferentAttributes.into());
         }
         // A ban is terminal; fail fast instead of leaving the caller polling a user that will
         // never activate.

smartcontract/programs/doublezero-serviceability/src/processors/user/create_subscribe.rs

@@ -12,7 +12,6 @@ use solana_program::{
     account_info::{next_account_info, AccountInfo},
     entrypoint::ProgramResult,
     msg,
-    program_error::ProgramError,
     pubkey::Pubkey,
 };
 use std::net::Ipv4Addr;
@@ -141,8 +140,11 @@ pub fn process_create_subscribe_user(
     )?
     else {
         // A duplicate is an error here, not a no-op: falling through would tick a second feed
-        // seat and push a duplicate feed_pks entry that delete would double-release.
-        return Err(ProgramError::AccountAlreadyInitialized);
+        // seat and push a duplicate feed_pks entry that delete would double-release. Named
+        // distinctly from the mismatch case in create_user_core, which shares neither cause nor
+        // remedy: this request matched an existing user exactly, so the subscription is already
+        // in place and the caller has nothing to change.
+        return Err(DoubleZeroError::SubscribeUserAlreadyExists.into());
     };
 
     // EdgeSeat multicast metro gate: the group must be joinable via a feed on the pass serving the

smartcontract/programs/doublezero-serviceability/tests/create_subscribe_user_test.rs

@@ -48,7 +48,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;
@@ -2543,3 +2548,114 @@ async fn test_publisher_disconnect_delete_decrements_publishers_count() {
         "subscribers_count must NOT change — user was created as publisher"
     );
 }
+
+// ============================================================================
+// Duplicate subscribe
+// ============================================================================
+
+/// Re-sending an identical CreateSubscribeUser is refused, and with its own error code.
+///
+/// For CreateUser an exact match is an idempotent no-op, but here falling through would tick a
+/// second feed seat and push a duplicate feed_pks entry that delete would double-release. Both
+/// this and the attribute-mismatch case in create_user_core used to return
+/// AccountAlreadyInitialized, which left a caller unable to tell "you are already subscribed"
+/// (nothing to do) from "that client IP belongs to another device" (pick another IP) — different
+/// causes with different remedies. This pins the codes apart.
+#[tokio::test]
+async fn test_create_subscribe_user_duplicate_is_refused_distinctly() {
+    let client_ip = [100, 0, 0, 9];
+    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 args = 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(),
+    };
+    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),
+    ];
+
+    let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap();
+    execute_transaction(
+        &mut banks_client,
+        recent_blockhash,
+        program_id,
+        DoubleZeroInstruction::CreateSubscribeUser(args.clone()),
+        accounts.clone(),
+        &payer,
+    )
+    .await;
+
+    // The retry flips `publisher`, for a reason worth stating: an instruction identical to the
+    // first would compile to identical transaction bytes against the same blockhash, so the
+    // runtime would dedupe it by signature and hand back the first one's success without ever
+    // reaching the program. `create_user_core` compares owner, device, user type and tenant —
+    // not the publisher/subscriber flags — so this still matches an existing user exactly and
+    // still lands in the duplicate branch, while being a distinct transaction.
+    let recent_blockhash = banks_client.get_latest_blockhash().await.unwrap();
+    let err = execute_transaction_expect_failure(
+        &mut banks_client,
+        recent_blockhash,
+        program_id,
+        DoubleZeroInstruction::CreateSubscribeUser(UserCreateSubscribeArgs {
+            publisher: true,
+            ..args
+        }),
+        accounts,
+        &payer,
+    )
+    .await
+    .expect_err("a duplicate subscribe must be refused");
+
+    match err {
+        BanksClientError::TransactionError(TransactionError::InstructionError(
+            0,
+            // SubscribeUserAlreadyExists, not the mismatch code.
+            InstructionError::Custom(code),
+        )) if code == custom_code(DoubleZeroError::SubscribeUserAlreadyExists) => {}
+        other => panic!("expected SubscribeUserAlreadyExists, got {other:?}"),
+    }
+
+    // The subscription is untouched: exactly one seat, not two.
+    let mgroup = get_account_data(&mut banks_client, mgroup_pubkey)
+        .await
+        .expect("MulticastGroup should exist")
+        .get_multicastgroup()
+        .unwrap();
+    assert_eq!(mgroup.subscriber_count, 1);
+    let user = get_account_data(&mut banks_client, user_pubkey)
+        .await
+        .expect("User should exist")
+        .get_user()
+        .unwrap();
+    assert_eq!(user.subscribers, vec![mgroup_pubkey]);
+}

smartcontract/programs/doublezero-serviceability/tests/feed_subscription_test.rs

@@ -1125,9 +1125,11 @@ async fn test_duplicate_create_subscribe_user_rejected() {
     match err {
         BanksClientError::TransactionError(TransactionError::InstructionError(
             0,
-            InstructionError::AccountAlreadyInitialized,
-        )) => {}
-        other => panic!("expected AccountAlreadyInitialized, got {other:?}"),
+            // SubscribeUserAlreadyExists: this request matched an existing user exactly, unlike
+            // UserExistsWithDifferentAttributes, which means the IP belongs to another device.
+            InstructionError::Custom(code),
+        )) if code == custom_code(DoubleZeroError::SubscribeUserAlreadyExists) => {}
+        other => panic!("expected SubscribeUserAlreadyExists, got {other:?}"),
     }
 
     assert_eq!(seat_users(&read_pass(&mut f).await, &feed), 1);

smartcontract/programs/doublezero-serviceability/tests/test_helpers.rs

@@ -1,4 +1,6 @@
 use borsh::to_vec;
+// Re-exported so the glob that pulls in `custom_code` also pulls in its argument type.
+pub use doublezero_serviceability::error::DoubleZeroError;
 use doublezero_serviceability::{
     entrypoint::process_instruction,
     instructions::*,
@@ -21,6 +23,7 @@ use doublezero_serviceability::{
 use solana_program_test::*;
 use solana_sdk::{
     instruction::{AccountMeta, Instruction},
+    program_error::ProgramError,
     pubkey::Pubkey,
     signature::{Keypair, Signer},
     transaction::Transaction,
@@ -62,6 +65,23 @@ pub fn test_payer() -> Keypair {
     Keypair::try_from(&TEST_PAYER_BYTES[..]).unwrap()
 }
 
+/// The `Custom` code a `DoubleZeroError` reaches a client as.
+///
+/// Derived through the conversion rather than written down, so a test cannot
+/// drift from `impl From<DoubleZeroError> for ProgramError`. Casting the variant
+/// would be wrong here: `DoubleZeroError` has no `#[repr(u32)]`, and its codes
+/// come from that impl's match rather than from declaration order. The two
+/// disagree — `InvalidExchangePubkey` is the third variant declared but maps to
+/// `Custom(3)` while `InvalidLocationPubkey`, declared fifth, maps to
+/// `Custom(2)` — so `as u32` would silently produce the wrong code.
+#[allow(dead_code)]
+pub fn custom_code(error: DoubleZeroError) -> u32 {
+    match ProgramError::from(error.clone()) {
+        ProgramError::Custom(code) => code,
+        other => panic!("{error:?} does not map to a custom error code: {other:?}"),
+    }
+}
+
 #[allow(dead_code)]
 pub async fn init_test() -> (BanksClient, Pubkey, Keypair, solana_program::hash::Hash) {
     let program_id = Pubkey::new_unique();

smartcontract/programs/doublezero-serviceability/tests/user_tests.rs

@@ -1672,9 +1672,11 @@ async fn test_user_create_existing_different_device_rejected() {
     match err {
         BanksClientError::TransactionError(TransactionError::InstructionError(
             0,
-            InstructionError::AccountAlreadyInitialized,
-        )) => {}
-        other => panic!("expected AccountAlreadyInitialized, got {other:?}"),
+            // Distinct from the exact-duplicate subscribe case. A pattern cannot
+            // hold a computed value, so the code is bound and compared.
+            InstructionError::Custom(code),
+        )) if code == custom_code(DoubleZeroError::UserExistsWithDifferentAttributes) => {}
+        other => panic!("expected UserExistsWithDifferentAttributes, got {other:?}"),
     }
 
     // The user still points at the original device.
@@ -1771,9 +1773,11 @@ async fn test_user_create_existing_different_tenant_rejected() {
     match err {
         BanksClientError::TransactionError(TransactionError::InstructionError(
             0,
-            InstructionError::AccountAlreadyInitialized,
-        )) => {}
-        other => panic!("expected AccountAlreadyInitialized, got {other:?}"),
+            // Distinct from the exact-duplicate subscribe case. A pattern cannot
+            // hold a computed value, so the code is bound and compared.
+            InstructionError::Custom(code),
+        )) if code == custom_code(DoubleZeroError::UserExistsWithDifferentAttributes) => {}
+        other => panic!("expected UserExistsWithDifferentAttributes, got {other:?}"),
     }
 
     // The user still has no tenant.
@@ -1873,9 +1877,11 @@ async fn test_user_create_existing_different_owner_rejected() {
     match err {
         BanksClientError::TransactionError(TransactionError::InstructionError(
             0,
-            InstructionError::AccountAlreadyInitialized,
-        )) => {}
-        other => panic!("expected AccountAlreadyInitialized, got {other:?}"),
+            // Distinct from the exact-duplicate subscribe case. A pattern cannot
+            // hold a computed value, so the code is bound and compared.
+            InstructionError::Custom(code),
+        )) if code == custom_code(DoubleZeroError::UserExistsWithDifferentAttributes) => {}
+        other => panic!("expected UserExistsWithDifferentAttributes, got {other:?}"),
     }
 
     // The user still belongs to the original owner.