Skip to content

feat(Escrow): add custom buyer option #117

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion contracts/foundry.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[profile.default]
src = 'src'
out = 'out'
libs = ['../node_modules', 'lib']
libs = ['../node_modules', 'lib']
via_ir = true
31 changes: 31 additions & 0 deletions contracts/src/EscrowUniversal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,37 @@ contract EscrowUniversal is IEscrow, IArbitrableV2 {
);
}

/// @inheritdoc IEscrow
function createERC20TransactionCustomBuyer(
uint256 _amount,
IERC20 _token,
uint256 _deadline,
string memory _transactionUri,
address payable _buyer,
address payable _seller
) external override shouldNotExceedCap(_token, _amount) returns (uint256 transactionID) {
// Transfers token from sender wallet to contract. Note that the amount is provided by the caller, not by the buyer.
if (!_token.safeTransferFrom(msg.sender, address(this), _amount)) revert TokenTransferFailed();
Transaction storage transaction = transactions.push();
transaction.buyer = _buyer;
transaction.seller = _seller;
transaction.amount = _amount;
transaction.token = _token;
transaction.deadline = _deadline;

transactionID = transactions.length - 1;

emit ERC20TransactionCreated(
transactionID,
_transactionUri,
_buyer,
_seller,
_token,
_amount,
transaction.deadline
);
}

/// @inheritdoc IEscrow
function pay(uint256 _transactionID, uint256 _amount) external override {
Transaction storage transaction = transactions[_transactionID];
Expand Down
17 changes: 17 additions & 0 deletions contracts/src/interfaces/IEscrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,23 @@ interface IEscrow {
address payable _seller
) external returns (uint256 transactionID);

/// @dev Create an ERC20 transaction with custom buyer address.
/// @param _amount The amount of tokens in this transaction.
/// @param _token The ERC20 token contract.
/// @param _deadline Time after which a party can automatically execute the arbitrable transaction.
/// @param _transactionUri The IPFS Uri Hash of the transaction.
/// @param _buyer Buyer's address.
/// @param _seller The recipient of the transaction.
/// @return transactionID The index of the transaction.
function createERC20TransactionCustomBuyer(
uint256 _amount,
IERC20 _token,
uint256 _deadline,
string memory _transactionUri,
address payable _buyer,
address payable _seller
) external returns (uint256 transactionID);

/// @dev Pay seller. To be called if the good or service is provided.
/// @param _transactionID The index of the transaction.
/// @param _amount Amount to pay in wei.
Expand Down
43 changes: 43 additions & 0 deletions contracts/test/EscrowUniversal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,49 @@ contract EscrowUniversalTest is Test {
assertEq(escrowToken.balanceOf(seller), 0, "Balance of seller should be 0");
}

function test_createERC20TransactionCustomBuyer() public {
uint256 deadline = block.timestamp + txTimeout;
uint256 txId;
vm.prank(buyer);
vm.expectEmit(true, true, true, true);
// Pay a bit less to differ between tests
emit ERC20TransactionCreated(txId, txUri, other, seller, escrowToken, 0.4 ether, deadline);
escrow.createERC20TransactionCustomBuyer(0.4 ether, escrowToken, deadline, txUri, payable(other), payable(seller));

(
address escrowBuyer,
address escrowSeller,
uint256 amount,
uint256 settlementBuyer,
uint256 settlementSeller,
uint256 escrowdeadline,
uint256 disputeId,
uint256 buyerFee,
uint256 sellerFee,
uint256 lastFeePaymentTime,
Status status,
IERC20 token
) = escrow.transactions(txId);

assertEq(escrowBuyer, other, "Wrong custom buyer address");
assertEq(escrowSeller, seller, "Wrong seller address");
assertEq(amount, 0.4 ether, "Wrong escrow amount");
assertEq(settlementBuyer, 0, "settlementBuyer should be 0");
assertEq(settlementSeller, 0, "settlementSeller should be 0");
assertEq(escrowdeadline, block.timestamp + 1000, "Wrong deadline");
assertEq(disputeId, 0, "disputeId should be 0");
assertEq(sellerFee, 0, "sellerFee should be 0");
assertEq(lastFeePaymentTime, 0, "lastFeePaymentTime should be 0");
assertEq(uint256(status), uint256(Status.NoDispute), "Wrong status");
assertEq(address(token), address(escrowToken), "Wrong token address");
assertEq(escrow.getTransactionCount(), 1, "Wrong transaction count");

assertEq(escrowToken.balanceOf(address(escrow)), 0.4 ether, "Wrong token balance of the escrow");
assertEq(escrowToken.balanceOf(buyer), 0.6 ether, "Wrong token balance of buyer"); // tx creator
assertEq(escrowToken.balanceOf(seller), 0, "Balance of seller should be 0");
assertEq(escrowToken.balanceOf(other), 0, "Balance of custom buyer should be 0"); // custom buyer did not have balance of his own
}
Comment on lines +383 to +424
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance test coverage for edge cases and security scenarios.

The test covers the basic happy path but lacks coverage for important edge cases:

Add these additional test cases to ensure comprehensive coverage:

function test_createERC20TransactionCustomBuyer_zeroAddresses() public {
    uint256 deadline = block.timestamp + txTimeout;
    
    // Test zero buyer address
    vm.expectRevert(IEscrow.ZeroBuyerAddress.selector);
    vm.prank(buyer);
    escrow.createERC20TransactionCustomBuyer(
        0.4 ether, 
        escrowToken, 
        deadline, 
        txUri, 
        payable(address(0)), 
        payable(seller)
    );
    
    // Test zero seller address
    vm.expectRevert(IEscrow.ZeroSellerAddress.selector);
    vm.prank(buyer);
    escrow.createERC20TransactionCustomBuyer(
        0.4 ether, 
        escrowToken, 
        deadline, 
        txUri, 
        payable(other), 
        payable(address(0))
    );
}

function test_createERC20TransactionCustomBuyer_sameBuyerSeller() public {
    uint256 deadline = block.timestamp + txTimeout;
    
    vm.expectRevert(IEscrow.BuyerAndSellerCannotBeSame.selector);
    vm.prank(buyer);
    escrow.createERC20TransactionCustomBuyer(
        0.4 ether, 
        escrowToken, 
        deadline, 
        txUri, 
        payable(other), 
        payable(other)
    );
}
🤖 Prompt for AI Agents
In contracts/test/EscrowUniversal.t.sol around lines 383 to 424, the existing
test only covers the basic successful scenario for
createERC20TransactionCustomBuyer. To improve test coverage and security, add
two new test functions: one to check that the function reverts when the buyer or
seller address is zero, and another to verify it reverts when the buyer and
seller addresses are the same. Use vm.expectRevert with the appropriate error
selectors before calling createERC20TransactionCustomBuyer with these invalid
inputs to ensure the contract correctly handles these edge cases.


function test_createNativeTransaction_approvalMissing() public {
uint256 deadline = block.timestamp + txTimeout;

Expand Down