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

Add Compound V3 client contract #46

Merged
merged 30 commits into from
May 26, 2023
Merged

Add Compound V3 client contract #46

merged 30 commits into from
May 26, 2023

Conversation

davidlaprade
Copy link
Contributor

This PR adds a flex voting client contract that is compatible with Compound V3. This was developed through support from a grant from Compound here

Currently this PR depends upon a fork of Compound V3 (split from main on May 15, 2023). The only change was to make Comet.updateBasePrincipal virtual so that we could add checkpointing in the client. My intention is to upstream this change after we're aligned on the architecture of the present PR and it is approved/merged.

This PR also abstracts the flex voting client logic into a separate abstract contract called FlexVotingClient that can be re-used for client implementations in the future. Doing so allows us to greatly simplify the ATokenFlexVoting implementation of previous PRs, e.g. #11.

Note that this PR leaves a few things unfinished, e.g.:

  • more thorough testing should be done (see TODO's at the bottom of the test file)
  • natspec comments (both for FlexVotingClient and for CometFlexVoting)
  • the Comet dependency should be updated to https://github.com/compound-finance/comet once the change is merged

@davidlaprade davidlaprade requested review from apbendi and mds1 May 15, 2023 20:17
* we are way over the size limit for Comet now -- need to look into why

* scopelint doesn't give us the option to selectively ignore naming
conventions on specific lines; because we are overriding certain Comet
functions we don't have the option to change their names
src/interfaces/IFractionalGovernor.sol Outdated Show resolved Hide resolved
src/ATokenFlexVoting.sol Outdated Show resolved Hide resolved
src/FlexVotingClient.sol Show resolved Hide resolved
src/FlexVotingClient.sol Outdated Show resolved Hide resolved
src/FlexVotingClient.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
src/ATokenFlexVoting.sol Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Outdated Show resolved Hide resolved
@davidlaprade davidlaprade requested a review from mds1 May 22, 2023 17:58
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 great @davidlaprade! A few small things, but overall this is excellent work and I'm excited to have a clean client abstraction, in addition to the Comet implementation.

src/FlexVotingClient.sol Show resolved Hide resolved
src/FlexVotingClient.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
src/CometFlexVoting.sol Show resolved Hide resolved
test/CometFlexVotingFork.t.sol Show resolved Hide resolved
src/CometFlexVoting.sol Show resolved Hide resolved
Ported over to #49
src/CometFlexVoting.sol Outdated Show resolved Hide resolved
@davidlaprade davidlaprade merged commit 2fa3de3 into master May 26, 2023
kevincheng96 pushed a commit to compound-finance/comet that referenced this pull request Jun 8, 2023
My team received a grant from CGP 2.0 to add Flexible Voting support to Compound V3.

In order to implement our extension we need to be able to determine, for any given block:

how much principal a given account has supplied up to that time, and
how much total principal has been supplied to Comet up to that time
Our belief is that the easiest way to do this involves overriding the Comet.updateBasePrincipal function to store a couple checkpoints. You can see the PR for this work in ScopeLift/flexible-voting#46, and the specific overrides we need to make here. Since updateBasePrincipal is the single way that principal balances are updated by the system, being able to add checkpointing to it greatly simplifies our task.

Thanks in advance!
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