-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update rewards apis #30
Conversation
✅ Heimdall Review Status
|
7c86eaa
to
0dc54f3
Compare
|
||
amount := big.NewInt(0) | ||
|
||
if model.GetAmount() != "" { |
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.
Is this because we are sometimes returning an empty value instead of a 0 value for a balance check?
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.
Exactly
unbondedBalance: 'Balance { amount: '' asset: 'Asset { networkId: '' assetId: '' contractAddress: '' decimals: '0' }' }
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.
AH makes sense, for solana its not a 0 value, but an nil one? If were just having the SDKs make nil -> 0, should we just return 0 from the API?
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 making it nil on the api would be better
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.
We need to bump the version in pkg/auth/transport.go
and update the Changelog & we should be g2g!
Align with api changes - StakingRewards's Date field is now a full timestamp - StakingBalance's Date field is now a full timestamp
0dc54f3
to
9e2e6f1
Compare
Approved review 2309739654 from deangalvin-cb is now dismissed due to new commit. Re-request for approval.
What changed? Why?
Align with api changes
Qualified Impact