Feat/key rotation

[KevinMB0220]

feat: payment vault key rotation (transfer_admin + set_oracle)

• Closes Payment Vault Key Rotation #34
• Added tests (if necessary)
• Ran cargo test (All tests passed)
• Evidence attached (Screenshots, Logs, or Transaction Hashes)
• Commented the code

---

📌 Type of Change

• 📚 Documentation (updates to README, docs, or comments)
• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ Enhancement (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to change)
• 🏗️ Refactor (code improvement/cleanup without logical changes)

---

📝 Changes Description

Adds key rotation to the payment-vault-contract so compromised keys can be replaced without redeploying. Without this, a leaked admin or oracle key permanently breaks the contract.

• transfer_admin(new_admin) — current admin signs, DataKey::Admin is overwritten; old key instantly loses all privileges (pause, set_oracle, transfer_admin)
• set_oracle(new_oracle) — admin signs, DataKey::Oracle is overwritten; old oracle can no longer call finalize_session
• Both operations emit on-chain events (admin_transferred, oracle_updated) so off-chain monitors can react to key changes

No breaking changes — existing storage layout and all prior entry points are unchanged.

---

📸 Evidence

running 34 tests
test test::test_initialization ... ok
test test::test_transfer_admin_success ... ok
test test::test_new_admin_can_pause_after_transfer ... ok
test test::test_old_admin_loses_privileges_after_transfer ... ok
test test::test_set_oracle_success ... ok
test test::test_non_admin_cannot_transfer_admin ... ok
test test::test_non_admin_cannot_set_oracle ... ok
test test::test_partial_duration_scenario ... ok
test test::test_full_duration_no_refund ... ok
test test::test_double_finalization_protection ... ok
test test::test_oracle_authorization_enforcement ... ok
test test::test_zero_duration_finalization ... ok
test test::test_booking_not_found ... ok
test test::test_book_session_balance_transfer ... ok
test test::test_get_user_and_expert_bookings ... ok
test test::test_reclaim_stale_session_too_early ... ok
test test::test_reclaim_stale_session_success ... ok
test test::test_reclaim_stale_session_wrong_user ... ok
test test::test_reclaim_already_finalized ... ok
test test::test_expert_rejects_pending_session ... ok
test test::test_user_cannot_reject_session ... ok
test test::test_reject_already_complete_session ... ok
test test::test_reject_already_reclaimed_session ... ok
test test::test_wrong_expert_cannot_reject ... ok
test test::test_reject_nonexistent_booking ... ok
test test::test_expert_can_set_and_update_rate ... ok
test test::test_book_session_calculates_correct_deposit ... ok
test test::test_book_session_fails_if_expert_rate_not_set ... ok
test test::test_pause_blocks_book_session ... ok
test test::test_pause_blocks_finalize_session ... ok
test test::test_pause_blocks_reclaim_stale_session ... ok
test test::test_pause_blocks_reject_session ... ok
test test::test_unpause_resumes_operations ... ok
test test::test_read_only_functions_work_while_paused ... ok

test result: ok. 34 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

---

🌌 Comments

Key points for reviewers:

• transfer_admin reads the current stored admin before overwriting — there is no window where both keys are valid simultaneously.
• set_oracle follows the same pattern as transfer_admin for consistency.
• Edge case covered: calling either function on an uninitialized contract returns VaultError::NotInitialized instead of panicking.

Summary by CodeRabbit

• New Features

  • Administrators can now transfer admin privileges to another address
  • Oracle address can now be updated through dedicated functions

• Tests

  • Added comprehensive tests for admin transfer and oracle rotation scenarios

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds admin and oracle key rotation functionality to the payment vault contract. It introduces two new externally callable functions—transfer_admin and set_oracle—that allow the current admin to securely rotate privileged keys, with appropriate authorization checks and event emissions.

Changes

Cohort / File(s)	Summary
Core Admin & Oracle Management contracts/payment-vault-contract/src/contract.rs, contracts/payment-vault-contract/src/events.rs, contracts/payment-vault-contract/src/lib.rs	Implemented transfer_admin() and set_oracle() with admin auth enforcement, storage updates, and event emissions. Added corresponding contract entrypoints and event-emission functions to expose functionality and log key rotations.
Key Rotation Tests contracts/payment-vault-contract/src/test.rs	Added comprehensive test coverage for admin transfer, oracle rotation, and authorization enforcement, verifying that old keys lose privileges immediately and new keys gain full access after rotation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• Identity Registry Key Rotation #39 — Implements the same admin key-rotation pattern (transfer_admin in contract.rs and lib.rs with admin_transferred emission), so changes address identical objectives.

Possibly related PRs

• feat: implement contract pausability (circuit breaker) #30 — Modifies the same payment-vault-contract module and adds admin-controlled management entrypoints (pause/unpause in prior PR vs. transfer_admin/set_oracle in this PR), forming a cohesive admin management layer.

Poem

🐰 A vault needs guardians wise and true,
But keys grow old, and times they change—
So transfer rights to those brand new,
While oracles spin their range,
Old locks retire; new ones reign! 🔐

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Feat/key rotation' is concise and directly summarizes the main change of adding key rotation functionality to the payment vault contract.
Description check	✅ Passed	The PR description is comprehensive, covering all template sections including issue closure, testing confirmation, type of change, detailed changes explanation, test evidence, and reviewer comments. All required checkboxes are marked completed.
Linked Issues check	✅ Passed	All coding requirements from issue #34 are met: transfer_admin and set_oracle functions implemented in contract.rs with admin auth checks, exposed in lib.rs with correct signatures, events added to events.rs, and comprehensive tests added to test.rs verifying admin transfer and privilege loss.
Out of Scope Changes check	✅ Passed	All changes directly align with issue #34 requirements. New entry points (transfer_admin, set_oracle), event emissions (admin_transferred, oracle_updated), and tests are all in-scope for key rotation functionality with no extraneous modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 94.44% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

contracts/payment-vault-contract/src/test.rs (1)

700-831: Good test coverage for key rotation functionality.

The tests cover the essential scenarios: successful transfers, new principals gaining privileges, and authorization enforcement. A few observations:

1. test_new_admin_can_pause_after_transfer - With mock_all_auths(), this confirms the storage was updated and pause succeeds. Combined with test_old_admin_loses_privileges_after_transfer, you get confidence in the auth model.

2. test_old_admin_loses_privileges_after_transfer - Correctly verifies that auth is required, though the test name suggests testing the old admin specifically. What's actually verified is that no auth = failure. This is still valuable coverage.

Consider adding tests for edge cases if desired:

📝 Suggested additional tests (optional)

#[test]
fn test_transfer_admin_not_initialized() {
    let env = Env::default();
    let client = create_client(&env);
    let new_admin = Address::generate(&env);
    
    // Should fail with NotInitialized
    let result = client.try_transfer_admin(&new_admin);
    assert!(result.is_err());
}

#[test]
fn test_old_oracle_cannot_finalize_after_rotation() {
    let env = Env::default();
    env.mock_all_auths();

    let admin = Address::generate(&env);
    let oracle_old = Address::generate(&env);
    let oracle_new = Address::generate(&env);
    let user = Address::generate(&env);
    let expert = 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_old);

    client.set_my_rate(&expert, &10_i128);
    let booking_id = client.book_session(&user, &expert, &100);

    // Rotate oracle
    client.set_oracle(&oracle_new);

    // Clear auths - now only explicit auth will work
    env.set_auths(&[]);

    // Without auth, finalize should fail (proves oracle_new auth is required)
    let result = client.try_finalize_session(&booking_id, &50);
    assert!(result.is_err());
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@contracts/payment-vault-contract/src/test.rs` around lines 700 - 831, Add two
edge-case tests: (1) test_transfer_admin_not_initialized — create an Env and
client via create_client but do not call client.init, call
client.try_transfer_admin with a generated Address and assert it returns Err
(expecting NotInitialized behavior); (2)
test_old_oracle_cannot_finalize_after_rotation — follow the pattern in
test_set_oracle_success to init with oracle_old, book a session (use
client.set_my_rate and client.book_session), call client.set_oracle(&oracle_new)
to rotate, clear auths via env.set_auths(&[]), then call
client.try_finalize_session(&booking_id, &50) and assert it returns Err to
ensure the old oracle cannot finalize after rotation; use create_token_contract
and token.mint as in other tests to fund the booking.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@contracts/payment-vault-contract/src/test.rs`:
- Around line 700-831: Add two edge-case tests: (1)
test_transfer_admin_not_initialized — create an Env and client via create_client
but do not call client.init, call client.try_transfer_admin with a generated
Address and assert it returns Err (expecting NotInitialized behavior); (2)
test_old_oracle_cannot_finalize_after_rotation — follow the pattern in
test_set_oracle_success to init with oracle_old, book a session (use
client.set_my_rate and client.book_session), call client.set_oracle(&oracle_new)
to rotate, clear auths via env.set_auths(&[]), then call
client.try_finalize_session(&booking_id, &50) and assert it returns Err to
ensure the old oracle cannot finalize after rotation; use create_token_contract
and token.mint as in other tests to fund the booking.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bb9cafc7-13a6-4100-a43a-93b00d0431ac

📥 Commits

Reviewing files that changed from the base of the PR and between 01a3a5c and 61efe57.

📒 Files selected for processing (4)

• contracts/payment-vault-contract/src/contract.rs
• contracts/payment-vault-contract/src/events.rs
• contracts/payment-vault-contract/src/lib.rs
• contracts/payment-vault-contract/src/test.rs