Skip to content

Conversation

@silva-fj
Copy link
Contributor

Summary

Implements a CREATE2 factory system for deterministic contract deployments across multiple EVM chains.

Changes

  • Configure Foundry for deterministic bytecode (bytecode_hash = none, cbor_metadata = false)
  • Add Create2Factory contract with deployment tracking and redeployment prevention
  • Add comprehensive test suite
  • Add deployment scripts for factory and AA contracts via CREATE2
  • Add factory addresses registry for tracking deployments across networks
  • Document CREATE2 deployment strategy in DEPLOYMENT.md

- Set bytecode_hash = none
- Set cbor_metadata = false
- Pin evm_version = cancun

These settings ensure identical bytecode across builds, which is
essential for CREATE2 address determinism.
Implements a CREATE2 factory contract with:
- Deterministic address computation
- Deployment tracking and redeployment prevention
- Sender-protected and simple salt generation
- Address prediction helpers
@linear
Copy link

linear bot commented Oct 30, 2025

@vercel
Copy link

vercel bot commented Oct 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
heima-aa-demo-app Ignored Ignored Preview Nov 24, 2025 10:03am

@claude
Copy link

claude bot commented Oct 30, 2025

Claude finished @silva-fj's task —— View job


CREATE2 Factory Code Review

I've completed a comprehensive review of this CREATE2 factory implementation. Overall, this is a high-quality implementation with excellent testing and documentation. Here's my detailed feedback:

Strengths

Code Quality & Architecture

  • Clean, well-documented contract with comprehensive NatSpec comments
  • Excellent separation of concerns - factory focused solely on CREATE2 deployments
  • Robust error handling with custom errors and meaningful messages
  • Deterministic bytecode configuration properly set in foundry.toml
  • Comprehensive deployment scripts with extensive validation and logging

Security

  • Frontrunning protection via sender-protected salts
  • Redeployment prevention with both mapping tracking and code existence checks
  • Assembly usage is minimal and safe - standard CREATE2 pattern
  • No privileged functions - permissionless and decentralized design

Testing

  • Exceptional test coverage (409 lines) covering all functions and edge cases
  • Fuzz testing included for robustness
  • Integration tests demonstrating real-world usage
  • Edge case handling including empty bytecode and minimal contracts

🟡 Minor Issues & Suggestions

Code Quality Improvements

tee-worker/omni-executor/aa-contracts/src/core/Create2Factory.sol:42-44

// Check if address already has code (additional safety check)
if (deployed.code.length > 0) {
    revert AddressAlreadyDeployed(deployed);
}

This check is redundant since the deployments mapping already prevents redeployment. The CREATE2 opcode will revert if attempting to deploy to an address with existing code. Consider removing for gas efficiency or keep with a comment explaining it's defensive programming.

tee-worker/omni-executor/aa-contracts/src/core/Create2Factory.sol:115-122
The generateSimpleSalt function could benefit from a stronger warning in documentation:

/**
 * @notice Generates a simple salt for deployment (SECURITY WARNING: vulnerable to frontrunning)
 * @dev WARNING: Anyone can frontrun deployments using simple salts. Only use if frontrunning
 *      protection is not required. Consider using generateSalt() instead.
 */

tee-worker/omni-executor/aa-contracts/script/DeployCreate2Factory.s.sol:46-55
The nonce warning logic is good, but consider making the warning more prominent:

if (vm.getNonce(deployer) > 0) {
    console.log("⚠️  WARNING: Deployer has non-zero nonce!");
    console.log("⚠️  For consistent factory addresses across chains, use a fresh EOA");
    console.log("⚠️  Current nonce:", vm.getNonce(deployer));
    console.log("");
}

Performance Considerations

tee-worker/omni-executor/aa-contracts/src/core/Create2Factory.sol:70-75
The computeAddress function re-hashes the bytecode on every call. For frequently called scenarios, consider caching or using computeAddressWithHash when you already have the hash:

// More efficient if you already have the hash
bytes32 hash = keccak256(bytecode);
address predicted = factory.computeAddressWithHash(salt, hash);

Gas Optimization: The deployments mapping uses a bool which costs 20k gas for first write. Since this is a one-time cost per deployment and provides safety, it's acceptable, but worth noting.

Minor Script Improvements

tee-worker/omni-executor/aa-contracts/script/DeployWithCreate2.s.sol:108-147
The address prediction section could be refactored to reduce code duplication. Consider extracting a helper function:

function predictAndLogAddress(string memory name, bytes32 salt, bytes memory bytecode) internal view {
    address predicted = factory.computeAddress(salt, bytecode);
    console.log(string(abi.encodePacked(name, " (predicted):")), predicted);
}

🔍 Security Analysis

CREATE2 Implementation

  • ✅ Correct CREATE2 address computation formula
  • ✅ Proper salt generation preventing frontrunning
  • ✅ No privilege escalation vectors
  • ✅ Redeployment protection

Assembly Safety

  • ✅ Minimal assembly usage (only for CREATE2 call)
  • ✅ Standard pattern - no memory corruption risks
  • ✅ Proper error handling for failed deployments

Access Control

  • ✅ Permissionless design - no admin functions
  • ✅ No upgradeability concerns (immutable contract)

🧪 Test Coverage Assessment

Excellent coverage including:

  • ✅ Basic deployment functionality
  • ✅ Address computation and prediction
  • ✅ Redeployment prevention
  • ✅ Salt generation (both protected and simple)
  • ✅ Event emission
  • ✅ Edge cases (empty bytecode, minimal contracts)
  • ✅ Fuzz testing for robustness
  • ✅ Integration scenarios

Consider adding:

  • Gas consumption tests for optimization validation
  • Tests with very large bytecode (approaching contract size limits)
  • Tests demonstrating the frontrunning protection in action

🏗️ Architecture Evaluation

Design Decisions

  • Stateless factory design - excellent for decentralization
  • Separate deployment tracking - good safety practice
  • Dual salt generation strategies - good flexibility
  • Comprehensive logging and validation in deployment scripts

Multi-Chain Strategy

  • Well-documented deployment process for consistency
  • Registry file for tracking factory addresses
  • Environment-based deployment organization

📋 Minor Suggestions

  1. Documentation: Add a security considerations section to the contract explaining frontrunning protection
  2. Gas Optimization: Document gas costs for different operations
  3. Deployment: Consider adding a deployment verification step that checks predicted vs actual addresses
  4. Registry: The create2-factories.json could include deployment block numbers and timestamps

🎯 Overall Assessment

This is a production-ready implementation that follows best practices for CREATE2 factories. The code quality is excellent, security considerations are well-addressed, and the testing is comprehensive. The deployment scripts and documentation are particularly well-crafted, making this easy to use and maintain.

Recommendation: Approve

The implementation is ready for production use with only minor optimization opportunities identified above.


  • Read and analyze all changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Analyze test coverage

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thank you, I left some comments - mainly questions around the salt generation

Comment on lines +11 to +12
bytecode_hash = "none"
cbor_metadata = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tried to deploy contracts with create2 factory? I'm not sure if would cause any verification problem (or you'll need manually verify that) as etherscan probably expects some metadatas to verify the contract

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 tried deploying a contract locally on Anvil. I got these values from the docs here

@silva-fj silva-fj requested a review from a team November 5, 2025 19:46
silva-fj and others added 6 commits November 6, 2025 09:09
…y-contract

Resolved conflicts by keeping Create2Factory-related files from dev branch:
- tee-worker/omni-executor/contracts/aa/deployments/create2-factories.json
- tee-worker/omni-executor/contracts/aa/script/DeployContract.s.sol
- tee-worker/omni-executor/contracts/aa/script/DeployCreate2Factory.s.sol
- tee-worker/omni-executor/contracts/aa/script/DeployWithCreate2.s.sol
- tee-worker/omni-executor/contracts/aa/src/core/Create2FactoryV1.sol
- tee-worker/omni-executor/contracts/aa/test/Create2Factory.t.sol
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.

4 participants