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.

Sign up for GitHub

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

Jump to bottom

Sherlock fix https://github.com/sherlock-audit/2023-03-Y2K-judging/issues/293 #136

Merged
merged 9 commits into from
May 10, 2023
Merged

Sherlock fix https://github.com/sherlock-audit/2023-03-Y2K-judging/issues/293 #136

merged 9 commits into from
May 10, 2023

Conversation

3xHarry
Copy link
Contributor

@3xHarry 3xHarry commented Apr 10, 2023

No description provided.

@3xHarry 3xHarry mentioned this pull request Apr 11, 2023
Open
IAm0x52
IAm0x52 reviewed May 5, 2023
View reviewed changes
Copy link

@IAm0x52 IAm0x52 left a comment

Choose a reason for hiding this comment

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

Needs additional changes. L423 doesn't make sense to me. queue[index].assets is in shares and entitledAmount isn't but they are subtracted directly.

@3xHarry
Copy link
Contributor Author

3xHarry commented May 6, 2023

@IAm0x52 tagging you again here sorry: thx for your comment, basically since QueueItem.asset (later renamed to shares) represents share of the epoch, i converted relayerFee which is denominated in underlying asset to shares for that epoch.
Later i realized that Users only want to rollover their original deposit value, therefore i burn the original deposit Value and mint this value - relayerFeeInShares into the next epoch, the winnings sahres amount are left in the epoch to be withdrawn

@3xHarry
Copy link
Contributor Author

3xHarry commented May 10, 2023

@IAm0x52 thx for noticing the relayerFeeInShares Bug I will close this PR, but fix can be observed in 9165674

@3xHarry 3xHarry merged commit 700dfbf into sherlock-fix-11 May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IAm0x52 IAm0x52 IAm0x52 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@3xHarry @IAm0x52