diff --git a/.gas-snapshot b/.gas-snapshot index 54484d18..32dd89c2 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -22,18 +22,19 @@ Conditions:test_isPriceAbove() (gas: 19098) Conditions:test_isPriceBelow() (gas: 19092) Conditions:test_isTimestampAfter() (gas: 7711) Conditions:test_isTimestampBefore() (gas: 7579) -DeploymentTest:test_deploy() (gas: 4835682) -DeploymentTest:test_deploy_oracle_zero_address() (gas: 1903169) -DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 1903085) -DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 1903152) -DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 1903165) -DeploymentTest:test_deploy_trusted_forwarder_zero_address() (gas: 37498) -Deposit:test_depositEth() (gas: 40987) -Deposit:test_depositEth_event() (gas: 42547) -Deposit:test_depositEth_fuzz(uint256,uint128) (runs: 256, μ: 39369, ~: 40409) -Deposit:test_depositEth_via_trustedForwarder() (gas: 68338) -Deposit:test_depositEth_via_trustedForwarder_value_mismatch() (gas: 73201) -Deposit:test_depositEth_via_trustedForwarder_value_mismatch_allow_failure() (gas: 72445) +DeploymentTest:test_deploy() (gas: 4890231) +DeploymentTest:test_deploy_oracle_zero_address() (gas: 1903233) +DeploymentTest:test_deploy_perps_market_proxy_zero_address() (gas: 1903149) +DeploymentTest:test_deploy_spot_market_proxy_zero_address() (gas: 1903216) +DeploymentTest:test_deploy_susd_proxy_zero_address() (gas: 1903229) +DeploymentTest:test_deploy_trusted_forwarder_zero_address() (gas: 37562) +Deposit:test_depositEth() (gas: 55255) +Deposit:test_depositEth_Account_Doesnt_Exist() (gas: 34271) +Deposit:test_depositEth_event() (gas: 56815) +Deposit:test_depositEth_fuzz(uint256,uint128) (runs: 256, μ: 36476, ~: 34889) +Deposit:test_depositEth_via_trustedForwarder() (gas: 85374) +Deposit:test_depositEth_via_trustedForwarder_value_mismatch() (gas: 87469) +Deposit:test_depositEth_via_trustedForwarder_value_mismatch_allow_failure() (gas: 86713) DepositCollateral:test_depositCollateral() (gas: 258530) DepositCollateral:test_depositCollateral_availableMargin() (gas: 266098) DepositCollateral:test_depositCollateral_collateralAmount() (gas: 259106) @@ -45,9 +46,9 @@ Execute:test_execute_CannotExecuteOrder_invalid_settlementStrategyId() (gas: 968 Execute:test_execute_CannotExecuteOrder_too_leveraged() (gas: 361524) Execute:test_execute_event() (gas: 433933) Execute:test_execute_order_committed() (gas: 430425) -Fee:test_fee_exceeds_account_credit() (gas: 53639) -Fee:test_fee_exceeds_maxExecutorFee() (gas: 53218) -Fee:test_fee_imposed() (gas: 465846) +Fee:test_fee_exceeds_account_credit() (gas: 67907) +Fee:test_fee_exceeds_maxExecutorFee() (gas: 67486) +Fee:test_fee_imposed() (gas: 468614) MathLibTest:test_abs128() (gas: 448) MathLibTest:test_abs256() (gas: 480) MathLibTest:test_castU128() (gas: 350) @@ -73,12 +74,12 @@ VerifySignature:test_verifySignature(uint256) (runs: 256, μ: 24402, ~: 24402) VerifySignature:test_verifySignature_false_private_key() (gas: 27079) VerifySigner:test_verifySigner() (gas: 25862) VerifySigner:test_verifySigner_false() (gas: 28614) -Withdraw:test_withdrawEth() (gas: 50332) -Withdraw:test_withdrawEth_EthTransferFailed() (gas: 78823) -Withdraw:test_withdrawEth_InsufficientEthBalance() (gas: 57013) -Withdraw:test_withdrawEth_Unauthorized() (gas: 56652) -Withdraw:test_withdrawEth_event() (gas: 49576) -Withdraw:test_withdrawEth_fuzz(uint256) (runs: 256, μ: 68193, ~: 68743) +Withdraw:test_withdrawEth() (gas: 52547) +Withdraw:test_withdrawEth_EthTransferFailed() (gas: 93091) +Withdraw:test_withdrawEth_InsufficientEthBalance() (gas: 59781) +Withdraw:test_withdrawEth_Unauthorized() (gas: 59420) +Withdraw:test_withdrawEth_event() (gas: 51791) +Withdraw:test_withdrawEth_fuzz(uint256) (runs: 256, μ: 70961, ~: 71511) WithdrawCollateral:test_withdrawCollateral() (gas: 353254) WithdrawCollateral:test_withdrawCollateral_availableMargin() (gas: 354755) WithdrawCollateral:test_withdrawCollateral_collateralAmount() (gas: 353747) diff --git a/lcov.info b/lcov.info index 1cd11278..8b9298b6 100644 --- a/lcov.info +++ b/lcov.info @@ -1,21 +1,5 @@ TN: SF:script/Deploy.s.sol -FN:133,DeployOptimism_Synthetix.run -FNDA:0,DeployOptimism_Synthetix.run -DA:134,0 -DA:135,0 -DA:137,0 -DA:144,0 -FN:54,DeployBase_Synthetix.run -FNDA:0,DeployBase_Synthetix.run -DA:55,0 -DA:56,0 -DA:58,0 -DA:65,0 -FN:26,Setup.deploySystem -FNDA:5,Setup.deploySystem -DA:38,5 -DA:40,5 FN:73,DeployBaseGoerli_Synthetix.run FNDA:0,DeployBaseGoerli_Synthetix.run DA:74,0 @@ -34,6 +18,22 @@ DA:96,0 DA:97,0 DA:99,0 DA:106,0 +FN:133,DeployOptimism_Synthetix.run +FNDA:0,DeployOptimism_Synthetix.run +DA:134,0 +DA:135,0 +DA:137,0 +DA:144,0 +FN:54,DeployBase_Synthetix.run +FNDA:0,DeployBase_Synthetix.run +DA:55,0 +DA:56,0 +DA:58,0 +DA:65,0 +FN:26,Setup.deploySystem +FNDA:5,Setup.deploySystem +DA:38,5 +DA:40,5 FN:114,DeployBaseGoerli_Andromeda.run FNDA:0,DeployBaseGoerli_Andromeda.run DA:115,0 @@ -59,211 +59,215 @@ FN:147,Engine._isAccountOwnerOrDelegate FNDA:283,Engine._isAccountOwnerOrDelegate DA:152,283 FN:162,Engine.depositEth -FNDA:526,Engine.depositEth -DA:163,526 -DA:165,526 -FN:169,Engine.withdrawEth +FNDA:527,Engine.depositEth +DA:165,527 +BRDA:165,0,0,243 +BRDA:165,0,1,284 +DA:166,243 +DA:169,284 +DA:171,284 +FN:175,Engine.withdrawEth FNDA:261,Engine.withdrawEth -DA:173,261 -DA:175,261 -BRDA:175,0,0,1 -BRDA:175,0,1,260 -DA:177,260 -DA:179,258 -FN:186,Engine._withdrawEth +DA:179,261 +DA:181,261 +BRDA:181,1,0,1 +BRDA:181,1,1,260 +DA:183,260 +DA:185,258 +FN:192,Engine._withdrawEth FNDA:272,Engine._withdrawEth -DA:191,272 -BRDA:191,1,0,1 -BRDA:191,1,1,271 -DA:194,271 -DA:196,271 +DA:195,272 +BRDA:195,2,0,1 +BRDA:195,2,1,271 DA:198,271 -BRDA:198,2,0,1 -BRDA:198,2,1,270 -FN:206,Engine.invalidateUnorderedNonces +DA:200,271 +DA:202,271 +BRDA:202,3,0,1 +BRDA:202,3,1,270 +FN:210,Engine.invalidateUnorderedNonces FNDA:261,Engine.invalidateUnorderedNonces -DA:211,261 -BRDA:211,3,0,261 -BRDA:211,3,1,- -DA:217,261 -DA:219,261 -DA:221,0 -FN:226,Engine.hasUnorderedNonceBeenUsed +DA:215,261 +BRDA:215,4,0,261 +BRDA:215,4,1,- +DA:221,261 +DA:223,261 +DA:225,0 +FN:230,Engine.hasUnorderedNonceBeenUsed FNDA:260,Engine.hasUnorderedNonceBeenUsed -DA:232,279 DA:236,279 -DA:250,279 -FN:260,Engine._bitmapPositions +DA:240,279 +DA:254,279 +FN:264,Engine._bitmapPositions FNDA:291,Engine._bitmapPositions -DA:267,291 DA:271,291 -FN:277,Engine._useUnorderedNonce +DA:275,291 +FN:281,Engine._useUnorderedNonce FNDA:12,Engine._useUnorderedNonce -DA:278,12 DA:282,12 -DA:289,12 -DA:309,12 -BRDA:309,4,0,- -BRDA:309,4,1,12 -FN:317,Engine.modifyCollateral +DA:286,12 +DA:293,12 +DA:313,12 +BRDA:313,5,0,- +BRDA:313,5,1,12 +FN:321,Engine.modifyCollateral FNDA:17,Engine.modifyCollateral -DA:322,17 -DA:324,17 DA:326,17 -BRDA:326,5,0,11 -BRDA:326,5,1,6 -DA:327,11 -DA:331,6 -BRDA:331,6,0,- -BRDA:331,6,1,6 -DA:332,6 -FN:338,Engine._depositCollateral +DA:328,17 +DA:330,17 +BRDA:330,6,0,11 +BRDA:330,6,1,6 +DA:331,11 +DA:335,6 +BRDA:335,7,0,- +BRDA:335,7,1,6 +DA:336,6 +FN:342,Engine._depositCollateral FNDA:11,Engine._depositCollateral -DA:346,11 -DA:348,10 -DA:350,10 -FN:353,Engine._withdrawCollateral +DA:350,11 +DA:352,10 +DA:354,10 +FN:357,Engine._withdrawCollateral FNDA:6,Engine._withdrawCollateral -DA:360,6 -DA:363,4 -FN:369,Engine._getSynthAddress +DA:364,6 +DA:367,4 +FN:373,Engine._getSynthAddress FNDA:17,Engine._getSynthAddress -DA:374,17 -FN:384,Engine.commitOrder +DA:378,17 +FN:388,Engine.commitOrder FNDA:3,Engine.commitOrder -DA:398,3 -BRDA:398,7,0,3 -BRDA:398,7,1,- -DA:399,3 -DA:409,0 -FN:413,Engine._commitOrder +DA:402,3 +BRDA:402,8,0,3 +BRDA:402,8,1,- +DA:403,3 +DA:413,0 +FN:417,Engine._commitOrder FNDA:13,Engine._commitOrder -DA:422,13 -FN:440,Engine.execute +DA:426,13 +FN:444,Engine.execute FNDA:14,Engine.execute -DA:455,14 -BRDA:455,8,0,2 -BRDA:455,8,1,12 -DA:458,12 -DA:464,12 +DA:459,14 +BRDA:459,9,0,2 +BRDA:459,9,1,12 +DA:462,12 DA:468,12 -DA:471,12 -BRDA:471,9,0,2 -BRDA:471,9,1,3 -DA:472,5 -DA:477,5 -BRDA:477,10,0,1 -BRDA:477,10,1,4 -DA:478,1 -DA:482,4 -BRDA:482,11,0,1 -BRDA:482,11,1,3 -DA:483,1 -DA:489,3 -BRDA:489,12,0,2 -BRDA:489,12,1,3 -DA:499,2 -DA:504,10 -DA:514,7 -FN:522,Engine.canExecute +DA:472,12 +DA:475,12 +BRDA:475,10,0,2 +BRDA:475,10,1,3 +DA:476,5 +DA:481,5 +BRDA:481,11,0,1 +BRDA:481,11,1,4 +DA:482,1 +DA:486,4 +BRDA:486,12,0,1 +BRDA:486,12,1,3 +DA:487,1 +DA:493,3 +BRDA:493,13,0,2 +BRDA:493,13,1,3 +DA:503,2 +DA:508,10 +DA:518,7 +FN:526,Engine.canExecute FNDA:9,Engine.canExecute -DA:528,23 -BRDA:528,13,0,2 -BRDA:528,13,1,21 -DA:531,21 -BRDA:531,14,0,2 -BRDA:531,14,1,19 -DA:534,19 -BRDA:534,15,0,2 -BRDA:534,15,1,17 -DA:535,2 -DA:539,17 -BRDA:539,16,0,1 -BRDA:539,16,1,16 -DA:542,16 -BRDA:542,17,0,1 -BRDA:542,17,1,15 -DA:545,15 -BRDA:545,18,0,- -BRDA:545,18,1,- -DA:548,0 -BRDA:548,19,0,- -BRDA:548,19,1,- -DA:552,15 -BRDA:552,20,0,1 -BRDA:552,20,1,14 -DA:555,14 -FN:563,Engine.verifySigner +DA:532,23 +BRDA:532,14,0,2 +BRDA:532,14,1,21 +DA:535,21 +BRDA:535,15,0,2 +BRDA:535,15,1,19 +DA:538,19 +BRDA:538,16,0,2 +BRDA:538,16,1,17 +DA:539,2 +DA:543,17 +BRDA:543,17,0,1 +BRDA:543,17,1,16 +DA:546,16 +BRDA:546,18,0,1 +BRDA:546,18,1,15 +DA:549,15 +BRDA:549,19,0,- +BRDA:549,19,1,- +DA:552,0 +BRDA:552,20,0,- +BRDA:552,20,1,- +DA:556,15 +BRDA:556,21,0,1 +BRDA:556,21,1,14 +DA:559,14 +FN:567,Engine.verifySigner FNDA:2,Engine.verifySigner -DA:569,19 -FN:573,Engine.verifySignature +DA:573,19 +FN:577,Engine.verifySignature FNDA:257,Engine.verifySignature -DA:577,273 -FN:583,Engine.verifyConditions +DA:581,273 +FN:587,Engine.verifyConditions FNDA:4,Engine.verifyConditions -DA:589,4 -DA:590,4 -BRDA:590,21,0,1 -BRDA:590,21,1,3 -DA:591,1 -DA:594,3 -DA:595,13 -DA:596,13 +DA:593,4 +DA:594,4 +BRDA:594,22,0,1 +BRDA:594,22,1,3 +DA:595,1 +DA:598,3 DA:599,13 -DA:604,13 -DA:605,11 -DA:606,9 -DA:607,7 -DA:608,5 -DA:609,4 -DA:610,3 -DA:611,2 -BRDA:603,22,0,1 -BRDA:603,22,1,11 -DA:614,12 -DA:617,12 -BRDA:617,23,0,1 -BRDA:617,23,1,11 -DA:620,11 -DA:623,1 +DA:600,13 +DA:603,13 +DA:608,13 +DA:609,11 +DA:610,9 +DA:611,7 +DA:612,5 +DA:613,4 +DA:614,3 +DA:615,2 +BRDA:607,23,0,1 +BRDA:607,23,1,11 +DA:618,12 +DA:621,12 +BRDA:621,24,0,1 +BRDA:621,24,1,11 +DA:624,11 DA:627,1 -FN:635,Engine.isTimestampAfter +DA:631,1 +FN:639,Engine.isTimestampAfter FNDA:5,Engine.isTimestampAfter -DA:641,5 -FN:645,Engine.isTimestampBefore +DA:645,5 +FN:649,Engine.isTimestampBefore FNDA:5,Engine.isTimestampBefore -DA:651,5 -FN:655,Engine.isPriceAbove +DA:655,5 +FN:659,Engine.isPriceAbove FNDA:6,Engine.isPriceAbove -DA:660,6 -DA:665,6 -FN:669,Engine.isPriceBelow +DA:664,6 +DA:669,6 +FN:673,Engine.isPriceBelow FNDA:6,Engine.isPriceBelow -DA:674,6 -DA:679,6 -FN:683,Engine.isMarketOpen +DA:678,6 +DA:683,6 +FN:687,Engine.isMarketOpen FNDA:3,Engine.isMarketOpen -DA:689,3 -FN:693,Engine.isPositionSizeAbove +DA:693,3 +FN:697,Engine.isPositionSizeAbove FNDA:4,Engine.isPositionSizeAbove -DA:698,4 -DA:699,4 -DA:701,4 -FN:705,Engine.isPositionSizeBelow +DA:702,4 +DA:703,4 +DA:705,4 +FN:709,Engine.isPositionSizeBelow FNDA:4,Engine.isPositionSizeBelow -DA:710,4 -DA:711,4 -DA:713,4 -FN:717,Engine.isOrderFeeBelow +DA:714,4 +DA:715,4 +DA:717,4 +FN:721,Engine.isOrderFeeBelow FNDA:4,Engine.isOrderFeeBelow -DA:723,4 -DA:728,4 +DA:727,4 +DA:732,4 FNF:29 FNH:29 -LF:103 -LH:100 -BRF:48 -BRH:40 +LF:105 +LH:102 +BRF:50 +BRH:42 end_of_record TN: SF:src/libraries/ConditionalOrderHashLib.sol @@ -469,6 +473,12 @@ DA:117,0 DA:118,0 DA:125,0 DA:133,0 +FN:146,BootstrapOptimismGoerli.init +FNDA:0,BootstrapOptimismGoerli.init +DA:150,0 +DA:151,0 +DA:158,0 +DA:166,0 FN:41,Bootstrap.initializeOptimismGoerli FNDA:0,Bootstrap.initializeOptimismGoerli DA:42,0 @@ -507,12 +517,6 @@ DA:100,0 DA:101,0 DA:106,0 DA:108,0 -FN:146,BootstrapOptimismGoerli.init -FNDA:0,BootstrapOptimismGoerli.init -DA:150,0 -DA:151,0 -DA:158,0 -DA:166,0 FNF:4 FNH:0 LF:42 diff --git a/src/Engine.sol b/src/Engine.sol index 85f4b504..4cf0d021 100644 --- a/src/Engine.sol +++ b/src/Engine.sol @@ -160,6 +160,12 @@ contract Engine is IEngine, EIP712, EIP7412, ERC2771Context { /// @inheritdoc IEngine function depositEth(uint128 _accountId) external payable override { + // ensure account exists (i.e. owner is not the zero address) + /// @notice this does not check if the caller is the account owner + if (PERPS_MARKET_PROXY.getAccountOwner(_accountId) == address(0)) { + revert AccountDoesNotExist(); + } + ethBalances[_accountId] += msg.value; emit EthDeposit(_accountId, msg.value); @@ -170,7 +176,7 @@ contract Engine is IEngine, EIP712, EIP7412, ERC2771Context { external override { - address payable caller = payable(_msgSender()); + address caller = _msgSender(); if (!isAccountOwner(_accountId, caller)) revert Unauthorized(); @@ -183,11 +189,9 @@ contract Engine is IEngine, EIP712, EIP7412, ERC2771Context { /// @dev UNSAFE to call directly; use `withdrawEth` instead /// @param _caller the caller of the function /// @param _accountId the account id to debit ETH from - function _withdrawEth( - address payable _caller, - uint128 _accountId, - uint256 _amount - ) internal { + function _withdrawEth(address _caller, uint128 _accountId, uint256 _amount) + internal + { if (_amount > ethBalances[_accountId]) revert InsufficientEthBalance(); // decrement the ETH balance of the account prior to transferring ETH to the caller @@ -461,7 +465,7 @@ contract Engine is IEngine, EIP712, EIP7412, ERC2771Context { /// @dev the fee is denoted in ETH and is paid to the caller (conditional order executor) /// @dev the fee does not exceed the max fee set by the conditional order and /// this is enforced by the `canExecute` function - _withdrawEth(payable(_msgSender()), _co.orderDetails.accountId, _fee); + _withdrawEth(_msgSender(), _co.orderDetails.accountId, _fee); /// @notice get size delta from order details /// @dev up to the caller to not waste gas by passing in a size delta of zero diff --git a/src/interfaces/IEngine.sol b/src/interfaces/IEngine.sol index 6896da0e..902807db 100644 --- a/src/interfaces/IEngine.sol +++ b/src/interfaces/IEngine.sol @@ -77,6 +77,9 @@ interface IEngine { /// @notice thrown when attempting to transfer eth fails error EthTransferFailed(); + /// @notice thrown when attempting to deposit eth into an account that does not exist + error AccountDoesNotExist(); + /*////////////////////////////////////////////////////////////// EVENTS //////////////////////////////////////////////////////////////*/ diff --git a/test/EthManagement.t.sol b/test/EthManagement.t.sol index 201397c9..e0c29e35 100644 --- a/test/EthManagement.t.sol +++ b/test/EthManagement.t.sol @@ -29,6 +29,14 @@ contract Deposit is EthManagementTest { assertEq(address(engine).balance, AMOUNT); } + function test_depositEth_Account_Doesnt_Exist() public { + uint128 invalidAccountId = type(uint128).max; + assert(perpsMarketProxy.getAccountOwner(invalidAccountId) == address(0)); + + vm.expectRevert(IEngine.AccountDoesNotExist.selector); + engine.depositEth{value: AMOUNT}(invalidAccountId); + } + function test_depositEth_via_trustedForwarder() public { TrustedMulticallForwarder.Call3Value memory call = TrustedMulticallForwarder.Call3Value( @@ -112,10 +120,15 @@ contract Deposit is EthManagementTest { ) public { vm.assume(fuzzedEthAmount < address(this).balance - 1 ether); - engine.depositEth{value: fuzzedEthAmount}(fuzzedAccountId); + if (perpsMarketProxy.getAccountOwner(fuzzedAccountId) == address(0)) { + vm.expectRevert(IEngine.AccountDoesNotExist.selector); + engine.depositEth{value: fuzzedEthAmount}(fuzzedAccountId); + } else { + engine.depositEth{value: fuzzedEthAmount}(fuzzedAccountId); - assertEq(engine.ethBalances(fuzzedAccountId), fuzzedEthAmount); - assertEq(address(engine).balance, fuzzedEthAmount); + assertEq(engine.ethBalances(fuzzedAccountId), fuzzedEthAmount); + assertEq(address(engine).balance, fuzzedEthAmount); + } } function test_depositEth_event() public {