-
Notifications
You must be signed in to change notification settings - Fork 106
Adding CompClaimAdapter #88
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
Conversation
b1001ae
to
06bf2da
Compare
|
||
// Address of COMP token | ||
// https://compound.finance/docs#networks | ||
// address public immutable compToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to delete if it's not needed
|
||
/** | ||
* @title CompClaimAdapter | ||
* @author Set Protocol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Author is yourself!
const amount: number = 1; | ||
|
||
before(async () => { | ||
await comptroller.mock.compAccrued.returns(amount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow nice, I didn't realize waffle had the ability to mock contracts and mock values
const receipt = await claim.wait(); | ||
|
||
// Get RewardClaimed event dispatched in a ClaimModule#_claim call | ||
const rewardClaimed: any = receipt.events.find((e: any): any => e.event == "RewardClaimed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the actual claimComp doesn't transfer any tokens? Ideally it would be a mock to transfer reward token to the SetToken and check the balance of:
https://github.com/SetProtocol/set-protocol-v2/blob/master/test/protocol/modules/claimModule.spec.ts#L970
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I used ComptrollerMock that was already in the repo. It's claiming function is triggering a transfer on a mocked Comp ERC20 token. New test is added below and it's checking how the balance changes after the #claim is executed.
06bf2da
to
81b4f6b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a simple claim adapter that allows managers to claim COMP from assets deposited on Compound.
It's interfacing with Comptroller and conforms to the IClaimAdapter interface used by the Set Protocol V2 ClaimModule.
Tests are using mocked Comptroller contract and are limited to ClaimAdapter <> ClaimModule integration.