Legion

[Dargon789]

No description provided.

[sourcery-ai]

Sorry @Dargon789, your pull request is larger than the review limit of 150000 diff characters

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a comprehensive refactoring of the project's deployment infrastructure and core contract logic, primarily focusing on gas efficiency and maintainability. The deployment system has been modernized to leverage Foundry's vm.readFork* and stdToml for configuration, alongside a new JSON-based registry for deployed addresses. A significant internal change involves optimizing Intent processing within the Orchestrator and Simulator by moving to direct calldata parsing, which is expected to yield gas savings. Additionally, the project now integrates Changesets for better version management, and the Escrow contract has been enhanced with more flexible refund mechanisms.

Highlights

• Deployment System Refactoring: The deployment and configuration system has been significantly refactored to leverage Foundry's vm.readFork* cheatcodes and stdToml for enhanced flexibility and readability, replacing the older forge-std/Config.sol approach.
• Centralized Deployment Registry: Introduced JSON registry files (deploy/registry/deployment_*.json) to store deployed contract addresses, decoupling deployment state from the main config.toml and providing a clear record of deployments.
• Gas Optimization and Intent Handling: The Orchestrator and Simulator contracts underwent major changes to parse Intent data directly from calldata using a new IntentHelpers library, aiming for significant gas savings and more efficient processing by avoiding Solidity's ABI encoding/decoding overhead.
• Changesets Integration: Adopted Changesets for streamlined versioning and publishing, indicated by the addition of .changeset configuration files, which will help manage releases more effectively.
• Enhanced Escrow Refund Logic: The Escrow contract's refund mechanism was updated to allow early refunds by either the recipient or depositor, providing greater flexibility and control over escrowed funds.
• Custom EIP-7702 Implementation: A new ERC7821Ithaca.sol library was introduced to provide a custom EIP-7702 implementation, replacing the generic solady/accounts/ERC7821.sol and allowing for tailored execution modes.

Changelog

• .changeset/README.md
  • Added a README file for the Changesets tool.
• .changeset/config.json
  • Added configuration for Changesets, defining changelog generation and access settings.
• .env.example
  • Updated RPC endpoints, added verification API keys, and removed deprecated upgrade test variables.
• .gitmodules
  • Added a new submodule for lib/devtools.
• CHANGELOG.md
  • Removed deprecated changelog entries and updated a minor typo.
• deploy/ConfigureLayerZeroSettler.s.sol
  • Refactored to use vm.readFork* for configuration and removed forge-std/Config.sol dependency.
  • Removed l0SettlerSigner from LayerZeroChainConfig struct.
  • Updated loadChainConfig to read from fork variables and handle missing LayerZero settler addresses gracefully.
  • Modified resolveDVNAddresses to read from fork variables.
  • Introduced createForks function to explicitly create forks for configured chains.
  • Removed _selectFork helper function, replacing calls with direct vm.selectFork using forkIds.
  • Simplified configureChain by removing endpoint and L0SettlerSigner updates, and added a check to skip unconfigured destinations.
  • Removed LayerZeroChainConfig parameter from setExecutorConfig as it's no longer needed.
• deploy/DeployMain.s.sol
  • Refactored deployment logic to use stdToml for configuration parsing and introduced JSON registry files for deployed addresses.
  • Removed Config and Variable imports, replacing them with stdToml.
  • Updated ChainConfig struct by adding pauseAuthority and removing l0SettlerSigner, oldOrchestrators, expMinterAddress, and expMintAmount.
  • Removed expToken and exp2Token from DeployedContracts struct.
  • Modified run functions to use vm.readForkChainIds() and introduced initializeDeployment for setup.
  • Updated loadChainConfigFromFork to read configuration directly from fork variables using vm.readFork* functions.
  • Modified getAllContracts to no longer conditionally add ExpToken for testnets.
  • Refactored loadDeployedContracts to read from JSON registry files instead of config.
  • Removed tryGetAddress helper function.
  • Updated logLoadedConfigurations to reflect changes in ChainConfig.
  • Refactored saveDeployedContract to write to JSON registry files and introduced writeToRegistry and getRegistryFilename helpers.
  • Added tryReadAddress helper function for parsing addresses from JSON.
  • Modified deployment functions (deployOrchestrator, deployIthacaAccount, deployAccountProxy, deploySimulator, deploySimpleFunder, deployEscrow, deploySimpleSettler, deployLayerZeroSettler) to check if contracts are already deployed and to use the new ChainConfig structure and saveDeployedContract.
• deploy/FundSigners.s.sol
  • Refactored to use stdToml for configuration and removed the setOrchestrators function call.
  • Removed Config import and added stdToml import.
  • Updated ISimpleFunder interface by removing setOrchestrators and orchestrators functions.
  • Removed supportedOrchestrators from ChainFundingConfig struct.
  • Modified run functions to use vm.readForkChainIds() and introduced loadConfig.
  • Removed setOrchestratorsInSimpleFunder function.
  • Updated getChainFundingConfig to create forks and read configuration from fork variables using vm.readFork* functions.
• deploy/README.md
  • Updated documentation to reflect the new configuration structure, registry files, and removed ExpToken deployment details.
  • Adjusted configuration structure example to use [profile.deployment] and [forks.*] sections.
  • Removed automatic address writing to config.toml and introduced registry files section.
  • Updated available contracts list by removing ExpToken.
  • Removed sections related to ExpToken deployment and supporting bash scripts.
• deploy/config.toml
  • Updated configuration structure to use [forks.*] and [profile.deployment] sections, and removed exp_minter_address and exp_mint_amount.
• deploy/registry/deployment_11155420_0x0000000000000000000000000000000000000000000000000000000000000001.json
  • Added a JSON registry file for deployed contracts on Optimism Sepolia.
• deploy/registry/deployment_84532_0x0000000000000000000000000000000000000000000000000000000000000001.json
  • Added a JSON registry file for deployed contracts on Base Sepolia.
• foundry.toml
  • Updated to extend deploy/config.toml and adjusted filesystem permissions for the new registry directory.
• gas-snapshots/.gas-snapshot-main
  • Added a new gas snapshot file, capturing gas usage benchmarks for various operations.
• lib/LayerZero-v2
  • Updated the LayerZero-v2 submodule commit.
• lib/forge-std
  • Updated the forge-std submodule commit.
• src/Escrow.sol
  • Modified refund logic to allow early refunds by the recipient or depositor if block.timestamp is past the refundTimestamp or if the caller is the respective party.
• src/GuardedExecutor.sol
  • Removed direct import of ERC7821 from Solady, now importing ERC7821Ithaca.
  • Updated an error message from SuperAdminCanSpendAnything() to SuperAdminCanExecuteEverything().
  • Corrected typos in doc comments from overriden to overridden.
• src/IthacaAccount.sol
  • Bumped the contract version to 0.5.11.
  • Adjusted conditional checks for orchestrator calls to improve readability.
  • Formatted getKeys function signature for better readability.
• src/MultiSigSigner.sol
  • Corrected a typo in a doc comment from Calcualated to Calculated.
  • Clarified a signature validation detail in a doc comment from signaturs to signatures.
• src/Orchestrator.sol
  • Integrated IntentHelpers library for efficient calldata parsing of Intent structs.
  • Removed isMultichain, encodedPreCalls, encodedFundTransfers, funder, funderSignature, settler, settlerContext, paymentAmount, paymentRecipient, paymentSignature, and supportedAccountImplementation fields from the Intent struct, now handled dynamically via IntentHelpers.
  • Updated INTENT_TYPEHASH and SIGNED_CALL_TYPEHASH to reflect the simplified Intent struct.
  • Added MERKLE_VERIFICATION constant for nonce prefixing.
  • Changed execute function visibility from public to external.
  • Removed nonReentrant modifier from execute(bytes[] calldata) function.
  • Refactored simulateExecute to accept a single bytes calldata encodedIntent parameter, embedding isStateOverride and combinedGasOverride within it.
  • Removed _extractIntent function, as intent data is now accessed directly via IntentHelpers.
  • Updated _execute function to use IntentHelpers for accessing intent fields and removed direct Intent calldata i parameter.
  • Modified selfCallPayVerifyCall537021665 to use IntentHelpers for parsing intent data and handling fund data and pre-calls.
  • Removed _verifyMerkleSig function, as merkle verification is now integrated into the main _verify flow based on nonce prefix.
  • Updated _fund function signature to accept bytes[] calldata encodedFundTransfers.
  • Modified _pay function signature to accept individual payment-related parameters instead of the full Intent struct.
  • Updated _computeDigest functions for both SignedCall and Intent to align with the new struct definitions and calldata parsing approach.
  • Bumped the contract version to 0.5.6.
• src/SimpleFunder.sol
  • Bumped the contract version to 0.1.9.
  • Updated the approve call in _approve to use a 20-byte all-ones sentinel instead of type(uint256).max for consistency with ERC20 standards.
• src/Simulator.sol
  • Inherited IntentHelpers for direct calldata parsing of Intent structs.
  • Updated _updatePaymentAmounts to modify bytes memory u directly using assembly based on IntentHelpers offsets.
  • Modified _callOrchestratorCalldata and _callOrchestratorMemory to use abi.encodePacked for simulateExecute calls, embedding simulation flags directly into the encoded intent.
  • Updated simulateCombinedGas and simulateV1Logs to accept bytes calldata calldataIntent and use IntentHelpers for internal data access.
• src/interfaces/ICommon.sol
  • Removed the Intent struct definition, centralizing its handling in IntentHelpers.
• src/interfaces/IIthacaAccount.sol
  • Updated the pay function signature to include eoa, payer, paymentToken, paymentRecipient, and paymentSignature parameters.
• src/interfaces/IOrchestrator.sol
  • Updated the simulateExecute function signature to accept a single bytes calldata encodedIntent parameter.
• src/libraries/ERC7821Ithaca.sol
  • Added a new library providing a custom EIP-7702 implementation with various execution modes, including single batch, batch of batches, and common to address batching.
• src/libraries/IntentHelpers.sol
  • Added a new library to assist with parsing Intent data directly from calldata, defining offsets for various fields and providing helper functions to extract them efficiently.
• test/Base.t.sol
  • Updated imports to use ERC7821Ithaca instead of ERC7821 from Solady.
  • Added _ERC7821_BATCH_SANS_TO_EXECUTION_MODE constant.
• test/Benchmark.t.sol
  • Updated testERC20Transfer_IthacaAccount to testERC20Transfer_IthacaAccount1.
  • Modified _createIntents to use the new Intent struct and encodeIntent helper.
  • Added testERC20TransferViaPortoOrchestratorWithPasskey for benchmarking passkey-based transfers.
  • Updated testERC20Transfer_IthacaAccountWithSpendLimits to use the new Intent struct and encodeIntent helper.
• test/Escrow.t.sol
  • Added new test cases for early refund functionality, allowing recipients or depositors to refund before the refundTimestamp.
• test/GuardedExecutor.t.sol
  • Updated test functions to use the new Intent struct and encodeIntent helper for oc.execute calls.
  • Adjusted nonce handling in testSetAndRemoveSpendLimitRevertsForSuperAdmin and testSetAndRemoveSpendLimit.
• test/Orchestrator.t.sol
  • Updated test functions to use the new Intent struct and encodeIntent helper for oc.execute calls.
  • Modified _TestFullFlowTemps struct to use the new Intent struct.
  • Adjusted multi-chain intent signature encoding to include additional bytes for compatibility.
  • Removed isMultichain flag from Intent initialization in multi-chain tests, as it's now inferred from nonce.
• test/SimulateExecute.t.sol
  • Updated test functions to use the new Intent struct and encodeIntent helper for simulator.simulateV1Logs and oc.simulateExecute calls.
• test/SubAccounts.t.sol
  • Added a new test suite for sub-account functionality with DApp integration, covering parent-child spend permissions and fund transfers.
• test/utils/interfaces/IPimlicoPaymaster.sol
  • Corrected typos in doc comments from singature to signature.
• test/utils/mocks/MockOrchestrator.sol
  • Exposed internal _hashTypedData and _hashTypedDataSansChainId functions for testing purposes.
• test/utils/mocks/MockPayerWithSignature.sol
  • Updated the pay function signature and logic to align with IIthacaAccount changes, including new parameters and nonce handling.
• test/utils/mocks/MockPayerWithState.sol
  • Updated the pay function signature and logic to align with IIthacaAccount changes, including new parameters and nonce handling.

Ignored Files

• Ignored by pattern: .github/workflows/** (5)
  • .github/workflows/auto-assign.yaml
  • .github/workflows/ci.yaml
  • .github/workflows/claude-code.yml
  • .github/workflows/manual-deployment.yml
  • .github/workflows/version-check.yaml

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a significant refactoring of the deployment and configuration system, migrating from forge-std/Config to a more robust approach using Foundry's fork-based configuration and JSON registry files. This is a commendable improvement for maintainability and idempotency. The Orchestrator contract has also been heavily optimized for gas by using direct calldata access, which is an advanced and effective technique. However, this refactoring has introduced a critical security vulnerability by removing the settler address from the signed EIP-712 digest, allowing it to be tampered with by a malicious relayer. Additionally, a logic change in the payment flow could break multichain intents. I've also included a high-severity comment on a security best practice in the .env.example file and a medium-severity comment on improving the maintainability of the deployment script.

I am having trouble creating individual review comments. Click here to see my feedback.

src/Orchestrator.sol (494-504)

The settlerData, which contains the settler address and settlerContext, is no longer part of the EIP-712 signed digest. The previous INTENT_TYPEHASH included the settler address, ensuring its integrity. By moving it out of the signed data, a malicious relayer can now tamper with it, potentially redirecting the settlement message to a malicious contract. This could lead to theft of funds or other exploits in the cross-chain settlement process.

To fix this, the settler address and settlerContext (or a hash of it) must be included in the signed EIP-712 digest.

.env.example (8)

The PRIVATE_KEY is set to a default, well-known test key. While this is an example file, it's a security best practice to leave private key fields empty to prevent accidental commitment of real private keys. Developers might copy the file and forget to change it, leading to security risks if the repository is public or the key is used on a testnet with real value.

PRIVATE_KEY=

src/Orchestrator.sol (345-351)

The check for the payer's balance is now performed for all intents, including multichain intents. The previous implementation correctly skipped this check for multichain intents, as their funding can happen later inside the gas-limited selfCallPayVerifyCall537021665. This premature balance check will cause valid multichain intents to fail if the payer does not have the funds at this point but expects to be funded during execution. This appears to be a regression.

        uint256 nonce = _getNonce();
        // Early skip the entire pay-verify-call workflow if the payer lacks tokens,
        // so that less gas is wasted when the Intent fails.
        // For multi chain mode, we skip this check, as the funding happens inside the self call.
        if (nonce >> 240 != MERKLE_VERIFICATION && TokenTransferLib.balanceOf(_getPaymentToken(), payer) < _getPaymentAmount()) {
            err = PaymentError.selector;

            if (flags == _SIMULATION_MODE_FLAG) {
                revert PaymentError();
            }
        }

deploy/DeployMain.s.sol (459-530)

The writeToRegistry function manually constructs a JSON string using a series of string.concat calls. This approach is verbose, hard to maintain, and prone to formatting errors (like missing or extra commas).

Consider using Foundry's stdJson library to build the JSON object in a more structured and safer way. This would make the code cleaner, more robust, and easier to modify when new contracts are added.