feat: new e2e setup

[arnaubennassar]

🔄 Changes Summary

• Added comprehensive E2E testing framework with environment loader system
• Implemented Docker Compose-based test environments (starting with op-pp configuration)
• Added bridge flow tests covering both L1→L2 and L2→L1 token transfers
• Integrated E2E tests into CI with GitHub Actions workflow
• Added SimpleERC20 contract for token transfer testing
• Created reusable environment snapshots for fast test setup

⚠️ Breaking Changes

None

📋 Config Updates

None - E2E test configurations are self-contained in test/e2e/envs/

✅ Testing

• 🤖 Automatic:
  • GitHub Actions workflow runs E2E tests via make test-e2e
  • Tests can be triggered manually via workflow_dispatch or on schedule
  • Automated upload of test results and logs as artifacts
• 🖱️ Manual:
  1. Run make test-e2e locally to execute all E2E tests
  2. Tests will automatically start Docker Compose environments
  3. Verify bridge flows complete successfully (L1→L2 and L2→L1)
  4. Environment is automatically cleaned up after tests

🐞 Issues

• Closes #[issue-number]

🔗 Related PRs

None

📝 Notes

• E2E environments are generated using Kurtosis snapshots for fast and reproducible setup
• The op-pp environment includes a single OP PP network with ECDSA multisig consensus
• Environment loader system supports multiple environments (currently op-pp, more can be added)
• Tests use remote images for faster CI execution but can build locally if needed
• Timeout set to 45 minutes for CI jobs to accommodate environment startup and test execution

[claude]

Claude finished @arnaubennassar's task in 2m 37s —— View job

---

PR Review Complete

I've completed a comprehensive review of this E2E test infrastructure PR. Here's my detailed feedback:

---

🚀 What's New

This PR introduces a comprehensive E2E testing framework for aggkit with:

• Environment Loader Infrastructure: A dynamic environment loader (test/e2e/envs/loader.go) that can load pre-configured test environments from snapshot data
• OP-PP Test Environment: A complete Optimism-based pessimistic proof (OP-PP) test environment with L1 (Ethereum), L2 (OP Stack), and AggLayer components
• Bridge Tests: Full L1→L2 and L2→L1 bridge flow tests with native ETH
• Docker Compose Setup: Complete containerized environment with geth, beacon, validator, agglayer, op-geth, op-node, and aggkit services
• Smart Contract Test Utilities: SimpleERC20 contract for testing ERC20 token bridging
• GitHub Actions Workflow: CI workflow to run E2E tests (currently manual trigger only)

🐛 Bug Fixes

None - this is a new feature addition.

📋 Config Updates

• Makefile: Added test-e2e target to run E2E tests with 30-minute timeout
• go.mod: No new dependencies added (uses existing libraries)
• GitHub Actions: New workflow file .github/workflows/test-go-e2e.yml for running E2E tests

⚠️ Breaking Changes

None - this is purely additive.

---

Detailed Review Feedback

🔴 Required Issues (Security & Critical)

1. Hardcoded Passwords in Configuration Files

Severity: 🔴 Critical Security Issue

Multiple configuration files contain hardcoded passwords in plain text:

• test/e2e/envs/op-pp/config/001/aggkit-config.toml:24: SequencerPrivateKeyPassword = "pSnv6Dh5s9ahuzGzH9RoCDrKAMddaX3m"
• test/e2e/envs/op-pp/config/agglayer/config.toml: Contains keystore password in plain text

Impact: While these are test credentials, hardcoding passwords in version control is a security anti-pattern. If these credentials are reused or if the pattern is copied to production configs, it could lead to serious security vulnerabilities.

Recommendation:

• Add a clear comment in these files indicating these are TEST CREDENTIALS ONLY and should never be used in production
• Consider using environment variables or a secure credential management approach even for tests
• Document that these are throwaway test credentials in the README

File References:

• test/e2e/envs/op-pp/config/001/aggkit-config.toml:24
• test/e2e/envs/op-pp/config/agglayer/config.toml

2. SimpleERC20 Contract Missing Access Control

Severity: 🔴 Critical (for production use)

The mint() function in test/contracts/simpleerc20/SimpleERC20.sol:49-53 has no access control - anyone can mint unlimited tokens.

function mint(address _to, uint256 _amount) public {
    totalSupply += _amount;
    balanceOf[_to] += _amount;
    emit Transfer(address(0), _to, _amount);
}

Impact: While this is a test contract, it could be dangerous if accidentally deployed or used as a template for production code.

Recommendation:

• Add a comment clearly marking this as a TEST-ONLY contract
• Consider adding onlyOwner or similar access control even for test contracts to demonstrate best practices
• Alternatively, document in comments why access control is intentionally omitted for testing purposes

File Reference: test/contracts/simpleerc20/SimpleERC20.sol:49-53

3. Missing Integer Overflow Protection

Severity: 🔴 Medium-High

SimpleERC20 contract uses Solidity ^0.8.20 which has built-in overflow protection, but the contract doesn't follow check-effects-interactions pattern and has potential issues:

• Line 27-28: Balance updates without checks for zero address
• Line 42-44: Multiple state changes in sequence

Recommendation:

• Add zero address checks in transfer() and transferFrom()
• Follow checks-effects-interactions pattern more strictly
• Add input validation for mint() function

File Reference: test/contracts/simpleerc20/SimpleERC20.sol

---

🟡 Suggested Improvements (Code Quality)

4. Error Messages Could Be More Descriptive

Location: test/e2e/envs/loader.go (multiple locations)

Error wrapping is good, but some error messages could provide more context:

// Line 161: Generic error message
return nil, fmt.Errorf("find envs directory: %w", err)

// Line 206: Could specify what the expected format is
return nil, fmt.Errorf("parse L1 chain ID: %s", summary.Networks.L1.ChainID)

Recommendation: Add more contextual information to errors to help with debugging:

return nil, fmt.Errorf("failed to find envs directory (searched from %s): %w", cwd, err)
return nil, fmt.Errorf("parse L1 chain ID (expected decimal integer, got %s): %w", summary.Networks.L1.ChainID, err)

5. Magic Numbers Should Be Constants

Location: Multiple files

Several magic numbers appear throughout the code:

• test/e2e/bridge_test.go:91: bridgeAmount := big.NewInt(100000000000000) // 0.0001 ETH
• test/e2e/bridge_test.go:27: 20*time.Minute timeout
• test/e2e/envs/loader.go:424: 2 * time.Minute timeout
• test/e2e/bridge_test.go:127: maxBridgeRetries := 30

Recommendation: Extract these to named constants with clear documentation:

const (
    defaultBridgeAmount = 100000000000000 // 0.0001 ETH in wei
    e2eTestTimeout = 20 * time.Minute
    serviceReadyTimeout = 2 * time.Minute
    maxBridgeQueryRetries = 30
)

6. Repeated Code in Bridge Tests

Location: test/e2e/bridge_test.go

The L1→L2 and L2→L1 test functions share significant similar code:

• Proof preparation (lines 194-208 vs 351-365)
• Bridge polling logic (lines 127-147 vs 300-320)
• Claim transaction submission

Recommendation: Extract common functionality into helper functions:

func prepareClaim(claimProof *types.ClaimProof) ([32][32]byte, [32][32]byte, common.Hash, common.Hash) { ... }
func waitForBridgeIndexing(ctx context.Context, client *client.Client, txHash string, networkID uint32) (*types.BridgeResponse, error) { ... }

This would improve maintainability and reduce the risk of inconsistencies.

7. Function Comment Formatting

Location: test/e2e/envs/loader.go

Per CLAUDE.md guidelines, exported functions should have doc comments starting with the function name. Some functions follow this, but consistency could be improved:

// Line 156 - Good example
// LoadEnv loads an E2E test environment by name

// Line 331 - Could be more descriptive
// parsePrivateKey parses a private key from hex string (with or without 0x prefix)

Recommendation: Ensure all exported functions have complete doc comments following Go conventions and CLAUDE.md guidelines.

8. Test Timeout Configuration

Location: test/e2e/bridge_test.go:27, .github/workflows/test-go-e2e.yml:17

The test has a 20-minute context timeout, but the GitHub Actions workflow has a 45-minute job timeout. The Makefile also specifies 30 minutes.

Recommendation: Align these timeouts or document why they differ:

• Test context: 20 minutes
• Makefile: 30 minutes
• GitHub Actions: 45 minutes

Consider whether these are appropriate for the actual test duration.

9. Unused Variables and Accounts

Location: test/e2e/bridge_test.go:60-67

Variables l1ToL2PrivKey and l2ToL1PrivKey are generated but never used in the tests:

l1ToL2PrivKey, err := crypto.GenerateKey()
require.NoError(t, err, "failed to generate L1 to L2 key")
log.Infof("L1->L2 account: %s", crypto.PubkeyToAddress(l1ToL2PrivKey.PublicKey).Hex())
// Never used afterward

Recommendation: Either use these accounts in the tests or remove them if they were left over from earlier iterations.

---

🟢 Optional Improvements (Nice to Have)

10. Documentation Could Be Enhanced

Location: test/e2e/envs/README.md

The README references a TODO link: TODO: add link once pushed to kurtosis repo (line 3)

Recommendation: Complete the documentation with the proper link, or create a GitHub issue to track this documentation update.

11. WIP.md Should Be Removed or Converted

Location: test/e2e/envs/WIP.md

This file appears to be internal development notes. It contains the original implementation requirements.

Recommendation:

• Remove this file since the feature appears to be complete
• Or convert it to a proper design document if it's valuable for historical context

12. GitHub Actions Workflow Improvements

Location: .github/workflows/test-go-e2e.yml:25

The workflow specifies Go version 1.25.6, which seems unusually high. As of my knowledge cutoff, the latest Go version is around 1.22-1.23.

Recommendation: Verify this Go version is correct. If it's a typo, it should be something like 1.22.6 or 1.23.x.

13. Test Isolation and Cleanup

Location: test/e2e/envs/loader.go:348-354

The Stop() method only stops docker-compose if the loader started it. This is good defensive programming, but could lead to state leakage between tests if containers are left running.

Recommendation: Consider adding a force-cleanup option or documenting the cleanup behavior more clearly in the README.

14. Large Generated Files in Repo

Impact: Repository bloat

The PR adds 21,068 lines, with the L2 genesis file being 15,591 lines alone:

• test/e2e/envs/op-pp/config/001/l2-genesis.json (15,591 lines)
• test/e2e/envs/op-pp/snapshot.log (858 lines)
• test/contracts/simpleerc20/simpleerc20.go (781 lines - generated)

Recommendation:

• Consider using Git LFS for large binary-like files (genesis.json)
• Or compress these files if they don't need to be human-readable
• Document in .gitattributes that these are generated files
• The simpleerc20.go file should have a comment indicating it's generated

15. Service Wait Logic Could Be More Robust

Location: test/e2e/envs/loader.go:422-451

The waitForEthereumService function uses simple polling with fixed intervals.

Recommendation: Consider implementing exponential backoff for more efficient startup detection, especially for CI environments where startup times may vary.

---

Summary of Findings

Critical Issues: 3 (Security-related)

• Hardcoded passwords in config files
• Unprotected mint function in test contract
• Missing input validation in smart contract

Code Quality Issues: 12 (Suggested/Optional improvements)

• Error messages, magic numbers, code duplication, documentation gaps

Overall Assessment

This is a well-structured and comprehensive E2E testing framework. The code quality is generally high with:

✅ Strengths:

• Clean separation of concerns (loader, tests, configs)
• Proper error handling with context
• Good use of Go testing patterns with testify/require
• Comprehensive test coverage of bridge flows
• Well-documented environment setup

⚠️ Areas for Improvement:

• Security: Hardcoded credentials need clear documentation
• Code organization: Some duplication in test code
• Documentation: Missing links and WIP files need cleanup

Recommendation: This PR is ready to merge after addressing the critical security documentation issues (adding comments that these are test-only credentials). The code quality improvements can be addressed in follow-up PRs if desired.

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud