Skip to content

feat(identity-registry): add contract upgrade functionality and corre…#68

Open
Elizabeth-dev270 wants to merge 1 commit into
LightForgeHub:mainfrom
Elizabeth-dev270:identity-registry-upgradability
Open

feat(identity-registry): add contract upgrade functionality and corre…#68
Elizabeth-dev270 wants to merge 1 commit into
LightForgeHub:mainfrom
Elizabeth-dev270:identity-registry-upgradability

Conversation

@Elizabeth-dev270
Copy link
Copy Markdown

@Elizabeth-dev270 Elizabeth-dev270 commented Jun 1, 2026

Closes #40

Implemented identity registry contract WASM upgradability and regression coverage.

Changes made

  • contract.rs

    • Added upgrade_contract(env: &Env, new_wasm_hash: BytesN<32>) -> Result<(), RegistryError>
    • Requires admin auth and calls env.deployer().update_current_contract_wasm(new_wasm_hash)
  • lib.rs

    • Exposed pub fn upgrade_contract(env: Env, new_wasm_hash: BytesN<32>) -> Result<(), RegistryError> in the #[contractimpl]
  • test.rs

    • Added regression test test_upgrade_contract_preserves_expert_directory_state
    • Verifies expert status, index membership, and data_uri persistence across upgrade

Notes

  • Editor diagnostics report no errors in the modified files.
  • The upgrade entrypoint follows the same admin-controlled pattern as the other contracts in the workspace.

Made changes.

Summary by CodeRabbit

  • New Features

    • Added contract upgrade functionality with authorization controls to safely update deployed code.
  • Tests

    • Added integration tests verifying that data and state integrity are preserved during contract upgrades.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR implements standard WASM upgrade capability for the identity-registry-contract. It adds a new admin-only upgrade_contract entrypoint that accepts a 32-byte WASM hash, authorizes the caller, and invokes the deployer to switch the contract version. The function is exposed through the public contract interface and tested to ensure expert directory state and metadata survive the upgrade.

Changes

Contract Upgrade Implementation

Layer / File(s) Summary
Upgrade contract core implementation
contracts/identity-registry-contract/src/contract.rs
Imports BytesN from soroban_sdk and implements the admin-only upgrade_contract function that loads the stored admin, requires authorization, and calls env.deployer().update_current_contract_wasm(new_wasm_hash) to perform the WASM upgrade.
Public contract method
contracts/identity-registry-contract/src/lib.rs
Adds BytesN to the contract module imports and exposes IdentityRegistryContract::upgrade_contract as a public contract method that forwards the call to the core implementation and returns Result<(), RegistryError>.
State preservation test
contracts/identity-registry-contract/src/test.rs
Imports BytesN in test utilities and adds test_upgrade_contract_preserves_expert_directory_state to verify that calling try_upgrade_contract does not alter expert verification status, directory index mapping, or persisted metadata.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • #35: The upgrade contract implementation directly addresses the admin-only WASM upgrade requirement with the same function signature and authorization pattern.

Poem

🐇 A contract that upgrades with grace,
Admin holds the reins in place,
State preserved through the swap,
Not a byte of expertise lost atop,
WASM flies, the registry stays strong!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates the main change is adding contract upgrade functionality with regression coverage, directly aligned with the PR's primary objectives.
Description check ✅ Passed The description covers all key changes and maps to the template sections, though it deviates from the repository's structured template format with custom content instead of using provided checkboxes and sections.
Linked Issues check ✅ Passed All objectives from issue #40 are met: upgrade_contract is implemented in contract.rs with admin auth, exposed in lib.rs, and regression test verifies state preservation across upgrades.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #40 requirements: three files modified to implement upgrade functionality and add regression tests with no extraneous alterations.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% 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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
contracts/identity-registry-contract/src/test.rs (1)

53-82: 💤 Low value

Consider adding a negative test for unauthorized upgrade attempts.

The test correctly validates state preservation through the upgrade. For completeness, consider adding a test that verifies non-admin callers are rejected (similar to test_add_expert_unauthorized and test_ban_expert_unauthorized patterns in this file).

#[test]
#[should_panic]
fn test_upgrade_contract_unauthorized() {
    let env = Env::default();
    let contract_id = env.register(IdentityRegistryContract, ());
    let client = IdentityRegistryContractClient::new(&env, &contract_id);

    let admin = Address::generate(&env);
    client.init(&admin);

    // No auth mocked — should panic
    let new_wasm_hash = BytesN::from_array(&env, &[1u8; 32]);
    client.upgrade_contract(&new_wasm_hash);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/identity-registry-contract/src/test.rs` around lines 53 - 82, Add a
negative test that asserts non-admin upgrade attempts are rejected: create a
test function (e.g., test_upgrade_contract_unauthorized) that registers
IdentityRegistryContract, creates IdentityRegistryContractClient, calls
client.init(&admin) for some generated admin but does NOT call
env.mock_all_auths(), then calls client.upgrade_contract(&new_wasm_hash) (using
BytesN::from_array) and mark the test with #[should_panic] to ensure an
unauthorized caller causes a panic; follow the existing unauthorized-test
patterns like test_add_expert_unauthorized/test_ban_expert_unauthorized for
structure and naming.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@contracts/identity-registry-contract/src/test.rs`:
- Around line 53-82: Add a negative test that asserts non-admin upgrade attempts
are rejected: create a test function (e.g., test_upgrade_contract_unauthorized)
that registers IdentityRegistryContract, creates IdentityRegistryContractClient,
calls client.init(&admin) for some generated admin but does NOT call
env.mock_all_auths(), then calls client.upgrade_contract(&new_wasm_hash) (using
BytesN::from_array) and mark the test with #[should_panic] to ensure an
unauthorized caller causes a panic; follow the existing unauthorized-test
patterns like test_add_expert_unauthorized/test_ban_expert_unauthorized for
structure and naming.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0630949a-77ff-410c-b741-a61b49fc7a79

📥 Commits

Reviewing files that changed from the base of the PR and between 68b1a40 and 8ccc704.

📒 Files selected for processing (3)
  • contracts/identity-registry-contract/src/contract.rs
  • contracts/identity-registry-contract/src/lib.rs
  • contracts/identity-registry-contract/src/test.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Identity Registry Upgradability

1 participant