Main Audit Issue Fixes Branch#112
Draft
zaryab2000 wants to merge 55 commits into
Draft
Conversation
…dress(this).balance totalSupply() previously returned address(this).balance, which could be inflated by force-sent native PC (e.g. via selfdestruct). Now tracked via a private _totalSupply variable updated only in deposit() and withdraw(), restoring the ERC-20 invariant totalSupply == sum(balanceOf).
Replaced raw approve() calls with forceApprove() from SafeERC20 in swapAndBurnGas and _autoSwap. The using SafeERC20 for IERC20 declaration was already present but unused — now all approve interactions go through the safe wrapper.
depositPRC20Token was the only UE-module entrypoint missing reentrancy protection. Added nonReentrant for symmetry with depositPRC20WithAutoSwap, refundUnusedGas, and swapAndBurnGas.
…0.transferFrom Reordered transferFrom to check and deduct allowance before calling _transfer, following standard ERC-20 checks-effects-interactions pattern. Updated tests to reflect new event ordering and revert behavior.
…rrors in WPC Added WPCErrors library with InsufficientBalance and InsufficientAllowance custom errors. Replaced three require(..., "") statements in withdraw and transferFrom with structured custom error reverts for gas efficiency and failure observability.
Added events to 8 privileged setters that were mutating protocol state without emitting: setAutoSwapSupported, setWPC, setUniversalGatewayPC, setUniswapV3Addresses, setDefaultFeeTier, setSlippageTolerance in UniversalCore, and setName, setSymbol in PRC20. Address setters include old and new values. Tests updated with event emission assertions.
Removed uniswapV3Quoter entirely from UniversalCore (not yet deployed on mainnet, no storage layout to preserve). Removed from initialize params and setUniswapV3Addresses. In UniversalCoreV0 (deployed testnet), replaced with __deprecated_uniswapV3Quoter to preserve storage layout. Updated all test files to remove quoter references.
…TxGasAndFees Added ZeroBaseGasLimit error and explicit revert when baseGasLimitByChainNamespace is unconfigured (zero), aligning validation with getRescueFundsGasLimit which already reverts on zero. Prevents silently returning zero gas fees for unconfigured chains.
…rations Adds event declarations to IPRC20 and IUniversalCore interfaces that were missed in the F-2026-15527 and F-2026-15537 commits due to filesystem case-sensitivity (Interfaces/ vs interfaces/). No behavioral change — events were already emitted correctly from the implementations.
Added NatSpec to gasPCPoolByChainNamespace and setGasPCPool clarifying the stored pool is for off-chain observability only. Runtime swap flows resolve pools dynamically from the factory. No code change.
Added FEE_TIER_LOW (500), FEE_TIER_MEDIUM (3000), FEE_TIER_HIGH (10000), and MAX_SLIPPAGE_BPS (5000) constants in both UniversalCore and UniversalCoreV0. Replaced hardcoded literals in setDefaultFeeTier and setSlippageTolerance.
CEA only uses IERC20.balanceOf — no transfers or approvals that would need SafeERC20 wrappers. Removed the dead import and using statement.
…izeUEA UEAProxy now validates _logic != address(0) before storing the implementation, matching CEAProxy's initialization pattern.
… single call Capture return data from failed low-level calls in _handleMulticall and _handleSingleCall. When revert data is available, propagate it via assembly revert. Fall back to CEAErrors.ExecutionFailed() when return data is empty. Matches the pattern already used in UEA_EVM. Updated 26 tests across 5 test files to expect actual underlying revert reasons.
…lTxFromCEA The ERC-20 path in sendUniversalTxToUEA called the gateway without approving it to pull tokens. Added IERC20(token).approve(UNIVERSAL_GATEWAY, amount) before the gateway call so transferFrom succeeds. Updated 4 tests to reflect approval behavior.
_handleMigration now enforces recipient == address(this), matching UEA's self-targeting requirement for migrations. Previously the recipient param was ignored in the migration branch. Updated 25 tests across migration, integration, and fuzz suites to pass the CEA's own address as recipient.
Inline initializer defaultDeadlineMins = 20 ran in the implementation constructor context, not the proxy. Proxy storage read 0, making swap deadlines ineffective. Moved assignment into initialize() so the value writes to proxy storage.
isSupportedToken mapping and setSupportedToken setter were configured by admin but never checked in any execution path. Removed entirely from UniversalCore (not yet deployed on mainnet). Deprecated with __deprecated_ prefix in UniversalCoreV0 to preserve testnet storage layout. Removed from both interfaces and all related tests.
…osit PRC20.deposit() now checks if UNIVERSAL_CORE is paused before minting, preventing the UNIVERSAL_EXECUTOR_MODULE from bypassing pause via the direct deposit path. Added CorePaused custom error and 3 tests for pause propagation. Fixed stale 5-arg initialize calls in PRC20 tests.
setGasTokenPRC20 now zeroes gasPriceByChainNamespace for the affected chain, forcing explicit reconfiguration via setChainMeta. Prevents stale gas price data denominated in the old token from producing incorrect fee calculations. Updated test setUp ordering to configure gas token before gas price.
setChainMeta now reverts with ZeroGasPrice when price is 0, preventing storage of invalid gas price that breaks downstream fee quote functions. Fails at write time instead of read time. Updated tests to expect the revert and use fresh namespaces for zero-price scenarios.
Deposit event now uses abi.encodePacked(msg.sender) instead of the hardcoded UNIVERSAL_EXECUTOR_MODULE address. Off-chain indexers can now distinguish deposits initiated by UniversalCore vs the module directly.
Added FEE_TIER_LOWEST (100) constant for Uniswap V3's 1 bps pools used by highly correlated pairs. Included in setDefaultFeeTier validation in both UniversalCore and UniversalCoreV0. Updated tests to reflect the expanded valid tier set.
slippageTolerance mapping and setSlippageTolerance setter were configured by admin but never read in swap logic. Slippage protection is caller- controlled via minPCOut. Removed mapping, setter, event, MAX_SLIPPAGE_BPS constant, and InvalidSlippageTolerance error from UniversalCore. Deprecated in UniversalCoreV0 for storage layout compatibility.
.transfer forwards only 2300 gas, blocking smart contract recipients with non-trivial receive logic. Replaced with low-level call that forwards all available gas. State is updated before the external call so reentrancy is not a concern. Added TransferFailed custom error.
… in Utils.sol Replaced string revert messages in stringToExactUInt256 with custom errors EmptyString and NonDigitCharacter for gas efficiency and structured error data. WPC was already fixed in batch 1.
…le checks Moved registration logic into internal _registerUEA. Both registerUEA (single) and registerMultipleUEA (batch) now delegate to it. Batch operations no longer repeat the onlyRole(DEFAULT_ADMIN_ROLE) check per iteration, saving gas proportional to batch size.
…ature Documentation showed 3 arguments for initializeCEA but the implementation takes 4 (pushAccount, vault, universalGateway, factory). Updated 3_CEA.md to match.
Formatting-only pass — no logic changes. Isolated here so upcoming audit-fix commits contain only surgical code changes.
…salt Added Push Chain's block.chainid as the canonical EIP-712 `salt` field in both UEA_EVM and UEA_SVM domain separators. Prevents cross-deployment signature replay across Push Chain forks or parallel deployments. Uses the canonical 5-field EIP712Domain shape (version, chainId, verifyingContract, salt) for maximum compatibility with standard wallet signers and EIP-712 tooling. Updates tests to reflect the new typehashes and domain separator reconstruction.
Per EIP-712, dynamic types (string, bytes) must be keccak256-hashed when included in the domain/struct hash. The SVM domain separator declared string chainId in the typehash but passed the raw string into abi.encode, a spec violation. Now wraps with keccak256(bytes(...)) to comply. Typehash constant is unchanged; only the encoding is fixed.
Fund-parking in _handleSingleCall now requires BOTH an empty payload AND a zero recipient. Previously, empty payload alone was sufficient, which made the park-vs-forward outcome ambiguous when a recipient was set. Requiring the vault to explicitly signal parking with address(0) removes that implicit dual meaning. The unified single-call entrypoint (vault dispatches one interface for both park and forward outcomes based on input shape) is intentional and retained — only the parking condition is tightened.
…ate path registerUEA now reverts with UEAAlreadyRegistered if an implementation is already set for the given _vmHash. A new updateUEAImplementation external function (DEFAULT_ADMIN_ROLE) provides the explicit replace path and emits UEAImplementationUpdated(vmHash, previous, new) so off-chain systems can reconstruct implementation history. Tests updated to use fresh VM hashes for first-time registrations and updateUEAImplementation for the replace path. 6 new tests cover the guard and the new function.
UniversalCore previously wrote timestampObservedAtByChainNamespace on every setChainMeta but never read it. Fee-quote functions could therefore return results derived from arbitrarily old gas data if the update pipeline stalled. This adds an opt-in per-chain staleness guard: - New mapping maxStalenessByChainNamespace (MANAGER_ROLE setter, 0 disables) - New event SetMaxStalenessByChain - New error StaleGasData(observedAt, nowTimestamp, maxAge) - Both getOutboundTxGasAndFees and getRescueFundsGasLimit call a private _validateGasDataFreshness helper after the ZeroGasPrice check; fail-closed when block.timestamp > observedAt + maxAge Feature is strictly opt-in: maxStaleness defaults to 0 which skips the check. V0 testnet contract is not modified. Coverage: 13 unit tests + 3 fuzz tests.
…back Replaces the hardcoded "42101" chainId in UEAFactory.getOriginForUEA's synthetic fallback branch with a configurable state variable pushChainId. Mainnet UEAFactory seeds it via initialize (new 3rd param); testnet UEAFactoryV0 seeds it via the setPushChainId admin setter. Adds short NatSpec on getOriginForUEA clarifying that the fallback returns a synthetic identity (not a registered origin) and callers must check isUEA before trusting the returned account.
Align documentation with deployed contract signatures, events, and role ownership. Docs-only change; no contract behaviour modified. - setChainMeta: 3-arg (observedAt derived from block.timestamp) - swapAndBurnGas: 5-arg signature and event; clarify protocol fee is routed by UniversalGatewayPC to VaultPC, not through swapAndBurnGas - pause/unpause: PAUSER_ROLE only, not admin - setWPCContractAddress -> setWPC - refundUnusedGas: full 6-arg signature - setUniswapV3Addresses: 2-arg (quoter removed) - Remove setSupportedToken and setSlippageTolerance (removed from impl) - Add setMaxStalenessByChain, setUniversalGatewayPC, setPauserRole - Fix getRescueFundsGasLimit: does not return protocolFee - Correct src/Interfaces/ path casing
Add explicit return-value checks on all IPRC20.deposit() and IPRC20.burn() call sites in UniversalCore. Reverts with PRC20OperationFailed if the token returns false instead of reverting.
…olDefaultAdminRulesUpgradeable
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.