-
Notifications
You must be signed in to change notification settings - Fork 17
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
Adds more tests to ynLSD #35
Adds more tests to ynLSD #35
Conversation
This pull request has been linked to Shortcut Story #133: Add test coverage for all remaining modules to ensure close to 100% test coverage. |
…had/sc-133/more-yneth-coverage
test/foundry/integration/ynLSD.t.sol
Outdated
assertEq(balance, amount, "Amount not received"); | ||
|
||
uint256 previewDeposit = ynlsd.previewDeposit(asset, amount); | ||
assertTrue(amount - previewDeposit < 1e18, "Preview deposit does not match expected value"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertion should be stronger
can we use compareWithThreshold?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
test/foundry/integration/ynLSD.t.sol
Outdated
assertEq(balance, amount, "Amount not received"); | ||
|
||
uint256 ethAmount = ynlsd.convertToETH(asset, amount); | ||
assertTrue(amount - ethAmount < 1e15, "ETH amount should be greater than zero"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use compareWithThreshold?
and why is the range so wide here as much as 1e15?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
test/foundry/integration/ynLSD.t.sol
Outdated
// Note, STETH returns aprx 9.987e17 SETH for 1 ether | ||
|
||
uint256 previewDeposit = ynlsd.previewDeposit(asset, amount); | ||
assertTrue(compareWithThreshold(amount, previewDeposit, 1.2e16), "Preview deposit does not match expected value"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be strictly equal with
IERC20 asset = IERC20(chainAddresses.lsd.STETH_ADDRESS);
uint256 amount = 1 ether;
AggregatorV3Interface priceFeed = AggregatorV3Interface(chainAddresses.lsd.STETH_FEED_ADDRESS);
(, int256 price,,,) = priceFeed.latestRoundData();
uint256 stethPrice = uint256(price);
uint expectedDepositPreview = amount * stethPrice / 1e18;
uint256 previewDeposit = ynlsd.previewDeposit(asset, amount);
assertEq(previewDeposit, expectedDepositPreview, "Preview deposit does not match expected value");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
No description provided.