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.

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

Aave naive approach #11

Merged
merged 15 commits into from
Nov 22, 2022
Merged

Aave naive approach #11

merged 15 commits into from
Nov 22, 2022

Conversation

davidlaprade
Copy link
Contributor

@davidlaprade davidlaprade commented Oct 9, 2022

RE #7

Rough usecase:

  • there is an ERC20, call it GOV
  • GOV is the governance token issued by Protocol
  • Protocol inherits from GovernorCountingFractional and thus will accept fractional voting
  • I deposit some GOV into Aave and receive aGOV in return
  • aGOV implements FractionalPool-like functionality
  • as proposals are made to Protocol, I can express my voting preference on them directly to aGOV
  • aGOV will then cast fractionally weighted votes to Protocol according to the expressed voting preferences of its holders

@davidlaprade davidlaprade marked this pull request as draft October 9, 2022 20:00
@davidlaprade davidlaprade changed the base branch from master to aave-v3-test-harness November 1, 2022 19:02
@davidlaprade davidlaprade changed the base branch from aave-v3-test-harness to master November 1, 2022 19:02
@davidlaprade davidlaprade force-pushed the aave-naive-approach-#7 branch 6 times, most recently from eee963c to cdecbf1 Compare November 5, 2022 16:02
@davidlaprade davidlaprade marked this pull request as ready for review November 10, 2022 21:44
src/ATokenNaive.sol Show resolved Hide resolved
src/ATokenNaive.sol Show resolved Hide resolved
Copy link
Contributor

@mds1 mds1 left a comment

Choose a reason for hiding this comment

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

Have not reviewed the tests yet, but figured I left enought comments to submit those for now. Looks good and clean overall, nice work

src/ATokenNaive.sol Outdated Show resolved Hide resolved
test/AaveAtokenFork.t.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Show resolved Hide resolved
src/ATokenNaive.sol Show resolved Hide resolved
src/ATokenNaive.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Outdated Show resolved Hide resolved
src/ATokenNaive.sol Show resolved Hide resolved
Copy link
Member

@apbendi apbendi left a comment

Choose a reason for hiding this comment

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

Looks good on my first pass @davidlaprade, nice work. Left some thoughts on the ongoing discussions.

src/ATokenNaive.sol Outdated Show resolved Hide resolved
Co-authored-by: Matt Solomon <[email protected]>
Copy link
Member

@apbendi apbendi left a comment

Choose a reason for hiding this comment

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

Looks good to me @davidlaprade—nice job on this. Getting the test harness in place was obviously a pretty tedious lift, but you did a great job! Will be really helpful as we dive into the two other approaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants