From 77bf19f99ed7b049f0fb9d201be7cd2eaf9d780d Mon Sep 17 00:00:00 2001 From: CleverWild Date: Mon, 22 Jun 2026 14:55:32 +0200 Subject: [PATCH] fix(wallet): delete access entries by entry ID instead of wallet ID --- .../src/peers/operator/session/handlers.rs | 141 +++++++++++++++++- 1 file changed, 140 insertions(+), 1 deletion(-) diff --git a/server/crates/arbiter-server/src/peers/operator/session/handlers.rs b/server/crates/arbiter-server/src/peers/operator/session/handlers.rs index 034fff4..9565146 100644 --- a/server/crates/arbiter-server/src/peers/operator/session/handlers.rs +++ b/server/crates/arbiter-server/src/peers/operator/session/handlers.rs @@ -214,7 +214,7 @@ impl OperatorSession { use crate::db::schema::evm_wallet_access; for entry in entries { diesel::delete(evm_wallet_access::table) - .filter(evm_wallet_access::wallet_id.eq(entry)) + .filter(evm_wallet_access::id.eq(entry)) .execute(&mut *conn) .await?; } @@ -289,3 +289,142 @@ impl OperatorSession { Ok(clients) } } + +#[cfg(test)] +mod tests { + use crate::db::{self, models::NewEvmWalletAccess, schema::evm_wallet_access}; + use diesel::{ExpressionMethods as _, QueryDsl as _, SelectableHelper}; + use diesel_async::{AsyncConnection, RunQueryDsl}; + + /// Regression test: revocation must delete by access-entry `id`, not by `wallet_id`. + /// + /// Before the fix, revoking entry_id=1 would delete all rows where wallet_id=1, + /// wiping out every client's access to wallet #1. + #[tokio::test] + async fn revoke_deletes_by_entry_id_not_wallet_id() { + use crate::db::models::EvmWalletAccess; + + let pool = db::create_test_pool().await; + let mut conn = pool.get().await.expect("pool connection"); + + // Insert two access entries for the same wallet but different clients. + // entry A: id will be 1, wallet_id=1, client_id=10 + // entry B: id will be 2, wallet_id=1, client_id=20 + let entry_a = diesel::insert_into(evm_wallet_access::table) + .values(NewEvmWalletAccess { + wallet_id: 1, + client_id: 10, + }) + .returning(EvmWalletAccess::as_select()) + .get_result(&mut *conn) + .await + .expect("insert entry A"); + + let entry_b = diesel::insert_into(evm_wallet_access::table) + .values(NewEvmWalletAccess { + wallet_id: 1, + client_id: 20, + }) + .returning(EvmWalletAccess::as_select()) + .get_result(&mut *conn) + .await + .expect("insert entry B"); + + // Revoke only entry A by its primary key id. + conn.transaction(async |conn| { + diesel::delete(evm_wallet_access::table) + .filter(evm_wallet_access::id.eq(entry_a.id)) + .execute(&mut *conn) + .await + }) + .await + .expect("revoke entry A"); + + // Entry A must be gone. + let gone = evm_wallet_access::table + .filter(evm_wallet_access::id.eq(entry_a.id)) + .count() + .get_result::(&mut *conn) + .await + .expect("count entry A"); + assert_eq!(gone, 0, "revoked entry must be deleted"); + + // Entry B (same wallet, different client) must still exist. + let still_there = evm_wallet_access::table + .filter(evm_wallet_access::id.eq(entry_b.id)) + .count() + .get_result::(&mut *conn) + .await + .expect("count entry B"); + assert_eq!(still_there, 1, "unrelated entry must not be deleted"); + } + + /// Regression test: when entry_id and wallet_id differ, only the correct row is removed. + /// + /// This specifically catches the case where entry.id=5 and wallet_id=1 are different values; + /// the old bug would delete by wallet_id, potentially matching a completely different entry. + #[tokio::test] + async fn revoke_with_mismatched_wallet_and_entry_ids() { + use crate::db::models::EvmWalletAccess; + + let pool = db::create_test_pool().await; + let mut conn = pool.get().await.expect("pool connection"); + + // Insert entries to force auto-increment IDs to diverge from wallet_ids. + // We'll insert 5 placeholder entries first so that the real entry gets id=6. + for i in 1_i32..=5 { + diesel::insert_into(evm_wallet_access::table) + .values(NewEvmWalletAccess { + wallet_id: 99, + client_id: i, + }) + .execute(&mut *conn) + .await + .expect("insert placeholder"); + } + + // Real target: wallet_id=1, will get id=6. + let target = diesel::insert_into(evm_wallet_access::table) + .values(NewEvmWalletAccess { + wallet_id: 1, + client_id: 1, + }) + .returning(EvmWalletAccess::as_select()) + .get_result(&mut *conn) + .await + .expect("insert target"); + + // Sanity: target.id != target.wallet_id + assert_ne!( + target.id, target.wallet_id, + "test prerequisite: id and wallet_id must differ" + ); + + // Revoke by entry id. + conn.transaction(async |conn| { + diesel::delete(evm_wallet_access::table) + .filter(evm_wallet_access::id.eq(target.id)) + .execute(&mut *conn) + .await + }) + .await + .expect("revoke target"); + + let remaining = evm_wallet_access::table + .filter(evm_wallet_access::id.eq(target.id)) + .count() + .get_result::(&mut *conn) + .await + .expect("count target"); + assert_eq!(remaining, 0, "target must be deleted by its entry id"); + + // Placeholders for wallet_id=99 must be untouched. + let placeholders = evm_wallet_access::table + .filter(evm_wallet_access::wallet_id.eq(99)) + .count() + .get_result::(&mut *conn) + .await + .expect("count placeholders"); + assert_eq!(placeholders, 5, "unrelated entries must survive"); + } +}