diff --git a/.gitignore b/.gitignore index 8d9a44f..56cfecc 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ # testing /coverage +/cache # next.js /.next/ diff --git a/contracts/escrow_smart_contract/RefundProtocol.sol b/contracts/escrow_smart_contract/RefundProtocol.sol index 7f8a577..1e161c1 100644 --- a/contracts/escrow_smart_contract/RefundProtocol.sol +++ b/contracts/escrow_smart_contract/RefundProtocol.sol @@ -15,12 +15,16 @@ pragma solidity ^0.8.24; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; contract RefundProtocol is EIP712 { + using SafeERC20 for IERC20; + struct Payment { address to; uint256 amount; + // Informational only; withdrawals are intentionally available immediately. uint256 releaseTimestamp; address refundTo; uint256 withdrawnAmount; @@ -61,6 +65,7 @@ contract RefundProtocol is EIP712 { error WithdrawalHashAlreadyUsed(); error WithdrawalHashExpired(); error PaymentRefunded(uint256 paymentID); + error PaymentFullyWithdrawn(uint256 paymentID); error MismatchedEarlyWithdrawalArrays(); constructor(address _arbiter, address _usdc, string memory eip712Name, string memory eip712version) @@ -97,7 +102,7 @@ contract RefundProtocol is EIP712 { revert RefundToIsZeroAddress(); } - fiatToken.transferFrom(msg.sender, address(this), amount); + fiatToken.safeTransferFrom(msg.sender, address(this), amount); payments[nonce] = Payment(to, amount, block.timestamp, refundTo, 0, false); balances[to] += amount; @@ -116,15 +121,16 @@ contract RefundProtocol is EIP712 { revert CallerNotAllowed(); } + uint256 refundAmount = _refundableAmount(paymentID, payment); uint256 recipientBalance = balances[payment.to]; - if (payment.amount > recipientBalance) { + if (refundAmount > recipientBalance) { revert InsufficientFunds(); } - balances[payment.to] = recipientBalance - payment.amount; + balances[payment.to] = recipientBalance - refundAmount; - _executeRefund(paymentID, payment); + _executeRefund(paymentID, payment, refundAmount); } /** @@ -137,27 +143,29 @@ contract RefundProtocol is EIP712 { function refundByArbiter(uint256 paymentID) external onlyArbiter { Payment memory payment = payments[paymentID]; + uint256 refundAmount = _refundableAmount(paymentID, payment); uint256 recipientBalance = balances[payment.to]; - if (payment.amount <= recipientBalance) { - balances[payment.to] = recipientBalance - payment.amount; - return _executeRefund(paymentID, payment); + if (refundAmount <= recipientBalance) { + balances[payment.to] = recipientBalance - refundAmount; + return _executeRefund(paymentID, payment, refundAmount); } uint256 arbiterBalance = balances[arbiter]; - if (payment.amount > arbiterBalance) { + if (refundAmount > arbiterBalance) { revert InsufficientFunds(); } - balances[arbiter] = arbiterBalance - payment.amount; - debts[payment.to] += payment.amount; + balances[arbiter] = arbiterBalance - refundAmount; + debts[payment.to] += refundAmount; - _executeRefund(paymentID, payment); + _executeRefund(paymentID, payment, refundAmount); } /** - * A function to settle recipient debts. + * A permissionless function to settle recipient debts from their protocol balance. + * The caller cannot redirect or otherwise benefit from the settled funds. * @param recipient the recipient address */ function settleDebt(address recipient) external { @@ -170,7 +178,7 @@ contract RefundProtocol is EIP712 { * @param amount amount to deposit */ function depositArbiterFunds(uint256 amount) external onlyArbiter { - fiatToken.transferFrom(msg.sender, address(this), amount); + fiatToken.safeTransferFrom(msg.sender, address(this), amount); balances[arbiter] += amount; } @@ -186,7 +194,7 @@ contract RefundProtocol is EIP712 { } balances[arbiter] = arbiterBalance - amount; - fiatToken.transfer(arbiter, amount); + fiatToken.safeTransfer(arbiter, amount); } /** @@ -194,6 +202,7 @@ contract RefundProtocol is EIP712 { * It will fail if: * 1. The caller is not the recipient of the payment * 2. The payment has already been refunded + * The release timestamp is informational; payments are intentionally withdrawable immediately. * @param paymentIDs an array of payments to release */ function withdraw(uint256[] calldata paymentIDs) external { @@ -217,7 +226,7 @@ contract RefundProtocol is EIP712 { revert InsufficientFunds(); } balances[msg.sender] = recipientBalance - totalAmount; - fiatToken.transfer(msg.sender, totalAmount); + fiatToken.safeTransfer(msg.sender, totalAmount); emit Withdrawal(msg.sender, totalAmount); } @@ -272,7 +281,8 @@ contract RefundProtocol is EIP712 { Payment memory payment = payments[paymentID]; - if (withdrawalAmount > payment.amount) { + if (payment.withdrawnAmount > payment.amount || withdrawalAmount > payment.amount - payment.withdrawnAmount) + { revert InvalidWithdrawalAmount(paymentID, withdrawalAmount); } if (payment.to != recipient) { @@ -294,7 +304,7 @@ contract RefundProtocol is EIP712 { balances[recipient] = recipientBalance - totalAmount; balances[arbiter] += feeAmount; - fiatToken.transfer(recipient, totalAmount - feeAmount); + fiatToken.safeTransfer(recipient, totalAmount - feeAmount); emit Withdrawal(recipient, totalAmount); emit WithdrawalFeePaid(recipient, feeAmount); @@ -340,16 +350,27 @@ contract RefundProtocol is EIP712 { * Internal function to execute a refund * @param paymentID the payment ID to refund * @param payment the payment struct + * @param refundAmount the unwithdrawn amount to refund */ - function _executeRefund(uint256 paymentID, Payment memory payment) internal { - if (payment.refunded) { - revert PaymentRefunded(paymentID); - } - fiatToken.transfer(payment.refundTo, payment.amount); + function _executeRefund(uint256 paymentID, Payment memory payment, uint256 refundAmount) internal { + fiatToken.safeTransfer(payment.refundTo, refundAmount); payments[paymentID].refunded = true; - emit Refund(paymentID, payment.refundTo, payment.amount); + emit Refund(paymentID, payment.refundTo, refundAmount); + } + + /** + * Returns the portion of a payment that remains eligible for refund. + */ + function _refundableAmount(uint256 paymentID, Payment memory payment) internal pure returns (uint256) { + if (payment.refunded) { + revert PaymentRefunded(paymentID); + } + if (payment.withdrawnAmount >= payment.amount) { + revert PaymentFullyWithdrawn(paymentID); + } + return payment.amount - payment.withdrawnAmount; } /** @@ -382,8 +403,9 @@ contract RefundProtocol is EIP712 { uint256 expiry, uint256 salt ) internal view returns (bytes32) { - bytes32 structHash = - keccak256(abi.encode(EARLY_WITHDRAWAL_TYPEHASH, paymentIDs, withdrawalAmounts, feeAmount, expiry, salt)); + bytes32 structHash = keccak256( + abi.encode(EARLY_WITHDRAWAL_TYPEHASH, paymentIDs, withdrawalAmounts, feeAmount, expiry, salt) + ); return _hashTypedDataV4(structHash); } -} \ No newline at end of file +} diff --git a/foundry.toml b/foundry.toml new file mode 100644 index 0000000..d84cf33 --- /dev/null +++ b/foundry.toml @@ -0,0 +1,7 @@ +[profile.default] +src = "contracts" +test = "test" +libs = ["node_modules"] +solc_version = "0.8.24" +optimizer = true +optimizer_runs = 200 diff --git a/package-lock.json b/package-lock.json index 6a4f681..052d3b9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "workflow-escrow-refund-protocol", + "name": "arc-escrow-40", "lockfileVersion": 3, "requires": true, "packages": { @@ -9,6 +9,7 @@ "@circle-fin/smart-contract-platform": "^4.3.0", "@circle-fin/user-controlled-wallets": "^4.5.0", "@ethersproject/abi": "^5.7.0", + "@openzeppelin/contracts": "^5.6.1", "@radix-ui/react-alert-dialog": "^1.1.2", "@radix-ui/react-checkbox": "^1.1.1", "@radix-ui/react-dialog": "^1.1.2", @@ -768,6 +769,12 @@ "node": ">= 10" } }, + "node_modules/@openzeppelin/contracts": { + "version": "5.6.1", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-5.6.1.tgz", + "integrity": "sha512-Ly6SlsVJ3mj+b18W3R8gNufB7dTICT105fJhodGAGgyC2oqnBAhqSiNDJ8V8DLY05cCz81GLI0CU5vNYA1EC/w==", + "license": "MIT" + }, "node_modules/@pkgjs/parseargs": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", @@ -1917,7 +1924,6 @@ "version": "2.45.4", "resolved": "https://registry.npmjs.org/@supabase/supabase-js/-/supabase-js-2.45.4.tgz", "integrity": "sha512-E5p8/zOLaQ3a462MZnmnz03CrduA5ySH9hZyL03Y+QZLIOO4/Gs8Rdy4ZCKDHsN7x0xdanVEWWFN3pJFQr9/hg==", - "peer": true, "dependencies": { "@supabase/auth-js": "2.65.0", "@supabase/functions-js": "2.4.1", @@ -2249,7 +2255,6 @@ "resolved": "https://registry.npmjs.org/@types/react/-/react-18.2.46.tgz", "integrity": "sha512-nNCvVBcZlvX4NU1nRRNV/mFl1nNRuTuslAJglQsq+8ldXe5Xv0Wd2f7WTE3jOxhLH2BFfiZGC6GCp+kHQbgG+w==", "devOptional": true, - "peer": true, "dependencies": { "@types/prop-types": "*", "@types/scheduler": "*", @@ -2261,7 +2266,6 @@ "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.2.18.tgz", "integrity": "sha512-TJxDm6OfAX2KJWJdMEVTwWke5Sc/E/RlnPGvGfS0W7+6ocy2xhDVQVh/KvC2Uf7kACs+gDytdusDSdWfWkaNzw==", "devOptional": true, - "peer": true, "dependencies": { "@types/react": "*" } @@ -4087,7 +4091,6 @@ "version": "14.2.15", "resolved": "https://registry.npmjs.org/next/-/next-14.2.15.tgz", "integrity": "sha512-h9ctmOokpoDphRvMGnwOJAedT6zKhwqyZML9mDtspgf4Rh3Pn7UTYKqePNoDvhsWBAO5GoPNYshnAUGIazVGmw==", - "peer": true, "dependencies": { "@next/env": "14.2.15", "@swc/helpers": "0.5.5", @@ -4468,7 +4471,6 @@ "version": "18.2.0", "resolved": "https://registry.npmjs.org/react/-/react-18.2.0.tgz", "integrity": "sha512-/3IjMdb2L9QbBdWiW5e3P2/npwMBaU9mHCSCUzNln0ZCYbcfTsGbTJrU/kGemdH2IWmB2ioZ+zkxtmq6g09fGQ==", - "peer": true, "dependencies": { "loose-envify": "^1.1.0" }, @@ -4495,7 +4497,6 @@ "version": "18.2.0", "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.2.0.tgz", "integrity": "sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==", - "peer": true, "dependencies": { "loose-envify": "^1.1.0", "scheduler": "^0.23.0" @@ -5470,7 +5471,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/package.json b/package.json index 58891a4..3e3d3c2 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "@circle-fin/smart-contract-platform": "^4.3.0", "@circle-fin/user-controlled-wallets": "^4.5.0", "@ethersproject/abi": "^5.7.0", + "@openzeppelin/contracts": "^5.6.1", "@radix-ui/react-alert-dialog": "^1.1.2", "@radix-ui/react-checkbox": "^1.1.1", "@radix-ui/react-dialog": "^1.1.2", @@ -55,4 +56,4 @@ "tailwindcss": "^4.0.0", "typescript": "5.3.3" } -} \ No newline at end of file +} diff --git a/test/RefundProtocol.t.sol b/test/RefundProtocol.t.sol new file mode 100644 index 0000000..59aee2b --- /dev/null +++ b/test/RefundProtocol.t.sol @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.24; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {RefundProtocol} from "../contracts/escrow_smart_contract/RefundProtocol.sol"; + +interface Vm { + function addr(uint256 privateKey) external returns (address); + function expectRevert(bytes calldata revertData) external; + function prank(address msgSender) external; + function sign(uint256 privateKey, bytes32 digest) external returns (uint8 v, bytes32 r, bytes32 s); +} + +contract MockToken is ERC20 { + constructor() ERC20("Mock USDC", "USDC") {} + + function mint(address account, uint256 amount) external { + _mint(account, amount); + } +} + +contract FalseReturningToken is IERC20 { + function totalSupply() external pure returns (uint256) { + return 0; + } + + function balanceOf(address) external pure returns (uint256) { + return 0; + } + + function transfer(address, uint256) external pure returns (bool) { + return false; + } + + function allowance(address, address) external pure returns (uint256) { + return type(uint256).max; + } + + function approve(address, uint256) external pure returns (bool) { + return true; + } + + function transferFrom(address, address, uint256) external pure returns (bool) { + return false; + } +} + +contract RefundProtocolTest { + Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); + + uint256 private constant RECIPIENT_KEY = 0xA11CE; + address private constant PAYER = address(0x100); + address private constant REFUND_TO = address(0x300); + + MockToken private token; + RefundProtocol private protocol; + address private recipient; + + function setUp() public { + token = new MockToken(); + protocol = new RefundProtocol(address(this), address(token), "RefundProtocol", "1"); + recipient = vm.addr(RECIPIENT_KEY); + + token.mint(PAYER, 1_000); + vm.prank(PAYER); + token.approve(address(protocol), type(uint256).max); + } + + function testWithdrawnPaymentCannotSubsequentlyBeRefundedByRecipient() public { + _pay(100); + + uint256[] memory paymentIDs = new uint256[](1); + paymentIDs[0] = 0; + vm.prank(recipient); + protocol.withdraw(paymentIDs); + + // Fund a second payment so an insufficient balance cannot mask the closed-payment check. + _pay(100); + + vm.expectRevert(abi.encodeWithSelector(RefundProtocol.PaymentFullyWithdrawn.selector, 0)); + vm.prank(recipient); + protocol.refundByRecipient(0); + + _assertEq(token.balanceOf(REFUND_TO), 0, "refund address received withdrawn funds"); + _assertEq(protocol.balances(recipient), 100, "recipient protocol balance changed"); + } + + function testWithdrawnPaymentCannotSubsequentlyBeRefundedByArbiter() public { + _pay(100); + + uint256[] memory paymentIDs = new uint256[](1); + paymentIDs[0] = 0; + vm.prank(recipient); + protocol.withdraw(paymentIDs); + + token.mint(address(this), 100); + token.approve(address(protocol), 100); + protocol.depositArbiterFunds(100); + + vm.expectRevert(abi.encodeWithSelector(RefundProtocol.PaymentFullyWithdrawn.selector, 0)); + protocol.refundByArbiter(0); + + _assertEq(token.balanceOf(REFUND_TO), 0, "refund address received withdrawn funds"); + _assertEq(protocol.balances(address(this)), 100, "arbiter protocol balance changed"); + } + + function testPartiallyWithdrawnPaymentRefundsOnlyRemainingAmount() public { + _pay(100); + + uint256[] memory paymentIDs = new uint256[](1); + paymentIDs[0] = 0; + uint256[] memory withdrawalAmounts = new uint256[](1); + withdrawalAmounts[0] = 40; + uint256 expiry = block.timestamp + 1 days; + uint256 salt = 1; + + bytes32 digest = protocol.hashEarlyWithdrawalInfo(paymentIDs, withdrawalAmounts, 0, expiry, salt); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(RECIPIENT_KEY, digest); + protocol.earlyWithdrawByArbiter(paymentIDs, withdrawalAmounts, 0, expiry, salt, recipient, v, r, s); + + vm.prank(recipient); + protocol.refundByRecipient(0); + + _assertEq(token.balanceOf(recipient), 40, "recipient withdrawal amount"); + _assertEq(token.balanceOf(REFUND_TO), 60, "only unwithdrawn amount should be refunded"); + _assertEq(protocol.balances(recipient), 0, "recipient protocol balance"); + } + + function testSafeTransferFromRejectsFalseReturnValue() public { + FalseReturningToken falseToken = new FalseReturningToken(); + RefundProtocol falseTokenProtocol = + new RefundProtocol(address(this), address(falseToken), "RefundProtocol", "1"); + + vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, address(falseToken))); + falseTokenProtocol.pay(recipient, 100, REFUND_TO); + } + + function _pay(uint256 amount) private { + vm.prank(PAYER); + protocol.pay(recipient, amount, REFUND_TO); + } + + function _assertEq(uint256 actual, uint256 expected, string memory message) private pure { + require(actual == expected, message); + } +}