fix(operator): soft-revoke wallet access instead of deleting the row
This commit is contained in:
@@ -109,6 +109,7 @@ create table if not exists evm_wallet_access (
|
|||||||
id integer not null primary key,
|
id integer not null primary key,
|
||||||
wallet_id integer not null references evm_wallet (id) on delete cascade,
|
wallet_id integer not null references evm_wallet (id) on delete cascade,
|
||||||
client_id integer not null references program_client (id) on delete cascade,
|
client_id integer not null references program_client (id) on delete cascade,
|
||||||
|
revoked_at integer, -- unix timestamp when revoked, null = still active
|
||||||
created_at integer not null default(unixepoch ('now'))
|
created_at integer not null default(unixepoch ('now'))
|
||||||
) STRICT;
|
) STRICT;
|
||||||
|
|
||||||
|
|||||||
@@ -226,6 +226,7 @@ impl EvmActor {
|
|||||||
.select(models::EvmWalletAccess::as_select())
|
.select(models::EvmWalletAccess::as_select())
|
||||||
.filter(schema::evm_wallet_access::wallet_id.eq(wallet.id))
|
.filter(schema::evm_wallet_access::wallet_id.eq(wallet.id))
|
||||||
.filter(schema::evm_wallet_access::client_id.eq(client_id))
|
.filter(schema::evm_wallet_access::client_id.eq(client_id))
|
||||||
|
.filter(schema::evm_wallet_access::revoked_at.is_null())
|
||||||
.first(&mut conn)
|
.first(&mut conn)
|
||||||
.await
|
.await
|
||||||
.optional()
|
.optional()
|
||||||
@@ -261,6 +262,7 @@ impl EvmActor {
|
|||||||
.select(models::EvmWalletAccess::as_select())
|
.select(models::EvmWalletAccess::as_select())
|
||||||
.filter(schema::evm_wallet_access::wallet_id.eq(wallet.id))
|
.filter(schema::evm_wallet_access::wallet_id.eq(wallet.id))
|
||||||
.filter(schema::evm_wallet_access::client_id.eq(client_id))
|
.filter(schema::evm_wallet_access::client_id.eq(client_id))
|
||||||
|
.filter(schema::evm_wallet_access::revoked_at.is_null())
|
||||||
.first(&mut conn)
|
.first(&mut conn)
|
||||||
.await
|
.await
|
||||||
.optional()
|
.optional()
|
||||||
@@ -323,11 +325,19 @@ impl EvmActor {
|
|||||||
) -> Result<(), Error> {
|
) -> Result<(), Error> {
|
||||||
let mut conn = self.db.get().await.map_err(DatabaseError::from)?;
|
let mut conn = self.db.get().await.map_err(DatabaseError::from)?;
|
||||||
|
|
||||||
|
// Revives a previously revoked row instead of conflicting on it forever:
|
||||||
|
// `uniq_wallet_access` is a unique index on `(wallet_id, client_id)`.
|
||||||
insert_into(schema::evm_wallet_access::table)
|
insert_into(schema::evm_wallet_access::table)
|
||||||
.values((
|
.values((
|
||||||
schema::evm_wallet_access::wallet_id.eq(EvmWalletId::from_raw(settings.wallet_id)),
|
schema::evm_wallet_access::wallet_id.eq(EvmWalletId::from_raw(settings.wallet_id)),
|
||||||
schema::evm_wallet_access::client_id.eq(settings.client_id),
|
schema::evm_wallet_access::client_id.eq(settings.client_id),
|
||||||
))
|
))
|
||||||
|
.on_conflict((
|
||||||
|
schema::evm_wallet_access::wallet_id,
|
||||||
|
schema::evm_wallet_access::client_id,
|
||||||
|
))
|
||||||
|
.do_update()
|
||||||
|
.set(schema::evm_wallet_access::revoked_at.eq(None::<models::SqliteTimestamp>))
|
||||||
.execute(&mut conn)
|
.execute(&mut conn)
|
||||||
.await
|
.await
|
||||||
.map_err(DatabaseError::from)?;
|
.map_err(DatabaseError::from)?;
|
||||||
|
|||||||
@@ -164,7 +164,11 @@ pub async fn create_test_pool() -> DatabasePool {
|
|||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use diesel::{ExpressionMethods as _, dsl::insert_into};
|
use diesel::{
|
||||||
|
ExpressionMethods as _,
|
||||||
|
dsl::insert_into,
|
||||||
|
result::{DatabaseErrorKind, Error as DieselError},
|
||||||
|
};
|
||||||
use diesel_async::RunQueryDsl;
|
use diesel_async::RunQueryDsl;
|
||||||
|
|
||||||
/// `operator.id` references `operator_identity(id)`. Without `PRAGMA foreign_keys = ON`
|
/// `operator.id` references `operator_identity(id)`. Without `PRAGMA foreign_keys = ON`
|
||||||
@@ -184,9 +188,18 @@ mod tests {
|
|||||||
.execute(&mut conn)
|
.execute(&mut conn)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
|
// Specifically a foreign-key violation, not any error: a `NOT NULL` failure or a
|
||||||
|
// renamed column would also make `result.is_err()` true without proving the pragma
|
||||||
|
// is what rejected the insert.
|
||||||
assert!(
|
assert!(
|
||||||
result.is_err(),
|
matches!(
|
||||||
"insert with a dangling operator_identity reference was accepted"
|
result,
|
||||||
|
Err(DieselError::DatabaseError(
|
||||||
|
DatabaseErrorKind::ForeignKeyViolation,
|
||||||
|
_
|
||||||
|
))
|
||||||
|
),
|
||||||
|
"expected a foreign-key violation for a dangling operator_identity reference, got {result:?}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -263,19 +263,22 @@ pub struct EvmWallet {
|
|||||||
#[view(
|
#[view(
|
||||||
NewEvmWalletAccess,
|
NewEvmWalletAccess,
|
||||||
derive(Insertable),
|
derive(Insertable),
|
||||||
omit(id, created_at),
|
omit(id, created_at, revoked_at),
|
||||||
attributes_with = "deriveless"
|
attributes_with = "deriveless"
|
||||||
)]
|
)]
|
||||||
#[view(
|
#[view(
|
||||||
CoreEvmWalletAccess,
|
CoreEvmWalletAccess,
|
||||||
derive(Insertable),
|
derive(Insertable),
|
||||||
omit(created_at),
|
omit(created_at, revoked_at),
|
||||||
attributes_with = "deriveless"
|
attributes_with = "deriveless"
|
||||||
)]
|
)]
|
||||||
pub struct EvmWalletAccess {
|
pub struct EvmWalletAccess {
|
||||||
pub id: i32,
|
pub id: i32,
|
||||||
pub wallet_id: EvmWalletId,
|
pub wallet_id: EvmWalletId,
|
||||||
pub client_id: i32,
|
pub client_id: i32,
|
||||||
|
// Grants, transaction logs, and persistent-grant proposals reference this row
|
||||||
|
// `on delete restrict`, so revocation cannot delete it -- it marks it revoked instead.
|
||||||
|
pub revoked_at: Option<SqliteTimestamp>,
|
||||||
pub created_at: SqliteTimestamp,
|
pub created_at: SqliteTimestamp,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -135,6 +135,7 @@ diesel::table! {
|
|||||||
id -> Integer,
|
id -> Integer,
|
||||||
wallet_id -> Integer,
|
wallet_id -> Integer,
|
||||||
client_id -> Integer,
|
client_id -> Integer,
|
||||||
|
revoked_at -> Nullable<Integer>,
|
||||||
created_at -> Integer,
|
created_at -> Integer,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -381,6 +381,7 @@ mod tests {
|
|||||||
id: WALLET_ACCESS_ID,
|
id: WALLET_ACCESS_ID,
|
||||||
wallet_id: EvmWalletId::from_raw(5),
|
wallet_id: EvmWalletId::from_raw(5),
|
||||||
client_id: 20,
|
client_id: 20,
|
||||||
|
revoked_at: None,
|
||||||
created_at: SqliteTimestamp(Utc::now()),
|
created_at: SqliteTimestamp(Utc::now()),
|
||||||
},
|
},
|
||||||
chain: CHAIN_ID,
|
chain: CHAIN_ID,
|
||||||
@@ -478,6 +479,9 @@ mod tests {
|
|||||||
conn: &mut DatabaseConnection,
|
conn: &mut DatabaseConnection,
|
||||||
shared: &SharedGrantSettings,
|
shared: &SharedGrantSettings,
|
||||||
) -> EvmBasicGrant {
|
) -> EvmBasicGrant {
|
||||||
|
// The seeded id deliberately wins over `shared.wallet_access_id`: every other field
|
||||||
|
// below is read from `shared`, but a caller-supplied access id would almost never
|
||||||
|
// reference a row that actually exists under foreign-key enforcement.
|
||||||
let wallet_access_id = seed_wallet_access(conn).await;
|
let wallet_access_id = seed_wallet_access(conn).await;
|
||||||
|
|
||||||
#[expect(
|
#[expect(
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ fn ctx(to: Address, value: U256) -> EvalContext {
|
|||||||
id: WALLET_ACCESS_ID,
|
id: WALLET_ACCESS_ID,
|
||||||
wallet_id: EvmWalletId::from_raw(10),
|
wallet_id: EvmWalletId::from_raw(10),
|
||||||
client_id: 20,
|
client_id: 20,
|
||||||
|
revoked_at: None,
|
||||||
created_at: SqliteTimestamp(Utc::now()),
|
created_at: SqliteTimestamp(Utc::now()),
|
||||||
},
|
},
|
||||||
chain: CHAIN_ID,
|
chain: CHAIN_ID,
|
||||||
|
|||||||
@@ -48,6 +48,7 @@ fn ctx(to: Address, calldata: Bytes) -> EvalContext {
|
|||||||
id: WALLET_ACCESS_ID,
|
id: WALLET_ACCESS_ID,
|
||||||
wallet_id: EvmWalletId::from_raw(10),
|
wallet_id: EvmWalletId::from_raw(10),
|
||||||
client_id: 20,
|
client_id: 20,
|
||||||
|
revoked_at: None,
|
||||||
created_at: SqliteTimestamp(Utc::now()),
|
created_at: SqliteTimestamp(Utc::now()),
|
||||||
},
|
},
|
||||||
chain: CHAIN_ID,
|
chain: CHAIN_ID,
|
||||||
|
|||||||
@@ -177,20 +177,7 @@ impl OperatorSession {
|
|||||||
entries: Vec<NewEvmWalletAccess>,
|
entries: Vec<NewEvmWalletAccess>,
|
||||||
) -> Result<(), Error> {
|
) -> Result<(), Error> {
|
||||||
let mut conn = self.props.db.get().await?;
|
let mut conn = self.props.db.get().await?;
|
||||||
conn.transaction(async |conn| {
|
grant_wallet_access(&mut conn, entries).await?;
|
||||||
use crate::db::schema::evm_wallet_access;
|
|
||||||
|
|
||||||
for entry in entries {
|
|
||||||
diesel::insert_into(evm_wallet_access::table)
|
|
||||||
.values(&entry)
|
|
||||||
.on_conflict_do_nothing()
|
|
||||||
.execute(&mut *conn)
|
|
||||||
.await?;
|
|
||||||
}
|
|
||||||
|
|
||||||
Result::<_, Error>::Ok(())
|
|
||||||
})
|
|
||||||
.await?;
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -211,6 +198,7 @@ impl OperatorSession {
|
|||||||
use crate::db::schema::evm_wallet_access;
|
use crate::db::schema::evm_wallet_access;
|
||||||
let mut conn = self.props.db.get().await?;
|
let mut conn = self.props.db.get().await?;
|
||||||
let access_entries = evm_wallet_access::table
|
let access_entries = evm_wallet_access::table
|
||||||
|
.filter(evm_wallet_access::revoked_at.is_null())
|
||||||
.select(EvmWalletAccess::as_select())
|
.select(EvmWalletAccess::as_select())
|
||||||
.load::<_>(&mut conn)
|
.load::<_>(&mut conn)
|
||||||
.await?;
|
.await?;
|
||||||
@@ -218,16 +206,47 @@ impl OperatorSession {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Deletes access rows by their own id. The wire carries `WalletAccessEntry.id` values, so
|
/// Grants access, reviving a previously revoked row rather than leaving it shadowed:
|
||||||
/// filtering by `wallet_id` here would revoke every client's access to that wallet.
|
/// `uniq_wallet_access` is a unique index on `(wallet_id, client_id)`, so a plain insert
|
||||||
|
/// would conflict forever on a row that was revoked but never deleted.
|
||||||
|
pub(crate) async fn grant_wallet_access(
|
||||||
|
conn: &mut crate::db::DatabaseConnection,
|
||||||
|
entries: Vec<NewEvmWalletAccess>,
|
||||||
|
) -> Result<(), diesel::result::Error> {
|
||||||
|
use crate::db::{models::SqliteTimestamp, schema::evm_wallet_access};
|
||||||
|
|
||||||
|
conn.transaction(async |conn| {
|
||||||
|
for entry in entries {
|
||||||
|
diesel::insert_into(evm_wallet_access::table)
|
||||||
|
.values(&entry)
|
||||||
|
.on_conflict((evm_wallet_access::wallet_id, evm_wallet_access::client_id))
|
||||||
|
.do_update()
|
||||||
|
.set(evm_wallet_access::revoked_at.eq(None::<SqliteTimestamp>))
|
||||||
|
.execute(&mut *conn)
|
||||||
|
.await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Marks access rows revoked by their own id rather than deleting them. The wire carries
|
||||||
|
/// `WalletAccessEntry.id` values, so filtering by `wallet_id` here would revoke every
|
||||||
|
/// client's access to that wallet. Deleting is not an option: `evm_basic_grant`,
|
||||||
|
/// `evm_transaction_log`, and `proposal_persistent_grant` all reference this row
|
||||||
|
/// `on delete restrict`, so an access that was ever granted, signed with, or proposed
|
||||||
|
/// against can never be deleted -- only marked revoked.
|
||||||
pub(crate) async fn revoke_wallet_access(
|
pub(crate) async fn revoke_wallet_access(
|
||||||
conn: &mut crate::db::DatabaseConnection,
|
conn: &mut crate::db::DatabaseConnection,
|
||||||
ids: &[i32],
|
ids: &[i32],
|
||||||
) -> Result<usize, diesel::result::Error> {
|
) -> Result<usize, diesel::result::Error> {
|
||||||
use crate::db::schema::evm_wallet_access;
|
use crate::db::{models::SqliteTimestamp, schema::evm_wallet_access};
|
||||||
|
|
||||||
diesel::delete(evm_wallet_access::table)
|
diesel::update(evm_wallet_access::table)
|
||||||
.filter(evm_wallet_access::id.eq_any(ids))
|
.filter(evm_wallet_access::id.eq_any(ids))
|
||||||
|
.filter(evm_wallet_access::revoked_at.is_null())
|
||||||
|
.set(evm_wallet_access::revoked_at.eq(SqliteTimestamp::now()))
|
||||||
.execute(conn)
|
.execute(conn)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
@@ -385,18 +404,15 @@ impl OperatorSession {
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::revoke_wallet_access;
|
use super::{grant_wallet_access, revoke_wallet_access};
|
||||||
use crate::db::{self, models, schema};
|
use crate::db::{self, models, schema};
|
||||||
|
|
||||||
use diesel::{ExpressionMethods as _, QueryDsl as _, dsl::insert_into};
|
use diesel::{ExpressionMethods as _, QueryDsl as _, dsl::insert_into};
|
||||||
use diesel_async::RunQueryDsl;
|
use diesel_async::RunQueryDsl;
|
||||||
|
|
||||||
/// Two clients share one wallet. Revoking one access row must leave the other alone.
|
/// Inserts a fresh root key, an aead-encrypted wallet secret, and the wallet itself.
|
||||||
#[tokio::test]
|
/// Returns the wallet's id and its 20-byte address.
|
||||||
async fn revoking_one_access_leaves_the_other_client_alone() {
|
async fn seed_wallet(conn: &mut db::DatabaseConnection) -> (models::EvmWalletId, Vec<u8>) {
|
||||||
let pool = db::create_test_pool().await;
|
|
||||||
let mut conn = pool.get().await.unwrap();
|
|
||||||
|
|
||||||
let root_key_id: models::RootKeyHistoryId = insert_into(schema::root_key_history::table)
|
let root_key_id: models::RootKeyHistoryId = insert_into(schema::root_key_history::table)
|
||||||
.values(&models::NewRootKeyHistory {
|
.values(&models::NewRootKeyHistory {
|
||||||
ciphertext: vec![0u8; 32],
|
ciphertext: vec![0u8; 32],
|
||||||
@@ -407,7 +423,7 @@ mod tests {
|
|||||||
salt: vec![0u8; 16],
|
salt: vec![0u8; 16],
|
||||||
})
|
})
|
||||||
.returning(schema::root_key_history::id)
|
.returning(schema::root_key_history::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
@@ -421,81 +437,323 @@ mod tests {
|
|||||||
created_at: chrono::Utc::now().into(),
|
created_at: chrono::Utc::now().into(),
|
||||||
})
|
})
|
||||||
.returning(schema::aead_encrypted::id)
|
.returning(schema::aead_encrypted::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
|
let address = rand::random::<[u8; 20]>().to_vec();
|
||||||
let wallet_id: models::EvmWalletId = insert_into(schema::evm_wallet::table)
|
let wallet_id: models::EvmWalletId = insert_into(schema::evm_wallet::table)
|
||||||
.values((
|
.values((
|
||||||
schema::evm_wallet::address.eq(vec![0u8; 20]),
|
schema::evm_wallet::address.eq(address.clone()),
|
||||||
schema::evm_wallet::aead_encrypted_id.eq(aead_id),
|
schema::evm_wallet::aead_encrypted_id.eq(aead_id),
|
||||||
))
|
))
|
||||||
.returning(schema::evm_wallet::id)
|
.returning(schema::evm_wallet::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
let metadata_id: i32 = insert_into(schema::client_metadata::table)
|
(wallet_id, address)
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn seed_client_metadata(conn: &mut db::DatabaseConnection) -> i32 {
|
||||||
|
insert_into(schema::client_metadata::table)
|
||||||
.values(schema::client_metadata::name.eq("test"))
|
.values(schema::client_metadata::name.eq("test"))
|
||||||
.returning(schema::client_metadata::id)
|
.returning(schema::client_metadata::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
let first_client: i32 = insert_into(schema::program_client::table)
|
/// Inserts a `program_client` row under the given `client_metadata` row, keyed by its
|
||||||
|
/// own random public key.
|
||||||
|
async fn seed_client(conn: &mut db::DatabaseConnection, metadata_id: i32) -> i32 {
|
||||||
|
insert_into(schema::program_client::table)
|
||||||
.values((
|
.values((
|
||||||
schema::program_client::public_key.eq(vec![1u8; 32]),
|
schema::program_client::public_key.eq(rand::random::<[u8; 32]>().to_vec()),
|
||||||
schema::program_client::metadata_id.eq(metadata_id),
|
schema::program_client::metadata_id.eq(metadata_id),
|
||||||
))
|
))
|
||||||
.returning(schema::program_client::id)
|
.returning(schema::program_client::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
let second_client: i32 = insert_into(schema::program_client::table)
|
/// Inserts an access row directly, for fixtures that need one to already exist.
|
||||||
.values((
|
/// Production code grants access through [`grant_wallet_access`].
|
||||||
schema::program_client::public_key.eq(vec![2u8; 32]),
|
async fn insert_wallet_access(
|
||||||
schema::program_client::metadata_id.eq(metadata_id),
|
conn: &mut db::DatabaseConnection,
|
||||||
))
|
wallet_id: models::EvmWalletId,
|
||||||
.returning(schema::program_client::id)
|
client_id: i32,
|
||||||
.get_result(&mut conn)
|
) -> i32 {
|
||||||
.await
|
insert_into(schema::evm_wallet_access::table)
|
||||||
.unwrap();
|
|
||||||
|
|
||||||
let first_access: i32 = insert_into(schema::evm_wallet_access::table)
|
|
||||||
.values((
|
.values((
|
||||||
schema::evm_wallet_access::wallet_id.eq(wallet_id),
|
schema::evm_wallet_access::wallet_id.eq(wallet_id),
|
||||||
schema::evm_wallet_access::client_id.eq(first_client),
|
schema::evm_wallet_access::client_id.eq(client_id),
|
||||||
))
|
))
|
||||||
.returning(schema::evm_wallet_access::id)
|
.returning(schema::evm_wallet_access::id)
|
||||||
.get_result(&mut conn)
|
.get_result(conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
let _second_access: i32 = insert_into(schema::evm_wallet_access::table)
|
/// Two clients share one wallet. Revoking one access row must leave the other alone,
|
||||||
.values((
|
/// and must mark the row revoked rather than deleting it.
|
||||||
schema::evm_wallet_access::wallet_id.eq(wallet_id),
|
#[tokio::test]
|
||||||
schema::evm_wallet_access::client_id.eq(second_client),
|
async fn revoking_one_access_leaves_the_other_client_alone() {
|
||||||
))
|
let pool = db::create_test_pool().await;
|
||||||
.returning(schema::evm_wallet_access::id)
|
let mut conn = pool.get().await.unwrap();
|
||||||
.get_result(&mut conn)
|
|
||||||
.await
|
let (wallet_id, _address) = seed_wallet(&mut conn).await;
|
||||||
.unwrap();
|
let metadata_id = seed_client_metadata(&mut conn).await;
|
||||||
|
let first_client = seed_client(&mut conn, metadata_id).await;
|
||||||
|
let second_client = seed_client(&mut conn, metadata_id).await;
|
||||||
|
|
||||||
|
let first_access = insert_wallet_access(&mut conn, wallet_id, first_client).await;
|
||||||
|
let _second_access = insert_wallet_access(&mut conn, wallet_id, second_client).await;
|
||||||
|
|
||||||
let removed = revoke_wallet_access(&mut conn, &[first_access])
|
let removed = revoke_wallet_access(&mut conn, &[first_access])
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(removed, 1);
|
assert_eq!(removed, 1);
|
||||||
|
|
||||||
let survivors: Vec<i32> = schema::evm_wallet_access::table
|
let active: Vec<i32> = schema::evm_wallet_access::table
|
||||||
|
.filter(schema::evm_wallet_access::revoked_at.is_null())
|
||||||
.select(schema::evm_wallet_access::client_id)
|
.select(schema::evm_wallet_access::client_id)
|
||||||
.load(&mut conn)
|
.load(&mut conn)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
survivors,
|
active,
|
||||||
vec![second_client],
|
vec![second_client],
|
||||||
"revoking one access row removed another client's access"
|
"revoking one access row removed another client's access"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
let total: i64 = schema::evm_wallet_access::table
|
||||||
|
.count()
|
||||||
|
.get_result(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
total, 2,
|
||||||
|
"revoking an access row must mark it revoked, not delete it"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The bug this round fixes: once an access has been used for a grant, a signed
|
||||||
|
/// transaction, or a proposed persistent grant, three tables reference
|
||||||
|
/// `evm_wallet_access` `on delete restrict`, so deleting the row is no longer possible
|
||||||
|
/// once foreign keys are enforced. Revoking must still succeed by marking it revoked.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn revoking_an_access_with_grant_log_and_proposal_succeeds() {
|
||||||
|
use crate::db::proposal::{Proposal as _, persistent_grant, persistent_grant::PersistentGrant};
|
||||||
|
|
||||||
|
let pool = db::create_test_pool().await;
|
||||||
|
let mut conn = pool.get().await.unwrap();
|
||||||
|
|
||||||
|
let (wallet_id, _address) = seed_wallet(&mut conn).await;
|
||||||
|
let metadata_id = seed_client_metadata(&mut conn).await;
|
||||||
|
let client_id = seed_client(&mut conn, metadata_id).await;
|
||||||
|
let access_id = insert_wallet_access(&mut conn, wallet_id, client_id).await;
|
||||||
|
|
||||||
|
// A grant against this access...
|
||||||
|
let grant_id: i32 = insert_into(schema::evm_basic_grant::table)
|
||||||
|
.values(models::NewEvmBasicGrant {
|
||||||
|
wallet_access_id: access_id,
|
||||||
|
chain_id: 1u64.into(),
|
||||||
|
valid_from: None,
|
||||||
|
valid_until: None,
|
||||||
|
max_gas_fee_per_gas: None,
|
||||||
|
max_priority_fee_per_gas: None,
|
||||||
|
rate_limit_count: None,
|
||||||
|
rate_limit_window_secs: None,
|
||||||
|
revoked_at: None,
|
||||||
|
})
|
||||||
|
.returning(schema::evm_basic_grant::id)
|
||||||
|
.get_result(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
// ...a signed transaction against that grant...
|
||||||
|
insert_into(schema::evm_transaction_log::table)
|
||||||
|
.values(models::NewEvmTransactionLog {
|
||||||
|
grant_id,
|
||||||
|
wallet_access_id: access_id,
|
||||||
|
chain_id: 1u64.into(),
|
||||||
|
eth_value: vec![0u8; 32],
|
||||||
|
signed_at: models::SqliteTimestamp(chrono::Utc::now()),
|
||||||
|
})
|
||||||
|
.execute(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
// ...and a persistent-grant proposal that named this access before it was voted on.
|
||||||
|
let operator_id: models::OperatorIdentityId =
|
||||||
|
insert_into(schema::operator_identity::table)
|
||||||
|
.values(schema::operator_identity::public_key.eq(rand::random::<[u8; 32]>().to_vec()))
|
||||||
|
.returning(schema::operator_identity::id)
|
||||||
|
.get_result(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let proposal_id: models::ProposalId = insert_into(schema::proposal::table)
|
||||||
|
.values(&models::NewProposal {
|
||||||
|
kind: db::proposal::ProposalKindTag::ApprovePersistentGrant,
|
||||||
|
initiator_id: operator_id,
|
||||||
|
expires_at: models::SqliteTimestamp(chrono::Utc::now() + chrono::Duration::days(1)),
|
||||||
|
})
|
||||||
|
.returning(schema::proposal::id)
|
||||||
|
.get_result(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
PersistentGrant::insert(
|
||||||
|
proposal_id,
|
||||||
|
&persistent_grant::Settings {
|
||||||
|
wallet_access_id: access_id,
|
||||||
|
chain_id: 1,
|
||||||
|
valid_from_secs: None,
|
||||||
|
valid_until_secs: None,
|
||||||
|
max_gas_fee_per_gas: None,
|
||||||
|
max_priority_fee_per_gas: None,
|
||||||
|
rate_limit: None,
|
||||||
|
specific: persistent_grant::Specific::EtherTransfer {
|
||||||
|
targets: vec![[0u8; 20]],
|
||||||
|
limit: persistent_grant::VolumeLimit {
|
||||||
|
max_volume: [0u8; 32],
|
||||||
|
window_secs: 3600,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
&mut conn,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
// Before this round's fix, this would fail with a foreign-key violation.
|
||||||
|
let removed = revoke_wallet_access(&mut conn, &[access_id]).await.unwrap();
|
||||||
|
assert_eq!(removed, 1);
|
||||||
|
|
||||||
|
let revoked_at: Option<models::SqliteTimestamp> = schema::evm_wallet_access::table
|
||||||
|
.find(access_id)
|
||||||
|
.select(schema::evm_wallet_access::revoked_at)
|
||||||
|
.first(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
revoked_at.is_some(),
|
||||||
|
"the row must be marked revoked, not deleted"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A revoked access must no longer resolve through the lookup `shared_analyze_transaction`
|
||||||
|
/// and `client_sign_transaction` share -- otherwise the SDK client keeps signing after
|
||||||
|
/// the operator believes it has been cut off.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn revoked_access_no_longer_authorizes_signing() {
|
||||||
|
use crate::actors::{
|
||||||
|
GlobalActors,
|
||||||
|
evm::{EvmActor, SignTransactionError},
|
||||||
|
vault::Vault,
|
||||||
|
};
|
||||||
|
use alloy::{
|
||||||
|
consensus::TxEip1559,
|
||||||
|
eips::eip2930::AccessList,
|
||||||
|
primitives::{Address, Bytes, TxKind, U256},
|
||||||
|
};
|
||||||
|
use kameo::actor::Spawn as _;
|
||||||
|
|
||||||
|
let pool = db::create_test_pool().await;
|
||||||
|
let mut conn = pool.get().await.unwrap();
|
||||||
|
|
||||||
|
let (wallet_id, address) = seed_wallet(&mut conn).await;
|
||||||
|
let metadata_id = seed_client_metadata(&mut conn).await;
|
||||||
|
let client_id = seed_client(&mut conn, metadata_id).await;
|
||||||
|
let access_id = insert_wallet_access(&mut conn, wallet_id, client_id).await;
|
||||||
|
|
||||||
|
revoke_wallet_access(&mut conn, &[access_id]).await.unwrap();
|
||||||
|
drop(conn);
|
||||||
|
|
||||||
|
let vault = Vault::spawn(
|
||||||
|
Vault::new(pool.clone(), GlobalActors::spawn_message_bus())
|
||||||
|
.await
|
||||||
|
.unwrap(),
|
||||||
|
);
|
||||||
|
let mut evm_actor = EvmActor::new(vault, pool.clone());
|
||||||
|
|
||||||
|
let transaction = TxEip1559 {
|
||||||
|
chain_id: 1,
|
||||||
|
nonce: 0,
|
||||||
|
gas_limit: 21_000,
|
||||||
|
max_fee_per_gas: 0,
|
||||||
|
max_priority_fee_per_gas: 0,
|
||||||
|
to: TxKind::Call(Address::ZERO),
|
||||||
|
value: U256::ZERO,
|
||||||
|
input: Bytes::new(),
|
||||||
|
access_list: AccessList::default(),
|
||||||
|
};
|
||||||
|
let wallet_address = Address::from_slice(&address);
|
||||||
|
|
||||||
|
// Both lookups resolve access the same way; both must reject the revoked row before
|
||||||
|
// ever touching the vault (neither call bootstraps one).
|
||||||
|
let analyze_result = evm_actor
|
||||||
|
.shared_analyze_transaction(client_id, wallet_address, transaction.clone())
|
||||||
|
.await;
|
||||||
|
assert!(
|
||||||
|
matches!(analyze_result, Err(SignTransactionError::WalletNotFound)),
|
||||||
|
"a revoked access must not authorize shared_analyze_transaction: {analyze_result:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let sign_result = evm_actor
|
||||||
|
.client_sign_transaction(client_id, wallet_address, transaction)
|
||||||
|
.await;
|
||||||
|
assert!(
|
||||||
|
matches!(sign_result, Err(SignTransactionError::WalletNotFound)),
|
||||||
|
"a revoked access must not authorize client_sign_transaction: {sign_result:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Re-granting a revoked access must restore it rather than silently doing nothing:
|
||||||
|
/// `uniq_wallet_access` is a unique index on `(wallet_id, client_id)`, so a plain insert
|
||||||
|
/// would conflict on the revoked row forever.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn regranting_a_revoked_access_restores_it() {
|
||||||
|
let pool = db::create_test_pool().await;
|
||||||
|
let mut conn = pool.get().await.unwrap();
|
||||||
|
|
||||||
|
let (wallet_id, _address) = seed_wallet(&mut conn).await;
|
||||||
|
let metadata_id = seed_client_metadata(&mut conn).await;
|
||||||
|
let client_id = seed_client(&mut conn, metadata_id).await;
|
||||||
|
let access_id = insert_wallet_access(&mut conn, wallet_id, client_id).await;
|
||||||
|
|
||||||
|
revoke_wallet_access(&mut conn, &[access_id]).await.unwrap();
|
||||||
|
|
||||||
|
grant_wallet_access(
|
||||||
|
&mut conn,
|
||||||
|
vec![models::NewEvmWalletAccess {
|
||||||
|
wallet_id,
|
||||||
|
client_id,
|
||||||
|
}],
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let revoked_at: Option<models::SqliteTimestamp> = schema::evm_wallet_access::table
|
||||||
|
.find(access_id)
|
||||||
|
.select(schema::evm_wallet_access::revoked_at)
|
||||||
|
.first(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
revoked_at.is_none(),
|
||||||
|
"re-granting a revoked access must clear revoked_at"
|
||||||
|
);
|
||||||
|
|
||||||
|
let total: i64 = schema::evm_wallet_access::table
|
||||||
|
.count()
|
||||||
|
.get_result(&mut conn)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
total, 1,
|
||||||
|
"re-granting a revoked access must revive the existing row, not add a second one"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user