Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ETHEREUM-CONTRACTS] use eip 1967 slot for admin #1710

Merged
merged 3 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading