Skip to content

Conversation

shark0der
Copy link
Contributor

@shark0der shark0der commented Oct 4, 2022

Context

Fixes #299

When the pool is new, the function was always skipping the updates, never updating any value. Because of this early exit the function was never updating anything.

Changes proposed in this pull request

The new implementation will stil exit early without updating anything if a forced update was not requested (updateUntilCurrentTimestamp = true). A force update should be requested if the function calling updateTranches changes the share supply. If the share supply is not being changed - we should be fine skipping the update i.e. avoid any writes.

I'm not entirely sure there aren't any edge cases that I didn't account for. Please review carefully.

Test plan

No tests present for staking at the moment.

Checklist

  • Rebased the base branch
  • Attached corresponding Github issue
  • Prefixed the name with the type of change (i.e. feat, chore, test)
  • Performed a self-review of my own code
  • Followed the style guidelines of this project
  • Made corresponding changes to the documentation
  • Didn't generate new warnings
  • Didn't generate failures on existing tests
  • Added tests that prove my fix is effective or that my feature works

Review

When reviewing a PR, please indicate intention in comments using the following emojis:

  • 🍰 = Nice to have but not essential.
  • 💡 = Suggestion or a comment based on personal opinion
  • 🔨 = I believe this should be changed.
  • 🤔 = I don’t understand something, do you mind giving me more context?
  • 🚀 = Feedback

@shark0der shark0der marked this pull request as ready for review October 4, 2022 13:15
@shark0der shark0der changed the title Fix: update tranches Fix: update tranches not performing any updates Oct 4, 2022
@shark0der shark0der linked an issue Oct 4, 2022 that may be closed by this pull request
Copy link
Contributor

@0xdewy 0xdewy left a comment

Choose a reason for hiding this comment

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

Seems good to me 🚀

@shark0der shark0der force-pushed the fix/update-tranches branch 2 times, most recently from c48523c to 39122c4 Compare October 5, 2022 08:40
@roxdanila roxdanila force-pushed the fix/update-tranches branch from 39122c4 to bc8e964 Compare October 7, 2022 15:44
@roxdanila roxdanila merged commit 192fc21 into nexus-v2 Oct 7, 2022
@roxdanila roxdanila deleted the fix/update-tranches branch October 7, 2022 16:00
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.

Audit - Staking M2: No update will ever be possible for tranches
3 participants