-
Notifications
You must be signed in to change notification settings - Fork 75
feat: Add claimFor method to AcrossMerkleDistributor #200
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
Allows a whitelisted account to claim on behalf of a user. Can be used to atomically claim and perform some action for a user, such as claim and stake.
mrice32
left a comment
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!
| require(whitelistedClaimers[msg.sender] || _claim.account == msg.sender, "invalid claimer"); | ||
| super.claim(_claim); | ||
| } | ||
|
|
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.
Maybe this should be done in a separate PR, but it's a little confusing that whitelisted claimers can claim directly to their own address and call claimMulti to send tokens to the user.
If we end up going in this direction, do you think we should drop the overridden claimMulti and claim methods, just to reduce surface area and complexity?
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.
I think we still need to require that the claim.account is the caller so users can't reset each other's referral multipliers but i can remove the whitelist requirement. Thats more so to make testing easier but there's an easier way to do this
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.
Implemented the change where the whitelist only applies for claimFor: e21526a
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, that makes sense!
mrice32
left a comment
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!
| require(whitelistedClaimers[msg.sender] || _claim.account == msg.sender, "invalid claimer"); | ||
| super.claim(_claim); | ||
| } | ||
|
|
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, that makes sense!
| require(claims[i].account == msg.sender, "invalid claimer"); | ||
| } | ||
| uint256 claimCount = claims.length; | ||
| for (uint256 i = 0; i < claimCount; i++) { |
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.
Undoes changes in https://github.com/across-protocol/contracts-v2/pull/195/files that allow a whitelisted claimer to claim on behalf of claimer
| * @param _claim leaf to claim. | ||
| */ | ||
|
|
||
| function claimFor(Claim memory _claim) public { |
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.
New claimFor method that whitelisted claimer can call
Allows a whitelisted account to claim on behalf of a user. Can be used to atomically claim and perform some action for a user, such as claim and stake.