From d379f35d9006aeb763079300813c214598ece9c1 Mon Sep 17 00:00:00 2001 From: Daiana Bilbao Date: Sun, 7 May 2023 17:44:17 -0600 Subject: [PATCH] optimizations --- src/Gas.sol | 255 +++++++++++++++------------------------ src/Ownable.sol | 2 +- test/Gas.UnitTests.t.sol | 3 +- 3 files changed, 99 insertions(+), 161 deletions(-) diff --git a/src/Gas.sol b/src/Gas.sol index aa95425..4d658a1 100644 --- a/src/Gas.sol +++ b/src/Gas.sol @@ -1,20 +1,21 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.0; +pragma solidity ^0.8.4; import "./Ownable.sol"; +error NotAdminOrOwner(); +error NotWhitelisted(); + contract Constants { - uint256 public tradeFlag = 1; - uint256 public basicFlag = 0; - uint256 public dividendFlag = 1; + bytes1 public tradeFlag = hex"01"; + bytes1 public basicFlag = hex"00"; + bytes1 public dividendFlag = hex"01"; } contract GasContract is Ownable, Constants { - uint256 public totalSupply = 0; // cannot be updated uint256 public paymentCounter = 0; mapping(address => uint256) public balances; uint256 public tradePercent = 12; - address public contractOwner; uint256 public tradeMode = 0; mapping(address => Payment[]) public payments; mapping(address => uint256) public whitelist; @@ -29,15 +30,16 @@ contract GasContract is Ownable, Constants { } PaymentType constant defaultPayment = PaymentType.Unknown; - History[] public paymentHistory; // when a payment was updated + History[] public paymentHistory; + mapping(address => bool) public isAdmin; struct Payment { PaymentType paymentType; uint256 paymentID; bool adminUpdated; - string recipientName; // max 8 characters + string recipientName; address recipient; - address admin; // administrators address + address admin; uint256 amount; } @@ -48,12 +50,12 @@ contract GasContract is Ownable, Constants { } uint256 wasLastOdd = 1; mapping(address => uint256) public isOddWhitelistUser; - + struct ImportantStruct { uint256 amount; - uint256 valueA; // max 3 digits + uint256 valueA; uint256 bigValue; - uint256 valueB; // max 3 digits + uint256 valueB; bool paymentStatus; address sender; } @@ -61,30 +63,18 @@ contract GasContract is Ownable, Constants { event AddedToWhitelist(address userAddress, uint256 tier); - modifier onlyAdminOrOwner() { - address senderOfTx = msg.sender; - if (checkForAdmin(senderOfTx)) { - require( - checkForAdmin(senderOfTx), - "Gas Contract Only Admin Check- Caller not admin" - ); - _; - } else if (senderOfTx == contractOwner) { - _; - } else { - revert( - "Error in Gas contract - onlyAdminOrOwner modifier : revert happened because the originator of the transaction was not the admin, and furthermore he wasn't the owner of the contract, so he cannot run this function" - ); + function onlyAdminOrOwner() private view { + if (!checkForAdmin(msg.sender) && msg.sender != owner()) { + revert NotAdminOrOwner(); } } - modifier checkIfWhiteListed(address sender) { - address senderOfTx = msg.sender; + function checkIfWhiteListed(address sender) private view { require( - senderOfTx == sender, + msg.sender == sender, "Gas Contract CheckIfWhiteListed modifier : revert happened because the originator of the transaction was not the sender" ); - uint256 usersTier = whitelist[senderOfTx]; + uint256 usersTier = whitelist[msg.sender]; require( usersTier > 0, "Gas Contract CheckIfWhiteListed modifier : revert happened because the user is not whitelisted" @@ -93,7 +83,6 @@ contract GasContract is Ownable, Constants { usersTier < 4, "Gas Contract CheckIfWhiteListed modifier : revert happened because the user's tier is incorrect, it cannot be over 4 as the only tier we have are: 1, 2, 3; therfore 4 is an invalid tier for the whitlist of this contract. make sure whitlist tiers were set correctly" ); - _; } event supplyChanged(address indexed, uint256 indexed); @@ -107,20 +96,18 @@ contract GasContract is Ownable, Constants { event WhiteListTransfer(address indexed); constructor(address[] memory _admins, uint256 _totalSupply) { - contractOwner = msg.sender; - totalSupply = _totalSupply; - - for (uint256 ii = 0; ii < administrators.length; ii++) { + for (uint8 ii = 0; ii < administrators.length; ii++) { if (_admins[ii] != address(0)) { administrators[ii] = _admins[ii]; - if (_admins[ii] == contractOwner) { - balances[contractOwner] = totalSupply; + isAdmin[_admins[ii]] = true; + if (_admins[ii] == msg.sender) { + balances[msg.sender] = _totalSupply; } else { balances[_admins[ii]] = 0; } - if (_admins[ii] == contractOwner) { - emit supplyChanged(_admins[ii], totalSupply); - } else if (_admins[ii] != contractOwner) { + if (_admins[ii] == msg.sender) { + emit supplyChanged(_admins[ii], _totalSupply); + } else if (_admins[ii] != msg.sender) { emit supplyChanged(_admins[ii], 0); } } @@ -135,53 +122,33 @@ contract GasContract is Ownable, Constants { return paymentHistory; } - function checkForAdmin(address _user) public view returns (bool admin_) { - bool admin = false; - for (uint256 ii = 0; ii < administrators.length; ii++) { - if (administrators[ii] == _user) { - admin = true; - } - } - return admin; + function checkForAdmin(address _user) private view returns (bool admin_) { + return isAdmin[_user]; } function balanceOf(address _user) public view returns (uint256 balance_) { - uint256 balance = balances[_user]; - return balance; + return balances[_user]; } - function getTradingMode() public view returns (bool mode_) { - bool mode = false; - if (tradeFlag == 1 || dividendFlag == 1) { - mode = true; - } else { - mode = false; - } - return mode; + function getTradingMode() public view returns (bool) { + return (uint8(tradeFlag) == 1 || uint8(dividendFlag) == 1); } - - function addHistory(address _updateAddress, bool _tradeMode) - public - returns (bool status_, bool tradeMode_) - { + function addHistory( + address _updateAddress, + bool _tradeMode + ) public returns (bool status_, bool tradeMode_) { History memory history; history.blockNumber = block.number; history.lastUpdate = block.timestamp; history.updatedBy = _updateAddress; paymentHistory.push(history); - bool[] memory status = new bool[](tradePercent); - for (uint256 i = 0; i < tradePercent; i++) { - status[i] = true; - } - return ((status[0] == true), _tradeMode); + return (true, _tradeMode); } - function getPayments(address _user) - public - view - returns (Payment[] memory payments_) - { + function getPayments( + address _user + ) public view returns (Payment[] memory payments_) { require( _user != address(0), "Gas Contract - getPayments function - User must have a valid non zero address" @@ -194,40 +161,40 @@ contract GasContract is Ownable, Constants { uint256 _amount, string calldata _name ) public returns (bool status_) { - address senderOfTx = msg.sender; require( - balances[senderOfTx] >= _amount, + balances[msg.sender] >= _amount, "Gas Contract - Transfer function - Sender has insufficient Balance" ); require( bytes(_name).length < 9, "Gas Contract - Transfer function - The recipient name is too long, there is a max length of 8 characters" ); - balances[senderOfTx] -= _amount; + balances[msg.sender] -= _amount; balances[_recipient] += _amount; emit Transfer(_recipient, _amount); - Payment memory payment; - payment.admin = address(0); - payment.adminUpdated = false; - payment.paymentType = PaymentType.BasicPayment; - payment.recipient = _recipient; - payment.amount = _amount; - payment.recipientName = _name; - payment.paymentID = ++paymentCounter; - payments[senderOfTx].push(payment); - bool[] memory status = new bool[](tradePercent); - for (uint256 i = 0; i < tradePercent; i++) { - status[i] = true; - } - return (status[0] == true); + uint256 paymentID = ++paymentCounter; + userPayments[msg.sender][paymentID] = Payment({ + admin: address(0), + adminUpdated: false, + paymentType: PaymentType.BasicPayment, + recipient: _recipient, + amount: _amount, + recipientName: _name, + paymentID: paymentID + }); + return true; } + mapping(address => mapping(uint256 => Payment)) public userPayments; + function updatePayment( address _user, uint256 _ID, uint256 _amount, PaymentType _type - ) public onlyAdminOrOwner { + ) public { + onlyAdminOrOwner(); + require( _ID > 0, "Gas Contract - Update Payment function - ID must be greater than 0" @@ -241,92 +208,64 @@ contract GasContract is Ownable, Constants { "Gas Contract - Update Payment function - Administrator must have a valid non zero address" ); - address senderOfTx = msg.sender; - - for (uint256 ii = 0; ii < payments[_user].length; ii++) { - if (payments[_user][ii].paymentID == _ID) { - payments[_user][ii].adminUpdated = true; - payments[_user][ii].admin = _user; - payments[_user][ii].paymentType = _type; - payments[_user][ii].amount = _amount; - bool tradingMode = getTradingMode(); - addHistory(_user, tradingMode); - emit PaymentUpdated( - senderOfTx, - _ID, - _amount, - payments[_user][ii].recipientName - ); - } - } + Payment storage payment = userPayments[_user][_ID]; + payment.adminUpdated = true; + payment.admin = _user; + payment.paymentType = _type; + payment.amount = _amount; + addHistory(_user, getTradingMode()); + emit PaymentUpdated(msg.sender, _ID, _amount, payment.recipientName); } - function addToWhitelist(address _userAddrs, uint256 _tier) - public - onlyAdminOrOwner - { + function addToWhitelist(address _userAddrs, uint256 _tier) public { + onlyAdminOrOwner(); require( _tier < 255, - "Gas Contract - addToWhitelist function - tier level should not be greater than 255" + "Gas Contract - addToWhitelist function - tier level should not be greater than 255" ); - whitelist[_userAddrs] = _tier; - if (_tier > 3) { - whitelist[_userAddrs] -= _tier; - whitelist[_userAddrs] = 3; - } else if (_tier == 1) { - whitelist[_userAddrs] -= _tier; - whitelist[_userAddrs] = 1; - } else if (_tier > 0 && _tier < 3) { - whitelist[_userAddrs] -= _tier; - whitelist[_userAddrs] = 2; - } - uint256 wasLastAddedOdd = wasLastOdd; - if (wasLastAddedOdd == 1) { - wasLastOdd = 0; - isOddWhitelistUser[_userAddrs] = wasLastAddedOdd; - } else if (wasLastAddedOdd == 0) { - wasLastOdd = 1; - isOddWhitelistUser[_userAddrs] = wasLastAddedOdd; - } else { - revert("Contract hacked, imposible, call help"); - } + whitelist[_userAddrs] = (_tier > 3) ? 3 : ((_tier == 1) ? 1 : 2); + isOddWhitelistUser[_userAddrs] = (wasLastOdd == 1) ? 0 : 1; + wasLastOdd = isOddWhitelistUser[_userAddrs]; emit AddedToWhitelist(_userAddrs, _tier); } - function whiteTransfer( - address _recipient, - uint256 _amount - ) public checkIfWhiteListed(msg.sender) { - address senderOfTx = msg.sender; - whiteListStruct[senderOfTx] = ImportantStruct(_amount, 0, 0, 0, true, msg.sender); - + function whiteTransfer(address _recipient, uint256 _amount) public { + checkIfWhiteListed(msg.sender); + whiteListStruct[msg.sender] = ImportantStruct( + _amount, + 0, + 0, + 0, + true, + msg.sender + ); + require( - balances[senderOfTx] >= _amount, + balances[msg.sender] >= _amount, "Gas Contract - whiteTransfers function - Sender has insufficient Balance" ); require( _amount > 3, "Gas Contract - whiteTransfers function - amount to send have to be bigger than 3" ); - balances[senderOfTx] -= _amount; + balances[msg.sender] -= _amount; balances[_recipient] += _amount; - balances[senderOfTx] += whitelist[senderOfTx]; - balances[_recipient] -= whitelist[senderOfTx]; - - emit WhiteListTransfer(_recipient); - } - + balances[msg.sender] += whitelist[msg.sender]; + balances[_recipient] -= whitelist[msg.sender]; - function getPaymentStatus(address sender) public returns (bool, uint256) { - return (whiteListStruct[sender].paymentStatus, whiteListStruct[sender].amount); + emit WhiteListTransfer(_recipient); } - receive() external payable { - payable(msg.sender).transfer(msg.value); + function getPaymentStatus( + address sender + ) public view returns (bool, uint256) { + return ( + whiteListStruct[sender].paymentStatus, + whiteListStruct[sender].amount + ); } + receive() external payable {} - fallback() external payable { - payable(msg.sender).transfer(msg.value); - } -} \ No newline at end of file + fallback() external payable {} +} diff --git a/src/Ownable.sol b/src/Ownable.sol index a8736c8..26a8ef5 100644 --- a/src/Ownable.sol +++ b/src/Ownable.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.0; +pragma solidity ^0.8.4; /** * @dev Provides information about the current execution context, including the diff --git a/test/Gas.UnitTests.t.sol b/test/Gas.UnitTests.t.sol index 3ee8423..a772e5e 100644 --- a/test/Gas.UnitTests.t.sol +++ b/test/Gas.UnitTests.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import "forge-std/Test.sol"; -import "../../src/Gas.sol"; +import "../src/Gas.sol"; contract GasTest is Test { GasContract public gas; @@ -84,7 +84,6 @@ contract GasTest is Test { vm.prank(_sender); gas.whiteTransfer(_recipient, _amount); (bool a, uint256 b) = gas.getPaymentStatus(address(_sender)); - console.log(a); assertEq(a, true); assertEq(b, _amount); }