Summary
In the Escrow.sol contract, there are three modifiers that are used only once and are not inherited by any other contract. Inlining these modifiers in their respective functions can lead to gas savings.
Vulnerability Details
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L58-L63
modifier onlyBuyer() {
if (msg.sender != i_buyer) {
revert Escrow__OnlyBuyer();
}
_;
}
onlyBuyer modifier is not inherited and only used in this Escrow.sol::confirmReceipt() function it's good to inline this modifier in the function to save gas
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L94-L99
function confirmReceipt() external onlyBuyer inState(State.Created) {
s_state = State.Confirmed;
emit Confirmed(i_seller);
i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)));
}
Recommendations
@@ -1,7 +1,4 @@
- function confirmReceipt() external onlyBuyer inState(State.Created) {
s_state = State.Confirmed;
emit Confirmed(i_seller);
+ function confirmReceipt() external inState(State.Created) {
+ if (msg.sender != i_buyer) {
+ revert Escrow__OnlyBuyer();
+ }
s_state = State.Confirmed;
emit Confirmed(i_seller);
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L66-L71
modifier onlyBuyerOrSeller() {
if (msg.sender != i_buyer && msg.sender != i_seller) {
revert Escrow__OnlyBuyerOrSeller();
}
_;
}
The above modifer is only used in the following:
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L102
function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();
s_state = State.Disputed;
emit Disputed(msg.sender);
}
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L74-L79
modifier onlyArbiter() {
if (msg.sender != i_arbiter) {
revert Escrow__OnlyArbiter();
}
_;
}
The above modifer is only used in the following:
https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L109-L129
function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
uint256 totalFee = buyerAward + i_arbiterFee;
if (totalFee > tokenBalance) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}
s_state = State.Resolved
emit Resolved(i_buyer, i_seller);
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}
}
