Skip to content

Commit

Permalink
Misc tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xGh
0xGh committed Dec 26, 2023
1 parent 40a93ee commit 4f06093
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 66 deletions.
24 changes: 12 additions & 12 deletions contracts/ERC721Baseline.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ import {IERC721Baseline} from "./IERC721Baseline.sol";
* @custom:version v0.1.0-alpha.7
* @notice A minimal proxy contract for ERC721BaselineImplementation.
*
* @dev !!WARNING!! Be careful when defining variables in your proxy
* as these might clash with the implementation ones.
* @dev ERC721BaselineImplementation uses ERC-7201 (Namespaced Storage Layout)
* to prevent collisions with the proxies storage.
* See https://eips.ethereum.org/EIPS/eip-7201.
*
* This contract makes the assumption that you will define storage slots
* similarly to EIP-1967 (https://eips.ethereum.org/EIPS/eip-1967)
* defining a struct with your variables and storing it at a specific location
* eg. bytes32(uint256(keccak256("erc721baseline.storage")) - 1)).
* Alternatively you can fork this contract and add a gap at the beginning,
* although this approach is discouraged.
* Proxies are encouraged, but not required, to use a similar pattern for storage.
*/
contract ERC721Baseline is Proxy {

struct ImplementationSlot {
address value;
}

/**
* @dev Storage slot with the address of the current implementation.
* This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1.
*/
bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;

constructor(
Expand All @@ -39,14 +39,14 @@ contract ERC721Baseline is Proxy {
}
implementation.value = ERC721BaselineImplementation;

(bool success, bytes memory result) = ERC721BaselineImplementation.delegatecall(
(bool success, bytes memory reason) = ERC721BaselineImplementation.delegatecall(
abi.encodeCall(IERC721Baseline.initialize, (name, symbol))
);

if (!success) {
if (result.length == 0) revert("Initialization Failed.");
if (success == false) {
if (reason.length == 0) revert("Initialization Failed.");
assembly {
revert(add(32, result), mload(result))
revert(add(32, reason), mload(reason))
}
}
}
Expand Down
60 changes: 29 additions & 31 deletions contracts/ERC721BaselineImplementation.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ import {Utils} from "./Utils.sol";
contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {

/**
* @dev ERC721Baseline uses ERC-7201 (Namespaced Storage Layout) for storage
* to prevent collisions with proxies storage.
* Proxies are encouraged, but not required, to use a similar pattern for storage.
* @dev ERC721Baseline uses ERC-7201 (Namespaced Storage Layout)
* to prevent collisions with the proxies storage.
* See https://eips.ethereum.org/EIPS/eip-7201.
*
* Proxies are encouraged, but not required, to use a similar pattern for storage.
*
* @custom:storage-location erc7201:erc721baseline.implementation.storage
*/
struct ERC721BaselineStorage {
Expand All @@ -38,8 +39,8 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
/**
* Royalties
*/
address __royaltiesReceiver;
uint256 __royaltiesBps;
address payable _royaltiesReceiver;
uint16 _royaltiesBps;

/**
* @dev Tracks whether the proxy's `_beforeTokenTransfer` hook is enabled or not.
Expand Down Expand Up @@ -70,17 +71,17 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
bytes32 private constant ERC721BaselineStorageLocation = 0xd70e9a647412bf72add39fd1ab5a6a89bfb0d778061be5e3d13cfa60d9d90b00;

/**
* @dev Convenience method to access storage at ERC721BaselineStorageLocation location.
* @dev Convenience method to access the storage at ERC721BaselineStorageLocation location.
*
* Usage:
*
* ERC721BaselineStorage storage $ = _getStorage();
*
* if ($.__royaltiesReceiver != address(0)) {
* $.__royaltiesReceiver = address(0);
* if ($._royaltiesReceiver != address(0)) {
* $._royaltiesReceiver = address(0);
* }
*
* @return $ a reference to the storage slot for reading and writing
* @return $ a reference to the storage at ERC721BaselineStorageLocation location for reading and writing
*/
function _getStorage() private pure returns (ERC721BaselineStorage storage $) {
assembly {
Expand Down Expand Up @@ -137,9 +138,6 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
* @inheritdoc IERC721Baseline
*/
function initialize(string memory name, string memory symbol) external initializer {
if (address(this).code.length != 0) {
revert Unauthorized();
}
__ERC721_init(name, symbol);
_setAdmin(_msgSender(), true);
_transferOwnership(_msgSender());
Expand Down Expand Up @@ -246,15 +244,15 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
/**
* @inheritdoc IERC721Baseline
*/
function __royaltiesReceiver() external view returns (address) {
return _getStorage().__royaltiesReceiver;
function royaltiesReceiver() external view returns (address) {
return _getStorage()._royaltiesReceiver;
}

/**
* @inheritdoc IERC721Baseline
*/
function __royaltiesBps() external view returns (uint256) {
return _getStorage().__royaltiesBps;
function royaltiesBps() external view returns (uint256) {
return _getStorage()._royaltiesBps;
}

/**
Expand All @@ -266,8 +264,8 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
) external view returns (address, uint256) {
ERC721BaselineStorage storage $ = _getStorage();

if ($.__royaltiesBps > 0 && $.__royaltiesReceiver != address(0)) {
return ($.__royaltiesReceiver, salePrice * $.__royaltiesBps / 10000);
if ($._royaltiesBps > 0 && $._royaltiesReceiver != address(0)) {
return ($._royaltiesReceiver, salePrice * $._royaltiesBps / 10000);
}

return (address(0), 0);
Expand All @@ -276,15 +274,17 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
/**
* @inheritdoc IERC721Baseline
*/
function __configureRoyalties(address receiver, uint256 bps) external onlyProxy {
function configureRoyalties(address payable receiver, uint16 bps) external {
this.requireAdmin(_msgSender());

ERC721BaselineStorage storage $ = _getStorage();

if (receiver != $.__royaltiesReceiver) {
$.__royaltiesReceiver = receiver;
if (receiver != $._royaltiesReceiver) {
$._royaltiesReceiver = receiver;
}

if (bps != $.__royaltiesBps) {
$.__royaltiesBps = bps;
if (bps != $._royaltiesBps) {
$._royaltiesBps = bps;
}
}

Expand Down Expand Up @@ -370,7 +370,7 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
address auth
) internal override returns (address) {
if (_getStorage()._beforeTokenTransferHookEnabled == true) {
(bool success, ) = address(this).delegatecall(
(bool success, bytes memory reason) = address(this).delegatecall(
abi.encodeWithSignature(
"_beforeTokenTransfer(address,address,address,uint256)",
_msgSender(),
Expand All @@ -380,12 +380,11 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
)
);

assembly {
switch success
case 0 {
returndatacopy(0, 0, returndatasize())
revert(0, returndatasize())
}
if (success == false) {
if (reason.length == 0) revert("_beforeTokenTransfer");
assembly {
revert(add(32, reason), mload(reason))
}
}
}

Expand All @@ -411,7 +410,6 @@ contract ERC721BaselineImplementation is ERC721Upgradeable, IERC721Baseline {
if (reason.length == 0) {
revert ERC721InvalidReceiver(to);
} else {
/// @solidity memory-safe-assembly
assembly {
revert(add(32, reason), mload(reason))
}
Expand Down
26 changes: 19 additions & 7 deletions contracts/IERC721Baseline.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {IERC2981} from "@openzeppelin/contracts/interfaces/IERC2981.sol";
interface IERC721Baseline is IERC721, IERC2981 {

/**
* @dev The version of the implementation contract.
* @notice The version of the implementation contract.
* @return string the implementation version
*/
function VERSION() external view returns (string memory);

Expand Down Expand Up @@ -71,6 +72,8 @@ interface IERC721Baseline is IERC721, IERC2981 {
* @notice The total minted supply.
* @dev The supply is decreased when a token is burned.
* Generally it is recommended to use a separate counter variable to track the supply available for minting.
*
* @return uint256 the existing tokens' supply
*/
function totalSupply() external view returns (uint256);

Expand All @@ -89,6 +92,7 @@ interface IERC721Baseline is IERC721, IERC2981 {
* @notice Returns the token URI for a token ID.
*
* @param tokenId token ID
* @return string the token URI for the token ID
*/
function __tokenURI(uint256 tokenId) external view returns (string memory);

Expand All @@ -97,7 +101,6 @@ interface IERC721Baseline is IERC721, IERC2981 {
* @dev Emits EIP-4906's `MetadataUpdate` event with the `tokenId`.
* This method is internal and only the proxy contract can call it.
*
*
* @param tokenId token ID
* @param tokenURI URI pointing to the metadata
*/
Expand All @@ -106,6 +109,8 @@ interface IERC721Baseline is IERC721, IERC2981 {
/**
* @notice Returns the shared URI for the tokens.
* @dev This method is internal and only the proxy contract can call it.
*
* @return string the shared URI
*/
function __sharedURI() external view returns (string memory);

Expand All @@ -124,6 +129,8 @@ interface IERC721Baseline is IERC721, IERC2981 {
/**
* @notice Returns the base URI for the tokens.
* @dev When set this URI is prepended to the token ID.
*
* @return string the base URI
*/
function __baseURI() external view returns (string memory);

Expand All @@ -145,23 +152,28 @@ interface IERC721Baseline is IERC721, IERC2981 {
************************************************/

/**
* @dev The address of the royalties receiver.
* @notice The address of the royalties receiver.
*
* @return address the address of the royalties receiver
*/
function __royaltiesReceiver() external view returns (address);
function royaltiesReceiver() external view returns (address);

/**
* @dev The royalties rate in basis points (100 bps = 1%).
* @notice The royalties rate in basis points (100 bps = 1%).
*
* @return uint256 the royalties rate
*/
function __royaltiesBps() external view returns (uint256);
function royaltiesBps() external view returns (uint256);

/**
* @notice Configures royalties receiver and bps for all the tokens.
* @dev Bps stants for basis points where 100 bps = 1%.
* The sender must be an admin.
*
* @param receiver address for the royalties receiver
* @param bps (basis points) royalties rate
*/
function __configureRoyalties(address receiver, uint256 bps) external;
function configureRoyalties(address payable receiver, uint16 bps) external;


/************************************************
Expand Down
4 changes: 0 additions & 4 deletions contracts/mocks/ERC721ProxyMock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ contract ERC721ProxyMock is ERC721Baseline {
baseline().__transferOwnership(newOwner);
}

function onlyProxy_configureRoyalties(address receiver, uint256 bps) external {
baseline().__configureRoyalties(receiver, bps);
}

function uri(uint256 tokenId) external view returns (string memory) {
return baseline().__tokenURI(tokenId);
}
Expand Down
29 changes: 17 additions & 12 deletions test/ERC721Baseline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,6 @@ contract(

await expectRevert(proxy.init(), "InvalidInitialization");
});

it("fails when calling initialize outside of the constructor", async () => {
const proxy = await InitializationMock.new(
implementation.address,
false,
);

await expectRevert(proxy.init(), "Unauthorized");
});
});

describe("onlyProxy methods", async () => {
Expand Down Expand Up @@ -527,6 +518,15 @@ contract(
});

describe("Royalties", () => {
it("works only for admins", async () => {
await expectRevert(
proxyDelegate.configureRoyalties(ZERO_ADDRESS, 1000, {
from: attacker,
}),
"Unauthorized",
);
});

it("returns zero address and 0 amount when unset", async () => {
const { 0: receiver, 1: amount } = await proxyDelegate.royaltyInfo(
10,
Expand All @@ -537,7 +537,7 @@ contract(
});

it("returns zero address and 0 amount when only one is set", async () => {
await proxy.onlyProxy_configureRoyalties(ZERO_ADDRESS, 1000);
await proxyDelegate.configureRoyalties(ZERO_ADDRESS, 1000);

let { 0: receiver, 1: amount } = await proxyDelegate.royaltyInfo(
10,
Expand All @@ -546,7 +546,7 @@ contract(
assert.equal(receiver, ZERO_ADDRESS);
assert.equal(amount, 0);

await proxy.onlyProxy_configureRoyalties(deployer, 0);
await proxyDelegate.configureRoyalties(deployer, 0);

({ 0: receiver, 1: amount } = await proxyDelegate.royaltyInfo(
10,
Expand All @@ -557,7 +557,10 @@ contract(
});

it("returns right amount of royalties when configured", async () => {
await proxy.onlyProxy_configureRoyalties(deployer, 1500);
await proxyDelegate.configureRoyalties(deployer, 1500);

assert.equal(deployer, await proxyDelegate.royaltiesReceiver());
assert.equal(1500, await proxyDelegate.royaltiesBps());

const salePrice = web3.utils.toWei("12.25");
const { 0: receiver, 1: amount } = await proxyDelegate.royaltyInfo(
Expand Down Expand Up @@ -647,6 +650,8 @@ contract(
switch (arg.type) {
case "uint256":
return 1;
case "uint16":
return 1;
case "string":
return "test";
case "address":
Expand Down

0 comments on commit 4f06093

Please sign in to comment.