Skip to content

feat: refactor contract owner to diamond storage (CPL-188)#249

Merged
GTC6244 merged 4 commits intonextfrom
feature/cpl-188-refactor-contract-owner-to-diamond-storage-v2
Mar 31, 2026
Merged

feat: refactor contract owner to diamond storage (CPL-188)#249
GTC6244 merged 4 commits intonextfrom
feature/cpl-188-refactor-contract-owner-to-diamond-storage-v2

Conversation

@GTC6244
Copy link
Copy Markdown
Contributor

@GTC6244 GTC6244 commented Mar 31, 2026

Summary

  • Rename owner storage field to configOperator in AppStorage, separating config management from diamond ownership
  • Add setConfigOperator setter (gated by revertIfNotConfigOperatorOrOwner) and configOperator() view
  • Replace s.owner references in SecurityLib with LibDiamond.contractOwner() for authorization checks
  • Regenerate Rust ABI bindings

Test plan

  • Verify contract compilation with npx hardhat compile
  • Verify setConfigOperator is only callable by current config operator or diamond owner
  • Verify configOperator() view returns correct address after initialization
  • Verify existing revertIfNotApiPayerOrOwner checks now use diamond owner correctly

🤖 Generated with Claude Code

GTC6244 and others added 3 commits March 31, 2026 17:52
…dings

Use revertIfNotConfigOperatorOrOwner instead of revertIfNotApiPayerOrOwner
for setConfigOperator so only the config operator or diamond owner can
reassign the role. Fix "Orwner" typo in SecurityLib. Regenerate Rust ABI
bindings via make generate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors AccountConfig’s “owner” concept by introducing a configOperator role in AppStorage, while switching authorization checks that previously relied on s.owner to use the canonical diamond owner (LibDiamond.contractOwner()), and updates generated ABI/bindings accordingly.

Changes:

  • Add configOperator() view and setConfigOperator(address) setter (with a new OnlyConfigOperatorOrOwner error).
  • Replace s.owner authorization checks in SecurityLib with LibDiamond.contractOwner().
  • Regenerate the ABI JSON and Rust ethers bindings to include the new function/error.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lit-api-server/src/accounts/contracts/AccountConfig.json Adds ABI entries for configOperator, setConfigOperator, and OnlyConfigOperatorOrOwner.
lit-api-server/src/accounts/contracts/account_config_contract.rs Regenerates Rust bindings to expose config_operator() / set_config_operator() and the new error decoding.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/ViewsFacet.sol Exposes configOperator() view over diamond storage.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/SecurityLib.sol Adds revertIfNotConfigOperatorOrOwner and switches “owner” checks to LibDiamond.contractOwner().
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/AppStorage.sol Renames the storage field from owner to configOperator and adds a corresponding error.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/APIConfigFacet.sol Adds setConfigOperator(address) state mutation.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfig.sol Initializes configOperator in the constructor instead of owner.
Comments suppressed due to low confidence (1)

lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/SecurityLib.sol:148

  • configOperator is introduced as a distinct role (see AppStorage.AccountConfigStorage.configOperator), but the existing “owner” authorization paths (revertIfNotApiPayerOrOwner / checkIfApiPayerOrOwner) now only consider LibDiamond.contractOwner(). As a result, calling setConfigOperator() will not grant the new operator permission to perform the config actions currently guarded by these helpers (e.g., setRequestedApiPayerCount, setApiPayers, setNodeConfiguration). If the intent is that the config operator retains the old s.owner capabilities, update these checks to include s.configOperator; otherwise, consider clarifying/renaming to avoid a role that has no effect beyond setConfigOperator itself.
    function revertIfNotApiPayerOrOwner(address caller) internal view {
        AppStorage.AccountConfigStorage storage s = AppStorage.getStorage();
        if (
            !s.api_payers.contains(caller) &&
            caller != LibDiamond.contractOwner() &&
            caller != s.adminApiPayerAccount
        ) {
            revert AppStorage.OnlyApiPayerOrOwner(caller);
        }
    }

    function checkIfApiPayerOrPricingOperator(address caller) internal view {
        AppStorage.checkIfApiPayerOrPricingOperator(
            AppStorage.getStorage(),
            caller
        );
    }

    function checkIfApiPayer(address caller) internal view {
        AppStorage.AccountConfigStorage storage s = AppStorage.getStorage();
        if (!s.api_payers.contains(caller)) {
            revert AppStorage.OnlyApiPayer(caller);
        }
    }

    function checkIfApiPayerOrOwner(address caller) internal view {
        AppStorage.AccountConfigStorage storage s = AppStorage.getStorage();
        if (
            !s.api_payers.contains(caller) &&
            caller != LibDiamond.contractOwner() &&
            caller != s.adminApiPayerAccount
        ) {
            revert AppStorage.OnlyApiPayerOrOwner(caller);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…nfigFacets/SecurityLib.sol

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@GTC6244 GTC6244 merged commit 19cb22d into next Mar 31, 2026
8 checks passed
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.

2 participants