Skip to content

Instantly share code, notes, and snippets.

@Falilah

Falilah/paycrestStaticReport.md

Created March 4, 2024 12:06
Show Gist options
  • Save Falilah/b2931e7cda6f4484cb2ca779e623c009 to your computer and use it in GitHub Desktop.
Save Falilah/b2931e7cda6f4484cb2ca779e623c009 to your computer and use it in GitHub Desktop.
Download ZIP
Raw
paycrestStaticReport.md

Report

Gas Optimizations

Issue Instances
GAS-1 Using bools for storage incurs overhead 1
GAS-2 For Operations that will not overflow, you could use unchecked 64
GAS-3 Use Custom Errors 12
GAS-4 Don't initialize variables with default value 2
GAS-5 Functions guaranteed to revert when called by normal users can be marked payable 8
GAS-6 ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 2
GAS-7 Use != 0 instead of > 0 for unsigned integer comparison 6

[GAS-1] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (1):

File: PaycrestSettingManager.sol

23:     mapping(address => bool) internal _isTokenSupported;

[GAS-2] For Operations that will not overflow, you could use unchecked

Instances (64):

File: Paycrest.sol

4: import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

4: import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

5: import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

6: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

8: import {PaycrestSettingManager} from "./PaycrestSettingManager.sol";

9: import {IPaycrest, IERC20} from "./interfaces/IPaycrest.sol";

9: import {IPaycrest, IERC20} from "./interfaces/IPaycrest.sol";

10: import {SharedStructs} from "./libraries/SharedStructs.sol";

10: import {SharedStructs} from "./libraries/SharedStructs.sol";

53:     ################################################################## */

53:     ################################################################## */

70:     ################################################################## */

70:     ################################################################## */

90:         IERC20(_token).transferFrom(msg.sender, address(this), _amount + _senderFee);

93:         _nonce[msg.sender] ++;

93:         _nonce[msg.sender] ++;

100:         uint256 _protocolFee = (_amount * protocolFeePercent) / MAX_BPS;

100:         uint256 _protocolFee = (_amount * protocolFeePercent) / MAX_BPS;

111:             amount: _amount - _protocolFee

140:     ################################################################## */

140:     ################################################################## */

156:         order[_orderId].currentBPS -= _settlePercent;

200:         uint256 refundAmount = order[_orderId].amount - _fee;

216:     ################################################################## */

216:     ################################################################## */

241:                 i++;

241:                 i++;

File: PaycrestSettingManager.sol

9: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

9: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

9: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

9: import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

11: import {SharedStructs} from "./libraries/SharedStructs.sol";

11: import {SharedStructs} from "./libraries/SharedStructs.sol";

36:     ################################################################## */

36:     ################################################################## */

69:                 i++;

69:                 i++;

File: interfaces/IPaycrest.sol

4: import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

4: import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

6: import {SharedStructs} from "../libraries/SharedStructs.sol";

6: import {SharedStructs} from "../libraries/SharedStructs.sol";

16:     ################################################################## */

16:     ################################################################## */

56:     ################################################################## */

56:     ################################################################## */

101:     ################################################################## */

101:     ################################################################## */

File: mocks/MockUSDC.sol

4: import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

4: import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

4: import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

4: import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

[GAS-3] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (12):

File: Paycrest.sol

47:         require(msg.sender == _aggregatorAddress, "OnlyAggregator");

87:         require(bytes(messageHash).length > 0, "InvalidMessageHash");

128:         require(_isTokenSupported[_token], "TokenNotSupported");

129:         require(_amount > 0, "AmountIsZero");

130:         require(_refundAddress != address(0), "ThrowZeroAddress");

131:         require(supportedInstitutionsByCode[_institutionCode].name != bytes32(0), "InvalidInstitutionCode");

134:             require(_senderFeeRecipient != address(0), "InvalidSenderFeeRecipient");

150:         require(!order[_orderId].isFulfilled, "OrderFulfilled");

197:         require(!order[_orderId].isFulfilled, "OrderFulfilled");

File: PaycrestSettingManager.sol

47:         require(value != address(0), "Paycrest: zero address");

91:         require(value != address(0), "Paycrest: zero address");

File: mocks/MockUSDC.sol

22:         require(_balanceOf > 0, "MockUSDT: Nothing to burn");

[GAS-4] Don't initialize variables with default value

Instances (2):

File: Paycrest.sol

238:         for (uint256 i = 0; i < length; ) {

File: PaycrestSettingManager.sol

63:         for (uint i = 0; i < length; ) {

[GAS-5] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Instances (8):

File: Paycrest.sol

57:     function pause() external onlyOwner {

64:     function unpause() external onlyOwner {

195:     function refund(uint256 _fee, bytes32 _orderId, bytes32 _label) external onlyAggregator() returns(bool) {

File: PaycrestSettingManager.sol

46:     function settingManagerBool(bytes32 what, address value, bool status) external onlyOwner {

59:     function setSupportedInstitutions(bytes32 currency, SharedStructs.Institution[] memory institutions) external onlyOwner {

78:     function updateProtocolFees(uint64 _protocolFeePercent) external onlyOwner {

90:     function updateProtocolAddresses(bytes32 what, address value) external onlyOwner {

103:     function updateProtocolAggregator(bytes calldata aggregator) external onlyOwner {

[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (2):

File: Paycrest.sol

241:                 i++;

File: PaycrestSettingManager.sol

69:                 i++;

[GAS-7] Use != 0 instead of > 0 for unsigned integer comparison

Instances (6):

File: Paycrest.sol

87:         require(bytes(messageHash).length > 0, "InvalidMessageHash");

129:         require(_amount > 0, "AmountIsZero");

133:         if (_senderFee > 0) {

162:             if (order[_orderId].senderFee > 0) {

167:             if (order[_orderId].protocolFee > 0) {

File: mocks/MockUSDC.sol

22:         require(_balanceOf > 0, "MockUSDT: Nothing to burn");

Low Issues

Issue Instances
L-1 Initializers could be front-run 4
L-2 Unsafe ERC20 operation(s) 6

[L-1] Initializers could be front-run

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Instances (4):

File: Paycrest.sol

37:     function initialize() external initializer {

37:     function initialize() external initializer {

39:         __Ownable_init();

40:         __Pausable_init();

[L-2] Unsafe ERC20 operation(s)

Instances (6):

File: Paycrest.sol

90:         IERC20(_token).transferFrom(msg.sender, address(this), _amount + _senderFee);

169:                 IERC20(token).transfer(treasuryAddress, order[_orderId].protocolFee);

174:         IERC20(token).transfer(_liquidityProvider, order[_orderId].amount);

189:         IERC20(order[_orderId].token).transfer(recipient, _fee);

201:         IERC20(order[_orderId].token).transfer(treasuryAddress, _fee);

208:         IERC20(order[_orderId].token).transfer(order[_orderId].refundAddress, refundAmount);

Medium Issues

Issue Instances
M-1 Centralization Risk for trusted owners 7

[M-1] Centralization Risk for trusted owners

Impact:

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

Instances (7):

File: Paycrest.sol

57:     function pause() external onlyOwner {

64:     function unpause() external onlyOwner {

File: PaycrestSettingManager.sol

46:     function settingManagerBool(bytes32 what, address value, bool status) external onlyOwner {

59:     function setSupportedInstitutions(bytes32 currency, SharedStructs.Institution[] memory institutions) external onlyOwner {

78:     function updateProtocolFees(uint64 _protocolFeePercent) external onlyOwner {

90:     function updateProtocolAddresses(bytes32 what, address value) external onlyOwner {

103:     function updateProtocolAggregator(bytes calldata aggregator) external onlyOwner {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment