ELO-335 -- lowering claim tokenAddresses to optional; using distribution to only query claimable tokens. #223
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bdchatham
force-pushed
the
FromTheRain-elo-335-optional-claim-token-addresses
branch
2 times, most recently
from
e367f44
to
e6086ae
Compare
pkg/rewards/claim.go
Outdated
| ) | ||
|
|
||
| const zero_address = "0x0000000000000000000000000000000000000000" |
There was a problem hiding this comment.
There was a problem hiding this comment.
also just for future. Go variables should never have _ - idiomatic is CamelCase
bdchatham
force-pushed
the
FromTheRain-elo-335-optional-claim-token-addresses
branch
from
c467b0c
to
1594d0e
Compare
pkg/rewards/claim.go
Outdated
| @@ -121,10 +123,15 @@ func Claim(cCtx *cli.Context, p utils.Prompter) error { | |||
| return eigenSdkUtils.WrapError("failed to fetch claim amounts for date", err) | |||
| } | |||
|
|
|||
| claimableTokens, present := proofData.Distribution.GetTokensForEarner(config.EarnerAddress) | |||
| if !present { | |||
| return eigenSdkUtils.WrapError("no tokens claimable by earner", nil) | |||
There was a problem hiding this comment.
nit: you can just do errors.New since you are anyway passing nil and there' no need to wrap errors.
…ry tokens which have pending rewards.
bdchatham
force-pushed
the
FromTheRain-elo-335-optional-claim-token-addresses
branch
from
ab28791
to
7881374
Compare
shrimalmadhur
approved these changes
Oct 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
#220
Prior to this change, users trying to claim rewards needed to know the token addresses they intend to claim rewards for. When no rewards are present, they receive errors and logs indicating the issue. They must iteratively remove token addresses from their input until they've reduced the set to addresses with claimable rewards. Also, they are required to know the token address of the rewards they want to claim. Users need a mechanism in the CLI to address these pain points.
What Changed?
This change brings two behavioral changes to the API.
tokenAddressesis provided, or it's empty, the CLI uses the public rewards distribution to claim all available rewards.tokenAddressesare provided, the CLI uses the public rewards distribution to claim rewards only for the token addresses with claimable rewards.Testing
Added unit test coverage.