fix: solving Uniswap User Ops not submitted on-chain by bundler problem #416
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 11 commits
October 13, 2025 10:40
… execution - Add is_simulated boolean field to RunNodeWithInputsReq protobuf message - Update RunNodeImmediatelyRPC to read is_simulated from request and pass to RunNodeImmediately - Modify RunNodeImmediately to accept variadic useSimulation parameter (defaults to true) - Enable real on-chain UserOp execution when is_simulated=false for testing contract interactions - Update smart wallet validation to allow addresses derivable from owner+salt (0-4) even if not in DB
…code Critical fixes for ERC-4337 UserOp handling: 1. Signature Format (EIP-191): - Ensure all UserOp signing uses signer.SignMessage for EIP-191 prefixed signatures - SimpleAccount contract expects EIP-191 format via toEthSignedMessageHash() - Consistent signature format in BuildUserOpWithPaymaster and sendUserOpCore 2. Nonce Management: - BuildUserOpWithPaymaster now explicitly fetches and sets nonce before signing - Add nonceOverride parameter to BuildUserOpWithPaymaster for sequential UserOps - Prevent nonce refresh in SendUserOp when already set to avoid invalidating signatures - Fix nonce collision issues for sequential transactions 3. Paymaster Signature Preservation: - Skip gas re-estimation when PaymasterAndData is present - Prevents invalidation of paymaster signatures after initial signing - Fixes paymaster signature validation failures 4. Gas Limits for Deployment: - Define gas limit constants (DEFAULT_*, DEPLOYMENT_VERIFICATION_GAS_LIMIT) - Increase verificationGasLimit to 1.6M for UUPS proxy deployment - Covers factory execution, proxy deployment, initialization, and validation 5. Code Refactoring: - Extract sendUserOpCore to eliminate ~350 lines of duplicated code - Reduce builder.go from ~900 to 718 lines (20% reduction) - Single source of truth for retry logic, nonce management, gas estimation - Both SendUserOp and SendUserOpWithWsClient now use shared core logic 6. Paymaster Integration: - Fix vm_runner_contract_write.go to use paymasterReq instead of nil - Enable paymaster sponsorship for contract writes when configured 7. Documentation: - Add bundler configuration guide for testing (--min_stake 0) - Document resolution for 'insufficient stake' errors with AAConfig - Explain ERC-4337 staking requirements and testing workarounds
…arameter order - Fixed timestamp packing from 32-byte ABI encoding to proper 6-byte big-endian uint48 format - Fixed PaymasterAndData field order: validAfter (6 bytes) then validUntil (6 bytes) - Fixed GetHash call to use (userOp, validUntil, validAfter) matching contract ABI - Added extensive debug logging for paymaster signature validation - PaymasterAndData now correctly: address(20) + validAfter(6) + validUntil(6) + signature(65) = 97 bytes - Still debugging AA33 paymaster validation error
… nonce tracking - Fixed PaymasterAndData encoding to use abi.encode(validUntil, validAfter) with 32-byte padding per Solidity contract - Changed order to validUntil FIRST, validAfter SECOND to match parsePaymasterAndData abi.decode - Added paymaster nonce fetching and logging (contract maintains per-sender nonce) - PaymasterAndData now correctly: address(20) + abi.encode(uint48, uint48)(64) + signature(65) = 149 bytes - First UserOp (approval) now successfully accepted by bundler! - Updated bundler URL to use SSH tunnel (localhost:4437) for testing - TODO: Fix paymaster nonce collision for sequential UserOps
- Fixed both sendUserOperationHTTP and sendUserOperationRPC to use checksummed EntryPoint address - Bundler was rejecting lowercase EntryPoint addresses with 'invalid UserOperation struct/fields' - Hardcoded the EIP-55 checksummed v0.6 EntryPoint: 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789 - Both HTTP and RPC fallback now use the same checksummed format - Still debugging remaining 'invalid UserOperation struct/fields' error
…undle trigger - Add gas estimation before generating paymaster signatures to prevent silent rejections - Implement manual bundle trigger (debug_bundler_sendBundleNow) for immediate bundling - Update shouldUsePaymaster() to check ETH balance with 20% buffer - Add SendBundleNow() method to BundlerClient - Add 5-second wait between sequential contract writes in Base test - Fix BuildUserOpWithPaymaster() test calls to include new gas override parameters - Create comprehensive documentation in docs/Account-Abstraction-Improvements.md These changes fix the root cause of UserOps being rejected by bundlers due to incorrect gas limits baked into paymaster signatures. Gas is now estimated accurately before signing, and manual bundling ensures immediate transaction confirmation (2-5 seconds instead of minutes).
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
chrisli30
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on November 10
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
added 4 commits
October 14, 2025 20:56
…rypoint parameter
…sses - Fix TestUniswapApprovalAmountSimulation to use OWNER_EOA environment variable and derive smart wallet address - Fix TestEndToEndValuePropagation to use derived smart wallet address instead of hardcoded runner - Update TestUniswapApprovalAmountSimulation_PrintAddresses to use OWNER_EOA approach - Add proper wallet registration in database for both tests - Resolves CI test failures caused by address mismatches after OWNER_EOA migration
- Rename TestUniswapApprovalAmountSimulation -> TestExecuteTask_UniswapApprovalAndSwap_DifferentApprovalAmounts_Sepolia - Rename TestEndToEndValuePropagation -> TestContractWrite_WETHDeposit_WithETHValue_Sepolia - Remove CI skip logic - GitHub Actions has Tenderly access and funded test wallets - Test names now clearly indicate: what they test, that they execute real tasks, and network (Sepolia) - Refactor WETH deposit test to use OWNER_EOA with salt:0 derivation - Fix aa.GetSenderAddress call signature (3 params instead of 5) Both tests now run in CI and local environments with same OWNER_EOA-derived smart wallet
chrisli30
added a commit
that referenced
this pull request
Oct 17, 2025
…em (#416) * fix: add is_simulated parameter to runNodeImmediately for real UserOp execution - Add is_simulated boolean field to RunNodeWithInputsReq protobuf message - Update RunNodeImmediatelyRPC to read is_simulated from request and pass to RunNodeImmediately - Modify RunNodeImmediately to accept variadic useSimulation parameter (defaults to true) - Enable real on-chain UserOp execution when is_simulated=false for testing contract interactions - Update smart wallet validation to allow addresses derivable from owner+salt (0-4) even if not in DB * fix: resolve bundler UserOp submission issues and refactor duplicate code Critical fixes for ERC-4337 UserOp handling: 1. Signature Format (EIP-191): - Ensure all UserOp signing uses signer.SignMessage for EIP-191 prefixed signatures - SimpleAccount contract expects EIP-191 format via toEthSignedMessageHash() - Consistent signature format in BuildUserOpWithPaymaster and sendUserOpCore 2. Nonce Management: - BuildUserOpWithPaymaster now explicitly fetches and sets nonce before signing - Add nonceOverride parameter to BuildUserOpWithPaymaster for sequential UserOps - Prevent nonce refresh in SendUserOp when already set to avoid invalidating signatures - Fix nonce collision issues for sequential transactions 3. Paymaster Signature Preservation: - Skip gas re-estimation when PaymasterAndData is present - Prevents invalidation of paymaster signatures after initial signing - Fixes paymaster signature validation failures 4. Gas Limits for Deployment: - Define gas limit constants (DEFAULT_*, DEPLOYMENT_VERIFICATION_GAS_LIMIT) - Increase verificationGasLimit to 1.6M for UUPS proxy deployment - Covers factory execution, proxy deployment, initialization, and validation 5. Code Refactoring: - Extract sendUserOpCore to eliminate ~350 lines of duplicated code - Reduce builder.go from ~900 to 718 lines (20% reduction) - Single source of truth for retry logic, nonce management, gas estimation - Both SendUserOp and SendUserOpWithWsClient now use shared core logic 6. Paymaster Integration: - Fix vm_runner_contract_write.go to use paymasterReq instead of nil - Enable paymaster sponsorship for contract writes when configured 7. Documentation: - Add bundler configuration guide for testing (--min_stake 0) - Document resolution for 'insufficient stake' errors with AAConfig - Explain ERC-4337 staking requirements and testing workarounds * Ignore the file/folder that is constantly added by Cursor * fix: correct PaymasterAndData uint48 timestamp encoding and GetHash parameter order - Fixed timestamp packing from 32-byte ABI encoding to proper 6-byte big-endian uint48 format - Fixed PaymasterAndData field order: validAfter (6 bytes) then validUntil (6 bytes) - Fixed GetHash call to use (userOp, validUntil, validAfter) matching contract ABI - Added extensive debug logging for paymaster signature validation - PaymasterAndData now correctly: address(20) + validAfter(6) + validUntil(6) + signature(65) = 97 bytes - Still debugging AA33 paymaster validation error * fix: use ABI-encoded timestamps in PaymasterAndData and add paymaster nonce tracking - Fixed PaymasterAndData encoding to use abi.encode(validUntil, validAfter) with 32-byte padding per Solidity contract - Changed order to validUntil FIRST, validAfter SECOND to match parsePaymasterAndData abi.decode - Added paymaster nonce fetching and logging (contract maintains per-sender nonce) - PaymasterAndData now correctly: address(20) + abi.encode(uint48, uint48)(64) + signature(65) = 149 bytes - First UserOp (approval) now successfully accepted by bundler! - Updated bundler URL to use SSH tunnel (localhost:4437) for testing - TODO: Fix paymaster nonce collision for sequential UserOps * fix: use EIP-55 checksummed EntryPoint address in bundler RPC calls - Fixed both sendUserOperationHTTP and sendUserOperationRPC to use checksummed EntryPoint address - Bundler was rejecting lowercase EntryPoint addresses with 'invalid UserOperation struct/fields' - Hardcoded the EIP-55 checksummed v0.6 EntryPoint: 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789 - Both HTTP and RPC fallback now use the same checksummed format - Still debugging remaining 'invalid UserOperation struct/fields' error * Updated config yaml for aggregator and operator * Add make aggregator-sepolia command and remove the old make dev-agg * Minor updates to the config/aggregator.example.yaml * fix: implement gas estimation before paymaster signature and manual bundle trigger - Add gas estimation before generating paymaster signatures to prevent silent rejections - Implement manual bundle trigger (debug_bundler_sendBundleNow) for immediate bundling - Update shouldUsePaymaster() to check ETH balance with 20% buffer - Add SendBundleNow() method to BundlerClient - Add 5-second wait between sequential contract writes in Base test - Fix BuildUserOpWithPaymaster() test calls to include new gas override parameters - Create comprehensive documentation in docs/Account-Abstraction-Improvements.md These changes fix the root cause of UserOps being rejected by bundlers due to incorrect gas limits baked into paymaster signatures. Gas is now estimated accurately before signing, and manual bundling ensures immediate transaction confirmation (2-5 seconds instead of minutes). * Added waiting for contract write in simulateTask * fix: not treat nonce 0 of a smart wallet as invalid case * Set sepolia yaml config as the default for tests * fix: replacing the hardcoded EntryPointV06Address with the actual entrypoint parameter * fix: make sure the protobuf field means true by default while it is unset * Updated default yaml config of tests * fix: Debug bundler AA33 revert issue - add paymaster nonce tracking and improve logging * Minor fix of removing the unused GetTestPrivateKey from test utils * Added docs of AA33 reverted issue fix * minor fix of removing a deprecated lien in config/aggregator.example.yaml * fix: migrate all tests from TEST_PRIVATE_KEY to OWNER_EOA - Replace TEST_PRIVATE_KEY with OWNER_EOA in all test files - Update GitHub Actions to use OWNER_EOA variable instead of secret - Remove TEST_PRIVATE_KEY from testutil and CI scripts - Add new userops_withdraw_test.go with OWNER_EOA and RECIPIENT_ADDRESS support - Update RunNodeImmediately to accept shouldUsePaymasterOverride parameter - Add shouldUsePaymasterOverride field to VM struct - Remove deprecated withdrawal integration tests - Update documentation for controller-based signing pattern This improves security by using controller delegation pattern where: - OWNER_EOA: Used for smart wallet address derivation (salt:0) - Controller private key: Signs UserOps on behalf of the smart wallet - No need to expose owner's private key in tests or CI * fix: add shouldUsePaymasterOverride support to ETHTransfer and ContractWrite nodes - ETHTransfer: Add shouldUsePaymasterOverride check (priority 0) - ContractWrite: Add shouldUsePaymasterOverride check (priority 0) - Both nodes now respect explicit paymaster override from RunNodeImmediately - Improve gas cost estimation to use real-time gas prices (not hardcoded 20 gwei) - Add detailed logging for paymaster decision-making - Update ContractWrite to check EntryPoint deposit for self-funding eligibility This enables explicit control over paymaster usage in tests via: engine.RunNodeImmediately(nodeType, config, vars, user, false, &usePaymaster) Critical for testing paymaster sponsorship vs self-funded UserOps. * fix: correct paymaster data packing to use ABI encoding (AA33 solution) Critical AA33 reverted fix - paymaster timestamp packing: BEFORE (12-byte compact packing): validUntil + validAfter = 12 bytes compact Total paymasterAndData: 97 bytes Contract tried to decode at offset 84 → read into signature bytes → FAIL AFTER (64-byte ABI encoding): abi.Arguments.Pack(uint48, uint48) = 64 bytes (32+32 padded) Total paymasterAndData: 149 bytes Contract decodes cleanly at offset 20-84 → SUCCESS Additional fixes: - Refactor SendUserOp/SendUserOpWithWsClient into shared sendUserOpShared() - Fix gas estimation to use EXACT UserOp structure (including paymaster) - Increase DEFAULT_VERIFICATION_GAS_LIMIT from 150k to 1M (conservative) - Add UseLocalGasEstimation flag to skip bundler estimation if needed - Improve logging for gas estimation and paymaster signing This resolves the persistent AA33 errors by ensuring client-side packing matches the VerifyingPaymasterFixed.sol contract's expectations. * chore: minor cleanups and documentation updates - Remove legacy TEST_PRIVATE_KEY references in comments - Update gas estimation logging to remove emoji (per user preference) - Improve EIP-1559 gas price calculation logging - Clean up testutil comments for OWNER_EOA migration No functional changes - documentation and logging improvements only. * fix: implement robust nonce polling for AA25 error resolution - Replace fixed 1-second delay with adaptive polling mechanism - Poll for fresh nonce with 15-second timeout and 500ms intervals - Only waits as long as needed for RPC node synchronization - Matches existing polling patterns used throughout codebase - Prevents AA25 invalid account nonce errors in back-to-back test runs - Includes progress logging and timeout fallback for robustness * fix: add hardcoded gas fallback for self-funded UserOps and diagnostic checks - Add hardcoded gas fallback to self-funded UserOps when gas estimation fails with STF error - Self-funded UserOps now fall back to hardcoded gas limits (same as paymaster UserOps) - Add 4 diagnostic checks to verify allowance spender is correct before swap execution - Both RunNodeImmediately and deployed workflow execution now handle STF gas estimation failures - Resolves STF error in Base mainnet Uniswap swaps by using hardcoded gas limits as fallback * fix: consolidate Uniswap swap test files for multi-chain support - Consolidate run_node_uniswap_swap_test.go and run_node_uniswap_swap_base_test.go into single file - Add environment-based chain detection via TEST_CHAIN env var (sepolia/base) - Add Sepolia constants and configuration support - Create getChainConfig() function to select chain-specific addresses and settings - Update all test functions to use setup.chain.* instead of hardcoded BASE_* constants - Rename SEPOLIA_CHAIN_ID to TENDERLY_SEPOLIA_CHAIN_ID in tenderly_client_test.go to avoid conflicts - All 4 test variants (Base, SelfFunded, WithQuoter, CheckSpender) now work for both chains - Simplifies maintenance - single source of truth for Uniswap swap testing logic * fix: resolve failing unit tests after paymaster timestamp changes - Fix TestContractWrite_WithSettingsAndUserAuth nil pointer dereference by creating eth client before calling GetSenderAddress - Update TestGetHash expected hash to match new ABI-encoded timestamp format (64 bytes instead of compact 12 bytes) - Add missing ethclient import to run_node_immediately_settings_test.go * fix: change IsSimulated default to true for safety - Change GetIsSimulated() default return value from false to true when IsSimulated is nil - This prevents unintended real executions when simulation mode was intended - Aligns with documented safety default behavior where unset is_simulated should default to simulation mode - Critical safety fix to prevent accidental real transactions * Revert "fix: change IsSimulated default to true for safety" This reverts commit 0bc3016. * fix: update failing tests to use OWNER_EOA instead of hardcoded addresses - Fix TestUniswapApprovalAmountSimulation to use OWNER_EOA environment variable and derive smart wallet address - Fix TestEndToEndValuePropagation to use derived smart wallet address instead of hardcoded runner - Update TestUniswapApprovalAmountSimulation_PrintAddresses to use OWNER_EOA approach - Add proper wallet registration in database for both tests - Resolves CI test failures caused by address mismatches after OWNER_EOA migration * fix: rename integration tests for clarity and enable in CI - Rename TestUniswapApprovalAmountSimulation -> TestExecuteTask_UniswapApprovalAndSwap_DifferentApprovalAmounts_Sepolia - Rename TestEndToEndValuePropagation -> TestContractWrite_WETHDeposit_WithETHValue_Sepolia - Remove CI skip logic - GitHub Actions has Tenderly access and funded test wallets - Test names now clearly indicate: what they test, that they execute real tasks, and network (Sepolia) - Refactor WETH deposit test to use OWNER_EOA with salt:0 derivation - Fix aa.GetSenderAddress call signature (3 params instead of 5) Both tests now run in CI and local environments with same OWNER_EOA-derived smart wallet --------- Co-authored-by: Chris Li <[email protected]>
chrisli30
added a commit
that referenced
this pull request
Oct 17, 2025
…em (#416) * fix: add is_simulated parameter to runNodeImmediately for real UserOp execution - Add is_simulated boolean field to RunNodeWithInputsReq protobuf message - Update RunNodeImmediatelyRPC to read is_simulated from request and pass to RunNodeImmediately - Modify RunNodeImmediately to accept variadic useSimulation parameter (defaults to true) - Enable real on-chain UserOp execution when is_simulated=false for testing contract interactions - Update smart wallet validation to allow addresses derivable from owner+salt (0-4) even if not in DB * fix: resolve bundler UserOp submission issues and refactor duplicate code Critical fixes for ERC-4337 UserOp handling: 1. Signature Format (EIP-191): - Ensure all UserOp signing uses signer.SignMessage for EIP-191 prefixed signatures - SimpleAccount contract expects EIP-191 format via toEthSignedMessageHash() - Consistent signature format in BuildUserOpWithPaymaster and sendUserOpCore 2. Nonce Management: - BuildUserOpWithPaymaster now explicitly fetches and sets nonce before signing - Add nonceOverride parameter to BuildUserOpWithPaymaster for sequential UserOps - Prevent nonce refresh in SendUserOp when already set to avoid invalidating signatures - Fix nonce collision issues for sequential transactions 3. Paymaster Signature Preservation: - Skip gas re-estimation when PaymasterAndData is present - Prevents invalidation of paymaster signatures after initial signing - Fixes paymaster signature validation failures 4. Gas Limits for Deployment: - Define gas limit constants (DEFAULT_*, DEPLOYMENT_VERIFICATION_GAS_LIMIT) - Increase verificationGasLimit to 1.6M for UUPS proxy deployment - Covers factory execution, proxy deployment, initialization, and validation 5. Code Refactoring: - Extract sendUserOpCore to eliminate ~350 lines of duplicated code - Reduce builder.go from ~900 to 718 lines (20% reduction) - Single source of truth for retry logic, nonce management, gas estimation - Both SendUserOp and SendUserOpWithWsClient now use shared core logic 6. Paymaster Integration: - Fix vm_runner_contract_write.go to use paymasterReq instead of nil - Enable paymaster sponsorship for contract writes when configured 7. Documentation: - Add bundler configuration guide for testing (--min_stake 0) - Document resolution for 'insufficient stake' errors with AAConfig - Explain ERC-4337 staking requirements and testing workarounds * Ignore the file/folder that is constantly added by Cursor * fix: correct PaymasterAndData uint48 timestamp encoding and GetHash parameter order - Fixed timestamp packing from 32-byte ABI encoding to proper 6-byte big-endian uint48 format - Fixed PaymasterAndData field order: validAfter (6 bytes) then validUntil (6 bytes) - Fixed GetHash call to use (userOp, validUntil, validAfter) matching contract ABI - Added extensive debug logging for paymaster signature validation - PaymasterAndData now correctly: address(20) + validAfter(6) + validUntil(6) + signature(65) = 97 bytes - Still debugging AA33 paymaster validation error * fix: use ABI-encoded timestamps in PaymasterAndData and add paymaster nonce tracking - Fixed PaymasterAndData encoding to use abi.encode(validUntil, validAfter) with 32-byte padding per Solidity contract - Changed order to validUntil FIRST, validAfter SECOND to match parsePaymasterAndData abi.decode - Added paymaster nonce fetching and logging (contract maintains per-sender nonce) - PaymasterAndData now correctly: address(20) + abi.encode(uint48, uint48)(64) + signature(65) = 149 bytes - First UserOp (approval) now successfully accepted by bundler! - Updated bundler URL to use SSH tunnel (localhost:4437) for testing - TODO: Fix paymaster nonce collision for sequential UserOps * fix: use EIP-55 checksummed EntryPoint address in bundler RPC calls - Fixed both sendUserOperationHTTP and sendUserOperationRPC to use checksummed EntryPoint address - Bundler was rejecting lowercase EntryPoint addresses with 'invalid UserOperation struct/fields' - Hardcoded the EIP-55 checksummed v0.6 EntryPoint: 0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789 - Both HTTP and RPC fallback now use the same checksummed format - Still debugging remaining 'invalid UserOperation struct/fields' error * Updated config yaml for aggregator and operator * Add make aggregator-sepolia command and remove the old make dev-agg * Minor updates to the config/aggregator.example.yaml * fix: implement gas estimation before paymaster signature and manual bundle trigger - Add gas estimation before generating paymaster signatures to prevent silent rejections - Implement manual bundle trigger (debug_bundler_sendBundleNow) for immediate bundling - Update shouldUsePaymaster() to check ETH balance with 20% buffer - Add SendBundleNow() method to BundlerClient - Add 5-second wait between sequential contract writes in Base test - Fix BuildUserOpWithPaymaster() test calls to include new gas override parameters - Create comprehensive documentation in docs/Account-Abstraction-Improvements.md These changes fix the root cause of UserOps being rejected by bundlers due to incorrect gas limits baked into paymaster signatures. Gas is now estimated accurately before signing, and manual bundling ensures immediate transaction confirmation (2-5 seconds instead of minutes). * Added waiting for contract write in simulateTask * fix: not treat nonce 0 of a smart wallet as invalid case * Set sepolia yaml config as the default for tests * fix: replacing the hardcoded EntryPointV06Address with the actual entrypoint parameter * fix: make sure the protobuf field means true by default while it is unset * Updated default yaml config of tests * fix: Debug bundler AA33 revert issue - add paymaster nonce tracking and improve logging * Minor fix of removing the unused GetTestPrivateKey from test utils * Added docs of AA33 reverted issue fix * minor fix of removing a deprecated lien in config/aggregator.example.yaml * fix: migrate all tests from TEST_PRIVATE_KEY to OWNER_EOA - Replace TEST_PRIVATE_KEY with OWNER_EOA in all test files - Update GitHub Actions to use OWNER_EOA variable instead of secret - Remove TEST_PRIVATE_KEY from testutil and CI scripts - Add new userops_withdraw_test.go with OWNER_EOA and RECIPIENT_ADDRESS support - Update RunNodeImmediately to accept shouldUsePaymasterOverride parameter - Add shouldUsePaymasterOverride field to VM struct - Remove deprecated withdrawal integration tests - Update documentation for controller-based signing pattern This improves security by using controller delegation pattern where: - OWNER_EOA: Used for smart wallet address derivation (salt:0) - Controller private key: Signs UserOps on behalf of the smart wallet - No need to expose owner's private key in tests or CI * fix: add shouldUsePaymasterOverride support to ETHTransfer and ContractWrite nodes - ETHTransfer: Add shouldUsePaymasterOverride check (priority 0) - ContractWrite: Add shouldUsePaymasterOverride check (priority 0) - Both nodes now respect explicit paymaster override from RunNodeImmediately - Improve gas cost estimation to use real-time gas prices (not hardcoded 20 gwei) - Add detailed logging for paymaster decision-making - Update ContractWrite to check EntryPoint deposit for self-funding eligibility This enables explicit control over paymaster usage in tests via: engine.RunNodeImmediately(nodeType, config, vars, user, false, &usePaymaster) Critical for testing paymaster sponsorship vs self-funded UserOps. * fix: correct paymaster data packing to use ABI encoding (AA33 solution) Critical AA33 reverted fix - paymaster timestamp packing: BEFORE (12-byte compact packing): validUntil + validAfter = 12 bytes compact Total paymasterAndData: 97 bytes Contract tried to decode at offset 84 → read into signature bytes → FAIL AFTER (64-byte ABI encoding): abi.Arguments.Pack(uint48, uint48) = 64 bytes (32+32 padded) Total paymasterAndData: 149 bytes Contract decodes cleanly at offset 20-84 → SUCCESS Additional fixes: - Refactor SendUserOp/SendUserOpWithWsClient into shared sendUserOpShared() - Fix gas estimation to use EXACT UserOp structure (including paymaster) - Increase DEFAULT_VERIFICATION_GAS_LIMIT from 150k to 1M (conservative) - Add UseLocalGasEstimation flag to skip bundler estimation if needed - Improve logging for gas estimation and paymaster signing This resolves the persistent AA33 errors by ensuring client-side packing matches the VerifyingPaymasterFixed.sol contract's expectations. * chore: minor cleanups and documentation updates - Remove legacy TEST_PRIVATE_KEY references in comments - Update gas estimation logging to remove emoji (per user preference) - Improve EIP-1559 gas price calculation logging - Clean up testutil comments for OWNER_EOA migration No functional changes - documentation and logging improvements only. * fix: implement robust nonce polling for AA25 error resolution - Replace fixed 1-second delay with adaptive polling mechanism - Poll for fresh nonce with 15-second timeout and 500ms intervals - Only waits as long as needed for RPC node synchronization - Matches existing polling patterns used throughout codebase - Prevents AA25 invalid account nonce errors in back-to-back test runs - Includes progress logging and timeout fallback for robustness * fix: add hardcoded gas fallback for self-funded UserOps and diagnostic checks - Add hardcoded gas fallback to self-funded UserOps when gas estimation fails with STF error - Self-funded UserOps now fall back to hardcoded gas limits (same as paymaster UserOps) - Add 4 diagnostic checks to verify allowance spender is correct before swap execution - Both RunNodeImmediately and deployed workflow execution now handle STF gas estimation failures - Resolves STF error in Base mainnet Uniswap swaps by using hardcoded gas limits as fallback * fix: consolidate Uniswap swap test files for multi-chain support - Consolidate run_node_uniswap_swap_test.go and run_node_uniswap_swap_base_test.go into single file - Add environment-based chain detection via TEST_CHAIN env var (sepolia/base) - Add Sepolia constants and configuration support - Create getChainConfig() function to select chain-specific addresses and settings - Update all test functions to use setup.chain.* instead of hardcoded BASE_* constants - Rename SEPOLIA_CHAIN_ID to TENDERLY_SEPOLIA_CHAIN_ID in tenderly_client_test.go to avoid conflicts - All 4 test variants (Base, SelfFunded, WithQuoter, CheckSpender) now work for both chains - Simplifies maintenance - single source of truth for Uniswap swap testing logic * fix: resolve failing unit tests after paymaster timestamp changes - Fix TestContractWrite_WithSettingsAndUserAuth nil pointer dereference by creating eth client before calling GetSenderAddress - Update TestGetHash expected hash to match new ABI-encoded timestamp format (64 bytes instead of compact 12 bytes) - Add missing ethclient import to run_node_immediately_settings_test.go * fix: change IsSimulated default to true for safety - Change GetIsSimulated() default return value from false to true when IsSimulated is nil - This prevents unintended real executions when simulation mode was intended - Aligns with documented safety default behavior where unset is_simulated should default to simulation mode - Critical safety fix to prevent accidental real transactions * Revert "fix: change IsSimulated default to true for safety" This reverts commit 0bc3016. * fix: update failing tests to use OWNER_EOA instead of hardcoded addresses - Fix TestUniswapApprovalAmountSimulation to use OWNER_EOA environment variable and derive smart wallet address - Fix TestEndToEndValuePropagation to use derived smart wallet address instead of hardcoded runner - Update TestUniswapApprovalAmountSimulation_PrintAddresses to use OWNER_EOA approach - Add proper wallet registration in database for both tests - Resolves CI test failures caused by address mismatches after OWNER_EOA migration * fix: rename integration tests for clarity and enable in CI - Rename TestUniswapApprovalAmountSimulation -> TestExecuteTask_UniswapApprovalAndSwap_DifferentApprovalAmounts_Sepolia - Rename TestEndToEndValuePropagation -> TestContractWrite_WETHDeposit_WithETHValue_Sepolia - Remove CI skip logic - GitHub Actions has Tenderly access and funded test wallets - Test names now clearly indicate: what they test, that they execute real tasks, and network (Sepolia) - Refactor WETH deposit test to use OWNER_EOA with salt:0 derivation - Fix aa.GetSenderAddress call signature (3 params instead of 5) Both tests now run in CI and local environments with same OWNER_EOA-derived smart wallet --------- Co-authored-by: Chris Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors the test configuration and build system to improve clarity and flexibility for integration tests and local development. It replaces the use of a test private key with an owner EOA address for smart wallet testing, updates environment variable usage and documentation, and restructures the Makefile to better support multi-chain deployments and local development workflows. Additionally, it removes the legacy withdrawal integration test file and updates the CI setup accordingly.
Test configuration and environment variable changes:
test-private-keyinput and environment variable withowner-eoathroughout the CI workflow, documentation, and setup scripts to clarify its role as the owner address for smart wallet derivation. (.github/actions/setup-test-config/action.yml,.github/workflows/run-test-on-pr.yml,README.md) [1] [2] [3] [4] [5].github/scripts/generate-test-config.shto copy and modify example config files for Sepolia, replacing direct YAML generation and removing references to the test private key. (.github/scripts/generate-test-config.sh)Build system and Makefile improvements:
build-prod) and local development (build) build targets, and added explicit commands for running aggregator and operator services on different chains, improving usability for multi-chain deployments. (Makefile) [1] [2]Codebase cleanup:
aggregator/withdrawal_integration_test.gofile, which relied on the old test private key pattern. (aggregator/withdrawal_integration_test.go)Minor improvements:
paymasterNonceOverride) to internal calls inaggregator/rpc_server.gofor clarity and correctness. (aggregator/rpc_server.go) [1] [2]