Skip to content

Commit

Permalink
[ETHEREUM-CONTRACTS] use eip 1967 slot for admin (#1710)
Browse files Browse the repository at this point in the history
* use eip 1967 slot for admin

* Update CHANGELOG.md

* Update SuperToken.sol
  • Loading branch information
0xdavinchee authored Oct 4, 2023
1 parent ae7f923 commit c10bd28
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 101 deletions.
7 changes: 4 additions & 3 deletions packages/ethereum-contracts/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Expose `_toUnderlyingAmount(uint256 amount)` with `toUnderlyingAmount(uint256 amount)`
- `batchCall` supports payable `OPERATION_TYPE_SUPERFLUID_CALL_APP_ACTION`: only the first `OPERATION_TYPE_SUPERFLUID_CALL_APP_ACTION` will be payable
- Added two new functions to `SuperfluidGovernanceBase.sol`: `changeSuperTokenAdmin` and `batchChangeSuperTokenAdmin`
- `Superfluid.changeSuperTokenAdmin()` function added to be called via governance for tokens with no admin override
- Added an overloaded `initialize` to `SuperToken.sol`, which additionally takes `address adminOverride` if you want to initialize the token with an admin override
- `Superfluid.changeSuperTokenAdmin()` function added to be called via governance for tokens with no admin address
- Added an overloaded `initialize` to `SuperToken.sol`, which additionally takes `address admin` if you want to initialize the token with an admin address
- `SuperToken.changeAdmin(address newAdmin)` added which is only callable by the current admin, the "admin" of a SuperToken can change the admin and update the proxy contract's pointer to a logic contract
> Note that the default admin (when address(0)) is the host contract as is currently the case
- `SuperToken.getAdminOverride()` added to retrieve the AdminOverride struct (currently only holds one property: `address admin`), but is future-proofed with 12 additional bits that can be packed in the struct
- Note that the admin is stored in the EIP-1967 admin storage slot (`0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103`)
- `SuperToken.getAdmin()` added to retrieve the admin address
- `SuperTokenFactory.createERC20Wrapper()` overloads added to create a SuperToken AND explicitly initialize a SuperToken with an admin

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ import { IConstantInflowNFT } from "./IConstantInflowNFT.sol";
*/
interface ISuperToken is ISuperfluidToken, IERC20Metadata, IERC777 {

struct AdminOverride {
address admin;
/// @note we use a struct so the 12 remaining
/// packed bytes may be used later on
}

/**************************************************************************
* Errors
*************************************************************************/
Expand Down Expand Up @@ -48,30 +42,30 @@ interface ISuperToken is ISuperfluidToken, IERC20Metadata, IERC777 {
) external;

/**
* @dev Initialize the contract with admin override
* @dev Initialize the contract with an admin
*/
function initializeWithAdminOverride(
function initializeWithAdmin(
IERC20 underlyingToken,
uint8 underlyingDecimals,
string calldata n,
string calldata s,
address adminOverride
address admin
) external;

/**
* @notice Changes the admin override for the SuperToken
* @notice Changes the admin for the SuperToken
* @dev Only the current admin can call this function
* if adminOverride is address(0), it is implicitly the host address
* @param newAdmin New admin override address
* if admin is address(0), it is implicitly the host address
* @param newAdmin New admin address
*/
function changeAdmin(address newAdmin) external;

event AdminChanged(address indexed oldAdmin, address indexed newAdmin);

/**
* @dev Returns the admin override struct for the SuperToken
* @dev Returns the admin address for the SuperToken
*/
function getAdminOverride() external view returns (AdminOverride memory);
function getAdmin() external view returns (address admin);

/**************************************************************************
* Immutable variables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface ISuperTokenFactory {
* @param upgradability Upgradability mode
* @param name Super token name
* @param symbol Super token symbol
* @param adminOverride Admin address override
* @param admin Admin address
* @return superToken The deployed and initialized wrapper super token
*/
function createERC20Wrapper(
Expand All @@ -64,7 +64,7 @@ interface ISuperTokenFactory {
Upgradability upgradability,
string calldata name,
string calldata symbol,
address adminOverride
address admin
)
external
returns (ISuperToken superToken);
Expand Down Expand Up @@ -94,15 +94,15 @@ interface ISuperTokenFactory {
* @param upgradability Upgradability mode
* @param name Super token name
* @param symbol Super token symbol
* @param adminOverride Admin address override
* @param admin Admin address
* @return superToken The deployed and initialized wrapper super token
*/
function createERC20Wrapper(
IERC20Metadata underlyingToken,
Upgradability upgradability,
string calldata name,
string calldata symbol,
address adminOverride
address admin
)
external
returns (ISuperToken superToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ interface ISuperfluid {
event SuperTokenLogicUpdated(ISuperToken indexed token, address code);

/**
* @notice Change the SuperToken admin override
* @dev The admin override is the only account allowed to update the token logic
* @notice Change the SuperToken admin address
* @dev The admin is the only account allowed to update the token logic
* For backward compatibility, the "host" is the default "admin" if unset (address(0)).
*/
function changeSuperTokenAdmin(ISuperToken token, address newAdmin) external;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,8 @@ contract SuperTokenStorageLayoutTester is SuperToken {
require (slot == 18 && offset == 0, "_operators changed location");
// uses 4 slots

assembly { slot:= _adminOverride.slot offset := _adminOverride.offset }
require (slot == 22 && offset == 0, "_adminOverride changed location");

assembly { slot:= _reserve23.slot offset := _reserve23.offset }
require (slot == 23 && offset == 0, "_reserve23 changed location");
assembly { slot:= _reserve22.slot offset := _reserve22.offset }
require (slot == 22 && offset == 0, "_reserve22 changed location");

assembly { slot:= _reserve31.slot offset := _reserve31.offset }
require (slot == 31 && offset == 0, "_reserve31 changed location");
Expand Down
51 changes: 34 additions & 17 deletions packages/ethereum-contracts/contracts/superfluid/SuperToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ contract SuperToken is
using ERC777Helper for ERC777Helper.Operators;
using SafeERC20 for IERC20;

// See: https://eips.ethereum.org/EIPS/eip-1967#admin-address
bytes32 constant private _ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;

uint8 constant private _STANDARD_DECIMALS = 18;

// solhint-disable-next-line var-name-mixedcase
Expand Down Expand Up @@ -69,17 +72,15 @@ contract SuperToken is
/// @dev ERC777 operators support data
ERC777Helper.Operators internal _operators;

/// @dev A struct which contains the address of the admin override
AdminOverride internal _adminOverride;

// NOTE: for future compatibility, these are reserved solidity slots
// The sub-class of SuperToken solidity slot will start after _reserve22

// NOTE: Whenever modifying the storage layout here it is important to update the validateStorageLayout
// function in its respective mock contract to ensure that it doesn't break anything or lead to unexpected
// behaviors/layout when upgrading

uint256 internal _reserve23;
uint256 internal _reserve22;
uint256 private _reserve23;
uint256 private _reserve24;
uint256 private _reserve25;
uint256 private _reserve26;
Expand Down Expand Up @@ -130,13 +131,13 @@ contract SuperToken is
_initialize(underlyingToken, underlyingDecimals, n, s, address(0));
}

/// @dev Initialize the Super Token proxy with an admin override
function initializeWithAdminOverride(
/// @dev Initialize the Super Token proxy with an admin
function initializeWithAdmin(
IERC20 underlyingToken,
uint8 underlyingDecimals,
string calldata n,
string calldata s,
address adminOverride
address admin
)
external
virtual
Expand All @@ -147,7 +148,7 @@ contract SuperToken is
// deployment of the proxy contract.

// initialize the Super Token
_initialize(underlyingToken, underlyingDecimals, n, s, adminOverride);
_initialize(underlyingToken, underlyingDecimals, n, s, admin);
}

function proxiableUUID() public pure virtual override returns (bytes32) {
Expand All @@ -156,7 +157,7 @@ contract SuperToken is

/**
* @notice Updates the logic contract the proxy is pointing at
* @dev Only the admin can call this function (host if adminOverride.admin == address(0))
* @dev Only the admin can call this function (host if admin == address(0))
* @param newAddress Address of the new logic contract
*/
function updateCode(address newAddress) external virtual override onlyAdmin {
Expand All @@ -176,16 +177,31 @@ contract SuperToken is
}

function changeAdmin(address newAdmin) external override onlyAdmin {
address oldAdmin = _adminOverride.admin;
_adminOverride.admin = newAdmin;
address oldAdmin = _getAdmin();
_setAdmin(newAdmin);

emit AdminChanged(oldAdmin, newAdmin);
}

function getAdminOverride() external view override returns (AdminOverride memory) {
return _adminOverride;
function getAdmin() external view override returns (address) {
return _getAdmin();
}

function _getAdmin() internal view returns (address admin) {
assembly {
// solium-disable-line
admin := sload(_ADMIN_SLOT)
}
}

function _setAdmin(address newAdmin) internal {
assembly {
// solium-disable-line
sstore(_ADMIN_SLOT, newAdmin)
}
}


/**************************************************************************
* ERC20 Token Info
*************************************************************************/
Expand All @@ -211,15 +227,15 @@ contract SuperToken is
uint8 underlyingDecimals,
string calldata n,
string calldata s,
address adminOverride
address admin
) internal {
_underlyingToken = underlyingToken;
_underlyingDecimals = underlyingDecimals;

_name = n;
_symbol = s;

_adminOverride.admin = adminOverride;
_setAdmin(admin);

// register interfaces
ERC777Helper.register(address(this));
Expand All @@ -228,7 +244,7 @@ contract SuperToken is
emit Transfer(address(0), address(0), 0);

// previous admin will always be the zero address in an uninitialized contract
emit AdminChanged(address(0), adminOverride);
emit AdminChanged(address(0), admin);
}

/**
Expand Down Expand Up @@ -865,7 +881,8 @@ contract SuperToken is
* override address
*/
modifier onlyAdmin() {
address admin = _adminOverride.admin == address(0) ? address(_host) : _adminOverride.admin;
address adminSlotAdmin = _getAdmin();
address admin = adminSlotAdmin == address(0) ? address(_host) : adminSlotAdmin;
if (msg.sender != admin) revert SUPER_TOKEN_ONLY_ADMIN();
_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ abstract contract SuperTokenFactoryBase is
Upgradability upgradability,
string calldata name,
string calldata symbol,
address adminOverride
address admin
) public override returns (ISuperToken superToken) {
if (address(underlyingToken) == address(0)) {
revert SUPER_TOKEN_FACTORY_ZERO_ADDRESS();
Expand All @@ -232,12 +232,12 @@ abstract contract SuperTokenFactoryBase is
}

// initialize the token
superToken.initializeWithAdminOverride(
superToken.initializeWithAdmin(
underlyingToken,
underlyingDecimals,
name,
symbol,
adminOverride
admin
);

emit SuperTokenCreated(superToken);
Expand Down Expand Up @@ -270,7 +270,7 @@ abstract contract SuperTokenFactoryBase is
Upgradability upgradability,
string calldata name,
string calldata symbol,
address adminOverride
address admin
)
external override
returns (ISuperToken superToken)
Expand All @@ -281,7 +281,7 @@ abstract contract SuperTokenFactoryBase is
upgradability,
name,
symbol,
adminOverride
admin
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,21 +161,21 @@ contract SuperfluidFrameworkDeployer is SuperfluidFrameworkDeploymentSteps {
/// @param _underlyingSymbol The token symbol
/// @param _decimals The token decimals
/// @param _mintLimit The mint limit of the underlying token
/// @param adminOverride The admin override address for the Super Token
/// @param _admin The admin address for the Super Token
/// @return underlyingToken and superToken
function deployWrapperSuperToken(
string calldata _underlyingName,
string calldata _underlyingSymbol,
uint8 _decimals,
uint256 _mintLimit,
address adminOverride
address _admin
)
external
requiresSuperTokenFactory
deploySuperTokenRequires1820
returns (TestToken underlyingToken, SuperToken superToken)
{
return _deployWrapperSuperToken(_underlyingName, _underlyingSymbol, _decimals, _mintLimit, adminOverride);
return _deployWrapperSuperToken(_underlyingName, _underlyingSymbol, _decimals, _mintLimit, _admin);
}

/// @notice Deploys an ERC20 and a Wrapper Super Token for the ERC20 and lists both in the resolver
Expand Down Expand Up @@ -257,7 +257,7 @@ contract SuperfluidFrameworkDeployer is SuperfluidFrameworkDeploymentSteps {
string calldata _underlyingSymbol,
uint8 _decimals,
uint256 _mintLimit,
address _adminOverride
address _admin
) internal returns (TestToken underlyingToken, SuperToken superToken) {
underlyingToken =
TokenDeployerLibrary.deployTestToken(_underlyingName, _underlyingSymbol, _decimals, _mintLimit);
Expand All @@ -272,7 +272,7 @@ contract SuperfluidFrameworkDeployer is SuperfluidFrameworkDeploymentSteps {
ISuperTokenFactory.Upgradability.SEMI_UPGRADABLE,
string.concat("Super ", _underlyingSymbol),
superTokenSymbol,
_adminOverride
_admin
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,14 @@ contract FoundrySuperfluidTester is Test {
uint8 underlyingDecimals,
string memory name,
string memory symbol,
address adminOverride
address admin
) internal returns (SuperToken localSuperToken) {
localSuperToken = new SuperToken(
sf.host,
previousSuperToken.CONSTANT_OUTFLOW_NFT(),
previousSuperToken.CONSTANT_INFLOW_NFT()
);
localSuperToken.initializeWithAdminOverride(underlyingToken, underlyingDecimals, name, symbol, adminOverride);
localSuperToken.initializeWithAdmin(underlyingToken, underlyingDecimals, name, symbol, admin);
}

// Write Helpers - ConstantFlowAgreementV1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract SuperfluidGovernanceIntegrationTest is FoundrySuperfluidTester {
sf.governance.changeSuperTokenAdmin(sf.host, superToken, newAdmin);
vm.stopPrank();

assertEq(superToken.getAdminOverride().admin, newAdmin, "Superfluid.t: super token admin not changed");
assertEq(superToken.getAdmin(), newAdmin, "Superfluid.t: super token admin not changed");
}

function testRevertChangeSuperTokenAdminWhenCallerIsNotNotGovOwner(address newAdmin) public {
Expand Down Expand Up @@ -64,7 +64,7 @@ contract SuperfluidGovernanceIntegrationTest is FoundrySuperfluidTester {
sf.governance.batchChangeSuperTokenAdmin(sf.host, superTokens, newAdmins);
vm.stopPrank();

assertEq(superToken.getAdminOverride().admin, newAdmin, "Superfluid.t: super token admin not changed");
assertEq(superToken.getAdmin(), newAdmin, "Superfluid.t: super token admin not changed");
}

function testRevertBatchChangeSuperTokenAdminWhenHostNotAdmin(address newAdmin) public {
Expand Down
Loading

0 comments on commit c10bd28

Please sign in to comment.