Submission Details
Severity:
`newScrow()` could be simplified
Summary
The implementation of newScrow() in EscrowFactory.sol can be simplified removing the call to computeEscrowAddress() and sending the tokens to the escrow contract address after the contract is created.
Doing so, the function computeEscrowAddress() can be removed, the contract balance validation in the escrow constructor can be removed as well.
Vulnerability Details
Impact
Gas could be saved and code could be simplified (ignore revert cases that would be catched before sending tx because the revert will be predicted)
testSameSaltReverts() (gas: 39172 (0.000%))
testConstructorTokenZeroReverts() (gas: -59 (-0.100%))
testConstructorBuyerZeroReverts() (gas: -81 (-0.136%))
testComputedAddressEqualsDeployedAddress() (gas: -1929 (-0.256%))
testRevertIfFeeGreaterThanPrice() (gas: 1011 (0.717%))
testSellerZeroReverts() (gas: 1011 (0.721%))
testConfirmReceiptRevertsOnTokenTxFail() (gas: -11796 (-0.830%))
testResolveDisputeWithBuyerAward() (gas: -11796 (-1.390%))
testResolveDisputeTransfersTokens() (gas: -11796 (-1.390%))
testResolveDisputeZeroSellerTransfer() (gas: -11796 (-1.471%))
testResolveDisputeChangesState() (gas: -11796 (-1.478%))
testCanOnlyResolveInDisputedState() (gas: -11796 (-1.523%))
testCanOnlyInitiateDisputeInConfirmedState() (gas: -11774 (-1.536%))
testTransfersTokenOutOfContract() (gas: -11774 (-1.536%))
testCanOnlyConfirmInCreatedState() (gas: -11796 (-1.537%))
testConfirmReceiptEmitsEvent() (gas: -11774 (-1.538%))
testStateChangesOnConfirmedReceipt() (gas: -11796 (-1.541%))
testResolveDisputeFeeExceedsBalance() (gas: -11796 (-1.548%))
testInitiateDisputeEmitsEvent() (gas: -11774 (-1.551%))
testInitiateDisputeChangesState() (gas: -11774 (-1.553%))
testCreatingEscrowHasBuyerActuallyBeBuyer() (gas: -11774 (-1.563%))
testDeployEscrowFromFactory() (gas: -11774 (-1.595%))
testOnlyArbiterCanCallResolveDispute(address) (gas: -11796 (-1.599%))
testConfirmReceiptOnlyByBuyer() (gas: -11774 (-1.599%))
testInitiateDisputeWithoutArbiterReverts() (gas: -11796 (-1.601%))
testConfirmReceiptOnlyByBuyerFuzz(address) (gas: -11774 (-1.602%))
testOnlyBuyerOrSellerCanCallinitiateDispute(address) (gas: -11796 (-1.604%))
testCreatingEscrowEmitsEvent() (gas: -12615 (-1.629%))
Tools Used
Manual Review
Recommendations
Change the newScrow() implementation like this
diff --git a/src/Escrow.sol b/src/Escrow.sol
index 5b5fd1f..5ada292 100644
--- a/src/Escrow.sol
+++ b/src/Escrow.sol
contract Escrow is IEscrow, ReentrancyGuard {
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
- if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
+ // if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
diff --git a/src/EscrowFactory.sol b/src/EscrowFactory.sol
index 08af965..3c8dafb 100644
--- a/src/EscrowFactory.sol
+++ b/src/EscrowFactory.sol
contract EscrowFactory is IEscrowFactory {
address arbiter,
uint256 arbiterFee,
bytes32 salt
- ) external returns (IEscrow) {
- address computedAddress = computeEscrowAddress(
- type(Escrow).creationCode,
- address(this),
- uint256(salt),
- price,
- tokenContract,
- msg.sender,
- seller,
- arbiter,
- arbiterFee
- );
- tokenContract.safeTransferFrom(msg.sender, computedAddress, price);
- Escrow escrow = new Escrow{salt: salt}(
+ ) external returns (IEscrow escrow) {
+ // address computedAddress = computeEscrowAddress(
+ // type(Escrow).creationCode,
+ // address(this),
+ // uint256(salt),
+ // price,
+ // tokenContract,
+ // msg.sender,
+ // seller,
+ // arbiter,
+ // arbiterFee
+ // );
+ escrow = new Escrow{salt: salt}(
price,
tokenContract,
msg.sender,
contract EscrowFactory is IEscrowFactory {
arbiter,
arbiterFee
);
- if (address(escrow) != computedAddress) {
- revert EscrowFactory__AddressesDiffer();
- }
+ tokenContract.safeTransferFrom(msg.sender, address(escrow), price);
+ // if (address(escrow) != computedAddress) {
+ // revert EscrowFactory__AddressesDiffer();
+ // }
emit EscrowCreated(address(escrow), msg.sender, seller, arbiter);
- return escrow;
+ // return escrow;
}
/// @dev See https://docs.soliditylang.org/en/latest/control-structures.html#salted-contract-creations-create2
Prize pool breakdown
Total prize
40,000 USDC
nSLOC
186215 USDC / LOC
37,000 USDC
3,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.