pimlicolabs / erc20-paymaster

An ERC-4337 Paymaster contract by Pimlico that is able to sponsor gas fees in exchange for ERC-20 tokens
https://pimlico.io
MIT License
72 stars 19 forks source link

Update `Magic Numbers` with `constants` and Update `public` variables with `private` for gas-efficiency #20

Open allwin199 opened 6 months ago

allwin199 commented 6 months ago

This codebase currently uses magic numbers in certain areas. Replacing these magic numbers with descriptive constants will significantly improve code readability and maintainability.

https://github.com/pimlicolabs/erc20-paymaster/blob/8e37933ea0125352d89659cb623a959071bf879e/src/ERC20Paymaster.sol#L79

1e6 magic number can be updated with PRICE_DENOMINATOR in the below line https://github.com/pimlicolabs/erc20-paymaster/blob/8e37933ea0125352d89659cb623a959071bf879e/src/ERC20Paymaster.sol#L134-L136

https://github.com/pimlicolabs/erc20-paymaster/blob/8e37933ea0125352d89659cb623a959071bf879e/src/ERC20Paymaster.sol#L140-L142

The above lines can be updated with the below lines of code.

uint256 private constant DECIMAL_PRECISION = 8;

.
.
.

if (_tokenOracle.decimals() != DECIMAL_PRECISION || _nativeAssetOracle.decimals() != DECIMAL_PRECISION) { 
     revert OracleDecimalsInvalid(); 
 } 

some more magic numbers are used. It can be updated with constants

Also, Instead of declaring all variables as public. All variables can be declared as private and we can write external getter functions to get the values. Which can prove to be gas-efficient.

-  uint256 public constant PRICE_DENOMINATOR = 1e6; 
+ uint256 private constant PRICE_DENOMINATOR = 1e6; 

External Getter function

function getPriceDenominator() external view returns(uint256){
     return PRICE_DENOMINATOR;
}
allwin199 commented 6 months ago

@kristofgazso Making these changes will help in code readability and gas cost.

Making these changes will break the test cases. I am willing to work on it and make sure everything works as intended.

Narendra-Reddy1 commented 1 month ago

Hey @allwin199 are you still working on this issue?? If not I'll take up this issue