-
Notifications
You must be signed in to change notification settings - Fork 8
feat: implement contract pausability (circuit breaker) #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3689703
7c73ace
6ffc8ca
c9513fb
8cb6616
d69571d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -697,6 +697,8 @@ fn test_reject_nonexistent_booking() { | |
| assert!(result.is_err()); | ||
| } | ||
|
|
||
| // ==================== Expert Rate Tests ==================== | ||
|
|
||
| #[test] | ||
| fn test_expert_can_set_and_update_rate() { | ||
| let env = Env::default(); | ||
|
|
@@ -749,7 +751,7 @@ fn test_book_session_calculates_correct_deposit() { | |
| let max_duration = 100_u64; | ||
| let expected_deposit = stored_rate * (max_duration as i128); // 1500 tokens | ||
|
|
||
| let booking_id = client.book_session(&user, &expert, &max_duration); | ||
| let _booking_id = client.book_session(&user, &expert, &max_duration); | ||
|
|
||
| // Verify correct deposit was extracted | ||
| assert_eq!(token.balance(&user), initial_balance - expected_deposit); | ||
|
|
@@ -781,3 +783,210 @@ fn test_book_session_fails_if_expert_rate_not_set() { | |
|
|
||
| assert!(res.is_err()); | ||
| } | ||
|
|
||
| // ==================== Pausability (Circuit Breaker) Tests ==================== | ||
|
|
||
| #[test] | ||
| fn test_pause_blocks_book_session() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Set expert rate before pausing | ||
| client.set_my_rate(&expert, &10_i128); | ||
|
|
||
| // Admin pauses the contract | ||
| let result = client.try_pause(); | ||
| assert!(result.is_ok()); | ||
|
|
||
| // User tries to book a session while paused (should fail) | ||
| let result = client.try_book_session(&user, &expert, &100); | ||
| assert!(result.is_err()); | ||
|
|
||
| // Verify user's balance is unchanged | ||
| assert_eq!(token.balance(&user), 10_000); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pause_blocks_finalize_session() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Book session while unpaused | ||
| let booking_id = { | ||
| client.set_my_rate(&expert, &10_i128); | ||
| client.book_session(&user, &expert, &100) | ||
| }; | ||
|
|
||
| // Admin pauses the contract | ||
| client.pause(); | ||
|
|
||
| // Oracle tries to finalize while paused (should fail) | ||
| let result = client.try_finalize_session(&booking_id, &50); | ||
| assert!(result.is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pause_blocks_reclaim_stale_session() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Book session while unpaused | ||
| let booking_id = { | ||
| client.set_my_rate(&expert, &10_i128); | ||
| client.book_session(&user, &expert, &100) | ||
| }; | ||
|
|
||
| // Advance time past reclaim timeout | ||
| env.ledger() | ||
| .set_timestamp(env.ledger().timestamp() + 90_000); | ||
|
|
||
| // Admin pauses the contract | ||
| client.pause(); | ||
|
|
||
| // User tries to reclaim while paused (should fail) | ||
| let result = client.try_reclaim_stale_session(&user, &booking_id); | ||
| assert!(result.is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_pause_blocks_reject_session() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Book session while unpaused | ||
| let booking_id = { | ||
| client.set_my_rate(&expert, &10_i128); | ||
| client.book_session(&user, &expert, &100) | ||
| }; | ||
|
|
||
| // Admin pauses the contract | ||
| client.pause(); | ||
|
|
||
| // Expert tries to reject while paused (should fail) | ||
| let result = client.try_reject_session(&expert, &booking_id); | ||
| assert!(result.is_err()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_unpause_resumes_operations() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Set expert rate | ||
| client.set_my_rate(&expert, &10_i128); | ||
|
|
||
| // Admin pauses the contract | ||
| client.pause(); | ||
|
|
||
| // Booking should fail while paused | ||
| let result = client.try_book_session(&user, &expert, &100); | ||
| assert!(result.is_err()); | ||
|
|
||
| // Admin unpauses the contract | ||
| let result = client.try_unpause(); | ||
| assert!(result.is_ok()); | ||
|
|
||
| // Booking should succeed after unpause | ||
| let booking_id = client.book_session(&user, &expert, &100); | ||
| assert_eq!(booking_id, 1); | ||
| assert_eq!(token.balance(&user), 9_000); | ||
| assert_eq!(token.balance(&client.address), 1_000); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_read_only_functions_work_while_paused() { | ||
| let env = Env::default(); | ||
| env.mock_all_auths(); | ||
|
|
||
| let admin = Address::generate(&env); | ||
| let user = Address::generate(&env); | ||
| let expert = Address::generate(&env); | ||
| let oracle = Address::generate(&env); | ||
|
|
||
| let token_admin = Address::generate(&env); | ||
| let token = create_token_contract(&env, &token_admin); | ||
| token.mint(&user, &10_000); | ||
|
|
||
| let client = create_client(&env); | ||
| client.init(&admin, &token.address, &oracle); | ||
|
|
||
| // Book a session before pausing | ||
| let booking_id = { | ||
| client.set_my_rate(&expert, &10_i128); | ||
| client.book_session(&user, &expert, &100) | ||
| }; | ||
|
|
||
| // Admin pauses the contract | ||
| client.pause(); | ||
|
|
||
| // Read-only functions should still work | ||
| let booking = client.get_booking(&booking_id); | ||
| assert!(booking.is_some()); | ||
| assert_eq!(booking.unwrap().id, booking_id); | ||
|
|
||
| let user_bookings = client.get_user_bookings(&user); | ||
| assert_eq!(user_bookings.len(), 1); | ||
|
|
||
| let expert_bookings = client.get_expert_bookings(&expert); | ||
| assert_eq!(expert_bookings.len(), 1); | ||
| } | ||
|
Comment on lines
+789
to
+992
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a test verifying that unauthorized callers cannot pause/unpause. All pausability tests use 💡 Suggested test skeleton#[test]
fn test_non_admin_cannot_pause() {
let env = Env::default();
env.mock_all_auths(); // set up contract
let admin = Address::generate(&env);
let non_admin = Address::generate(&env);
let token = Address::generate(&env);
let oracle = Address::generate(&env);
let client = create_client(&env);
client.init(&admin, &token, &oracle);
// Clear mocked auths; supply only non_admin's auth
env.set_auths(&[/* non_admin auth for pause */]);
let result = client.try_pause();
assert!(result.is_err());
}🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pause/unpauseshould guard against redundant state transitions.Calling
pause()when the contract is already paused silently succeeds and emits a spuriouspaused=trueevent;unpause()when already unpaused does the same. This creates misleading audit trails for indexers and monitoring tools and gives callers no signal that the call was a no-op.Consider returning an error (or at minimum skipping the event) when the requested state matches the current state:
🛡️ Proposed fix for idempotency guards
pub fn pause(env: &Env) -> Result<(), VaultError> { let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?; admin.require_auth(); + if storage::is_paused(env) { + return Err(VaultError::ContractPaused); // already paused + } storage::set_paused(env, true); events::contract_paused(env, true); Ok(()) } pub fn unpause(env: &Env) -> Result<(), VaultError> { let admin = storage::get_admin(env).ok_or(VaultError::NotInitialized)?; admin.require_auth(); + if !storage::is_paused(env) { + return Err(VaultError::NotInitialized); // reuse or add a dedicated NotPaused variant + } storage::set_paused(env, false); events::contract_paused(env, false); Ok(()) }Alternatively, add a dedicated
VaultError::NotPaused = 9variant for clarity instead of reusingNotInitialized.🤖 Prompt for AI Agents