Merge pull request #18 from pnetwork-association/chore/required-changes

Refactors some part reported to a recent feedback
pnetwork-association · Jul 29, 2024 · c442ece · c442ece
2 parents 7f7853d + fa813e8
commit c442ece
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 13 deletions.
diff --git a/solidity/src/Adapter.sol b/solidity/src/Adapter.sol
@@ -41,7 +41,7 @@ contract Adapter is IAdapter, Ownable {
     error AlreadyProcessed(bytes32 operationId);
     error UnsupportedProtocolId(bytes1 protocolId);
     error InvalidEventContentLength(uint256 length);
-    error UnsufficientAmount(uint256 amount, uint256 fees);
+    error InsufficientAmount(uint256 amount, uint256 fees);
     error InvalidMessageId(uint256 actual, uint256 expected);
     error InvalidDestinationChainId(uint256 destinationChainId);
 

diff --git a/solidity/src/ptoken-v2/PTokenV2.sol b/solidity/src/ptoken-v2/PTokenV2.sol
@@ -356,7 +356,7 @@ contract PTokenV2 is
             );
         }
 
-        if (fees > _amount) revert("UnsufficientAmount");
+        if (fees > _amount) revert("InsufficientAmount");
 
         uint256 netAmount = _amount - fees;
 

diff --git a/solidity/src/ptoken-v2/PTokenV2NoGSN.sol b/solidity/src/ptoken-v2/PTokenV2NoGSN.sol
@@ -393,7 +393,7 @@ contract PTokenV2NoGSN is
             );
         }
 
-        if (fees > _amount) revert("UnsufficientAmount");
+        if (fees > _amount) revert("InsufficientAmount");
 
         uint256 netAmount = _amount - fees;
 

diff --git a/solidity/src/xerc20/XERC20.sol b/solidity/src/xerc20/XERC20.sol
@@ -32,7 +32,7 @@ contract XERC20 is ERC20, Ownable, IXERC20, ERC20Permit {
     mapping(address => address) public adapterToPAM;
 
     error OnlyFeesManager();
-    error UnsufficientAmount();
+    error InsufficientAmount();
     error NotAContract(address addr);
 
     event FeesManagerChanged(address newAddress);
@@ -52,14 +52,21 @@ contract XERC20 is ERC20, Ownable, IXERC20, ERC20Permit {
         address _factory
     ) ERC20(_name, _symbol) ERC20Permit(_name) Ownable(msg.sender) {}
 
-    /// @inheritdoc IXERC20
-    function setFeesManager(address newAddress) public {
-        if (feesManager == address(0)) {
-            feesManager = newAddress;
-        } else if (msg.sender != feesManager) revert OnlyFeesManager();
+    modifier onlyContractAddress(address anAddress) {
+        if (anAddress.code.length == 0) revert NotAContract(anAddress);
+        _;
+    }
 
-        if (newAddress.code.length == 0) revert NotAContract(newAddress);
+    modifier onlyFeesManager() {
+        if (feesManager != address(0) && msg.sender != feesManager)
+            revert OnlyFeesManager();
+        _;
+    }
 
+    /// @inheritdoc IXERC20
+    function setFeesManager(
+        address newAddress
+    ) public onlyFeesManager onlyContractAddress(newAddress) {
         feesManager = newAddress;
 
         emit FeesManagerChanged(newAddress);
@@ -75,7 +82,7 @@ contract XERC20 is ERC20, Ownable, IXERC20, ERC20Permit {
     function setPAM(
         address adapterAddress,
         address pamAddress
-    ) external onlyOwner {
+    ) external onlyOwner onlyContractAddress(pamAddress) {
         adapterToPAM[adapterAddress] = pamAddress;
         emit PAMChanged(pamAddress);
     }
@@ -357,15 +364,15 @@ contract XERC20 is ERC20, Ownable, IXERC20, ERC20Permit {
         uint256 _amount
     ) internal {
         uint256 fees;
-        // is local?
+        // We don't make an internal call to isLocal() in order to save gas
         if (lockbox != address(0) && feesManager != address(0)) {
             fees = IFeesManager(feesManager).calculateFee(
                 address(this),
                 _amount
             );
         }
 
-        if (fees > _amount) revert UnsufficientAmount();
+        if (fees > _amount) revert InsufficientAmount();
 
         uint256 netAmount = _amount - fees;
-Original file line number
+Diff line change
@@ Expand Up / @@ -356,7 +356,7 @@ contract PTokenV2 is @@
                 );
             }
-            if (fees > _amount) revert("UnsufficientAmount");
+            if (fees > _amount) revert("InsufficientAmount");
             uint256 netAmount = _amount - fees;
@@ Expand Down @@