Doubt in the CEI model.

[ebok21]

Why do we perform the transfer operation after the updating of the mapping? If the transaction were to fail, the mapping would hold wrong info or does the revert undo the changes made? And even if the changes are undone, is the gas spent recovered? Why not just perform the interactions before the effects? @PatrickAlphaC

// Follows CEI
// Checks in function definition using modifiers
function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
        external
        moreThanZero(amountCollateral)
        isAllowedToken(tokenCollateralAddress)
        nonReentrant
    {
       // Effects - Update the mapping and emit event
        s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral;
        emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);

       // Perform the token transfer
        bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
        if (!success) {
            revert DSCEngine__TransferFailed();
        }
    }

[omarhibshi]

I can answer the first question for certain if the line revert DSCEngine__TransferFailed(); reverts, this will undo everything above it in that function.

As for the second question, I don't think the gas spent is redeemable. Third question, This is more of common practice than any thing else.

[ebok21]

As for the second question, I don't think the gas spent is redeemable.

I wonder if the gas tradeoff is worth the common practice though. If we had a lot of "Effects" to perform, the gas lost in the case of a revert could be significant.

[omarhibshi]

I agree with you. There maybe another reason for why we do this first before calling the transfer() function. I will have to go back and check that section of the video where Patrick explains this, I may have overlooked something. I'll keep you posted

[usmanfarooq91]

I wonder if the gas tradeoff is worth the common practice though. If we had a lot of "Effects" to perform, the gas lost in the case of a revert could be significant.

This tradeoff is being made to prevent reentrancy attack, the gas tradeoff is much smaller compared to the implications of reentrancy attack.

On this current repo, there are not many people following to answer questions. Please feel free to continue the discussion here if you have further questions. We would love to help you.

[PatrickAlphaC]

@usmanfarooq91 is spot on.