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

[ZNS ZChain] String Resolver #106

Merged
merged 19 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
26 changes: 26 additions & 0 deletions contracts/resolver/IZNSStringResolver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;


interface IZNSStringResolver {
/**
* @param newString The new domain owner
Copy link
Collaborator

Choose a reason for hiding this comment

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

change the order of these.
also, newString comment is not correct. it's not the owner, it's the string that owner assigns which a domain will resolve to

* @param domainHash The identifying hash of a domain's name
*/
event StringSet(bytes32 indexed domainHash, string indexed newString);

function supportsInterface(bytes4 interfaceId) external view returns (bool);

function resolveDomainString(bytes32 domainHash) external view returns (string memory);

function setString(
bytes32 domainHash,
string calldata newString
) external;

function getInterfaceId() external pure returns (bytes4);

function setRegistry(address _registry) external;

function initialize(address _accessController, address _registry) external;
}
109 changes: 109 additions & 0 deletions contracts/resolver/ZNSStringResolver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import { IZNSStringResolver } from "./IZNSStringResolver.sol";
import { AAccessControlled } from "../access/AAccessControlled.sol";
import { ARegistryWired } from "../registry/ARegistryWired.sol";


/**
* @title The specific Resolver for ZNS that maps domain hashes to strings.
*/
contract ZNSStringResolver is
UUPSUpgradeable,
AAccessControlled,
ARegistryWired,
ERC165,
IZNSStringResolver {

mapping(bytes32 domainHash => string resolvedString) internal domainStrings;
Copy link
Collaborator

Choose a reason for hiding this comment

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

the name domainStrings could be a bit misleading in that it implies the resolved strings are the domains themselves, not what they resolve to, maybe resolvedStrings similar to the name you already are using in the mapping key/value pair

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Called them resolvedString and resolvedStrings, which I dont like, to be honest. Tell me, if I'm wrong and have to find another name)


/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
_disableInitializers();
}

/**
* @notice Initializer for the `ZNSStringResolver` proxy.
* Note that setter functions are used instead of direct state variable assignments
* to use access control at deploy time. Only ADMIN can call this function.
* @param accessController_ The address of the `ZNSAccessController` contract
* @param registry_ The address of the `ZNSRegistry` contract
*/
function initialize(address accessController_, address registry_) external override initializer {
_setAccessController(accessController_);
setRegistry(registry_);
}

/**
* @dev Returns string associated with a given domain name hash.
* @param domainHash The identifying hash of a domain's name
*/
function resolveDomainString(
bytes32 domainHash
) external view override returns (string memory) {
return domainStrings[domainHash];
}

/**
* @dev Sets the string for a domain name hash.
* @param domainHash The identifying hash of a domain's name
* @param newString The new string to map the domain to
*/
function setString(
bytes32 domainHash,
string calldata newString
) external override {
// only owner or operator of the current domain can set the string
require(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using custom errors is more gas efficient. I know they aren't used in the other contracts but they have already been deployed, so maybe we can update that at some point but for now use them instead of require and do the opposite logic of what you're checking here

if (!registry.IsOwnerOrOperator(domainHash, msg.sender)) {
    revert CustomErrorName();
}

registry.isOwnerOrOperator(domainHash, msg.sender) ||
// TODO: decide, whether the registrar can set a string
accessController.isRegistrar(msg.sender),
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, remove the comment and L63. Registrar will not be able to set a string. only owner or operator can.

"ZNSStringResolver: Not authorized for this domain"
);

domainStrings[domainHash] = newString;

emit StringSet(domainHash, newString);
}

/**
* @dev ERC-165 check for implementation identifier
* @dev Supports interfaces IZNSStringResolver and IERC165
* @param interfaceId ID to check, XOR of the first 4 bytes of each function signature
*/
function supportsInterface(
bytes4 interfaceId
) public view virtual override(ERC165, IZNSStringResolver) returns (bool) {
return
interfaceId == getInterfaceId() ||
super.supportsInterface(interfaceId);
}

/**
* @dev Exposes IZNSStringResolver interfaceId
*/
function getInterfaceId() public pure override returns (bytes4) {
return type(IZNSStringResolver).interfaceId;
}

/**
* @dev Sets the address of the `ZNSRegistry` contract that holds all crucial data
* for every domain in the system. This function can only be called by the ADMIN.
* @param _registry The address of the `ZNSRegistry` contract
*/
function setRegistry(address _registry) public override(ARegistryWired, IZNSStringResolver) onlyAdmin {
_setRegistry(_registry);
}

/**
* @notice To use UUPS proxy we override this function and revert if `msg.sender` isn't authorized
* @param newImplementation The implementation contract to upgrade to
*/
// solhint-disable-next-line no-unused-vars
function _authorizeUpgrade(address newImplementation) internal view override {
accessController.checkGovernor(msg.sender);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

import { ZNSStringResolver } from "../../resolver/ZNSStringResolver.sol";
import { UpgradeMock } from "../UpgradeMock.sol";

/* solhint-disable-next-line */
contract ZNSStringResolverUpgradeMock is ZNSStringResolver, UpgradeMock {}
2 changes: 2 additions & 0 deletions src/deploy/campaign/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
ZNSSubRegistrar,
ZNSTreasury,
MeowToken,
ZNSStringResolver,
} from "../../../typechain";

export type IZNSSigner = HardhatEthersSigner | DefenderRelaySigner | SignerWithAddress;
Expand Down Expand Up @@ -47,6 +48,7 @@ export type ZNSContract =
MeowTokenMock |
MeowToken |
ZNSAddressResolver |
ZNSStringResolver |
ZNSCurvePricer |
ZNSTreasury |
ZNSRootRegistrar |
Expand Down
3 changes: 3 additions & 0 deletions src/deploy/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ export const EXECUTOR_ROLE = ethers.solidityPackedKeccak256(

export const ResolverTypes = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this would be better as an enum, then it would resolve the "TODO" comment you have below as these are just given values 0, 1, 2 etc. unless you specifically assign them something.

export enum ResolverTypes {
  ADDRESS,
  STRING
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

that would require a Registry contract upgrade...

address: "address",
// TODO: Which word to use for a string type of resolver??
// eslint-disable-next-line id-blacklist
string: "string",
};
67 changes: 67 additions & 0 deletions src/deploy/missions/contracts/string-resolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { BaseDeployMission, TDeployArgs } from "@zero-tech/zdc";
import { HardhatRuntimeEnvironment } from "hardhat/types";
import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";
import { DefenderRelayProvider } from "@openzeppelin/defender-sdk-relay-signer-client/lib/ethers";
import { IZNSContracts } from "../../campaign/types";
import { ProxyKinds, ResolverTypes } from "../../constants";
import { znsNames } from "./names";


export class ZNSStringResolverDM extends BaseDeployMission<
HardhatRuntimeEnvironment,
SignerWithAddress,
DefenderRelayProvider,
IZNSContracts
> {
proxyData = {
isProxy: true,
kind: ProxyKinds.uups,
};

contractName = znsNames.stringResolver.contract;
instanceName = znsNames.stringResolver.instance;

async deployArgs () : Promise<TDeployArgs> {
const { accessController, registry } = this.campaign;

return [
await accessController.getAddress(),
await registry.getAddress(),
];
}

async needsPostDeploy () {
const {
registry,
stringResolver,
} = this.campaign;

const resolverInReg = await registry.getResolverType(
ResolverTypes.string,
);

const needs = resolverInReg !== await stringResolver.getAddress();
const msg = needs ? "needs" : "doesn't need";

this.logger.debug(`${this.contractName} ${msg} post deploy sequence`);

return needs;
}

async postDeploy () {
const {
registry,
stringResolver,
config: {
deployAdmin,
},
} = this.campaign;

await registry.connect(deployAdmin).addResolverType(
ResolverTypes.string,
await stringResolver.getAddress(),
);

this.logger.debug(`${this.contractName} post deploy sequence completed`);
}
}
10 changes: 6 additions & 4 deletions test/DeployCampaignInt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
ZNSDomainTokenDM, ZNSFixedPricerDM,
ZNSRegistryDM, ZNSRootRegistrarDM, ZNSSubRegistrarDM, ZNSTreasuryDM,
} from "../src/deploy/missions/contracts";
import { ZNSStringResolverDM } from "../src/deploy/missions/contracts/string-resolver";
import { znsNames } from "../src/deploy/missions/contracts/names";
import { runZnsCampaign } from "../src/deploy/zns-campaign";
import { MeowMainnet } from "../src/deploy/missions/contracts/meow-token/mainnet-data";
Expand Down Expand Up @@ -313,11 +314,11 @@ describe("Deploy Campaign Test", () => {
const { curVersion: nextDbVersion } = dbAdapter;
expect(nextDbVersion).to.equal(initialDbVersion);

// state should have 10 contracts in it
// state should have 11 contracts in it
const { state } = nextCampaign;
expect(Object.keys(state.contracts).length).to.equal(10);
expect(Object.keys(state.instances).length).to.equal(10);
expect(state.missions.length).to.equal(10);
expect(Object.keys(state.contracts).length).to.equal(11);
expect(Object.keys(state.instances).length).to.equal(11);
expect(state.missions.length).to.equal(11);
// it should deploy AddressResolver
expect(await state.contracts.addressResolver.getAddress()).to.be.properAddress;

Expand Down Expand Up @@ -430,6 +431,7 @@ describe("Deploy Campaign Test", () => {
FailingZNSAddressResolverDM, // failing DM
ZNSCurvePricerDM,
ZNSTreasuryDM,
ZNSStringResolverDM,
ZNSRootRegistrarDM,
ZNSFixedPricerDM,
ZNSSubRegistrarDM,
Expand Down
Loading