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

FuseController #318

Open
wants to merge 15 commits into
base: l2-registry
Choose a base branch
from
309 changes: 309 additions & 0 deletions contracts/l2/FuseController.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,309 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.17;

import "./L2Registry.sol";
import "./IFuseController.sol";
import "./IControllerUpgrade.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

error Unauthorised(bytes32 node, address addr);
error CannotUpgrade();
error nameExpired(bytes32 node);

/**
* @dev A simple ENS registry controller. Names are permanently owned by a single account.
* Name data is structured as follows:
* - Byte 0: controller (address)
* - Byte 20: owner (address)
* - Byte 40: resolver (address)
* _ Byte 60: expiry (uint64)
* - Byte 68: fuses (uint96)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you reduce this to a uint64, the whole thing will be 96 bytes - saving a storage slot.

* - Byte 80: renewalController (address)
*/
contract FuseController is Ownable, IFuseController {
L2Registry immutable registry;

IControllerUpgrade upgradeContract;

// A struct to hold the unpacked data
struct TokenData {
address owner;
address resolver;
uint64 expiry;
uint96 fuses;
address renewalController;
}

constructor(L2Registry _registry) {
registry = _registry;
}

/*************************
* IController functions *
*************************/

function ownerOfWithData(
bytes calldata tokenData
) external pure returns (address) {
(address owner, , , , ) = _unpack(tokenData);
return owner;
}

function ownerOf(bytes32 node) external view returns (address) {
//get the tokenData
bytes memory tokenData = registry.getData(uint256(node));
(address owner, , , , ) = _unpack(tokenData);
return owner;
}

function safeTransferFrom(
bytes calldata tokenData,
address operator,
address from,
address to,
uint256 /*id*/,
uint256 value,
bytes calldata /*data*/,
bool operatorApproved
) external view returns (bytes memory) {
TokenData memory td;

(
td.owner,
td.resolver,
td.expiry,
td.fuses,
td.renewalController
) = _unpack(tokenData);

require(value == 1);
require(from == td.owner);
require(operator == td.owner || operatorApproved);

return
_pack(
address(this),
to,
td.resolver,
td.expiry,
td.fuses,
td.renewalController
);
}

function balanceOf(
bytes calldata tokenData,
address _owner,
uint256 /*id*/
) external pure returns (uint256) {
(address owner, , , , ) = _unpack(tokenData);
return _owner == owner ? 1 : 0;
}

function resolverFor(
bytes calldata tokenData
) external pure returns (address) {
(, address resolver, , , ) = _unpack(tokenData);
return resolver;
}

function expiryOf(bytes32 node) external view returns (uint64) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A generic data method may be more useful here, as well as being more gas efficient.

// get the tokenData
bytes memory tokenData = registry.getData(uint256(node));
(, , uint64 expiry, , ) = _unpack(tokenData);
return expiry;
}

function fusesOf(bytes32 node) external view returns (uint96) {
// get the tokenData
bytes memory tokenData = registry.getData(uint256(node));
(, , , uint96 fuses, ) = _unpack(tokenData);
return fuses;
}

function renewalControllerOf(bytes32 node) external view returns (address) {
// get the tokenData
bytes memory tokenData = registry.getData(uint256(node));
(, , , , address renewalController) = _unpack(tokenData);
return renewalController;
}

function upgrade(bytes32 node, bytes calldata extraData) public {
// Make sure the upgrade contract is set.
if (address(upgradeContract) == address(0)) {
revert CannotUpgrade();
}

// Unpack the tokenData of the node.
bytes memory tokenData = registry.getData(uint256(node));
(
address owner,
address resolver,
uint64 expiry,
uint96 fuses,
address renewalController
) = _unpack(tokenData);

bool isAuthorized = registry.getAuthorization(
uint256(node),
owner,
msg.sender
);

if (owner != msg.sender && !isAuthorized) {
revert Unauthorised(node, msg.sender);
}

if (_isExpired(tokenData)) {
revert nameExpired(node);
}

// Change the controller to the upgrade contract.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to instead allow the upgrade contract to specify the new data, so that a data format migration could take place at the same time as a contract upgrade.

Copy link
Contributor Author

@nxt3d nxt3d Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upgrade target contract won't have permissions to make any changes if we don't change the controller here? There might be a way to make this more efficient, however.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but we could have a function on the upgrade contract that we call with the old data, and it returns the new data to set on the registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an opinion on this, could be either way.

registry.setNode(
uint256(node),
_pack(
address(upgradeContract),
owner,
resolver,
expiry,
fuses,
renewalController
)
);

// Call the new contract to notify it of the upgrade.
upgradeContract.upgradeFrom(node, extraData);
}

/*******************
* Node Owner functions *
*******************/

function setResolver(uint256 id, address newResolver) external {
// get tokenData
bytes memory tokenData = registry.getData(id);
(
address owner,
,
uint64 expiry,
uint96 fuses,
address renewalController
) = _unpack(tokenData);
bool isAuthorized = registry.getAuthorization(id, owner, msg.sender);

if (owner != msg.sender && !isAuthorized) {
revert Unauthorised(bytes32(id), msg.sender);
}

registry.setNode(
id,
_pack(
address(this),
owner,
newResolver,
expiry,
fuses,
renewalController
)
);
}

function setSubnode(
bytes32 node,
uint256 label,
address subnodeOwner,
address subnodeResolver,
uint64 subnodeExpiry,
uint96 subnodeFuses,
address subnodeRenewalController
) external {
bytes memory tokenData = registry.getData(uint256(node));
(address owner, , , , ) = _unpack(tokenData);
bool isAuthorized = registry.getAuthorization(
uint256(node),
owner,
msg.sender
);

if (owner != msg.sender && !isAuthorized) {
revert Unauthorised(node, msg.sender);
}

registry.setSubnode(
uint256(node),
label,
_pack(
address(this),
subnodeOwner,
subnodeResolver,
subnodeExpiry,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be some enforcement here that a subnode can't expire after its parent?

subnodeFuses,
subnodeRenewalController
),
msg.sender,
subnodeOwner
);
}

/*******************
* Owner only functions *
*******************/

// A function that sets the upgrade contract.
function setUpgradeController(
IControllerUpgrade _upgradeContract
) external onlyOwner {
upgradeContract = _upgradeContract;
}

/**********************
* Internal functions *
**********************/

function _isExpired(bytes memory tokenData) internal view returns (bool) {
(, , uint64 expiry, , ) = _unpack(tokenData);
return expiry <= block.timestamp;
}

function _unpack(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned before - just use calldata instead of memory and you don't need assembly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using memory allows for the function ownerOfWithData, which saves an external call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? ownerOfWithData can specify that its argument is calldata too, and pass it straight to _unpack.

Copy link
Contributor Author

@nxt3d nxt3d Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example in this function in the controller:

function ownerOf(bytes32 node) external view returns (address) {
        //get the tokenData
        bytes memory tokenData = registry.getData(uint256(node));
        (address owner, , , , ) = _unpack(tokenData);
        return owner;
    }

I just get the data from the registry and then call _unpack, but tokenData in this case is memory and not calldata.

bytes memory tokenData
)
internal
pure
returns (
address owner,
address resolver,
uint64 expiry,
uint96 fuses,
address renewalController
)
{
assembly {
owner := mload(add(tokenData, 40))
resolver := mload(add(tokenData, 60))
expiry := mload(add(tokenData, 68))
fuses := mload(add(tokenData, 80))
renewalController := mload(add(tokenData, 92))
}
}

function _pack(
address controller,
address owner,
address resolver,
uint64 expiry,
uint96 fuses,
address renewalController
) internal view returns (bytes memory /*tokenData*/) {
return
abi.encodePacked(
controller,
owner,
resolver,
expiry,
fuses,
renewalController
);
}
}
8 changes: 6 additions & 2 deletions contracts/l2/IController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
pragma solidity ^0.8.17;

interface IController {
function ownerOf(bytes calldata tokenData) external view returns (address);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this change? Requiring controllers to implement both methods complicates them, and many will have to implement ownerOf simply by calling the registry back to fetch their data from it anywy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It saves an external call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what circumstances?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function ownerOf(bytes32 node) external view returns (address) {
        //get the tokenData
        bytes memory tokenData = registry.getData(uint256(node));
        (address owner, , , , ) = _unpack(tokenData);
        return owner;
    }

function ownerOfWithData(
bytes calldata tokenData
) external view returns (address);

function ownerOf(bytes32 node) external view returns (address);

function safeTransferFrom(
bytes calldata tokenData,
Expand All @@ -13,7 +17,7 @@ interface IController {
uint256 id,
uint256 value,
bytes calldata data,
bool operatorApproved
bool isApproved
) external returns (bytes memory);

function balanceOf(
Expand Down
9 changes: 9 additions & 0 deletions contracts/l2/IControllerUpgrade.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.17;

import "./IController.sol";

interface IControllerUpgrade is IController {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be something like IUpgradeTarget?

function upgradeFrom(bytes32 node, bytes calldata extraData) external;
}
19 changes: 19 additions & 0 deletions contracts/l2/IFuseController.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "./IController.sol";
import "./IControllerUpgrade.sol";

interface IFuseController is IController {
function expiryOf(bytes32 node) external view returns (uint64);

function fusesOf(bytes32 node) external view returns (uint96);

function renewalControllerOf(bytes32 node) external view returns (address);

function upgrade(bytes32 node, bytes calldata extraData) external;

function setUpgradeController(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this an upgrade target, rather than any kind of controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setUpgradeControllerTarget? It's a controller to upgrade to.

IControllerUpgrade _upgradeController
) external;
}
Loading
Loading