Merge branch 'master' into msg-sender-trick

ItsNickBarry · ItsNickBarry · commit 29b712924526 · 2025-03-18T12:44:19.000-06:00
diff --git a/contracts/factory/Factory.sol b/contracts/factory/Factory.sol
@@ -46,7 +46,7 @@ library Factory {
     }
 
     /**
-     * @notice calculate the _deployMetamorphicContract deployment address for a given salt
+     * @notice calculate the deployment address for a given salt
      * @param initCodeHash hash of contract initialization code
      * @param salt input for deterministic address calculation
      * @return deployment address
diff --git a/contracts/proxy/IProxy.sol b/contracts/proxy/IProxy.sol
@@ -5,7 +5,5 @@ pragma solidity ^0.8.20;
 import { _IProxy } from './_IProxy.sol';
 
 interface IProxy is _IProxy {
-    error Proxy__ImplementationIsNotContract();
-
     fallback() external payable;
 }
diff --git a/contracts/proxy/Proxy.sol b/contracts/proxy/Proxy.sol
@@ -2,46 +2,18 @@
 
 pragma solidity ^0.8.20;
 
-import { AddressUtils } from '../utils/AddressUtils.sol';
 import { IProxy } from './IProxy.sol';
 import { _Proxy } from './_Proxy.sol';
 
 /**
  * @title Base proxy contract
  */
 abstract contract Proxy is IProxy, _Proxy {
-    using AddressUtils for address;
-
     /**
      * @notice delegate all calls to implementation contract
-     * @dev reverts if implementation address contains no code, for compatibility with metamorphic contracts
-     * @dev memory location in use by assembly may be unsafe in other contexts
+     * @dev reverts if implementation address contains no code
      */
-    fallback() external payable virtual {
-        address implementation = _getImplementation();
-
-        if (!implementation.isContract())
-            revert Proxy__ImplementationIsNotContract();
-
-        assembly {
-            calldatacopy(0, 0, calldatasize())
-            let result := delegatecall(
-                gas(),
-                implementation,
-                0,
-                calldatasize(),
-                0,
-                0
-            )
-            returndatacopy(0, 0, returndatasize())
-
-            switch result
-            case 0 {
-                revert(0, returndatasize())
-            }
-            default {
-                return(0, returndatasize())
-            }
-        }
+    fallback() external payable {
+        _fallback();
     }
 }
diff --git a/contracts/proxy/_IProxy.sol b/contracts/proxy/_IProxy.sol
@@ -2,4 +2,6 @@
 
 pragma solidity ^0.8.20;
 
-interface _IProxy {}
+interface _IProxy {
+    error Proxy__ImplementationIsNotContract();
+}
diff --git a/contracts/proxy/_Proxy.sol b/contracts/proxy/_Proxy.sol
@@ -16,14 +16,9 @@ abstract contract _Proxy is _IProxy {
         virtual
         returns (address implementation)
     {
-        // inline storage layout retrieval uses less gas
-        ProxyStorage.Layout storage $;
-        bytes32 slot = ProxyStorage.DEFAULT_STORAGE_SLOT;
-        assembly {
-            $.slot := slot
-        }
-
-        implementation = $.implementation;
+        implementation = ProxyStorage
+            .layout(ProxyStorage.DEFAULT_STORAGE_SLOT)
+            .implementation;
     }
 
     /**
@@ -35,4 +30,42 @@ abstract contract _Proxy is _IProxy {
             .layout(ProxyStorage.DEFAULT_STORAGE_SLOT)
             .implementation = implementation;
     }
+
+    /**
+     * @notice delegate all calls to implementation contract
+     * @dev memory location in use by assembly may be unsafe in other contexts
+     * @dev function declares no return value, but data is returned via assembly
+     */
+    function _fallback() internal virtual {
+        address implementation = _getImplementation();
+
+        assembly {
+            calldatacopy(0, 0, calldatasize())
+
+            let result := delegatecall(
+                gas(),
+                implementation,
+                0,
+                calldatasize(),
+                0,
+                0
+            )
+
+            returndatacopy(0, 0, returndatasize())
+
+            if iszero(result) {
+                revert(0, returndatasize())
+            }
+
+            if returndatasize() {
+                return(0, returndatasize())
+            }
+
+            if extcodesize(implementation) {
+                return(0, returndatasize())
+            }
+        }
+
+        revert Proxy__ImplementationIsNotContract();
+    }
 }
diff --git a/contracts/proxy/diamond/_DiamondProxy.sol b/contracts/proxy/diamond/_DiamondProxy.sol
@@ -25,14 +25,24 @@ abstract contract _DiamondProxy is _IDiamondProxy, _Proxy {
         override
         returns (address implementation)
     {
-        // inline storage layout retrieval uses less gas
-        ERC2535Storage.Layout storage $;
-        bytes32 slot = ERC2535Storage.DEFAULT_STORAGE_SLOT;
-        assembly {
-            $.slot := slot
-        }
+        implementation = _facetAddress(msg.sig);
+    }
 
-        implementation = address(bytes20($.selectorInfo[msg.sig]));
+    /**
+     * @notice get the address of the facet associated with given selector
+     * @param selector function selector to query
+     * @return facet facet address (zero address if not found)
+     */
+    function _facetAddress(
+        bytes4 selector
+    ) internal view returns (address facet) {
+        facet = address(
+            bytes20(
+                ERC2535Storage
+                    .layout(ERC2535Storage.DEFAULT_STORAGE_SLOT)
+                    .selectorInfo[selector]
+            )
+        );
     }
 
     /**
diff --git a/contracts/proxy/diamond/readable/_DiamondProxyReadable.sol b/contracts/proxy/diamond/readable/_DiamondProxyReadable.sol
@@ -200,21 +200,4 @@ abstract contract _DiamondProxyReadable is
             mstore(addresses, numFacets)
         }
     }
-
-    /**
-     * @notice get the address of the facet associated with given selector
-     * @param selector function selector to query
-     * @return facet facet address (zero address if not found)
-     */
-    function _facetAddress(
-        bytes4 selector
-    ) internal view returns (address facet) {
-        facet = address(
-            bytes20(
-                ERC2535Storage
-                    .layout(ERC2535Storage.DEFAULT_STORAGE_SLOT)
-                    .selectorInfo[selector]
-            )
-        );
-    }
 }
diff --git a/contracts/token/non_fungible/_NonFungibleToken.sol b/contracts/token/non_fungible/_NonFungibleToken.sol
@@ -245,7 +245,7 @@ abstract contract _NonFungibleToken is
                 tokenId,
                 data
             ),
-            'NonFungibleToken: transfer to non ERC721Receiver implementer'
+            NonFungibleToken__ERC721ReceiverNotImplemented.selector
         );
 
         bytes4 returnValue = abi.decode(returnData, (bytes4));
diff --git a/contracts/utils/AddressUtils.sol b/contracts/utils/AddressUtils.sol
@@ -10,6 +10,9 @@ library AddressUtils {
     error AddressUtils__InsufficientBalance();
     error AddressUtils__NotContract();
     error AddressUtils__SendValueFailed();
+    error AddressUtils__FailedCall();
+    error AddressUtils__FailedCallWithValue();
+    error AddressUtils__FailedDelegatecall();
 
     function toString(address account) internal pure returns (string memory) {
         return uint256(uint160(account)).toHexString(20);
@@ -32,14 +35,13 @@ library AddressUtils {
         address target,
         bytes memory data
     ) internal returns (bytes memory) {
-        return
-            functionCall(target, data, 'AddressUtils: failed low-level call');
+        return functionCall(target, data, AddressUtils__FailedCall.selector);
     }
 
     function functionCall(
         address target,
         bytes memory data,
-        string memory error
+        bytes4 error
     ) internal returns (bytes memory) {
         return _functionCallWithValue(target, data, 0, error);
     }
@@ -54,21 +56,43 @@ library AddressUtils {
                 target,
                 data,
                 value,
-                'AddressUtils: failed low-level call with value'
+                AddressUtils__FailedCallWithValue.selector
             );
     }
 
     function functionCallWithValue(
         address target,
         bytes memory data,
         uint256 value,
-        string memory error
+        bytes4 error
     ) internal returns (bytes memory) {
         if (value > address(this).balance)
             revert AddressUtils__InsufficientBalance();
         return _functionCallWithValue(target, data, value, error);
     }
 
+    function functionDelegateCall(
+        address target,
+        bytes memory data
+    ) internal returns (bytes memory) {
+        return
+            functionDelegateCall(
+                target,
+                data,
+                AddressUtils__FailedDelegatecall.selector
+            );
+    }
+
+    function functionDelegateCall(
+        address target,
+        bytes memory data,
+        bytes4 error
+    ) internal returns (bytes memory) {
+        (bool success, bytes memory returnData) = target.delegatecall(data);
+        _verifyCallResultFromTarget(target, success, returnData, error);
+        return returnData;
+    }
+
     /**
      * @notice execute arbitrary external call with limited gas usage and amount of copied return data
      * @dev derived from https://github.com/nomad-xyz/ExcessivelySafeCall (MIT License)
@@ -120,23 +144,37 @@ library AddressUtils {
         address target,
         bytes memory data,
         uint256 value,
-        string memory error
+        bytes4 error
     ) private returns (bytes memory) {
-        if (!isContract(target)) revert AddressUtils__NotContract();
-
         (bool success, bytes memory returnData) = target.call{ value: value }(
             data
         );
+        _verifyCallResultFromTarget(target, success, returnData, error);
+        return returnData;
+    }
 
-        if (success) {
-            return returnData;
-        } else if (returnData.length > 0) {
-            assembly {
-                let returnData_size := mload(returnData)
-                revert(add(32, returnData), returnData_size)
+    function _verifyCallResultFromTarget(
+        address target,
+        bool success,
+        bytes memory returnData,
+        bytes4 error
+    ) private view {
+        if (!success) {
+            if (returnData.length == 0) {
+                assembly {
+                    mstore(0, error)
+                    revert(0, 4)
+                }
+            } else {
+                assembly {
+                    let returnData_size := mload(returnData)
+                    revert(add(32, returnData), returnData_size)
+                }
             }
-        } else {
-            revert(error);
         }
+
+        // code check is only required if call is successful and without return data
+        if (returnData.length == 0 && !isContract(target))
+            revert AddressUtils__NotContract();
     }
 }
diff --git a/contracts/utils/SafeERC20.sol b/contracts/utils/SafeERC20.sol
@@ -91,7 +91,7 @@ library SafeERC20 {
     function _callOptionalReturn(IERC20 token, bytes memory data) private {
         bytes memory returndata = address(token).functionCall(
             data,
-            'SafeERC20: low-level call failed'
+            SafeERC20__OperationFailed.selector
         );
 
         if (returndata.length > 0) {
diff --git a/spec/token/non_fungible/NonFungibleToken.behavior.ts b/spec/token/non_fungible/NonFungibleToken.behavior.ts
@@ -378,8 +378,9 @@ export function describeBehaviorOfNonFungibleToken(
               [
                 'safeTransferFrom(address,address,uint256)'
               ](holder.address, await instance.getAddress(), tokenId),
-          ).to.be.revertedWith(
-            'NonFungibleToken: transfer to non ERC721Receiver implementer',
+          ).to.be.revertedWithCustomError(
+            instance,
+            'NonFungibleToken__ERC721ReceiverNotImplemented',
           );
         });
 
@@ -510,8 +511,9 @@ export function describeBehaviorOfNonFungibleToken(
               [
                 'safeTransferFrom(address,address,uint256,bytes)'
               ](holder.address, await instance.getAddress(), tokenId, '0x'),
-          ).to.be.revertedWith(
-            'NonFungibleToken: transfer to non ERC721Receiver implementer',
+          ).to.be.revertedWithCustomError(
+            instance,
+            'NonFungibleToken__ERC721ReceiverNotImplemented',
           );
         });
 
diff --git a/test/token/non_fungible/NonFungibleToken.ts b/test/token/non_fungible/NonFungibleToken.ts
diff --git a/test/utils/AddressUtils.ts b/test/utils/AddressUtils.ts

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ library Factory {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`/**`
`49`		`- * @notice calculate the _deployMetamorphicContract deployment address for a given salt`
	`49`	`+ * @notice calculate the deployment address for a given salt`
`50`	`50`	`* @param initCodeHash hash of contract initialization code`
`51`	`51`	`* @param salt input for deterministic address calculation`
`52`	`52`	`* @return deployment address`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,5 @@ pragma solidity ^0.8.20;`
`5`	`5`	`import { _IProxy } from './_IProxy.sol';`
`6`	`6`
`7`	`7`	`interface IProxy is _IProxy {`
`8`		`- error Proxy__ImplementationIsNotContract();`
`9`		`-`
`10`	`8`	`fallback() external payable;`
`11`	`9`	`}`