Skip to content

feat: Develop SingleRequestProxy Smart Contracts #1453

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
e2567f4
feat: develop for native currencies
aimensahnoun Sep 12, 2024
c624474
feat: develop for ERC20 currencies
aimensahnoun Sep 12, 2024
917d472
feat: create `EthereumSingleRequestProxy` factory contract
aimensahnoun Sep 12, 2024
d5e4728
feat: create `ERC20SingleRequestProxy` factory contract
aimensahnoun Sep 12, 2024
0543092
refactor: merge the single request proxy factories into one factory
aimensahnoun Sep 13, 2024
2bd3495
fix: infinite loop between `EthSingleRequestProxy` and `EthFeeProxy`
aimensahnoun Sep 13, 2024
19b5e40
refactor: refactor `nonReentrant` to match openzeppelin implementation
aimensahnoun Sep 16, 2024
c1d3a77
fix: update `ERC20SingleRequestProxy` to use `balance`
aimensahnoun Sep 16, 2024
1376582
fix: update `SingleRequestProxyFactory` to call Ownable constructor
aimensahnoun Sep 16, 2024
c2f55ee
chore: update contracts to use `0.8.9` solidity version
aimensahnoun Sep 17, 2024
bc922e8
test: add test for `EthereumSingleRequestProxy`
aimensahnoun Sep 18, 2024
8aef234
fix: `ERC20SingleRequestProxy` to factor in fee amount
aimensahnoun Sep 18, 2024
0769aa7
test: `ERC20SingleRequestProxy` functionality
aimensahnoun Sep 18, 2024
3bd9e38
test: `SingleRequestProxyFactory` functionality
aimensahnoun Sep 18, 2024
ed429ae
chore: update `EthereumSingleRequestProxy` tests
aimensahnoun Sep 18, 2024
d9a4484
Merge branch 'master' into 1394-develop-smart-contracts-nativesingler…
aimensahnoun Sep 18, 2024
a341ce1
feat: write deployment script for `SingleRequestProxyFactory`
aimensahnoun Sep 23, 2024
10974b5
Merge branch '1394-develop-smart-contracts-nativesinglerequestproxy-e…
aimensahnoun Sep 23, 2024
2cc82a0
refactor: rewrite deployment to use `CREATE2` schema
aimensahnoun Sep 23, 2024
bee1f58
fix: add SingleRequestProxyFactory `create2ContractDeploymentList`
aimensahnoun Sep 23, 2024
aab6711
chore: include SingleRequestProxyFactory in transfer ownership cases …
aimensahnoun Sep 23, 2024
0c68eba
refactor: rename updateEthereumFeeProxy to setEthereumFeeProxy for cl…
aimensahnoun Sep 23, 2024
796f2b9
Merge branch 'master' into 1394-develop-smart-contracts-nativesingler…
aimensahnoun Sep 23, 2024
0482f6d
fix: type and add new events
aimensahnoun Sep 26, 2024
b8183f3
Merge branch '1394-develop-smart-contracts-nativesinglerequestproxy-e…
aimensahnoun Sep 26, 2024
85fdc13
Merge branch 'master' of github.com:RequestNetwork/requestNetwork int…
aimensahnoun Oct 16, 2024
9033066
docs: add more documentation to nonReentrant modifier
aimensahnoun Oct 16, 2024
cd91ec2
feat: add `rescueFunds` to `EthereumSingleRequestProxy`
aimensahnoun Oct 16, 2024
f944199
docs: add more documentation to `SingleRequestProxyFactory`
aimensahnoun Oct 16, 2024
c77c1eb
fix: use `safeApprove` instead of `approve`
aimensahnoun Oct 16, 2024
41e43b5
feat: add rescue funds method
aimensahnoun Oct 17, 2024
01c28d2
test: add more ownership tests
aimensahnoun Oct 17, 2024
cfba1b9
test: add partial payment and non-standard ERC20 tests
aimensahnoun Oct 17, 2024
7241e0c
test: add rescue-funds tests
aimensahnoun Oct 17, 2024
4abe9e9
test: refactor tests
aimensahnoun Oct 17, 2024
90b413f
test: add await back
aimensahnoun Oct 17, 2024
c493758
feat: add triggerERC20Payment
aimensahnoun Oct 17, 2024
06e015a
chore: add require for zero address
aimensahnoun Oct 17, 2024
c778b17
feat: add rescue methods for ERC20 and native tokens
aimensahnoun Oct 17, 2024
b45607a
test: add tests for both rescue methods
aimensahnoun Oct 17, 2024
c58dfc0
fix: rename rescueFunds to rescueERC20Funds
aimensahnoun Oct 17, 2024
9ee661a
fix: typo in "receive"
aimensahnoun Oct 18, 2024
0141d22
Merge branch 'master' of github.com:RequestNetwork/requestNetwork int…
aimensahnoun Oct 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ export const computeCreate2DeploymentAddressesFromList = async (
case 'BatchConversionPayments':
case 'ERC20SwapToPay':
case 'ERC20SwapToConversion':
case 'ERC20TransferableReceivable': {
case 'ERC20TransferableReceivable':
case 'SingleRequestProxyFactory': {
try {
const constructorArgs = getConstructorArgs(contract, chain);
address = await computeCreate2DeploymentAddress({ contract, constructorArgs }, hre);
Expand Down
11 changes: 11 additions & 0 deletions packages/smart-contracts/scripts-create2/constructor-args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ export const getConstructorArgs = (
const erc20FeeProxyAddress = erc20FeeProxy.getAddress(network);
return ['Request Network Transferable Receivable', 'tREC', erc20FeeProxyAddress];
}
case 'SingleRequestProxyFactory': {
if (!network) {
throw new Error('SingleRequestProxyFactory requires network parameter');
}
const erc20FeeProxy = artifacts.erc20FeeProxyArtifact;
const erc20FeeProxyAddress = erc20FeeProxy.getAddress(network);
const ethereumFeeProxy = artifacts.ethereumFeeProxyArtifact;
const ethereumFeeProxyAddress = ethereumFeeProxy.getAddress(network);

return [ethereumFeeProxyAddress, erc20FeeProxyAddress];
}
default:
return [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export const transferOwnership = async (
case 'Erc20ConversionProxy':
case 'BatchConversionPayments':
case 'ERC20SwapToPay':
case 'ERC20SwapToConversion': {
case 'ERC20SwapToConversion':
case 'SingleRequestProxyFactory': {
await updateOwner({ contract, hre, signWithEoa });
break;
}
Expand Down
1 change: 1 addition & 0 deletions packages/smart-contracts/scripts-create2/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const create2ContractDeploymentList = [
'BatchConversionPayments',
'ERC20EscrowToPay',
'ERC20TransferableReceivable',
'SingleRequestProxyFactory',
];

/**
Expand Down
91 changes: 91 additions & 0 deletions packages/smart-contracts/src/contracts/ERC20SingleRequestProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import '@openzeppelin/contracts/token/ERC20/IERC20.sol';
import './interfaces/ERC20FeeProxy.sol';
import './lib/SafeERC20.sol';

/**
* @title ERC20SingleRequestProxy
* @notice This contract is used to send a single request to a payee with a fee sent to a third address for ERC20
*/

contract ERC20SingleRequestProxy {
address public payee;
address public tokenAddress;
address public feeAddress;
uint256 public feeAmount;
bytes public paymentReference;
IERC20FeeProxy public erc20FeeProxy;

constructor(
address _payee,
address _tokenAddress,
address _feeAddress,
uint256 _feeAmount,
bytes memory _paymentReference,
address _erc20FeeProxy
) {
payee = _payee;
tokenAddress = _tokenAddress;
feeAddress = _feeAddress;
feeAmount = _feeAmount;
paymentReference = _paymentReference;
erc20FeeProxy = IERC20FeeProxy(_erc20FeeProxy);
}

receive() external payable {
require(msg.value == 0, 'This function is only for triggering the transfer');
_processPayment();
}

function triggerERC20Payment() external {
_processPayment();
}

function _processPayment() internal {
IERC20 token = IERC20(tokenAddress);
uint256 balance = token.balanceOf(address(this));
uint256 paymentAmount = balance;
if (feeAmount > 0 && feeAddress != address(0)) {
require(balance > feeAmount, 'Insufficient balance to cover fee');
paymentAmount = balance - feeAmount;
}

require(SafeERC20.safeApprove(token, address(erc20FeeProxy), balance), 'Approval failed');

erc20FeeProxy.transferFromWithReferenceAndFee(
tokenAddress,
payee,
paymentAmount,
paymentReference,
feeAmount,
feeAddress
);
}
Comment on lines +46 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add Reentrancy Guard to _processPayment Function

The _processPayment() function involves external calls and token transfers without any reentrancy protection. This could expose the contract to reentrancy attacks, especially since it interacts with external contracts.

Consider implementing a reentrancy guard to secure this function. You can use a custom reentrancy guard similar to the one used in the EthereumSingleRequestProxy contract, which allows reentrancy from trusted contracts if necessary.

Apply this diff to add a reentrancy guard:

+  bool private locked;

   function _processPayment() internal {
+    require(!locked, "Reentrant call detected");
+    locked = true;

     IERC20 token = IERC20(tokenAddress);
     uint256 balance = token.balanceOf(address(this));
     uint256 paymentAmount = balance;
     if (feeAmount > 0 && feeAddress != address(0)) {
       require(balance > feeAmount, 'Insufficient balance to cover fee');
       paymentAmount = balance - feeAmount;
     }

     SafeERC20.safeApprove(token, address(erc20FeeProxy), balance);

     erc20FeeProxy.transferFromWithReferenceAndFee(
       tokenAddress,
       payee,
       paymentAmount,
       paymentReference,
       feeAmount,
       feeAddress
     );

+    locked = false;
   }

Committable suggestion was skipped due to low confidence.


/**
* @notice Rescues any trapped funds by sending them to the payee
* @dev Can be called by anyone, but funds are always sent to the payee
*/
function rescueERC20Funds(address _tokenAddress) external {
require(_tokenAddress != address(0), 'Invalid token address');
IERC20 token = IERC20(_tokenAddress);
uint256 balance = token.balanceOf(address(this));
require(balance > 0, 'No funds to rescue');
bool success = SafeERC20.safeTransfer(token, payee, balance);
require(success, 'ERC20 rescue failed');
}

Comment on lines +71 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add Reentrancy Guard to rescueERC20Funds Function

The rescueERC20Funds() function can be called by anyone and performs an external token transfer. Without reentrancy protection, this function could be exploited through reentrancy attacks.

Implement a reentrancy guard to secure this function. You can reuse the same guard applied in _processPayment().

Apply this diff to add the guard:

   function rescueERC20Funds(address _tokenAddress) external {
+    require(!locked, "Reentrant call detected");
+    locked = true;

     require(_tokenAddress != address(0), 'Invalid token address');
     IERC20 token = IERC20(_tokenAddress);
     uint256 balance = token.balanceOf(address(this));
     require(balance > 0, 'No funds to rescue');

     SafeERC20.safeTransfer(token, payee, balance);

+    locked = false;
   }

Committable suggestion was skipped due to low confidence.

/**
* @notice Rescues any trapped funds by sending them to the payee
* @dev Can be called by anyone, but funds are always sent to the payee
*/
function rescueNativeFunds() external {
uint256 balance = address(this).balance;
require(balance > 0, 'No funds to rescue');

(bool success, ) = payable(payee).call{value: balance}('');
require(success, 'Rescue failed');
}
Comment on lines +84 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add Reentrancy Guard to rescueNativeFunds Function

The rescueNativeFunds() function allows anyone to transfer Ether held by the contract to the payee. Without reentrancy protection, this function may be vulnerable to reentrancy attacks.

Add a reentrancy guard to this function to enhance security.

Apply this diff to add the guard:

   function rescueNativeFunds() external {
+    require(!locked, "Reentrant call detected");
+    locked = true;

     uint256 balance = address(this).balance;
     require(balance > 0, 'No funds to rescue');

     (bool success, ) = payable(payee).call{value: balance}('');
     require(success, 'Rescue failed');

+    locked = false;
   }

Committable suggestion was skipped due to low confidence.

}
114 changes: 114 additions & 0 deletions packages/smart-contracts/src/contracts/EthereumSingleRequestProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import './interfaces/EthereumFeeProxy.sol';
import '@openzeppelin/contracts/token/ERC20/IERC20.sol';
import './lib/SafeERC20.sol';

/**
* @title EthereumSingleRequestProxy
* @notice This contract is used to send a single request to a payee with a fee sent to a third address
*/
contract EthereumSingleRequestProxy {
address public payee;
bytes public paymentReference;
address public feeAddress;
uint256 public feeAmount;
IEthereumFeeProxy public ethereumFeeProxy;

address private originalSender;

/**
* @dev Custom reentrancy guard.
* Similar to OpenZeppelin's ReentrancyGuard, but allows reentrancy from ethereumFeeProxy.
* This enables controlled callbacks from ethereumFeeProxy while protecting against other reentrancy attacks.
*/
uint256 private constant _NOT_ENTERED = 1;
uint256 private constant _ENTERED = 2;
uint256 private _status;

constructor(
address _payee,
bytes memory _paymentReference,
address _ethereumFeeProxy,
address _feeAddress,
uint256 _feeAmount
) {
payee = _payee;
paymentReference = _paymentReference;
feeAddress = _feeAddress;
feeAmount = _feeAmount;
ethereumFeeProxy = IEthereumFeeProxy(_ethereumFeeProxy);
_status = _NOT_ENTERED;
}

/**
* @dev Modified nonReentrant guard.
* Prevents reentrancy except for calls from ethereumFeeProxy.
*/
modifier nonReentrant() {
if (msg.sender != address(ethereumFeeProxy)) {
require(_status != _ENTERED, 'ReentrancyGuard: reentrant call');
_status = _ENTERED;
}
_;
if (msg.sender != address(ethereumFeeProxy)) {
_status = _NOT_ENTERED;
}
}

receive() external payable nonReentrant {
if (msg.sender == address(ethereumFeeProxy)) {
// Funds are being sent back from EthereumFeeProxy
require(originalSender != address(0), 'No original sender stored');

// Forward the funds to the original sender
(bool forwardSuccess, ) = payable(originalSender).call{value: msg.value}('');
require(forwardSuccess, 'Forwarding to original sender failed');

// Clear the stored original sender
originalSender = address(0);
} else {
require(originalSender == address(0), 'Another request is in progress');

originalSender = msg.sender;

bytes memory data = abi.encodeWithSignature(
'transferWithReferenceAndFee(address,bytes,uint256,address)',
payable(payee),
paymentReference,
feeAmount,
payable(feeAddress)
);

(bool callSuccess, ) = address(ethereumFeeProxy).call{value: msg.value}(data);
require(callSuccess, 'Call to EthereumFeeProxy failed');
}
}

/**
* @notice Rescues any trapped funds by sending them to the payee
* @dev Can be called by anyone, but funds are always sent to the payee
*/
function rescueNativeFunds() external nonReentrant {
uint256 balance = address(this).balance;
require(balance > 0, 'No funds to rescue');

(bool success, ) = payable(payee).call{value: balance}('');
require(success, 'Rescue failed');
}

/**
* @notice Rescues any trapped ERC20 funds by sending them to the payee
* @dev Can be called by anyone, but funds are always sent to the payee
* @param _tokenAddress The address of the ERC20 token to rescue
*/
function rescueERC20Funds(address _tokenAddress) external nonReentrant {
require(_tokenAddress != address(0), 'Invalid token address');
IERC20 token = IERC20(_tokenAddress);
uint256 balance = token.balanceOf(address(this));
require(balance > 0, 'No funds to rescue');
bool success = SafeERC20.safeTransfer(token, payee, balance);
require(success, 'Rescue failed');
}
Comment on lines +106 to +113
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect usage of SafeERC20.safeTransfer; it does not return a boolean.

The safeTransfer function from SafeERC20 does not return a boolean value; it reverts upon failure. Assigning its result to bool success will cause a compilation error. The subsequent require statement is unnecessary as any failure will already revert the transaction.

Apply this diff to fix the code:

-        bool success = SafeERC20.safeTransfer(token, payee, balance);
-        require(success, 'Rescue failed');
+        SafeERC20.safeTransfer(token, payee, balance);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function rescueERC20Funds(address _tokenAddress) external nonReentrant {
require(_tokenAddress != address(0), 'Invalid token address');
IERC20 token = IERC20(_tokenAddress);
uint256 balance = token.balanceOf(address(this));
require(balance > 0, 'No funds to rescue');
bool success = SafeERC20.safeTransfer(token, payee, balance);
require(success, 'Rescue failed');
}
function rescueERC20Funds(address _tokenAddress) external nonReentrant {
require(_tokenAddress != address(0), 'Invalid token address');
IERC20 token = IERC20(_tokenAddress);
uint256 balance = token.balanceOf(address(this));
require(balance > 0, 'No funds to rescue');
SafeERC20.safeTransfer(token, payee, balance);
}

}
117 changes: 117 additions & 0 deletions packages/smart-contracts/src/contracts/SingleRequestProxyFactory.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;

import '@openzeppelin/contracts/access/Ownable.sol';
import './ERC20SingleRequestProxy.sol';
import './EthereumSingleRequestProxy.sol';

/**
* @title SingleRequestProxyFactory
* @notice This contract is used to create SingleRequestProxy instances
*/
contract SingleRequestProxyFactory is Ownable {
/// @notice The address of the EthereumFeeProxy contract
/// @dev This proxy is used for handling Ethereum-based fee transactions
address public ethereumFeeProxy;

/// @notice The address of the ERC20FeeProxy contract
/// @dev This proxy is used for handling ERC20-based fee transactions
address public erc20FeeProxy;

event EthereumSingleRequestProxyCreated(
address indexed proxyAddress,
address indexed payee,
bytes indexed paymentReference
);

event ERC20SingleRequestProxyCreated(
address indexed proxyAddress,
address indexed payee,
address tokenAddress,
bytes indexed paymentReference
);

event ERC20FeeProxyUpdated(address indexed newERC20FeeProxy);
event EthereumFeeProxyUpdated(address indexed newEthereumFeeProxy);

constructor(address _ethereumFeeProxy, address _erc20FeeProxy) {
require(_ethereumFeeProxy != address(0), 'EthereumFeeProxy address cannot be zero');
require(_erc20FeeProxy != address(0), 'ERC20FeeProxy address cannot be zero');
ethereumFeeProxy = _ethereumFeeProxy;
erc20FeeProxy = _erc20FeeProxy;
}

/**
* @notice Creates a new EthereumSingleRequestProxy instance
* @param _payee The address of the payee
* @param _paymentReference The payment reference
* @param _feeAddress The address of the fee recipient
* @param _feeAmount The fee amount
* @return The address of the newly created proxy
*/
function createEthereumSingleRequestProxy(
address _payee,
bytes memory _paymentReference,
address _feeAddress,
uint256 _feeAmount
) external returns (address) {
EthereumSingleRequestProxy proxy = new EthereumSingleRequestProxy(
_payee,
_paymentReference,
ethereumFeeProxy,
_feeAddress,
_feeAmount
);
emit EthereumSingleRequestProxyCreated(address(proxy), _payee, _paymentReference);
return address(proxy);
}

/**
* @notice Creates a new ERC20SingleRequestProxy instance
* @param _payee The address of the payee
* @param _tokenAddress The address of the token
* @param _paymentReference The payment reference
* @param _feeAddress The address of the fee recipient
* @param _feeAmount The fee amount
* @return The address of the newly created proxy
*/
function createERC20SingleRequestProxy(
address _payee,
address _tokenAddress,
bytes memory _paymentReference,
address _feeAddress,
uint256 _feeAmount
) external returns (address) {
ERC20SingleRequestProxy proxy = new ERC20SingleRequestProxy(
_payee,
_tokenAddress,
_feeAddress,
_feeAmount,
_paymentReference,
erc20FeeProxy
);

emit ERC20SingleRequestProxyCreated(address(proxy), _payee, _tokenAddress, _paymentReference);
return address(proxy);
}

/**
* @notice Updates the ERC20FeeProxy address
* @param _newERC20FeeProxy The new ERC20FeeProxy address
*/
function setERC20FeeProxy(address _newERC20FeeProxy) external onlyOwner {
require(_newERC20FeeProxy != address(0), 'ERC20FeeProxy address cannot be zero');
erc20FeeProxy = _newERC20FeeProxy;
emit ERC20FeeProxyUpdated(_newERC20FeeProxy);
}

/**
* @notice Updates the EthereumFeeProxy address
* @param _newEthereumFeeProxy The new EthereumFeeProxy address
*/
function setEthereumFeeProxy(address _newEthereumFeeProxy) external onlyOwner {
require(_newEthereumFeeProxy != address(0), 'EthereumFeeProxy address cannot be zero');
ethereumFeeProxy = _newEthereumFeeProxy;
emit EthereumFeeProxyUpdated(_newEthereumFeeProxy);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract MockEthereumFeeProxy {
function transferWithReferenceAndFee(
address payable _to,
bytes calldata _paymentReference,
uint256 _feeAmount,
address payable _feeAddress
) external payable {
// Do nothing, just accept the funds
}

function sendFundsBack(address payable _to, uint256 _amount) external {
(bool success, ) = _to.call{value: _amount}('');
require(success, 'Failed to send funds back');
}

receive() external payable {}
}
Loading