search

pickle-finance / protocol

Pickle Protocols
MIT License
128 stars 74 forks source link

Save gas by skipping 0 value withdrawals and performance fees. #73

Closed bayesianmind closed 3 years ago

bayesianmind commented 3 years ago

The strategy base currently doesn't check if fees are 0 before doing transfers. This really racks up gas - the 0 value transfer is way more expensive than a "if 0, skip" check. These two withdrawal fees are currently 0, and seems like they plan to stay that way. Fixes #71.

bayesianmind commented 3 years ago

Here's the difference in gas costs for the dapp test run @ master vs this patch. For example: test_ethusdtv3_1_get_earn_harvest_rewards() (gas: 1276679 - > 1258989) test_ethusdtv3_1_withdraw_release() (gas: 1239813 -> 1231935)

Full wdiff:

[PASS] test_jar_converter_uni_curve_0_0() (gas: 268485922753888)268485922753931)
[PASS] test_jar_converter_uni_curve_2_2() (gas: 2621550)2622498)
[PASS] test_jar_converter_uni_curve_3_0() (gas: 2601309)2602257)
[PASS] test_jar_converter_uni_curve_1_2() (gas: 2640188)2641136)
[PASS] test_jar_converter_uni_curve_3_2() (gas: 2264520)2265468)
[PASS] test_jar_converter_uni_curve_0_2() (gas: 2595021)2595969)
[PASS] test_jar_converter_uni_curve_4_1() (gas: 2944584)2945532)
[PASS] test_jar_converter_uni_curve_1_0() (gas: 2641726)2642674)
[PASS] test_jar_converter_uni_curve_2_0() (gas: 2623065)2624013)
[PASS] test_jar_converter_uni_curve_1_1() (gas: 2955046)2955994)
[PASS] test_jar_converter_uni_curve_3_1() (gas: 2944517)2945465)
[PASS] test_jar_converter_uni_curve_0_1() (gas: 2930790)2931738)
[PASS] test_jar_converter_uni_curve_2_1() (gas: 2603554)2604502)
@@ -102,4 +102,4 @@ Running 4 tests for src/tests/jar-converters/uni-uni.test.sol:StrategyUniUniJarS
[PASS] test_jar_converter_uni_uni_2() (gas: 2517664)2518612)
[PASS] test_jar_converter_uni_uni_3() (gas: 2522984)2523932)
[PASS] test_jar_converter_uni_uni_0() (gas: 2535716)2536664)
[PASS] test_jar_converter_uni_uni_1() (gas: 2513734)2514682)
@@ -117 +117 @@ Running 2 tests for src/tests/strategies/barnbridge/strategy-bond-usdc-v1.sol:St
[PASS] test_bond_usdc_v1_earn_harvest_rewards() (gas: 1108786)1090640)
@@ -187 +187 @@ Running 11 tests for src/tests/strategies/compound/strategy-cmpnd-dai-v3.sol:Str
[PASS] test_cmpnd_dai_v3_withdrawSome() (gas: 4935022)4893795)
@@ -199 +199 @@ Running 3 tests for src/tests/strategies/convex/strategy-sushi-cvx-eth-lp.test.s
[PASS] test_ethcvx_get_earn_harvest_rewards() (gas: 1405938)1388248)
@@ -201 +201 @@ Running 3 tests for src/tests/strategies/convex/strategy-sushi-cvx-eth-lp.test.s
[PASS] test_ethcvx_withdraw_release() (gas: 1368854)1360976)
@@ -252 +252 @@ Running 2 tests for src/tests/strategies/curve/strategy-curve-stecrv.test.sol:St
[PASS] test_stecrv_get_earn_harvest_rewards() (gas: 2181074)2165014)
@@ -260,2 +260,2 @@ Running 3 tests for src/tests/strategies/fei/strategy-fei-tribe-lp.test.sol:Stra
[PASS] test_feitribev1_withdraw_release() (gas: 1275643)1267803)
[PASS] test_feitribev1_get_earn_harvest_rewards() (gas: 1312423)1294843)
@@ -278,2 +278,2 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-aapl-ust-lp.test
[PASS] test_aaplustv3_1_withdraw_release() (gas: 1390603)1382761)
[PASS] test_aaplustv3_1_get_earn_harvest_rewards() (gas: 1426977)1409395)
@@ -282,2 +282,2 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-baba-ust-lp.test
[PASS] test_babaustv3_1_withdraw_release() (gas: 1390843)1383001)
[PASS] test_babaustv3_1_get_earn_harvest_rewards() (gas: 1427195)1409613)
@@ -287 +287 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-mir-ust-lp.test.
[PASS] test_mirustv3_1_withdraw_release() (gas: 1295308)1287468)
@@ -289 +289 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-mir-ust-lp.test.
[PASS] test_mirustv3_1_get_earn_harvest_rewards() (gas: 1331594)1314014)
@@ -293,2 +293,2 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-qqq-ust-lp.test.
[PASS] test_qqqustv3_1_get_earn_harvest_rewards() (gas: 1427195)1409613)
[PASS] test_qqqustv3_1_withdraw_release() (gas: 1390909)1383067)
@@ -297 +297 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-slv-ust-lp.test.
[PASS] test_slvustv3_1_withdraw_release() (gas: 1390953)1383111)
@@ -299 +299 @@ Running 3 tests for src/tests/strategies/mirror/strategy-mirror-slv-ust-lp.test.
[PASS] test_slvustv3_1_get_earn_harvest_rewards() (gas: 1427283)1409701)
@@ -371,2 +371,2 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-dai-lp.tes
[PASS] test_ethdaiv3_1_withdraw_release() (gas: 1222267)1214389)
[PASS] test_ethdaiv3_1_get_earn_harvest_rewards() (gas: 1259089)1241399)
@@ -376,2 +376,2 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-sushi-lp.t
[PASS] test_ethsushiv3_1_get_earn_harvest_rewards() (gas: 1233088)1215398)
[PASS] test_ethsushiv3_1_withdraw_release() (gas: 1196222)1188344)
@@ -381,2 +381,2 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-usdc-lp.te
[PASS] test_ethusdcv3_1_withdraw_release() (gas: 1253903)1246025)
[PASS] test_ethusdcv3_1_get_earn_harvest_rewards() (gas: 1290747)1273057)
@@ -386,2 +386,2 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-usdt-lp.te
[PASS] test_ethusdtv3_1_withdraw_release() (gas: 1239813)1231935)
[PASS] test_ethusdtv3_1_get_earn_harvest_rewards() (gas: 1276679)1258989)
@@ -391,2 +391,2 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-wbtc-lp.te
[PASS] test_ethwbtcv1_withdraw_release() (gas: 1212897)1205019)
[PASS] test_ethwbtcv1_get_earn_harvest_rewards() (gas: 1249807)1232117)
@@ -396 +396 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-yfi-lp.tes
[PASS] test_ethyfiv1_get_earn_harvest_rewards() (gas: 1254071)1236381)
@@ -398 +398 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-yfi-lp.tes
[PASS] test_ethyfiv1_withdraw_release() (gas: 1217293)1209415)
@@ -416 +416 @@ Running 3 tests for src/tests/strategies/sushiswap/strategy-sushi-eth-yvecrv-lp.
[PASS] test_ethvecrvv3_1_get_earn_harvest_rewards() (gas: 855148)844281)
@@ -426 +426 @@ Running 3 tests for src/tests/strategies/uniswapv2/strategy-uni-eth-dai-lp-v4.te
[PASS] test_ethdaiv3_1_get_earn_harvest_rewards() (gas: 728483)717691)
@@ -437 +437 @@ Running 3 tests for src/tests/strategies/uniswapv2/strategy-uni-eth-usdc-lp-v4.t
[PASS] test_ethusdcv3_1_get_earn_harvest_rewards() (gas: 754818)744026)
@@ -448 +448 @@ Running 3 tests for src/tests/strategies/uniswapv2/strategy-uni-eth-usdt-lp-v4.t
[PASS] test_ethusdtv3_1_get_earn_harvest_rewards() (gas: 747276)736484)

As you can see, harvest() and withdrawals are cheaper, though the savings isn't as large as I thought it would be.

bayesianmind commented 3 years ago

@larrythecucumber321 I saw your comments in email, but not on github for some reason..

I made the logic more compact and minimally changed from the previous one. All it does now is check if the fee settings are > 0 before doing the erc20 transfer.