Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

609 enable lusd collateralization #627

Merged
merged 14 commits into from
May 17, 2023
Merged

609 enable lusd collateralization #627

merged 14 commits into from
May 17, 2023

Conversation

hashedMae
Copy link
Contributor

Resolves #609

@ubiquibot
Copy link

ubiquibot bot commented May 1, 2023

Deployment: Mon May 01 2023 19:47:21 (UTC)

@0x4007
Copy link
Member

0x4007 commented May 1, 2023 •
edited
Loading

Is there an issue with the CI? I'm on mobile so my capabilities are limited.

Could you request reviews from all the Solidity developers? It isn't easy for me to on the iOS client.

@zgorizzo69 @0xcodercrane @rndquu @Steveantor

@hashedMae
Copy link
Contributor Author

Is there an issue with the CI? I'm on mobile so my capabilities are limited.

"Compare Test Coverage" is failing, but I have no idea what that is for.

@0x4007
Copy link
Member

0x4007 commented May 3, 2023

Is there an issue with the CI? I'm on mobile so my capabilities are limited.

"Compare Test Coverage" is failing, but I have no idea what that is for.

Can you verify that the percentage of test coverage has not decreased with this pull request?

That is the intended purpose of that CI.

@hashedMae
Copy link
Contributor Author

It doesn't even get that far. Just Exit code 1

@rndquu
Copy link
Member

rndquu commented May 3, 2023

It doesn't even get that far. Just Exit code 1

I'll check it in few hours

@rndquu
Copy link
Member

rndquu commented May 3, 2023

@hashedMae

The CI action "Compare Test Coverage" fails due to decreased test coverage.

If you check the "Print coverages" step here you may see the development branch test coverage and PR's test coverage:

Development branch coverage: 38.17
PR branch coverage: 35.82

This PR makes the coverage error more verbose

Pls add more tests for the "LUSD collateralization" feature

@zgorizzo69 zgorizzo69 force-pushed the 609-enable-LUSD-collateralization branch from 2050318 to cab617d Compare May 12, 2023 17:08
@zgorizzo69 zgorizzo69 requested a review from 0x4007 May 16, 2023 07:52
hashedMae and others added 10 commits May 16, 2023 09:53
@hashedMae @zgorizzo69
feat: started forking frax contracts
bac51d7
@hashedMae @zgorizzo69
feat: fork frax pool
d935653 
- convert frax pool to diamond standard
- write facet for pool
@hashedMae @zgorizzo69
feat: frax pool fork
488ee02 
-write LibUbiquityPool
@hashedMae @zgorizzo69
feat: frax pool fork
37d331e 
-add notMintPaused modifier
-add notRedeemPaused modifier
-add setNotRedeemPaused function
-add setNotMintPaused function
@hashedMae @zgorizzo69
feat: frax pool fork
407f0b8 
-natspec
@hashedMae @zgorizzo69
feat: frax pool fork
3730d57 
-remove unneeded file
@hashedMae @zgorizzo69
fix: spelling
805eb94
@hashedMae @zgorizzo69
fix: spelling
8a36f8e
@zgorizzo69
feat(lusd): add test
70ba976
@zgorizzo69
feat(lusd): add tests
e26055e
0x4007
0x4007 requested changes May 16, 2023
View reviewed changes
Copy link
Member

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Token symbols should never be referenced in any identifiers. See more information here.

0x4007
0x4007 requested changes May 16, 2023
View reviewed changes
Copy link
Member

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Token symbols should never be referenced in any identifiers. See more information here.

@zgorizzo69 zgorizzo69 force-pushed the 609-enable-LUSD-collateralization branch from 10a4073 to c3bb34a Compare May 16, 2023 07:58
@zgorizzo69 zgorizzo69 requested a review from 0x4007 May 16, 2023 08:03
0x4007
0x4007 requested changes May 16, 2023
View reviewed changes
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Show resolved Hide resolved
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/src/dollar/interfaces/IUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/src/dollar/libraries/LibUbiquityPool.sol Outdated Show resolved Hide resolved
packages/contracts/test/diamond/DiamondTest.t.sol Show resolved Hide resolved
packages/contracts/test/diamond/DiamondTestSetup.sol Show resolved Hide resolved
packages/contracts/test/diamond/DiamondTestSetup.sol Show resolved Hide resolved
@zgorizzo69 zgorizzo69 requested a review from 0x4007 May 16, 2023 12:33
0x4007
0x4007 requested changes May 16, 2023
View reviewed changes
Copy link
Member

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good to merge after these last small adjustments. Thanks for implementing the code changes from before.

@0x4007 0x4007 enabled auto-merge May 16, 2023 16:52
@zgorizzo69 zgorizzo69 requested a review from 0x4007 May 17, 2023 16:16
@0x4007
Copy link
Member

0x4007 commented May 17, 2023

There are a few unresolved conversations. Do you know how to find them conveniently?
image

@zgorizzo69
Copy link
Contributor

There are a few unresolved conversations. Do you know how to find them conveniently? image

they are outdated so not relevant anymore

@0x4007
Copy link
Member

0x4007 commented May 17, 2023

You need to resolve all outstanding comments before GitHub merges.

@0x4007 0x4007 merged commit 8db0891 into development May 17, 2023
@0x4007 0x4007 deleted the 609-enable-LUSD-collateralization branch May 17, 2023 20:44
@0x4007
Copy link
Member

0x4007 commented May 17, 2023

image

Not sure about this curve3PoolTokenAddress

@zgorizzo69
Copy link
Contributor

image

Not sure about this curve3PoolTokenAddress

ubiquity-dollar/packages/contracts/src/dollar/facets/ManagerFacet.sol

Line 252 in 8db0891

function curve3PoolTokenAddress() external view returns (address) {
🧐 strange have you tried clean up and regenerate types again ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Steveantor Steveantor Steveantor left review comments

@zgorizzo69 zgorizzo69 zgorizzo69 left review comments

@0x4007 0x4007 0x4007 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable LUSD Collateralization
5 participants
@hashedMae @0x4007 @rndquu @zgorizzo69 @Steveantor