fix: smart contract security hardening from audit (CPL-199) by GTC6244 · Pull Request #265 · LIT-Protocol/chipotle

GTC6244 · 2026-04-03T15:07:42Z

Summary

Security hardening of the AccountConfig diamond proxy contracts based on a comprehensive audit (CPL-199). All 21 Solidity files reviewed, 0 critical vulnerabilities found, 2 high and 4 medium issues fixed.

Access Control Tightening:

setApiPayers restricted to owner-only (was api-payer-or-owner), preventing hostile payer takeover
setPricingOperator restricted to owner-only (was api-payer-or-pricing-operator), preventing privilege escalation

Input Validation:

Array-size bounds added to addGroup (max 10 cidHashes/pkpIds) matching existing updateGroup limits
Array-size bounds added to setUsageApiKey (max 50 per permission array)
Duplicate PKP registration check added to registerWalletDerivation

Code Cleanup:

Dead code ternary in debitApiKey/creditApiKey simplified (was unreachable due to revertIfNotMasterAccount guard)
Unused LibERC2771.sol meta-transaction library removed (no imports reference it)

Auditability:

Events added to all state-changing functions across WritesFacet, BillingFacet, and APIConfigFacet
receive() function added to AccountConfig to explicitly reject direct ETH transfers

Compiler:

Pragma pinned to =0.8.28 across all 21 contracts for deterministic compilation

Pre-Landing Review

No security vulnerabilities found in the diff. All changes are security-positive hardening measures. Two independent security reviews confirmed no issues.

Test plan

Hardhat compilation passes: 22 Solidity files successfully (evm target: paris)
No test regressions (no existing Solidity test suite)
Two independent security reviews passed (automated + manual)

Full audit report: .context/attachments/CPL-199-audit-report.md

🤖 Generated with Claude Code

- Restrict setApiPayers and setPricingOperator to owner-only (was api-payer-or-owner) - Add array-size bounds to addGroup (10) and setUsageApiKey (50) - Add duplicate PKP registration check in registerWalletDerivation - Add events to all state-changing functions for off-chain monitoring - Add receive() to reject direct ETH transfers - Simplify debitApiKey/creditApiKey by removing dead code branch - Remove unused LibERC2771.sol meta-transaction library - Pin pragma to =0.8.28 across all 21 contracts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Security hardening for the AccountConfig diamond proxy contracts based on audit CPL-199, including tighter access control, added input bounds, additional event emission for auditability, and deterministic compilation via pinned Solidity pragma.

Changes:

Restricted privileged configuration functions (e.g., setApiPayers, setPricingOperator) and added SecurityLib.revertIfNotOwner.
Added input size bounds and expanded event emission across state-changing facets for better auditability.
Pinned Solidity pragma to =0.8.28 and removed unused LibERC2771.sol; added receive() to explicitly reject ETH transfers.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
lit-api-server/blockchain/lit_node_express/libraries/LibERC2771.sol	Removed unused ERC-2771 helper library.
lit-api-server/blockchain/lit_node_express/libraries/LibDiamond.sol	Pinned compiler pragma for deterministic builds.
lit-api-server/blockchain/lit_node_express/libraries/diamond/OwnershipFacet.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/libraries/diamond/DiamondMultiInit.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/libraries/diamond/DiamondLoupeFacetNoERC165.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/libraries/diamond/DiamondLoupeFacet.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/libraries/diamond/DiamondInit.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/libraries/diamond/DiamondCutFacet.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/interfaces/IERC173.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/interfaces/IERC165.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/interfaces/IDiamondLoupe.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/interfaces/IDiamondCut.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/interfaces/IDiamond.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/WritesFacet.sol	Added events, input bounds, and duplicate PKP registration check.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/ViewsFacet.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/SecurityLib.sol	Added owner-only guard helper using `LibDiamond` ownership.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/DiamondInit.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/BillingFacet.sol	Simplified debit/credit logic, added events, tightened pricing-operator update access control.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/AppStorage.sol	Pinned compiler pragma.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/APIConfigFacet.sol	Added events and restricted `setApiPayers` to owner-only.
lit-api-server/blockchain/lit_node_express/contracts/AccountConfig.sol	Added `receive()` to explicitly reject ETH transfers and pinned pragma.

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

lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/WritesFacet.sol

Copilot · 2026-04-03T15:10:56Z

lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/WritesFacet.sol

+                "cidHashes must be less than 10 items"
+            );
+        }
+        if (pkpIds.length > 10) {
+            revert AppStorage.InvalidRequest(
+                "pkpIds must be less than 10 items"


The revert message is inconsistent with the bound check. The code permits pkpIds.length == 10 (reverts only when > 10), but the message says "less than 10 items". Align the message with the actual limit (e.g., "10 items or fewer") or change the condition to match the message.

Suggested change

"cidHashes must be less than 10 items"

);

}

if (pkpIds.length > 10) {

revert AppStorage.InvalidRequest(

"pkpIds must be less than 10 items"

"cidHashes must be 10 items or fewer"

);

}

if (pkpIds.length > 10) {

revert AppStorage.InvalidRequest(

"pkpIds must be 10 items or fewer"

Copilot · 2026-04-03T15:10:56Z

lit-api-server/blockchain/lit_node_express/contracts/AccountConfigFacets/WritesFacet.sol

        AppStorage.AccountConfigStorage storage s = AppStorage.getStorage();
        AppStorage.Account storage account = s.accounts[masterHash];
+        if (account.pkpData[pkpId].id != 0) {
+            revert AppStorage.InvalidRequest("PKP already registered");
+        }


Duplicate registration detection relies on account.pkpData[pkpId].id != 0, but pkpData is mapping(address => Metadata) where id is documented as the derivation path (AppStorage.sol:97). If derivationPath can be 0, the first registration stores id = 0 and subsequent calls will bypass this check, leading to duplicate entries and inconsistent pkpCounts/indexes. Consider either forbidding derivationPath == 0 at input validation, or tracking registration with a dedicated boolean/sentinel that cannot collide with valid derivationPath values.

Copilot · 2026-04-03T15:10:56Z

lit-api-server/blockchain/lit_node_express/contracts/AccountConfig.sol

+    /// @notice Reject any ETH sent directly to the contract.
+    receive() external payable {
+        revert();
+    }


receive() unconditionally calls revert() with no reason data, which makes it hard to diagnose failed transfers from tooling/wallets. Consider reverting with a custom error (preferred) or a short revert string so callers can tell the failure is specifically due to direct ETH transfers being unsupported.

…nfigFacets/WritesFacet.sol Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Run `make generate` to update the ABI and Rust ethers bindings to include new events, the NotContractOwner error, and updated function signatures from the security hardening commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Use custom error DirectETHTransferNotAllowed() in receive() for better diagnostics - Fix inconsistent error messages: "less than 10" -> "10 items or fewer" - Add derivationPath != 0 validation to prevent duplicate detection bypass - Regenerate Rust contract bindings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The previous commit restricted setApiPayers to owner-only, but the signer pool reconciliation (signer_pool.rs) calls this function via get_admin_api_payer_contract(), which signs with the admin API payer key, not the diamond owner. This would break signer pool updates in production. New access control: owner OR admin API payer (but NOT regular API payers), which still prevents the hostile payer takeover from H-2 while keeping the signer pool operational. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

GTC6244 requested review from a team and Copilot April 3, 2026 15:07

Copilot started reviewing on behalf of GTC6244 April 3, 2026 15:08 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

GTC6244 self-assigned this Apr 3, 2026

GTC6244 and others added 4 commits April 3, 2026 11:53

Update lit-api-server/blockchain/lit_node_express/contracts/AccountCo…

e7d8f99

…nfigFacets/WritesFacet.sol Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: smart contract security hardening from audit (CPL-199)#265

fix: smart contract security hardening from audit (CPL-199)#265
GTC6244 wants to merge 5 commits intonextfrom
feature/cpl-199-web3-contract-audit

GTC6244 commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

GTC6244 commented Apr 3, 2026

Summary

Pre-Landing Review

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants