Skip to content

TEERegistry: simplify PCR revocation, clean up naming#35

Merged
adambalogh merged 13 commits intomainfrom
aba/refac
Mar 10, 2026
Merged

TEERegistry: simplify PCR revocation, clean up naming#35
adambalogh merged 13 commits intomainfrom
aba/refac

Conversation

@adambalogh
Copy link
Contributor

@adambalogh adambalogh commented Mar 10, 2026

Summary

Simplifies the TEERegistry contract by removing unused complexity and standardizing naming across the contract, tests, CLI, and integration scripts.

PCR revocation: lazy → eager

  • Removed grace period from revokePCR. Revoked PCRs now take effect immediately.
  • revokePCR actively iterates the enabled list and disables all TEEs running the revoked image (safe backward iteration with swap-and-pop).
  • Removed expiresAt from ApprovedPCR, PCRExpired error, and lazy PCR enforcement from heartbeat.
  • isPCRApproved simplified to a single storage read.

Removed removeTEE endpoint

  • Permanent TEE deletion is no longer needed. TEEs can be disabled/enabled but not deleted.
  • Removed removeTEE function, TEERemoved event, and all associated tests, CLI commands, and client methods.

Naming cleanup

Before After Reason
active (TEEInfo field) enabled Avoids confusion with PCR/type active
activateTEE / deactivateTEE enableTEE / disableTEE Matches the field name
getActivatedTEEs getEnabledTEEs Consistency
getLiveTEEs / getHealthyTEEs getActiveTEEs Clearer three-tier naming: all → enabled → active
isHealthy / _isHealthy isTEEActive / _isTEEActive Disambiguates from PCR/type active
isEnabled isTEEEnabled Disambiguates — could be TEE, PCR, or type
getPublicKey getTEEPublicKey Disambiguates
lastUpdatedAt lastHeartbeatAt More precise
TEEDeactivated / TEEActivated TEEDisabled / TEEEnabled Matches function names
CLI tee healthy tee active Matches contract naming

Test fixes

  • Fixed disableTEE test to expect a revert (not a no-op) when disabling an already-disabled TEE, matching the contract's TEENotEnabled guard.
  • Fixed addTEEType test to check typeInfo.addedAt instead of non-existent typeInfo.active field (TEETypeInfo struct has name and addedAt, not active).

Other

  • Simplified onlyTEEOwnerOrAdmin modifier (clearer boolean logic).
  • Moved setAWSRootCertificate above TEE type management for better section ordering.
  • Removed lazy PCR check from heartbeat (no longer needed with eager revocation).
  • Fixed ABI JSON struct field names to match contract (activeenabled for TEEInfo).

Files changed

  • contracts/solidity/TEERegistry.sol — core contract changes
  • contracts/solidity/TEERegistry.json — updated ABI
  • contracts/solidity/TEEInferenceVerifier.sol — updated call sites
  • tests/solidity/suites/tee/ — updated test helper, lifecycle, registry, and settlement tests
  • scripts/tee-mgmt-cli/ — updated CLI commands and Go client
  • scripts/integration/local_tee_workflow.go — updated integration script

@adambalogh adambalogh changed the title TEERegistry cleanup TEERegistry: simplify PCR revocation, remove dead endpoints, rename for clarity Mar 10, 2026
@adambalogh adambalogh added the run-ci Run ETE Testing Suite label Mar 10, 2026
@adambalogh adambalogh changed the title TEERegistry: simplify PCR revocation, remove dead endpoints, rename for clarity TEERegistry: simplify PCR revocation, clean up naming Mar 10, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
@adambalogh adambalogh added run-ci Run ETE Testing Suite and removed run-ci Run ETE Testing Suite labels Mar 10, 2026
@adambalogh adambalogh requested a review from Copilot March 10, 2026 02:58
Copy link

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

This PR refactors the TEERegistry Solidity contract and related tooling to simplify PCR revocation (lazy → eager), remove the removeTEE endpoint, and standardize naming across contracts, tests, CLI, and integration scripts.

Changes:

  • Switch PCR revocation to immediate enforcement and actively disable TEEs using a revoked PCR.
  • Remove permanent TEE deletion (removeTEE) in favor of enable/disable only.
  • Rename “active/activate/deactivate” terminology to “enabled/enable/disable”, and adjust dependent tests and tooling.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
contracts/solidity/TEERegistry.sol Core contract refactor: eager PCR revocation + naming cleanup + removeTEE removal.
contracts/solidity/TEERegistry.json Updated ABI JSON (currently inconsistent with Solidity; see comments).
contracts/solidity/TEEInferenceVerifier.sol Updates registry calls to isTEEEnabled / getTEEPublicKey.
tests/solidity/suites/tee/test/registry.js Updates tests for PCR revocation + renamed PCR/TEE APIs.
tests/solidity/suites/tee/test/lifecycle.js Updates lifecycle tests for enable/disable semantics and eager PCR revocation behavior.
tests/solidity/suites/tee/test/settlementRelay.js Updates integration-style test flow to enable/disable.
tests/solidity/suites/tee/contracts/TEETestHelper.sol Updates helper wrappers to renamed APIs and new revokePCR signature.
tests/solidity/suites/tee/contracts/MockTEERegistry.sol Updates mock registry to new TEEInfo field names and enable flow.
scripts/tee-mgmt-cli/registry/client.go Updates Go client selectors/types for renamed endpoints (has build/runtime issues; see comments).
scripts/tee-mgmt-cli/cmd/tee.go CLI commands renamed to enable/disable/active and updated output fields.
scripts/tee-mgmt-cli/cmd/pcr.go CLI PCR revoke/list updated for eager revocation (return type mismatch risk; see comments).
scripts/tee-mgmt-cli/cmd/types.go Removes TEE type deactivation command.
scripts/tee-mgmt-cli/Readme.md Removes documentation reference to type deactivation.
scripts/integration/local_tee_workflow.go Updates integration workflow to new selectors/names (multiple selector/ABI mismatches; see comments).
precompiles/tee/README.md Documentation partially updated but still references removed APIs/old semantics (see comments).
Comments suppressed due to low confidence (3)

contracts/solidity/TEERegistry.sol:324

  • getApprovedPCRs() currently returns PCRKey[] (pcrHash + teeType), but the updated ABI JSON and Go clients/scripts in this PR treat it as returning a plain bytes32[] of hashes. This API mismatch will break clients. Decide on one contract-level return type and make the ABI + clients consistent (either change Solidity to bytes32[], or update ABI/clients to decode a tuple array).
    /// @notice Get all currently approved PCRs
    /// @return PCRKey[] Array of active PCR keys (pcrHash + teeType)
    function getApprovedPCRs() external view returns (PCRKey[] memory) {
        uint256 count = 0;
        for (uint256 i = 0; i < _pcrList.length; i++) {
            if (isPCRApproved(_pcrList[i].teeType, _pcrList[i].pcrHash)) count++;
        }

contracts/solidity/TEERegistry.json:578

  • contracts/solidity/TEERegistry.json is inconsistent with TEERegistry.sol and will break any consumer that relies on this ABI:
  • getApprovedPCRs is defined as returning bytes32[], but Solidity returns PCRKey[] (tuple array).
  • getActiveTEEs is listed with no inputs and returning bytes32[], but Solidity defines getActiveTEEs(uint8) returning TEEInfo[].
  • The ABI appears to be missing getEnabledTEEs(uint8) entirely.
  • isPCRApproved is shown as isPCRApproved(bytes32) but Solidity is isPCRApproved(uint8,bytes32).
    Please regenerate this ABI from the compiled contract (or update it manually to exactly match the Solidity signatures and return types).
      "inputs": [
        {
          "internalType": "bytes32",
          "name": "teeId",
          "type": "bytes32"
        }
      ],
      "name": "disableTEE",
      "outputs": [],
      "stateMutability": "nonpayable",
      "type": "function"
    },
    {
      "inputs": [],
      "name": "getApprovedPCRs",
      "outputs": [
        {
          "internalType": "bytes32[]",
          "name": "",
          "type": "bytes32[]"
        }
      ],
      "stateMutability": "view",
      "type": "function"
    },
    {
      "inputs": [],
      "name": "getActiveTEEs",
      "outputs": [
        {
          "internalType": "bytes32[]",
          "name": "",
          "type": "bytes32[]"
        }
      ],
      "stateMutability": "view",
      "type": "function"

scripts/tee-mgmt-cli/registry/client.go:249

  • GetApprovedPCRs() decodes the return value as a bytes32[], but the Solidity implementation of getApprovedPCRs() returns PCRKey[] (tuple array of {pcrHash, teeType}). This call will fail to decode or will produce incorrect results at runtime. Either change the contract/API to return only bytes32[], or update the CLI to decode a tuple array and print both the hash and type.
func (c *Client) GetApprovedPCRs() ([]string, error) {
	result, err := c.ethCall(selGetApprovedPCRs)
	if err != nil {
		return nil, err
	}
	return decodeBytes32Array(result)
}

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

///
/// 4. **Deactivation / Reactivation** — TEE owners or admins can toggle a TEE's active
/// status. `activateTEE` re-validates the TEE's PCR before reactivating.
/// 4. **Disable / Enable** — TEE owners or admins can toggle a TEE's enabled
Copy link
Collaborator

Choose a reason for hiding this comment

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

SDK should be updated to take into consideration the new naming

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does the SDK use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the CLI already

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have to add it to .gitignore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point!

/// @param gracePeriod Seconds until revocation takes effect (0 = immediate)
function revokePCR(bytes32 pcrHash, uint8 teeType, uint256 gracePeriod) external onlyRole(DEFAULT_ADMIN_ROLE) {
/// @param teeType The TEE type this PCR belongs to
function revokePCR(bytes32 pcrHash, uint8 teeType) external onlyRole(DEFAULT_ADMIN_ROLE) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will not consider a grace period, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, i think it's better to simplify the logic here

Copy link
Collaborator

@khalifaT khalifaT left a comment

Choose a reason for hiding this comment

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

I have added few comments:

  • Update to sdk will be needed
  • Revocation process: no need to grace period?

@adambalogh adambalogh merged commit 8d35b76 into main Mar 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Run ETE Testing Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants