Skip to content

Conversation

shark0der
Copy link
Contributor

@shark0der shark0der commented Dec 7, 2022

Context

An underflow issue in the StakingPool.getActiveAllocations function was hit during pricing tests.

Changes proposed in this pull request

  • The issue was caused by the wrong tranche id passed when calling updateExpiringCoverAmounts from allocate hence having tranches misaligned (stored active cover vs expirations in buckets). This led to getActiveAllocations attempting to expire the tranche that dind't have the allocated capacity.
  • This PR also fixes a todo where the indexes seemed wrong inside getActiveAllocationsWithoutCover function.
  • Additionally I'm including the commit that exposed a few pricing functions as pure functions. This does not change the logic itself.

Test plan

The test for this is on the test/pricing-unit-tests branch - PR #543 (and it passed) but I'm submitting the fixes separately to avoid any blockers on other PRs.

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 requested a review from danoctavian December 7, 2022 11:55
Copy link
Contributor

@danoctavian danoctavian left a comment

Choose a reason for hiding this comment

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

LGTM but i can't confidently validate the bug fix right now, some tests are needed at some point

@shark0der shark0der force-pushed the fix/staking-pool-expirations-buckets-underflow branch from 4dd60c0 to 55678eb Compare December 7, 2022 14:21
@shark0der shark0der force-pushed the fix/staking-pool-expirations-buckets-underflow branch from 55678eb to 3f54e69 Compare December 7, 2022 16:35
@shark0der shark0der merged commit 55b6054 into nexus-v2 Dec 7, 2022
@shark0der shark0der deleted the fix/staking-pool-expirations-buckets-underflow branch December 7, 2022 17:00
@0xdewy 0xdewy mentioned this pull request Dec 7, 2022
9 tasks
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