SIP-135 Refactor Collateral contracts for L2 shorts by barrasso · Pull Request #1448 · Synthetixio/synthetix

barrasso · 2021-08-05T22:12:55Z

Tracking tasks here: Synthetixio/issues#264

…alInternal

…to meb-short-refactor

* Add repayAndClose * Remove extra getLoanAndAccrueInterest * SIP-149 Removing Binary Options from the protocol (#1494) * Fixes for imports around VirtualSynth to prevent issues with IERC20 (#1497) * Update short tests * Fix shorts tests on l1 fork * Fix shorts tests on l1 fork * Update ovm-ignore.json * Update ovm-ignore.json * Rename closeLoanWithCollateral Co-authored-by: Mark Barrasso <mark@synthetix.io> Co-authored-by: jj <justin@synthetix.io>

…to meb-short-refactor

codecov · 2021-09-10T16:49:24Z

Codecov Report

Merging #1448 (e830cfc) into develop (2a3cd7e) will increase coverage by 0.25%.
The diff coverage is 96.15%.

@@             Coverage Diff             @@
##           develop    #1448      +/-   ##
===========================================
+ Coverage    95.21%   95.47%   +0.25%     
===========================================
  Files           76       75       -1     
  Lines         1733     1744      +11     
  Branches       557      545      -12     
===========================================
+ Hits          1650     1665      +15     
+ Misses          83       79       -4

Impacted Files	Coverage Δ
contracts/CollateralManager.sol	`89.55% <81.81%> (+5.42%)`	⬆️
contracts/Collateral.sol	`93.33% <96.87%> (-0.65%)`	⬇️
contracts/CollateralErc20.sol	`100.00% <100.00%> (ø)`
contracts/CollateralEth.sol	`100.00% <100.00%> (ø)`
contracts/CollateralShort.sol	`100.00% <100.00%> (+9.09%)`	⬆️
contracts/MixinSystemSettings.sol	`100.00% <100.00%> (ø)`
contracts/SystemSettings.sol	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a3cd7e...e830cfc. Read the comment docs.

barrasso · 2021-09-15T18:43:50Z

For ovm deployment:

Make sure interactionDelay is set to 0.
Make sure minCollateral is set to 0.
Make sure collapseFeeRate is set to ~~15 bps~~ 0 by default.
Ensure the CollateralManager has all Collateral contracts added. Only CollateralShort should be added to L2.
Deploy shorting rewards process
Is it necessary to filter shortable synths using getSynths?

For kovan-ovm:

Make sure to upgrade sUNI and sAAVE

jjgonecrypto · 2021-09-16T11:13:14Z

publish/releases.json

 		},
+		{
+			"sip": 135,
+			"layer": "both",


Instead of this, I suggest targeting layer: ovm and for the SystemSettings configs that are loans specific, put them in a conditional that checks if the SystemSettings contract interface has the function you expect (loaded up from the ABI). We don't want to tackle a multilayer deployment just for system setting configuration.

barrasso · 2021-09-16T15:46:59Z

Some gas usage statistics for L2 shorts:

short.open: 9366958
short.withdraw: 4044849
short.draw: 5553515
short.close: 4548746

* Remove CollateralState contract * Add check to avoid zero transfer * Use SafeERC20 * Remove unnecessary isLoanOpen check * Minor fix to comments * Resolve audit fixes for CollateralManager * Add short coverage * Add more coverage to manager * Add service fee for collapsing loans * Update ovm params * Add interaction delay override * Add collapse fee rate to defaults * Update sip-135 release layer * Update configure-loans.js * Update l2 integration tests

jjgonecrypto · 2021-09-20T10:56:16Z

publish/releases.json

+				"MixinSystemSettings",
+				"ShortingRewards",
+				"SystemSettings"
+			]


you will also need to add SynthsUSD, ETH and BTC here as you want them to be instances of MultiCollateralShort.

* Enforce skew limit max * Fix manager deployment * Fix unit tests * Add synths to releases * checkout develop publish/deployed * Sync ovm params * Sync ovm synths * Update ovm config * Update local config * Update ovm params * Revert json crap Co-authored-by: Mark Barrasso <barrassomark@gmail.com>

No longer relevant

jjgonecrypto · 2021-09-22T15:13:34Z

contracts/MixinSystemSettings.sol

+        return
+            flexibleStorage().getUIntValue(
+                SETTING_CONTRACT_NAME,
+                keccak256(abi.encodePacked(SETTING_MIN_CRATIO, collateral))


[np] would it be worth using a private function to do the keccak? It might reduce the contract size impact of all these.

nvm - did some testing and seems like it doesn't improve.

jjgonecrypto

💯

barrasso added 15 commits July 22, 2021 17:17

Add useCollateral check to repay function

654b0f0

WIP: replace repay with repayWithCollateral and add repayWithCollater…

dc3c453

…alInternal

implement repay with collateral function

92e7073

minor update to comments

eaf4d8a

remove getRewards function from shorts contract

2cfbc99

decrease require string sizes

bae2bad

use collateral manager interface directly

1d45dda

add collateral utility contract

4cf0265

update deploy scripts for util contract

c8a5ed7

update test setup

a9c320d

update deploy config

9893b0f

update shorting rewards unit test

21daa47

fix util test setup

ba0b548

initial settings refactor

d0a27ca

WIP: Move loan storage from external state pattern to internal mapping

baa4b40

barrasso force-pushed the meb-short-refactor branch from d1cffb6 to baa4b40 Compare August 5, 2021 22:20

barrasso mentioned this pull request Aug 5, 2021

[TASK]: Collateral contracts refactoring Synthetixio/issues#264

Closed

22 tasks

Move accrueInterest logic to collateral manager

802620f

barrasso self-assigned this Aug 6, 2021

barrasso added contracts optimism labels Aug 6, 2021

barrasso added this to the West 4 milestone Aug 6, 2021

barrasso added 8 commits August 6, 2021 12:31

Merge branch 'refactor-settings' into meb-short-refactor

a18e8c9

Update SystemSettings contract and fix mutative functions test

5cb298f

Update Collateral mutative function test

54817cc

Add missing functions to conform to manager interface

2fdb794

wip fix Collateral Manager tests

9ff04fc

Update ovm deployment config

abeacf5

Update deploy parameter factory

0874ae7

Update remaining deployment params

236a8b5

Mark Barrasso added 2 commits September 5, 2021 15:41

Update CollateralShort.js

13a7f73

Merge branch 'develop' of https://github.com/Synthetixio/synthetix in…

46e510d

…to meb-short-refactor

barrasso force-pushed the meb-short-refactor branch from d5f108d to 46e510d Compare September 5, 2021 19:56

barrasso and others added 5 commits September 6, 2021 00:40

Merge branch 'develop' into meb-short-refactor

9a78c64

Fix merge conflicts

f7818f5

Merge branch 'develop' of https://github.com/Synthetixio/synthetix in…

17c6f4c

…to meb-short-refactor

Fix l2 integration tests

58c2b71

Merge branch 'develop' into meb-short-refactor

36c93d3

jjgonecrypto reviewed Sep 16, 2021

View reviewed changes

barrasso and others added 3 commits September 16, 2021 11:47

Merge branch 'develop' into meb-short-refactor

f0d2b13

Merge branch 'develop' into meb-short-refactor

de8dc9b

jjgonecrypto reviewed Sep 20, 2021

View reviewed changes

jj and others added 8 commits September 21, 2021 14:15

Merge branch 'develop' into meb-short-refactor

3f08fbb

Update synths.json

9a02d30

Update synths.json

60b1df4

Update releases.json

60e194d

Update releases.json

8b2386f

Merge branch 'develop' into meb-short-refactor

0604893

Update CollateralShort.js

e830cfc

jjgonecrypto reviewed Sep 22, 2021

View reviewed changes

jjgonecrypto approved these changes Sep 22, 2021

View reviewed changes

jjgonecrypto merged commit 96be7e1 into develop Sep 22, 2021

jjgonecrypto deleted the meb-short-refactor branch September 22, 2021 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SIP-135 Refactor Collateral contracts for L2 shorts#1448

SIP-135 Refactor Collateral contracts for L2 shorts#1448
jjgonecrypto merged 135 commits intodevelopfrom
meb-short-refactor

barrasso commented Aug 5, 2021

Uh oh!

codecov bot commented Sep 10, 2021 •

edited

Loading

Uh oh!

barrasso commented Sep 15, 2021 •

edited

Loading

Uh oh!

jjgonecrypto Sep 16, 2021

Uh oh!

barrasso commented Sep 16, 2021

Uh oh!

jjgonecrypto Sep 20, 2021

Uh oh!

jjgonecrypto Sep 22, 2021

Uh oh!

jjgonecrypto Sep 22, 2021

Uh oh!

jjgonecrypto left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

barrasso commented Aug 5, 2021

Uh oh!

codecov bot commented Sep 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

barrasso commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jjgonecrypto Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

barrasso commented Sep 16, 2021

Uh oh!

jjgonecrypto Sep 20, 2021

Choose a reason for hiding this comment

Uh oh!

jjgonecrypto Sep 22, 2021

Choose a reason for hiding this comment

Uh oh!

jjgonecrypto Sep 22, 2021

Choose a reason for hiding this comment

Uh oh!

jjgonecrypto left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Sep 10, 2021 •

edited

Loading

barrasso commented Sep 15, 2021 •

edited

Loading