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

fix: correct handling of non-18 decimal reward tokens #35

Merged
merged 6 commits into from
Sep 4, 2024

Conversation

jparklev
Copy link
Contributor

@jparklev jparklev commented Sep 2, 2024

No description provided.

stevenvaleri and others added 3 commits September 2, 2024 11:20
* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>
Copy link
Contributor

@stevenvaleri stevenvaleri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

FixedPointMathLib.mulWadUp(redeemableWithFee, rewardsPerPToken), redemptionFee
);

fee = fee / scalingFactor; // Downscale to reward token decimals.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just calling out that we are rounding down here. Makes sense, and is nice to round in favor of the user here.

@jparklev jparklev changed the base branch from main to fix-review September 4, 2024 00:46
@jparklev jparklev merged commit bd0a06f into fix-review Sep 4, 2024
2 checks passed
jparklev added a commit that referenced this pull request Sep 7, 2024
* fix: correct handling of non-18 decimal reward tokens (#35)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* fix: ensure reward fees are only transferred if the reward token is set, sherlock 185

* fix: correct handling of non-18 decimal reward tokens

* fix: revert rewards collection change from different branch

* fix: revert rewards collection test change from different branch

* fix: correct reversion commit

---------

Co-authored-by: Steven Valeri <[email protected]>

* fix: ensure reward fees are only transferred if the reward token is set (#36)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* fix: ensure reward fees are only transferred if the reward token is set

* refactor: don't revert on collect fees if the reward token is unset, just skip the transfer

---------

Co-authored-by: Steven Valeri <[email protected]>

* chore: trusted claimer -> trusted receiver (#37)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* chore: trusted claimer -> trusted receiver

---------

Co-authored-by: Steven Valeri <[email protected]>

* fix: separate deposit cap tracking from contract balance changes (#38)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* fix: separate deposit cap tracking from contract balance changes

* chore: move interaction below effects for deposit

---------

Co-authored-by: Steven Valeri <[email protected]>

* Chore/agpl (#40)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* chore: AGPL

---------

Co-authored-by: Steven Valeri <[email protected]>

* Chore/collect fees after effects (#39)

* Feat/mapping comment update (#18)

* feat: update mapping comments

* feat: clean comment

* Update contracts/PointTokenVault.sol

Co-authored-by: Josh Levine <[email protected]>

---------

Co-authored-by: Josh Levine <[email protected]>

* chore: collect fees after effects

---------

Co-authored-by: Steven Valeri <[email protected]>

* feat: add ExecutionFailed error and improve error handling in execute function (#42)

* chore: lock sol version to 0.8.24 (#41)

* chore: add comment about not supporting > 18 decimals (#43)

---------

Co-authored-by: Steven Valeri <[email protected]>
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.

2 participants