🐞 fix(DSCEngineTest): fix testRevertsIfTransferFromFails function logic

[GrayJyyyyy]

Here's a clear explanation of the issue for my PR:

---

Issue in test_RevertIfTransferIsFailed function

The current implementation of the test_RevertIfTransferIsFailed test has several unnecessary and potentially confusing elements:

1. Incorrect DSC token parameter: The test incorrectly passes the mock token (mockCollateralToken) as the third parameter to the DSCEngine constructor, which should actually be the DSC token address. This is confusing as it mixes the collateral token and the stablecoin token roles.

2. Unnecessary ownership transfer: The test transfers ownership of mockCollateralToken to the mockDsce. This step is unnecessary since we're only testing the depositCollateral function, which doesn't require any owner-only permissions on the token.

3. Redundant address tracking: The test uses an explicit owner variable and vm.startPrank(owner) when deploying contracts, but this is unnecessary as the default sender in tests is already the test contract itself.

Proposed fix

The test can be simplified to focus clearly on its purpose - verifying that DSCEngine correctly handles a failed transferFrom operation during collateral deposit:

    function testRevertsIfTransferFromFails() public {
        // In the foundry test, the default caller is msg.sender.We do not need to specify particularly.
        MockFailedTransferFrom mockCollateralToken = new MockFailedTransferFrom();
        tokenAddresses = [address(mockCollateralToken)];
        feedAddresses = [ethUsdPriceFeed];
        // DSCEngine receives the third parameter as dscAddress, not the tokenAddress used as collateral.
        DSCEngine mockDsce = new DSCEngine(tokenAddresses, feedAddresses, address(dsc));
        mockCollateralToken.mint(user, amountCollateral);
        vm.startPrank(user);
        ERC20Mock(address(mockCollateralToken)).approve(address(mockDsce), amountCollateral);
        // Act / Assert
        vm.expectRevert(DSCEngine.DSCEngine__TransferFailed.selector);
        mockDsce.depositCollateral(address(mockCollateralToken), amountCollateral);
        vm.stopPrank();
    }

This simplified version:

• Uses the correct DSC token parameter
• Eliminates the unnecessary ownership transfer
• Maintains the core test functionality to ensure that transfer failures are properly handled

[GrayJyyyyy]

@PatrickAlphaC plz check this~

[EngrPips]

Why couldn't you edit the pull request if you want to make some changes but instead, you prefer making multiple pull request.

[GrayJyyyyy]

Why couldn't you edit the pull request if you want to make some changes but instead, you prefer making multiple pull request.

sry,it's my first time to create a pr.And I promise it won't be change anymore.I will do better next time.

[EngrPips]

No worries at all. I am just checking to know your reason for doing it that way. This will be attended to appropriately as soon as possible.

[PatrickAlphaC]

gm,

This is pretty good! Cool to see your first PR!

I made a comment, could you take a look? Let's try to keep the code as similar to what we originally had, and I can merge this.

[GrayJyyyyy]

gm,

This is pretty good! Cool to see your first PR!

I made a comment, could you take a look? Let's try to keep the code as similar to what we originally had, and I can merge this.

I can not see the comment,where can I see?

[PatrickAlphaC]

Can you scroll up in this page? You should be able to see this, let me know.