Skip to content

Fix: share calculations#105

Merged
Teolhyn merged 4 commits intomainfrom
fix/share-calculations
Mar 23, 2025
Merged

Fix: share calculations#105
Teolhyn merged 4 commits intomainfrom
fix/share-calculations

Conversation

@Teolhyn
Copy link
Copy Markdown
Member

@Teolhyn Teolhyn commented Mar 11, 2025

  • Shares were previously given equal to deposited amount
  • This did not consider if yield had already generated in to the pool, therefore giving new users access to previously generated yield.
  • Share calculation now accounts for the value of the pool.
  • Added integration test to test that the new user cannot withdraw generated yield.

@Teolhyn Teolhyn force-pushed the fix/share-calculations branch from 807d3a3 to 6cdf361 Compare March 12, 2025 07:12
@Teolhyn Teolhyn force-pushed the fix/share-calculations branch from 54ff353 to 3c25afd Compare March 12, 2025 08:07
Comment on lines +744 to +748
// Move in time
e.ledger().with_mut(|li| {
li.sequence_number = 100_000 + 100_000;
li.timestamp = 1 + 31_556_926;
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we make a helper function for this? Would improve readability and remove repetition.

}

#[test]
#[should_panic(expected = "Error(Contract, #12)")]
Copy link
Copy Markdown
Member

@kovipu kovipu Mar 12, 2025

Choose a reason for hiding this comment

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

I'd love to make asserting errors a bit more readable. Now I have to look in error.rs to see what the hell is error #12. 😂

But I tried doing that with the try_ contract calls and asserting the Result, but couldn't get that to work. Maybe we'll have to let this marinate for a bit.

@Teolhyn Teolhyn merged commit 567e5b7 into main Mar 23, 2025
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