From bdb5bde4cdcb03362d5454fd15d3ae85b85b2d37 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Fri, 8 Sep 2023 21:13:02 +0100 Subject: [PATCH 01/19] Add DelegatableResolver.sol --- contracts/resolvers/DelegatableResolver.sol | 114 ++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 contracts/resolvers/DelegatableResolver.sol diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol new file mode 100644 index 00000000..0270e53f --- /dev/null +++ b/contracts/resolvers/DelegatableResolver.sol @@ -0,0 +1,114 @@ +pragma solidity >=0.8.4; +import "@openzeppelin/contracts/access/Ownable.sol"; +import "./profiles/ABIResolver.sol"; +import "./profiles/AddrResolver.sol"; +import "./profiles/ContentHashResolver.sol"; +import "./profiles/DNSResolver.sol"; +import "./profiles/InterfaceResolver.sol"; +import "./profiles/NameResolver.sol"; +import "./profiles/PubkeyResolver.sol"; +import "./profiles/TextResolver.sol"; +import "./profiles/ExtendedResolver.sol"; + +/** + * A simple resolver anyone can use; only allows the owner of a node to set its + * address. + */ +contract DelegatableResolver is + ABIResolver, + AddrResolver, + ContentHashResolver, + DNSResolver, + InterfaceResolver, + NameResolver, + PubkeyResolver, + TextResolver, + ExtendedResolver +{ + using BytesUtils for bytes; + + constructor(address owner) { + operators[bytes32(0)][owner] = true; + } + + //node => (delegate => isAuthorised) + mapping(bytes32 => mapping(address => bool)) operators; + + // function approve(bytes32 node, address operator, bool approved) authorised(node){ + // operators[node][operator] = approved; + // // Add event + // } + function getAuthorizedNode( + bytes memory name, + uint256 offset, + address operator + ) internal returns (bytes32 node, bool authorized) { + uint256 len = name.readUint8(offset); + node = bytes32(0); + if (len > 0) { + bytes32 label = name.keccak(offset + 1, len); + (node, authorized) = getAuthorizedNode( + name, + offset + len + 1, + operator + ); + node = keccak256(abi.encodePacked(node, label)); + } + return (node, authorized || operators[node][operator]); + } + + function approve( + bytes memory name, + address operator, + bool approved + ) external { + (bytes32 node, bool authorized) = getAuthorizedNode( + name, + 0, + msg.sender + ); + // TODO throw custom error with the node info + require(authorized, "caller cannot authorise"); + operators[node][operator] = approved; + // Add event + } + + // Usage + // basenode = namehash('onl2.eth') + // - l2: l2resolver = DelegatableResolver.deploy(ownerAddress) + // - l1: l1resolver.setVerfierForNode(......, l2resolver) + // - l1: registry.setResolver(......, l1resolver.address) + // - l2: subnameregistrar = SubnameRegistar.deploy + // base = bytes32(0) + // name = encodename('onl2.eth') + // - l2: DelegatableResolver.approve(name, subnameregistrar, true) + // - l2: subnameregistrar.register('makoto') + // subname = encodename('makoto.onl2.eth') + // - lDelegatableResolver.approve(subname, owner, true) + + function isAuthorised(bytes32 node) internal view override returns (bool) { + // return msg.sender == owner(); + return operators[node][msg.sender]; + } + + function supportsInterface( + bytes4 interfaceID + ) + public + view + virtual + override( + ABIResolver, + AddrResolver, + ContentHashResolver, + DNSResolver, + InterfaceResolver, + NameResolver, + PubkeyResolver, + TextResolver + ) + returns (bool) + { + return super.supportsInterface(interfaceID); + } +} From 701bf66eaae48c0ced4b185915564789d7a8b23f Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Tue, 24 Oct 2023 12:30:55 +0100 Subject: [PATCH 02/19] WIP --- README.md | 4 +- test/resolvers/TestDelegatableResolver.js | 547 ++++++++++++++++++++++ 2 files changed, 549 insertions(+), 2 deletions(-) create mode 100644 test/resolvers/TestDelegatableResolver.js diff --git a/README.md b/README.md index e943715a..8a64dd69 100644 --- a/README.md +++ b/README.md @@ -176,8 +176,8 @@ yarn pub 5. Create a "Release Candidate" [release](https://docs.github.com/en/repositories/releasing-projects-on-github/about-releases) on GitHub. This will be of the form `v1.2.3-RC0`. This tagged commit is now subject to our bug bounty. 6. Have the tagged commit audited if necessary 7. If changes are required, make the changes and then once ready for review create another GitHub release with an incremented RC value `v1.2.3-RC0` -> `v.1.2.3-RC1`. Repeat as necessary. -8. Deploy to testnet. Open a pull request to merge the deploy artifacts into -the `feature` branch. Get someone to review and approve the deployment and then merge. You now MUST merge this branch into `staging` branch. +8. Deploy to testnet. Open a pull request to merge the deploy artifacts into + the `feature` branch. Get someone to review and approve the deployment and then merge. You now MUST merge this branch into `staging` branch. 9. Create GitHub release of the form `v1.2.3-testnet` from the commit that has the new deployment artifacts. 10. If any further changes are needed, you can either make them on the existing feature branch that is in sync or create a new branch, and follow steps 1 -> 9. Repeat as necessary. 11. Make Deployment to mainnet from `staging`. Commit build artifacts. You now MUST merge this branch into `main`. diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js new file mode 100644 index 00000000..ee2320c8 --- /dev/null +++ b/test/resolvers/TestDelegatableResolver.js @@ -0,0 +1,547 @@ +const ENS = artifacts.require('./registry/ENSRegistry.sol') +const DelegatableResolver = artifacts.require('DelegatableResolver.sol') +const NameWrapper = artifacts.require('DummyNameWrapper.sol') +const { deploy } = require('../test-utils/contracts') +const { labelhash } = require('../test-utils/ens') +const { + EMPTY_BYTES32: ROOT_NODE, + EMPTY_ADDRESS, +} = require('../test-utils/constants') + +const { expect } = require('chai') +const namehash = require('eth-ens-namehash') +const sha3 = require('web3-utils').sha3 + +const { exceptions } = require('../test-utils') + +contract('DelegatableResolver', function (accounts) { + let node + let ens, resolver, nameWrapper + let account + let signers + let result + + beforeEach(async () => { + signers = await ethers.getSigners() + account = await signers[0].getAddress() + node = namehash.hash('eth') + ens = await ENS.new() + nameWrapper = await NameWrapper.new() + + //setup reverse registrar + + resolver = await DelegatableResolver.new(account) + }) + + describe('supportsInterface function', async () => { + it('supports known interfaces', async () => { + assert.equal(await resolver.supportsInterface('0x3b3b57de'), true) // IAddrResolver + assert.equal(await resolver.supportsInterface('0xf1cb7e06'), true) // IAddressResolver + assert.equal(await resolver.supportsInterface('0x691f3431'), true) // INameResolver + assert.equal(await resolver.supportsInterface('0x2203ab56'), true) // IABIResolver + assert.equal(await resolver.supportsInterface('0xc8690233'), true) // IPubkeyResolver + assert.equal(await resolver.supportsInterface('0x59d1d43c'), true) // ITextResolver + assert.equal(await resolver.supportsInterface('0xbc1c58d1'), true) // IContentHashResolver + assert.equal(await resolver.supportsInterface('0xa8fa5682'), true) // IDNSRecordResolver + assert.equal(await resolver.supportsInterface('0x5c98042b'), true) // IDNSZoneResolver + assert.equal(await resolver.supportsInterface('0x01ffc9a7'), true) // IInterfaceResolver + }) + + it('does not support a random interface', async () => { + assert.equal(await resolver.supportsInterface('0x3b3b57df'), false) + }) + }) + + describe('recordVersion', async () => { + it('permits clearing records', async () => { + var tx = await resolver.clearRecords(node, { from: accounts[0] }) + assert.equal(tx.logs.length, 1) + assert.equal(tx.logs[0].event, 'VersionChanged') + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.newVersion, 1) + }) + }) + + describe('addr', async () => { + it.only('permits setting address by owner', async () => { + var tx = await resolver.methods['setAddr(bytes32,address)']( + node, + accounts[1], + { from: accounts[0] }, + ) + assert.equal(tx.logs.length, 2) + assert.equal(tx.logs[0].event, 'AddressChanged') + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) + assert.equal(tx.logs[1].event, 'AddrChanged') + assert.equal(tx.logs[1].args.node, node) + assert.equal(tx.logs[1].args.a, accounts[1]) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + }) + + it('can overwrite previously set address', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + + await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[0], + }) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[0]) + }) + + it('can overwrite to same address', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + }) + + it('forbids setting new address by non-owners', async () => { + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[1], + }), + ) + }) + + it('forbids writing same address by non-owners', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[1], + }), + ) + }) + + it('forbids overwriting existing address by non-owners', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[1], + }), + ) + }) + + it('returns zero when fetching nonexistent addresses', async () => { + assert.equal( + await resolver.methods['addr(bytes32)'](node), + '0x0000000000000000000000000000000000000000', + ) + }) + + it('permits setting and retrieving addresses for other coin types', async () => { + await resolver.methods['setAddr(bytes32,uint256,bytes)']( + node, + 123, + accounts[1], + { from: accounts[0] }, + ) + assert.equal( + await resolver.methods['addr(bytes32,uint256)'](node, 123), + accounts[1].toLowerCase(), + ) + }) + + it('returns ETH address for coin type 60', async () => { + var tx = await resolver.methods['setAddr(bytes32,address)']( + node, + accounts[1], + { from: accounts[0] }, + ) + assert.equal(tx.logs.length, 2) + assert.equal(tx.logs[0].event, 'AddressChanged') + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) + assert.equal(tx.logs[1].event, 'AddrChanged') + assert.equal(tx.logs[1].args.node, node) + assert.equal(tx.logs[1].args.a, accounts[1]) + assert.equal( + await resolver.methods['addr(bytes32,uint256)'](node, 60), + accounts[1].toLowerCase(), + ) + }) + + it('setting coin type 60 updates ETH address', async () => { + var tx = await resolver.methods['setAddr(bytes32,uint256,bytes)']( + node, + 60, + accounts[2], + { from: accounts[0] }, + ) + assert.equal(tx.logs.length, 2) + assert.equal(tx.logs[0].event, 'AddressChanged') + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.newAddress, accounts[2].toLowerCase()) + assert.equal(tx.logs[1].event, 'AddrChanged') + assert.equal(tx.logs[1].args.node, node) + assert.equal(tx.logs[1].args.a, accounts[2]) + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[2]) + }) + + it('forbids calls to the fallback function with 1 value', async () => { + await exceptions.expectFailure( + web3.eth.sendTransaction({ + from: accounts[0], + to: resolver.address, + gas: 3000000, + value: 1, + }), + ) + }) + }) + + describe('implementsInterface', async () => { + const basicSetInterface = async () => { + await resolver.setInterface(node, '0x12345678', accounts[0], { + from: accounts[0], + }) + assert.equal( + await resolver.interfaceImplementer(node, '0x12345678'), + accounts[0], + ) + } + + it('permits setting interface by owner', basicSetInterface) + + it('can update previously set interface', async () => { + await resolver.setInterface(node, '0x12345678', resolver.address, { + from: accounts[0], + }) + assert.equal( + await resolver.interfaceImplementer(node, '0x12345678'), + resolver.address, + ) + }) + + it('forbids setting interface by non-owner', async () => { + await exceptions.expectFailure( + resolver.setInterface(node, '0x12345678', accounts[1], { + from: accounts[1], + }), + ) + }) + + it('returns 0 when fetching unset interface', async () => { + assert.equal( + await resolver.interfaceImplementer(namehash.hash('foo'), '0x12345678'), + '0x0000000000000000000000000000000000000000', + ) + }) + + it('falls back to calling implementsInterface on addr', async () => { + // Set addr to the resolver itself, since it has interface implementations. + await resolver.methods['setAddr(bytes32,address)']( + node, + resolver.address, + { + from: accounts[0], + }, + ) + // Check the ID for `addr(bytes32)` + assert.equal( + await resolver.interfaceImplementer(node, '0x3b3b57de'), + resolver.address, + ) + }) + + it('returns 0 on fallback when target contract does not implement interface', async () => { + // Check an imaginary interface ID we know it doesn't support. + assert.equal( + await resolver.interfaceImplementer(node, '0x00000000'), + '0x0000000000000000000000000000000000000000', + ) + }) + + it('returns 0 on fallback when target contract does not support implementsInterface', async () => { + // Set addr to the ENS registry, which doesn't implement supportsInterface. + await resolver.methods['setAddr(bytes32,address)'](node, ens.address, { + from: accounts[0], + }) + // Check the ID for `supportsInterface(bytes4)` + assert.equal( + await resolver.interfaceImplementer(node, '0x01ffc9a7'), + '0x0000000000000000000000000000000000000000', + ) + }) + + it('returns 0 on fallback when target is not a contract', async () => { + // Set addr to an externally owned account. + await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[0], + }) + // Check the ID for `supportsInterface(bytes4)` + assert.equal( + await resolver.interfaceImplementer(node, '0x01ffc9a7'), + '0x0000000000000000000000000000000000000000', + ) + }) + + it('resets record on version change', async () => { + await basicSetInterface() + await resolver.clearRecords(node) + assert.equal( + await resolver.interfaceImplementer(node, '0x12345678'), + '0x0000000000000000000000000000000000000000', + ) + }) + }) + + describe('authorisations', async () => { + it('permits authorisations to be set', async () => { + await resolver.setApprovalForAll(accounts[1], true, { + from: accounts[0], + }) + assert.equal( + await resolver.isApprovedForAll(accounts[0], accounts[1]), + true, + ) + }) + + it('permits authorisations to be set', async () => { + await resolver.setApprovalForAll(accounts[1], true, { + from: accounts[0], + }) + assert.equal( + await resolver.isApprovedForAll(accounts[0], accounts[1]), + true, + ) + }) + + it('permits authorised users to make changes', async () => { + await resolver.setApprovalForAll(accounts[1], true, { + from: accounts[0], + }) + assert.equal( + await resolver.isApprovedForAll(await ens.owner(node), accounts[1]), + true, + ) + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[1], + }) + assert.equal(await resolver.addr(node), accounts[1]) + }) + + it('permits authorisations to be cleared', async () => { + await resolver.setApprovalForAll(accounts[1], false, { + from: accounts[0], + }) + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[1], + }), + ) + }) + + it('permits non-owners to set authorisations', async () => { + await resolver.setApprovalForAll(accounts[2], true, { + from: accounts[1], + }) + + // The authorisation should have no effect, because accounts[1] is not the owner. + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[2], + }), + ) + }) + + it('checks the authorisation for the current owner', async () => { + await resolver.setApprovalForAll(accounts[2], true, { + from: accounts[1], + }) + await ens.setOwner(node, accounts[1], { from: accounts[0] }) + + await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[2], + }) + assert.equal(await resolver.addr(node), accounts[0]) + }) + + it('trusted contract can bypass authorisation', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[9], { + from: accounts[9], + }) + assert.equal(await resolver.addr(node), accounts[9]) + }) + + it('emits an ApprovalForAll log', async () => { + var owner = accounts[0] + var operator = accounts[1] + var tx = await resolver.setApprovalForAll(operator, true, { + from: owner, + }) + assert.equal(tx.logs.length, 1) + assert.equal(tx.logs[0].event, 'ApprovalForAll') + assert.equal(tx.logs[0].args.owner, owner) + assert.equal(tx.logs[0].args.operator, operator) + assert.equal(tx.logs[0].args.approved, true) + }) + + it('reverts if attempting to approve self as an operator', async () => { + await expect( + resolver.setApprovalForAll(accounts[1], true, { from: accounts[1] }), + ).to.be.revertedWith('ERC1155: setting approval status for self') + }) + + it('permits name wrapper owner to make changes if owner is set to name wrapper address', async () => { + var owner = await ens.owner(node) + var operator = accounts[2] + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, owner, { + from: operator, + }), + ) + await ens.setOwner(node, nameWrapper.address, { from: owner }) + await expect( + resolver.methods['setAddr(bytes32,address)'](node, owner, { + from: operator, + }), + ) + }) + }) + + describe('token approvals', async () => { + it('permits delegate to be approved', async () => { + await resolver.approve(node, accounts[1], true, { + from: accounts[0], + }) + assert.equal( + await resolver.isApprovedFor(accounts[0], node, accounts[1]), + true, + ) + }) + + it('permits delegated users to make changes', async () => { + await resolver.approve(node, accounts[1], true, { + from: accounts[0], + }) + assert.equal( + await resolver.isApprovedFor(await ens.owner(node), node, accounts[1]), + true, + ) + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[1], + }) + assert.equal(await resolver.addr(node), accounts[1]) + }) + + it('permits delegations to be cleared', async () => { + await resolver.approve(node, accounts[1], false, { + from: accounts[0], + }) + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[1], + }), + ) + }) + + it('permits non-owners to set delegations', async () => { + await resolver.approve(node, accounts[2], true, { + from: accounts[1], + }) + + // The delegation should have no effect, because accounts[1] is not the owner. + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[2], + }), + ) + }) + + it('checks the delegation for the current owner', async () => { + await resolver.approve(node, accounts[2], true, { + from: accounts[1], + }) + await ens.setOwner(node, accounts[1], { from: accounts[0] }) + + await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[2], + }) + assert.equal(await resolver.addr(node), accounts[0]) + }) + + it('emits a Approved log', async () => { + var owner = accounts[0] + var delegate = accounts[1] + var tx = await resolver.approve(node, delegate, true, { + from: owner, + }) + assert.equal(tx.logs.length, 1) + assert.equal(tx.logs[0].event, 'Approved') + assert.equal(tx.logs[0].args.owner, owner) + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.delegate, delegate) + assert.equal(tx.logs[0].args.approved, true) + }) + + it('reverts if attempting to delegate self as an delegate', async () => { + await expect( + resolver.approve(node, accounts[1], true, { from: accounts[1] }), + ).to.be.revertedWith('Setting delegate status for self') + }) + }) + + describe('multicall', async () => { + it('allows setting multiple fields', async () => { + var addrSet = resolver.contract.methods['setAddr(bytes32,address)']( + node, + accounts[1], + ).encodeABI() + var textSet = resolver.contract.methods + .setText(node, 'url', 'https://ethereum.org/') + .encodeABI() + var tx = await resolver.multicall([addrSet, textSet], { + from: accounts[0], + }) + + assert.equal(tx.logs.length, 3) + assert.equal(tx.logs[0].event, 'AddressChanged') + assert.equal(tx.logs[0].args.node, node) + assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) + assert.equal(tx.logs[1].event, 'AddrChanged') + assert.equal(tx.logs[1].args.node, node) + assert.equal(tx.logs[1].args.a, accounts[1]) + assert.equal(tx.logs[2].event, 'TextChanged') + assert.equal(tx.logs[2].args.node, node) + assert.equal(tx.logs[2].args.key, 'url') + + assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + assert.equal(await resolver.text(node, 'url'), 'https://ethereum.org/') + }) + + it('allows reading multiple fields', async () => { + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) + await resolver.setText(node, 'url', 'https://ethereum.org/', { + from: accounts[0], + }) + var results = await resolver.multicall.call([ + resolver.contract.methods['addr(bytes32)'](node).encodeABI(), + resolver.contract.methods.text(node, 'url').encodeABI(), + ]) + assert.equal( + web3.eth.abi.decodeParameters(['address'], results[0])[0], + accounts[1], + ) + assert.equal( + web3.eth.abi.decodeParameters(['string'], results[1])[0], + 'https://ethereum.org/', + ) + }) + }) +}) From bbd083a12ac9feacc870f7c4f6a8a5c318534138 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:16:32 +0100 Subject: [PATCH 03/19] Add tests for DelegatableResolver --- contracts/resolvers/DelegatableResolver.sol | 20 +- script/deploy.js | 0 script/deploy.ts | 18 + script/hardhat.ts | 0 test/resolvers/TestDelegatableResolver.js | 353 ++------------------ 5 files changed, 71 insertions(+), 320 deletions(-) create mode 100644 script/deploy.js create mode 100644 script/deploy.ts create mode 100644 script/hardhat.ts diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 0270e53f..d73c2590 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -9,12 +9,14 @@ import "./profiles/NameResolver.sol"; import "./profiles/PubkeyResolver.sol"; import "./profiles/TextResolver.sol"; import "./profiles/ExtendedResolver.sol"; +import "./Multicallable.sol"; /** * A simple resolver anyone can use; only allows the owner of a node to set its * address. */ contract DelegatableResolver is + Multicallable, ABIResolver, AddrResolver, ContentHashResolver, @@ -26,6 +28,12 @@ contract DelegatableResolver is ExtendedResolver { using BytesUtils for bytes; + // Logged when an operator is added or removed. + event Approval( + bytes32 indexed node, + address indexed operator, + bool approved + ); constructor(address owner) { operators[bytes32(0)][owner] = true; @@ -42,7 +50,7 @@ contract DelegatableResolver is bytes memory name, uint256 offset, address operator - ) internal returns (bytes32 node, bool authorized) { + ) public view returns (bytes32 node, bool authorized) { uint256 len = name.readUint8(offset); node = bytes32(0); if (len > 0) { @@ -70,7 +78,7 @@ contract DelegatableResolver is // TODO throw custom error with the node info require(authorized, "caller cannot authorise"); operators[node][operator] = approved; - // Add event + emit Approval(node, operator, approved); } // Usage @@ -87,8 +95,11 @@ contract DelegatableResolver is // - lDelegatableResolver.approve(subname, owner, true) function isAuthorised(bytes32 node) internal view override returns (bool) { - // return msg.sender == owner(); - return operators[node][msg.sender]; + return isOwner(msg.sender) || operators[node][msg.sender]; + } + + function isOwner(address addr) public view returns (bool) { + return operators[bytes32(0)][addr]; } function supportsInterface( @@ -98,6 +109,7 @@ contract DelegatableResolver is view virtual override( + Multicallable, ABIResolver, AddrResolver, ContentHashResolver, diff --git a/script/deploy.js b/script/deploy.js new file mode 100644 index 00000000..e69de29b diff --git a/script/deploy.ts b/script/deploy.ts new file mode 100644 index 00000000..1e8e98f9 --- /dev/null +++ b/script/deploy.ts @@ -0,0 +1,18 @@ +import { ethers } from 'hardhat' + +async function main(hre) { + console.log({ hre }) + const lock = await ethers.deployContract('OwnedResolver') + // hre.storageLayout.export(); + + await lock.waitForDeployment() + + console.log(`Deployed to ${lock.target}`) +} + +// We recommend this pattern to be able to use async/await everywhere +// and properly handle errors. +main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/script/hardhat.ts b/script/hardhat.ts new file mode 100644 index 00000000..e69de29b diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index ee2320c8..ab3e343a 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -1,35 +1,20 @@ const ENS = artifacts.require('./registry/ENSRegistry.sol') const DelegatableResolver = artifacts.require('DelegatableResolver.sol') -const NameWrapper = artifacts.require('DummyNameWrapper.sol') -const { deploy } = require('../test-utils/contracts') -const { labelhash } = require('../test-utils/ens') -const { - EMPTY_BYTES32: ROOT_NODE, - EMPTY_ADDRESS, -} = require('../test-utils/constants') - -const { expect } = require('chai') -const namehash = require('eth-ens-namehash') -const sha3 = require('web3-utils').sha3 - +const { encodeName, namehash } = require('../test-utils/ens') const { exceptions } = require('../test-utils') contract('DelegatableResolver', function (accounts) { - let node - let ens, resolver, nameWrapper + let node, encodedname + let ens, resolver let account let signers - let result beforeEach(async () => { signers = await ethers.getSigners() account = await signers[0].getAddress() - node = namehash.hash('eth') + node = namehash('eth') + encodedname = encodeName('eth') ens = await ENS.new() - nameWrapper = await NameWrapper.new() - - //setup reverse registrar - resolver = await DelegatableResolver.new(account) }) @@ -52,54 +37,13 @@ contract('DelegatableResolver', function (accounts) { }) }) - describe('recordVersion', async () => { - it('permits clearing records', async () => { - var tx = await resolver.clearRecords(node, { from: accounts[0] }) - assert.equal(tx.logs.length, 1) - assert.equal(tx.logs[0].event, 'VersionChanged') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.newVersion, 1) - }) - }) - describe('addr', async () => { - it.only('permits setting address by owner', async () => { + it('permits setting address by owner', async () => { var tx = await resolver.methods['setAddr(bytes32,address)']( node, accounts[1], { from: accounts[0] }, ) - assert.equal(tx.logs.length, 2) - assert.equal(tx.logs[0].event, 'AddressChanged') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) - assert.equal(tx.logs[1].event, 'AddrChanged') - assert.equal(tx.logs[1].args.node, node) - assert.equal(tx.logs[1].args.a, accounts[1]) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) - }) - - it('can overwrite previously set address', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) - - await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[0], - }) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[0]) - }) - - it('can overwrite to same address', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) - - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) }) @@ -110,97 +54,6 @@ contract('DelegatableResolver', function (accounts) { }), ) }) - - it('forbids writing same address by non-owners', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) - - await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[1], - }), - ) - }) - - it('forbids overwriting existing address by non-owners', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) - - await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[1], - }), - ) - }) - - it('returns zero when fetching nonexistent addresses', async () => { - assert.equal( - await resolver.methods['addr(bytes32)'](node), - '0x0000000000000000000000000000000000000000', - ) - }) - - it('permits setting and retrieving addresses for other coin types', async () => { - await resolver.methods['setAddr(bytes32,uint256,bytes)']( - node, - 123, - accounts[1], - { from: accounts[0] }, - ) - assert.equal( - await resolver.methods['addr(bytes32,uint256)'](node, 123), - accounts[1].toLowerCase(), - ) - }) - - it('returns ETH address for coin type 60', async () => { - var tx = await resolver.methods['setAddr(bytes32,address)']( - node, - accounts[1], - { from: accounts[0] }, - ) - assert.equal(tx.logs.length, 2) - assert.equal(tx.logs[0].event, 'AddressChanged') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) - assert.equal(tx.logs[1].event, 'AddrChanged') - assert.equal(tx.logs[1].args.node, node) - assert.equal(tx.logs[1].args.a, accounts[1]) - assert.equal( - await resolver.methods['addr(bytes32,uint256)'](node, 60), - accounts[1].toLowerCase(), - ) - }) - - it('setting coin type 60 updates ETH address', async () => { - var tx = await resolver.methods['setAddr(bytes32,uint256,bytes)']( - node, - 60, - accounts[2], - { from: accounts[0] }, - ) - assert.equal(tx.logs.length, 2) - assert.equal(tx.logs[0].event, 'AddressChanged') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.newAddress, accounts[2].toLowerCase()) - assert.equal(tx.logs[1].event, 'AddrChanged') - assert.equal(tx.logs[1].args.node, node) - assert.equal(tx.logs[1].args.a, accounts[2]) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[2]) - }) - - it('forbids calls to the fallback function with 1 value', async () => { - await exceptions.expectFailure( - web3.eth.sendTransaction({ - from: accounts[0], - to: resolver.address, - gas: 3000000, - value: 1, - }), - ) - }) }) describe('implementsInterface', async () => { @@ -236,7 +89,7 @@ contract('DelegatableResolver', function (accounts) { it('returns 0 when fetching unset interface', async () => { assert.equal( - await resolver.interfaceImplementer(namehash.hash('foo'), '0x12345678'), + await resolver.interfaceImplementer(namehash('foo'), '0x12345678'), '0x0000000000000000000000000000000000000000', ) }) @@ -300,148 +153,45 @@ contract('DelegatableResolver', function (accounts) { }) describe('authorisations', async () => { - it('permits authorisations to be set', async () => { - await resolver.setApprovalForAll(accounts[1], true, { - from: accounts[0], - }) - assert.equal( - await resolver.isApprovedForAll(accounts[0], accounts[1]), - true, - ) - }) - - it('permits authorisations to be set', async () => { - await resolver.setApprovalForAll(accounts[1], true, { - from: accounts[0], - }) - assert.equal( - await resolver.isApprovedForAll(accounts[0], accounts[1]), - true, + it('approves multiple users', async () => { + await resolver.approve(encodedname, accounts[1], true) + await resolver.approve(encodedname, accounts[2], true) + const result = await resolver.getAuthorizedNode( + encodedname, + 0, + accounts[1], ) - }) - - it('permits authorised users to make changes', async () => { - await resolver.setApprovalForAll(accounts[1], true, { - from: accounts[0], - }) + assert.equal(result.node, node) + assert.equal(result.authorized, true) assert.equal( - await resolver.isApprovedForAll(await ens.owner(node), accounts[1]), + (await resolver.getAuthorizedNode(encodedname, 0, accounts[2])) + .authorized, true, ) - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[1], - }) - assert.equal(await resolver.addr(node), accounts[1]) }) - it('permits authorisations to be cleared', async () => { - await resolver.setApprovalForAll(accounts[1], false, { - from: accounts[0], - }) - await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + it('approves subnames', async () => { + const subname = 'a.b.c.eth' + await resolver.approve(encodeName(subname), accounts[1], true) + await resolver.methods['setAddr(bytes32,address)']( + namehash(subname), + accounts[1], + { from: accounts[1], - }), - ) - }) - - it('permits non-owners to set authorisations', async () => { - await resolver.setApprovalForAll(accounts[2], true, { - from: accounts[1], - }) - - // The authorisation should have no effect, because accounts[1] is not the owner. - await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[2], - }), - ) - }) - - it('checks the authorisation for the current owner', async () => { - await resolver.setApprovalForAll(accounts[2], true, { - from: accounts[1], - }) - await ens.setOwner(node, accounts[1], { from: accounts[0] }) - - await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[2], - }) - assert.equal(await resolver.addr(node), accounts[0]) - }) - - it('trusted contract can bypass authorisation', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[9], { - from: accounts[9], - }) - assert.equal(await resolver.addr(node), accounts[9]) - }) - - it('emits an ApprovalForAll log', async () => { - var owner = accounts[0] - var operator = accounts[1] - var tx = await resolver.setApprovalForAll(operator, true, { - from: owner, - }) - assert.equal(tx.logs.length, 1) - assert.equal(tx.logs[0].event, 'ApprovalForAll') - assert.equal(tx.logs[0].args.owner, owner) - assert.equal(tx.logs[0].args.operator, operator) - assert.equal(tx.logs[0].args.approved, true) - }) - - it('reverts if attempting to approve self as an operator', async () => { - await expect( - resolver.setApprovalForAll(accounts[1], true, { from: accounts[1] }), - ).to.be.revertedWith('ERC1155: setting approval status for self') - }) - - it('permits name wrapper owner to make changes if owner is set to name wrapper address', async () => { - var owner = await ens.owner(node) - var operator = accounts[2] - await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, owner, { - from: operator, - }), - ) - await ens.setOwner(node, nameWrapper.address, { from: owner }) - await expect( - resolver.methods['setAddr(bytes32,address)'](node, owner, { - from: operator, - }), - ) - }) - }) - - describe('token approvals', async () => { - it('permits delegate to be approved', async () => { - await resolver.approve(node, accounts[1], true, { - from: accounts[0], - }) - assert.equal( - await resolver.isApprovedFor(accounts[0], node, accounts[1]), - true, + }, ) }) - it('permits delegated users to make changes', async () => { - await resolver.approve(node, accounts[1], true, { - from: accounts[0], - }) - assert.equal( - await resolver.isApprovedFor(await ens.owner(node), node, accounts[1]), - true, - ) + it('approves users to make changes', async () => { + await resolver.approve(encodedname, accounts[1], true) await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { from: accounts[1], }) assert.equal(await resolver.addr(node), accounts[1]) }) - it('permits delegations to be cleared', async () => { - await resolver.approve(node, accounts[1], false, { - from: accounts[0], - }) + it('approves to be cleared', async () => { + await resolver.approve(encodedname, accounts[1], false) await exceptions.expectFailure( resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { from: accounts[1], @@ -449,50 +199,21 @@ contract('DelegatableResolver', function (accounts) { ) }) - it('permits non-owners to set delegations', async () => { - await resolver.approve(node, accounts[2], true, { - from: accounts[1], - }) - - // The delegation should have no effect, because accounts[1] is not the owner. + it('does not allow non owner to approve', async () => { await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[2], - }), + resolver.approve(encodedname, accounts[1], true, { from: accounts[1] }), ) }) - it('checks the delegation for the current owner', async () => { - await resolver.approve(node, accounts[2], true, { - from: accounts[1], - }) - await ens.setOwner(node, accounts[1], { from: accounts[0] }) - - await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[2], - }) - assert.equal(await resolver.addr(node), accounts[0]) - }) - - it('emits a Approved log', async () => { - var owner = accounts[0] - var delegate = accounts[1] - var tx = await resolver.approve(node, delegate, true, { - from: owner, - }) + it('emits an Approval log', async () => { + var operator = accounts[1] + var tx = await resolver.approve(encodedname, operator, true) assert.equal(tx.logs.length, 1) - assert.equal(tx.logs[0].event, 'Approved') - assert.equal(tx.logs[0].args.owner, owner) + assert.equal(tx.logs[0].event, 'Approval') assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.delegate, delegate) + assert.equal(tx.logs[0].args.operator, operator) assert.equal(tx.logs[0].args.approved, true) }) - - it('reverts if attempting to delegate self as an delegate', async () => { - await expect( - resolver.approve(node, accounts[1], true, { from: accounts[1] }), - ).to.be.revertedWith('Setting delegate status for self') - }) }) describe('multicall', async () => { From 16d5a79a02c3b4a1cdde46c4ccd0761247c06817 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:20:23 +0100 Subject: [PATCH 04/19] Add encodedname into the event --- contracts/resolvers/DelegatableResolver.sol | 7 ++----- test/resolvers/TestDelegatableResolver.js | 1 + 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index d73c2590..da6b0d86 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -32,6 +32,7 @@ contract DelegatableResolver is event Approval( bytes32 indexed node, address indexed operator, + bytes name, bool approved ); @@ -42,10 +43,6 @@ contract DelegatableResolver is //node => (delegate => isAuthorised) mapping(bytes32 => mapping(address => bool)) operators; - // function approve(bytes32 node, address operator, bool approved) authorised(node){ - // operators[node][operator] = approved; - // // Add event - // } function getAuthorizedNode( bytes memory name, uint256 offset, @@ -78,7 +75,7 @@ contract DelegatableResolver is // TODO throw custom error with the node info require(authorized, "caller cannot authorise"); operators[node][operator] = approved; - emit Approval(node, operator, approved); + emit Approval(node, operator, name, approved); } // Usage diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index ab3e343a..42788c89 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -212,6 +212,7 @@ contract('DelegatableResolver', function (accounts) { assert.equal(tx.logs[0].event, 'Approval') assert.equal(tx.logs[0].args.node, node) assert.equal(tx.logs[0].args.operator, operator) + assert.equal(tx.logs[0].args.name, encodedname) assert.equal(tx.logs[0].args.approved, true) }) }) From 2aee50be352f312a06b093f349f4d74ca4c307ac Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:26:12 +0100 Subject: [PATCH 05/19] Add comment --- contracts/resolvers/DelegatableResolver.sol | 22 ++++++++------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index da6b0d86..987ab3fe 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -12,7 +12,7 @@ import "./profiles/ExtendedResolver.sol"; import "./Multicallable.sol"; /** - * A simple resolver anyone can use; only allows the owner of a node to set its + * A delegated resolver that allows for the resolver owner to add an operator to update records of a node on behalf of the owner. * address. */ contract DelegatableResolver is @@ -28,6 +28,7 @@ contract DelegatableResolver is ExtendedResolver { using BytesUtils for bytes; + // Logged when an operator is added or removed. event Approval( bytes32 indexed node, @@ -43,6 +44,9 @@ contract DelegatableResolver is //node => (delegate => isAuthorised) mapping(bytes32 => mapping(address => bool)) operators; + /** + * @dev Check to see if the operator has been approved by the owner for the node. + */ function getAuthorizedNode( bytes memory name, uint256 offset, @@ -62,6 +66,9 @@ contract DelegatableResolver is return (node, authorized || operators[node][operator]); } + /** + * @dev Approve an operator to be able to updated records on a node. + */ function approve( bytes memory name, address operator, @@ -78,19 +85,6 @@ contract DelegatableResolver is emit Approval(node, operator, name, approved); } - // Usage - // basenode = namehash('onl2.eth') - // - l2: l2resolver = DelegatableResolver.deploy(ownerAddress) - // - l1: l1resolver.setVerfierForNode(......, l2resolver) - // - l1: registry.setResolver(......, l1resolver.address) - // - l2: subnameregistrar = SubnameRegistar.deploy - // base = bytes32(0) - // name = encodename('onl2.eth') - // - l2: DelegatableResolver.approve(name, subnameregistrar, true) - // - l2: subnameregistrar.register('makoto') - // subname = encodename('makoto.onl2.eth') - // - lDelegatableResolver.approve(subname, owner, true) - function isAuthorised(bytes32 node) internal view override returns (bool) { return isOwner(msg.sender) || operators[node][msg.sender]; } From 429fa38a660cc3b6a62738e880e63e03e195410e Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Thu, 26 Oct 2023 17:35:52 +0100 Subject: [PATCH 06/19] Add custom error --- contracts/resolvers/DelegatableResolver.sol | 7 +++++-- test/resolvers/TestDelegatableResolver.js | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 987ab3fe..f277f310 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -37,6 +37,8 @@ contract DelegatableResolver is bool approved ); + error NotAuthorized(bytes32 node); + constructor(address owner) { operators[bytes32(0)][owner] = true; } @@ -79,8 +81,9 @@ contract DelegatableResolver is 0, msg.sender ); - // TODO throw custom error with the node info - require(authorized, "caller cannot authorise"); + if (!authorized) { + revert NotAuthorized(node); + } operators[node][operator] = approved; emit Approval(node, operator, name, approved); } diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 42788c89..99166ebc 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -2,6 +2,7 @@ const ENS = artifacts.require('./registry/ENSRegistry.sol') const DelegatableResolver = artifacts.require('DelegatableResolver.sol') const { encodeName, namehash } = require('../test-utils/ens') const { exceptions } = require('../test-utils') +const { expect } = require('chai') contract('DelegatableResolver', function (accounts) { let node, encodedname @@ -200,9 +201,9 @@ contract('DelegatableResolver', function (accounts) { }) it('does not allow non owner to approve', async () => { - await exceptions.expectFailure( + await expect( resolver.approve(encodedname, accounts[1], true, { from: accounts[1] }), - ) + ).to.be.revertedWith('NotAuthorized') }) it('emits an Approval log', async () => { From afae77ec3bd4520edc84faa1f7639b3f61738ffa Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:03:42 +0100 Subject: [PATCH 07/19] Add IDelegatableResolver --- contracts/resolvers/DelegatableResolver.sol | 5 +- contracts/resolvers/IDelegatableResolver.sol | 18 +++ test/resolvers/TestDelegatableResolver.js | 109 +++---------------- 3 files changed, 35 insertions(+), 97 deletions(-) create mode 100644 contracts/resolvers/IDelegatableResolver.sol diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index f277f310..984ecc8d 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -10,6 +10,7 @@ import "./profiles/PubkeyResolver.sol"; import "./profiles/TextResolver.sol"; import "./profiles/ExtendedResolver.sol"; import "./Multicallable.sol"; +import "./IDelegatableResolver.sol"; /** * A delegated resolver that allows for the resolver owner to add an operator to update records of a node on behalf of the owner. @@ -115,6 +116,8 @@ contract DelegatableResolver is ) returns (bool) { - return super.supportsInterface(interfaceID); + return + interfaceID == type(IDelegatableResolver).interfaceId || + super.supportsInterface(interfaceID); } } diff --git a/contracts/resolvers/IDelegatableResolver.sol b/contracts/resolvers/IDelegatableResolver.sol new file mode 100644 index 00000000..29566c62 --- /dev/null +++ b/contracts/resolvers/IDelegatableResolver.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.8.4; + +interface IDelegatableResolver { + function approve( + bytes memory name, + address operator, + bool approved + ) external; + + function getAuthorizedNode( + bytes memory name, + uint256 offset, + address operator + ) external returns (bytes32 node, bool authorized); + + function isOwner(address addr) external returns (bool); +} diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 99166ebc..166828ab 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -31,6 +31,8 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.supportsInterface('0xa8fa5682'), true) // IDNSRecordResolver assert.equal(await resolver.supportsInterface('0x5c98042b'), true) // IDNSZoneResolver assert.equal(await resolver.supportsInterface('0x01ffc9a7'), true) // IInterfaceResolver + assert.equal(await resolver.supportsInterface('0x4fbf0433'), true) // IMulticallable + assert.equal(await resolver.supportsInterface('0xf21ce672'), true) // IDelegatable }) it('does not support a random interface', async () => { @@ -57,102 +59,6 @@ contract('DelegatableResolver', function (accounts) { }) }) - describe('implementsInterface', async () => { - const basicSetInterface = async () => { - await resolver.setInterface(node, '0x12345678', accounts[0], { - from: accounts[0], - }) - assert.equal( - await resolver.interfaceImplementer(node, '0x12345678'), - accounts[0], - ) - } - - it('permits setting interface by owner', basicSetInterface) - - it('can update previously set interface', async () => { - await resolver.setInterface(node, '0x12345678', resolver.address, { - from: accounts[0], - }) - assert.equal( - await resolver.interfaceImplementer(node, '0x12345678'), - resolver.address, - ) - }) - - it('forbids setting interface by non-owner', async () => { - await exceptions.expectFailure( - resolver.setInterface(node, '0x12345678', accounts[1], { - from: accounts[1], - }), - ) - }) - - it('returns 0 when fetching unset interface', async () => { - assert.equal( - await resolver.interfaceImplementer(namehash('foo'), '0x12345678'), - '0x0000000000000000000000000000000000000000', - ) - }) - - it('falls back to calling implementsInterface on addr', async () => { - // Set addr to the resolver itself, since it has interface implementations. - await resolver.methods['setAddr(bytes32,address)']( - node, - resolver.address, - { - from: accounts[0], - }, - ) - // Check the ID for `addr(bytes32)` - assert.equal( - await resolver.interfaceImplementer(node, '0x3b3b57de'), - resolver.address, - ) - }) - - it('returns 0 on fallback when target contract does not implement interface', async () => { - // Check an imaginary interface ID we know it doesn't support. - assert.equal( - await resolver.interfaceImplementer(node, '0x00000000'), - '0x0000000000000000000000000000000000000000', - ) - }) - - it('returns 0 on fallback when target contract does not support implementsInterface', async () => { - // Set addr to the ENS registry, which doesn't implement supportsInterface. - await resolver.methods['setAddr(bytes32,address)'](node, ens.address, { - from: accounts[0], - }) - // Check the ID for `supportsInterface(bytes4)` - assert.equal( - await resolver.interfaceImplementer(node, '0x01ffc9a7'), - '0x0000000000000000000000000000000000000000', - ) - }) - - it('returns 0 on fallback when target is not a contract', async () => { - // Set addr to an externally owned account. - await resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[0], - }) - // Check the ID for `supportsInterface(bytes4)` - assert.equal( - await resolver.interfaceImplementer(node, '0x01ffc9a7'), - '0x0000000000000000000000000000000000000000', - ) - }) - - it('resets record on version change', async () => { - await basicSetInterface() - await resolver.clearRecords(node) - assert.equal( - await resolver.interfaceImplementer(node, '0x12345678'), - '0x0000000000000000000000000000000000000000', - ) - }) - }) - describe('authorisations', async () => { it('approves multiple users', async () => { await resolver.approve(encodedname, accounts[1], true) @@ -218,6 +124,17 @@ contract('DelegatableResolver', function (accounts) { }) }) + describe('isOwner', async () => { + it('the deployer is the owner by default', async () => { + assert.equal(await resolver.isOwner(account), true) + }) + + it('can have multiple owner', async () => { + await resolver.approve(encodeName(''), accounts[1], true) + assert.equal(await resolver.isOwner(accounts[1]), true) + }) + }) + describe('multicall', async () => { it('allows setting multiple fields', async () => { var addrSet = resolver.contract.methods['setAddr(bytes32,address)']( From 5eb3d378d87f738f65db8a1bfad325bc8749b475 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:10:26 +0100 Subject: [PATCH 08/19] Remove script/deploy.ts --- script/deploy.js | 0 script/deploy.ts | 18 ------------------ 2 files changed, 18 deletions(-) delete mode 100644 script/deploy.js delete mode 100644 script/deploy.ts diff --git a/script/deploy.js b/script/deploy.js deleted file mode 100644 index e69de29b..00000000 diff --git a/script/deploy.ts b/script/deploy.ts deleted file mode 100644 index 1e8e98f9..00000000 --- a/script/deploy.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { ethers } from 'hardhat' - -async function main(hre) { - console.log({ hre }) - const lock = await ethers.deployContract('OwnedResolver') - // hre.storageLayout.export(); - - await lock.waitForDeployment() - - console.log(`Deployed to ${lock.target}`) -} - -// We recommend this pattern to be able to use async/await everywhere -// and properly handle errors. -main().catch((error) => { - console.error(error) - process.exitCode = 1 -}) From 6cc269f3cc9989a3b9cb133872f32f96beda9e85 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:18:02 +0100 Subject: [PATCH 09/19] Remove empty file --- script/hardhat.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 script/hardhat.ts diff --git a/script/hardhat.ts b/script/hardhat.ts deleted file mode 100644 index e69de29b..00000000 From 59b0cc74fd9ea40c143ec0c9129a0d0c6c4f95dc Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:43:03 +0000 Subject: [PATCH 10/19] Remove Ownable --- contracts/resolvers/DelegatableResolver.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 984ecc8d..a3a225fe 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -1,5 +1,4 @@ pragma solidity >=0.8.4; -import "@openzeppelin/contracts/access/Ownable.sol"; import "./profiles/ABIResolver.sol"; import "./profiles/AddrResolver.sol"; import "./profiles/ContentHashResolver.sol"; From 0abb7cd45d0f66b7c982e53d223f23a499b4dac1 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:45:19 +0000 Subject: [PATCH 11/19] Revert indentation From 2fadc67ab8703ad85bedecc8dc6b226a50e839ce Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:56:07 +0000 Subject: [PATCH 12/19] Fix test --- test/resolvers/TestDelegatableResolver.js | 24 ++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 166828ab..883fdffe 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -1,12 +1,12 @@ -const ENS = artifacts.require('./registry/ENSRegistry.sol') const DelegatableResolver = artifacts.require('DelegatableResolver.sol') const { encodeName, namehash } = require('../test-utils/ens') const { exceptions } = require('../test-utils') const { expect } = require('chai') contract('DelegatableResolver', function (accounts) { - let node, encodedname - let ens, resolver + let node + let encodedname + let resolver let account let signers @@ -15,7 +15,6 @@ contract('DelegatableResolver', function (accounts) { account = await signers[0].getAddress() node = namehash('eth') encodedname = encodeName('eth') - ens = await ENS.new() resolver = await DelegatableResolver.new(account) }) @@ -42,11 +41,9 @@ contract('DelegatableResolver', function (accounts) { describe('addr', async () => { it('permits setting address by owner', async () => { - var tx = await resolver.methods['setAddr(bytes32,address)']( - node, - accounts[1], - { from: accounts[0] }, - ) + await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { + from: accounts[0], + }) assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) }) @@ -97,8 +94,12 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.addr(node), accounts[1]) }) - it('approves to be cleared', async () => { - await resolver.approve(encodedname, accounts[1], false) + it('approves to be revoked', async () => { + await resolver.approve(encodedname, accounts[1], true) + resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { + from: accounts[1], + }), + await resolver.approve(encodedname, accounts[1], false) await exceptions.expectFailure( resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { from: accounts[1], @@ -131,6 +132,7 @@ contract('DelegatableResolver', function (accounts) { it('can have multiple owner', async () => { await resolver.approve(encodeName(''), accounts[1], true) + assert.equal(await resolver.isOwner(accounts[0]), true) assert.equal(await resolver.isOwner(accounts[1]), true) }) }) From 42acb3fea146e150eab948fe224dab048e932ac0 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 10:56:40 +0000 Subject: [PATCH 13/19] Remove test --- test/resolvers/TestDelegatableResolver.js | 50 ----------------------- 1 file changed, 50 deletions(-) diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 883fdffe..828f2ef5 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -136,54 +136,4 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.isOwner(accounts[1]), true) }) }) - - describe('multicall', async () => { - it('allows setting multiple fields', async () => { - var addrSet = resolver.contract.methods['setAddr(bytes32,address)']( - node, - accounts[1], - ).encodeABI() - var textSet = resolver.contract.methods - .setText(node, 'url', 'https://ethereum.org/') - .encodeABI() - var tx = await resolver.multicall([addrSet, textSet], { - from: accounts[0], - }) - - assert.equal(tx.logs.length, 3) - assert.equal(tx.logs[0].event, 'AddressChanged') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.newAddress, accounts[1].toLowerCase()) - assert.equal(tx.logs[1].event, 'AddrChanged') - assert.equal(tx.logs[1].args.node, node) - assert.equal(tx.logs[1].args.a, accounts[1]) - assert.equal(tx.logs[2].event, 'TextChanged') - assert.equal(tx.logs[2].args.node, node) - assert.equal(tx.logs[2].args.key, 'url') - - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) - assert.equal(await resolver.text(node, 'url'), 'https://ethereum.org/') - }) - - it('allows reading multiple fields', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], - }) - await resolver.setText(node, 'url', 'https://ethereum.org/', { - from: accounts[0], - }) - var results = await resolver.multicall.call([ - resolver.contract.methods['addr(bytes32)'](node).encodeABI(), - resolver.contract.methods.text(node, 'url').encodeABI(), - ]) - assert.equal( - web3.eth.abi.decodeParameters(['address'], results[0])[0], - accounts[1], - ) - assert.equal( - web3.eth.abi.decodeParameters(['string'], results[1])[0], - 'https://ethereum.org/', - ) - }) - }) }) From 7ecbe22d6d3cd02ab700ef64ff8acf3604e93af4 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 11:29:36 +0000 Subject: [PATCH 14/19] Revert to use OZ Ownable --- contracts/resolvers/DelegatableResolver.sol | 11 ++- contracts/resolvers/IDelegatableResolver.sol | 2 - test/resolvers/TestDelegatableResolver.js | 83 +++++++++----------- 3 files changed, 43 insertions(+), 53 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index a3a225fe..15c6378b 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -1,4 +1,5 @@ pragma solidity >=0.8.4; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import "./profiles/ABIResolver.sol"; import "./profiles/AddrResolver.sol"; import "./profiles/ContentHashResolver.sol"; @@ -12,10 +13,11 @@ import "./Multicallable.sol"; import "./IDelegatableResolver.sol"; /** - * A delegated resolver that allows for the resolver owner to add an operator to update records of a node on behalf of the owner. + * A delegated resolver that allows the resolver owner to add an operator to update records of a node on behalf of the owner. * address. */ contract DelegatableResolver is + Ownable, Multicallable, ABIResolver, AddrResolver, @@ -41,6 +43,7 @@ contract DelegatableResolver is constructor(address owner) { operators[bytes32(0)][owner] = true; + transferOwnership(owner); } //node => (delegate => isAuthorised) @@ -89,11 +92,7 @@ contract DelegatableResolver is } function isAuthorised(bytes32 node) internal view override returns (bool) { - return isOwner(msg.sender) || operators[node][msg.sender]; - } - - function isOwner(address addr) public view returns (bool) { - return operators[bytes32(0)][addr]; + return msg.sender == owner() || operators[node][msg.sender]; } function supportsInterface( diff --git a/contracts/resolvers/IDelegatableResolver.sol b/contracts/resolvers/IDelegatableResolver.sol index 29566c62..12dde116 100644 --- a/contracts/resolvers/IDelegatableResolver.sol +++ b/contracts/resolvers/IDelegatableResolver.sol @@ -13,6 +13,4 @@ interface IDelegatableResolver { uint256 offset, address operator ) external returns (bytes32 node, bool authorized); - - function isOwner(address addr) external returns (bool); } diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 828f2ef5..8b75b98c 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -7,15 +7,21 @@ contract('DelegatableResolver', function (accounts) { let node let encodedname let resolver - let account let signers + let deployer + let owner + let operator + let operator2 beforeEach(async () => { signers = await ethers.getSigners() - account = await signers[0].getAddress() + deployer = await signers[0].getAddress() + owner = await signers[1].getAddress() + operator = await signers[2].getAddress() + operator2 = await signers[3].getAddress() node = namehash('eth') encodedname = encodeName('eth') - resolver = await DelegatableResolver.new(account) + resolver = await DelegatableResolver.new(owner) }) describe('supportsInterface function', async () => { @@ -31,7 +37,7 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.supportsInterface('0x5c98042b'), true) // IDNSZoneResolver assert.equal(await resolver.supportsInterface('0x01ffc9a7'), true) // IInterfaceResolver assert.equal(await resolver.supportsInterface('0x4fbf0433'), true) // IMulticallable - assert.equal(await resolver.supportsInterface('0xf21ce672'), true) // IDelegatable + assert.equal(await resolver.supportsInterface('0xdd48591c'), true) // IDelegatable }) it('does not support a random interface', async () => { @@ -41,16 +47,16 @@ contract('DelegatableResolver', function (accounts) { describe('addr', async () => { it('permits setting address by owner', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[0], + await resolver.methods['setAddr(bytes32,address)'](node, operator, { + from: owner, }) - assert.equal(await resolver.methods['addr(bytes32)'](node), accounts[1]) + assert.equal(await resolver.methods['addr(bytes32)'](node), operator) }) it('forbids setting new address by non-owners', async () => { await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[1], + resolver.methods['setAddr(bytes32,address)'](node, operator, { + from: operator, }), ) }) @@ -58,17 +64,13 @@ contract('DelegatableResolver', function (accounts) { describe('authorisations', async () => { it('approves multiple users', async () => { - await resolver.approve(encodedname, accounts[1], true) - await resolver.approve(encodedname, accounts[2], true) - const result = await resolver.getAuthorizedNode( - encodedname, - 0, - accounts[1], - ) + await resolver.approve(encodedname, operator, true, { from: owner }) + await resolver.approve(encodedname, operator2, true, { from: owner }) + const result = await resolver.getAuthorizedNode(encodedname, 0, operator) assert.equal(result.node, node) assert.equal(result.authorized, true) assert.equal( - (await resolver.getAuthorizedNode(encodedname, 0, accounts[2])) + (await resolver.getAuthorizedNode(encodedname, 0, operator2)) .authorized, true, ) @@ -76,46 +78,49 @@ contract('DelegatableResolver', function (accounts) { it('approves subnames', async () => { const subname = 'a.b.c.eth' - await resolver.approve(encodeName(subname), accounts[1], true) + await resolver.approve(encodeName(subname), operator, true, { + from: owner, + }) await resolver.methods['setAddr(bytes32,address)']( namehash(subname), - accounts[1], + operator, { - from: accounts[1], + from: operator, }, ) }) it('approves users to make changes', async () => { - await resolver.approve(encodedname, accounts[1], true) - await resolver.methods['setAddr(bytes32,address)'](node, accounts[1], { - from: accounts[1], + await resolver.approve(encodedname, operator, true, { from: owner }) + await resolver.methods['setAddr(bytes32,address)'](node, operator, { + from: operator, }) - assert.equal(await resolver.addr(node), accounts[1]) + assert.equal(await resolver.addr(node), operator) }) it('approves to be revoked', async () => { - await resolver.approve(encodedname, accounts[1], true) - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[1], + await resolver.approve(encodedname, operator, true, { from: owner }) + resolver.methods['setAddr(bytes32,address)'](node, operator2, { + from: operator, }), - await resolver.approve(encodedname, accounts[1], false) + await resolver.approve(encodedname, operator, false, { from: owner }) await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, accounts[0], { - from: accounts[1], + resolver.methods['setAddr(bytes32,address)'](node, operator2, { + from: operator, }), ) }) it('does not allow non owner to approve', async () => { await expect( - resolver.approve(encodedname, accounts[1], true, { from: accounts[1] }), + resolver.approve(encodedname, operator, true, { from: operator }), ).to.be.revertedWith('NotAuthorized') }) it('emits an Approval log', async () => { - var operator = accounts[1] - var tx = await resolver.approve(encodedname, operator, true) + var tx = await resolver.approve(encodedname, operator, true, { + from: owner, + }) assert.equal(tx.logs.length, 1) assert.equal(tx.logs[0].event, 'Approval') assert.equal(tx.logs[0].args.node, node) @@ -124,16 +129,4 @@ contract('DelegatableResolver', function (accounts) { assert.equal(tx.logs[0].args.approved, true) }) }) - - describe('isOwner', async () => { - it('the deployer is the owner by default', async () => { - assert.equal(await resolver.isOwner(account), true) - }) - - it('can have multiple owner', async () => { - await resolver.approve(encodeName(''), accounts[1], true) - assert.equal(await resolver.isOwner(accounts[0]), true) - assert.equal(await resolver.isOwner(accounts[1]), true) - }) - }) }) From ed57aa8d60b692dca01a45e9afd563a5921aa60f Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 11:33:54 +0000 Subject: [PATCH 15/19] Add a test to check the owner --- test/resolvers/TestDelegatableResolver.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 8b75b98c..8e42e4db 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -63,6 +63,10 @@ contract('DelegatableResolver', function (accounts) { }) describe('authorisations', async () => { + it('owner is the owner', async () => { + assert.equal(await resolver.owner(), owner) + }) + it('approves multiple users', async () => { await resolver.approve(encodedname, operator, true, { from: owner }) await resolver.approve(encodedname, operator2, true, { from: owner }) From 093fbd2619efc297295865007be18c6c2892d09a Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:12:50 +0000 Subject: [PATCH 16/19] Add comments and more test --- contracts/resolvers/DelegatableResolver.sol | 9 +++- test/resolvers/TestDelegatableResolver.js | 46 +++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 15c6378b..3f28a9a5 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -49,8 +49,13 @@ contract DelegatableResolver is //node => (delegate => isAuthorised) mapping(bytes32 => mapping(address => bool)) operators; - /** - * @dev Check to see if the operator has been approved by the owner for the node. + /* + * Check to see if the operator has been approved by the owner for the node. + * @param name The ENS node to query + * @param offset The offset of the label to query recursively. Start from the 0 position and kepp adding the length of each label as it traverse. The function exits when len is 0. + * @param operator The address of the operator to query + * @return node The node of the name passed as an argument + * @return authorized The boolean state of whether the operator is approved to update record of the name */ function getAuthorizedNode( bytes memory name, diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 8e42e4db..71e02c30 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -60,6 +60,17 @@ contract('DelegatableResolver', function (accounts) { }), ) }) + + it('forbids approving wrong node', async () => { + encodedname = encodeName('a.b.c.eth') + const wrongnode = namehash('d.b.c.eth') + await resolver.approve(encodedname, operator, true, { from: owner }) + await exceptions.expectFailure( + resolver.methods['setAddr(bytes32,address)'](wrongnode, operator, { + from: operator, + }), + ) + }) }) describe('authorisations', async () => { @@ -67,6 +78,19 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.owner(), owner) }) + it('owner is ahtorised to update any names', async () => { + assert.equal( + (await resolver.getAuthorizedNode(encodeName('a.b.c'), 0, owner)) + .authorized, + true, + ) + assert.equal( + (await resolver.getAuthorizedNode(encodeName('x.y.z'), 0, owner)) + .authorized, + true, + ) + }) + it('approves multiple users', async () => { await resolver.approve(encodedname, operator, true, { from: owner }) await resolver.approve(encodedname, operator2, true, { from: owner }) @@ -94,6 +118,28 @@ contract('DelegatableResolver', function (accounts) { ) }) + it('only approves the subname and not its parent', async () => { + const subname = '1234.123' + const parentname = 'b.c.eth' + await resolver.approve(encodeName(subname), operator, true, { + from: owner, + }) + const result = await resolver.getAuthorizedNode( + encodeName(subname), + 0, + operator, + ) + assert.equal(result.node, namehash(subname)) + assert.equal(result.authorized, true) + const result2 = await resolver.getAuthorizedNode( + encodeName(parentname), + 0, + operator, + ) + assert.equal(result2.node, namehash(parentname)) + assert.equal(result2.authorized, false) + }) + it('approves users to make changes', async () => { await resolver.approve(encodedname, operator, true, { from: owner }) await resolver.methods['setAddr(bytes32,address)'](node, operator, { From d07e6d921964e2e3ade4d705cec7477d073d6c42 Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Wed, 1 Nov 2023 12:40:10 +0000 Subject: [PATCH 17/19] Rename getAuthorizedNode to getAuthorisedNode --- contracts/resolvers/DelegatableResolver.sol | 6 +++--- contracts/resolvers/IDelegatableResolver.sol | 2 +- test/resolvers/TestDelegatableResolver.js | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 3f28a9a5..92b8d54c 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -57,7 +57,7 @@ contract DelegatableResolver is * @return node The node of the name passed as an argument * @return authorized The boolean state of whether the operator is approved to update record of the name */ - function getAuthorizedNode( + function getAuthorisedNode( bytes memory name, uint256 offset, address operator @@ -66,7 +66,7 @@ contract DelegatableResolver is node = bytes32(0); if (len > 0) { bytes32 label = name.keccak(offset + 1, len); - (node, authorized) = getAuthorizedNode( + (node, authorized) = getAuthorisedNode( name, offset + len + 1, operator @@ -84,7 +84,7 @@ contract DelegatableResolver is address operator, bool approved ) external { - (bytes32 node, bool authorized) = getAuthorizedNode( + (bytes32 node, bool authorized) = getAuthorisedNode( name, 0, msg.sender diff --git a/contracts/resolvers/IDelegatableResolver.sol b/contracts/resolvers/IDelegatableResolver.sol index 12dde116..978c19db 100644 --- a/contracts/resolvers/IDelegatableResolver.sol +++ b/contracts/resolvers/IDelegatableResolver.sol @@ -8,7 +8,7 @@ interface IDelegatableResolver { bool approved ) external; - function getAuthorizedNode( + function getAuthorisedNode( bytes memory name, uint256 offset, address operator diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 71e02c30..58b2896b 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -80,12 +80,12 @@ contract('DelegatableResolver', function (accounts) { it('owner is ahtorised to update any names', async () => { assert.equal( - (await resolver.getAuthorizedNode(encodeName('a.b.c'), 0, owner)) + (await resolver.getAuthorisedNode(encodeName('a.b.c'), 0, owner)) .authorized, true, ) assert.equal( - (await resolver.getAuthorizedNode(encodeName('x.y.z'), 0, owner)) + (await resolver.getAuthorisedNode(encodeName('x.y.z'), 0, owner)) .authorized, true, ) @@ -94,11 +94,11 @@ contract('DelegatableResolver', function (accounts) { it('approves multiple users', async () => { await resolver.approve(encodedname, operator, true, { from: owner }) await resolver.approve(encodedname, operator2, true, { from: owner }) - const result = await resolver.getAuthorizedNode(encodedname, 0, operator) + const result = await resolver.getAuthorisedNode(encodedname, 0, operator) assert.equal(result.node, node) assert.equal(result.authorized, true) assert.equal( - (await resolver.getAuthorizedNode(encodedname, 0, operator2)) + (await resolver.getAuthorisedNode(encodedname, 0, operator2)) .authorized, true, ) @@ -124,14 +124,14 @@ contract('DelegatableResolver', function (accounts) { await resolver.approve(encodeName(subname), operator, true, { from: owner, }) - const result = await resolver.getAuthorizedNode( + const result = await resolver.getAuthorisedNode( encodeName(subname), 0, operator, ) assert.equal(result.node, namehash(subname)) assert.equal(result.authorized, true) - const result2 = await resolver.getAuthorizedNode( + const result2 = await resolver.getAuthorisedNode( encodeName(parentname), 0, operator, From 9ae228135ac290b384b54c5068c6541207968c1e Mon Sep 17 00:00:00 2001 From: Makoto Inoue <2630+makoto@users.noreply.github.com> Date: Wed, 1 Nov 2023 15:37:29 +0000 Subject: [PATCH 18/19] Delegatable resolver with factory (#289) * Use Factory * Add description and more tests * Add check to prevent people from using the resolver without owner * Apply suggestions from code review Co-authored-by: Nick Johnson Co-authored-by: Jeff Lau * Fix broken tests * Remove owner check --------- Co-authored-by: Nick Johnson Co-authored-by: Jeff Lau --- contracts/resolvers/DelegatableResolver.sol | 22 ++- .../resolvers/DelegatableResolverFactory.sol | 43 ++++++ contracts/resolvers/IDelegatableResolver.sol | 2 + package.json | 1 + test/resolvers/TestDelegatableResolver.js | 137 +++++++++++------- yarn.lock | 4 + 6 files changed, 146 insertions(+), 63 deletions(-) create mode 100644 contracts/resolvers/DelegatableResolverFactory.sol diff --git a/contracts/resolvers/DelegatableResolver.sol b/contracts/resolvers/DelegatableResolver.sol index 92b8d54c..92b0f3a7 100644 --- a/contracts/resolvers/DelegatableResolver.sol +++ b/contracts/resolvers/DelegatableResolver.sol @@ -1,5 +1,4 @@ pragma solidity >=0.8.4; -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import "./profiles/ABIResolver.sol"; import "./profiles/AddrResolver.sol"; import "./profiles/ContentHashResolver.sol"; @@ -11,13 +10,14 @@ import "./profiles/TextResolver.sol"; import "./profiles/ExtendedResolver.sol"; import "./Multicallable.sol"; import "./IDelegatableResolver.sol"; +import {Clone} from "clones-with-immutable-args/src/Clone.sol"; /** * A delegated resolver that allows the resolver owner to add an operator to update records of a node on behalf of the owner. * address. */ contract DelegatableResolver is - Ownable, + Clone, Multicallable, ABIResolver, AddrResolver, @@ -41,11 +41,6 @@ contract DelegatableResolver is error NotAuthorized(bytes32 node); - constructor(address owner) { - operators[bytes32(0)][owner] = true; - transferOwnership(owner); - } - //node => (delegate => isAuthorised) mapping(bytes32 => mapping(address => bool)) operators; @@ -72,6 +67,11 @@ contract DelegatableResolver is operator ); node = keccak256(abi.encodePacked(node, label)); + } else { + return ( + node, + authorized || operators[node][operator] || owner() == operator + ); } return (node, authorized || operators[node][operator]); } @@ -96,6 +96,14 @@ contract DelegatableResolver is emit Approval(node, operator, name, approved); } + /* + * Returns the owner address passed set by the Factory + * @return address The owner address + */ + function owner() public view returns (address) { + return _getArgAddress(0); + } + function isAuthorised(bytes32 node) internal view override returns (bool) { return msg.sender == owner() || operators[node][msg.sender]; } diff --git a/contracts/resolvers/DelegatableResolverFactory.sol b/contracts/resolvers/DelegatableResolverFactory.sol new file mode 100644 index 00000000..066a3997 --- /dev/null +++ b/contracts/resolvers/DelegatableResolverFactory.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.17; + +import "./DelegatableResolver.sol"; +import {ClonesWithImmutableArgs} from "clones-with-immutable-args/src/ClonesWithImmutableArgs.sol"; + +/** + * A resolver factory that creates a dedicated resolver for each user + */ + +contract DelegatableResolverFactory { + using ClonesWithImmutableArgs for address; + + DelegatableResolver public implementation; + event NewDelegatableResolver(address resolver, address owner); + + constructor(DelegatableResolver _implementation) { + implementation = _implementation; + } + + /* + * Create the unique address unique to the owner + * @param address The address of the resolver owner + * @return address The address of the newly created Resolver + */ + function create( + address owner + ) external returns (DelegatableResolver clone) { + bytes memory data = abi.encodePacked(owner); + clone = DelegatableResolver(address(implementation).clone2(data)); + emit NewDelegatableResolver(address(clone), owner); + } + + /* + * Returns the unique address unique to the owner + * @param address The address of the resolver owner + * @return address The address of the newly created Resolver + */ + function predictAddress(address owner) external returns (address clone) { + bytes memory data = abi.encodePacked(owner); + clone = address(implementation).predictAddress(data); + } +} diff --git a/contracts/resolvers/IDelegatableResolver.sol b/contracts/resolvers/IDelegatableResolver.sol index 978c19db..f01b0c35 100644 --- a/contracts/resolvers/IDelegatableResolver.sol +++ b/contracts/resolvers/IDelegatableResolver.sol @@ -13,4 +13,6 @@ interface IDelegatableResolver { uint256 offset, address operator ) external returns (bytes32 node, bool authorized); + + function owner() external view returns (address); } diff --git a/package.json b/package.json index 925fb889..e2e9f2e1 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "@ensdomains/buffer": "^0.1.1", "@ensdomains/solsha1": "0.0.3", "@openzeppelin/contracts": "^4.1.0", + "clones-with-immutable-args": "Arachnid/clones-with-immutable-args#feature/create2", "dns-packet": "^5.3.0" }, "directories": { diff --git a/test/resolvers/TestDelegatableResolver.js b/test/resolvers/TestDelegatableResolver.js index 58b2896b..a8a86d28 100644 --- a/test/resolvers/TestDelegatableResolver.js +++ b/test/resolvers/TestDelegatableResolver.js @@ -1,3 +1,6 @@ +const DelegatableResolverFactory = artifacts.require( + 'DelegatableResolverFactory.sol', +) const DelegatableResolver = artifacts.require('DelegatableResolver.sol') const { encodeName, namehash } = require('../test-utils/ens') const { exceptions } = require('../test-utils') @@ -6,22 +9,37 @@ const { expect } = require('chai') contract('DelegatableResolver', function (accounts) { let node let encodedname - let resolver + let resolver, operatorResolver let signers let deployer - let owner - let operator - let operator2 + let owner, ownerSigner + let operator, operatorSigner + let operator2, operator2Signer + let impl beforeEach(async () => { signers = await ethers.getSigners() - deployer = await signers[0].getAddress() - owner = await signers[1].getAddress() - operator = await signers[2].getAddress() - operator2 = await signers[3].getAddress() + deployer = await signers[0] + ownerSigner = await signers[1] + owner = await ownerSigner.getAddress() + operatorSigner = await signers[2] + operator = await operatorSigner.getAddress() + operator2Signer = await signers[3] + operator2 = await operator2Signer.getAddress() node = namehash('eth') encodedname = encodeName('eth') - resolver = await DelegatableResolver.new(owner) + impl = await DelegatableResolver.new() + factory = await DelegatableResolverFactory.new(impl.address) + const tx = await factory.create(owner) + const result = tx.logs[0].args.resolver + resolver = await (await ethers.getContractFactory('DelegatableResolver')) + .attach(result) + .connect(ownerSigner) + operatorResolver = await ( + await ethers.getContractFactory('DelegatableResolver') + ) + .attach(result) + .connect(operatorSigner) }) describe('supportsInterface function', async () => { @@ -37,7 +55,7 @@ contract('DelegatableResolver', function (accounts) { assert.equal(await resolver.supportsInterface('0x5c98042b'), true) // IDNSZoneResolver assert.equal(await resolver.supportsInterface('0x01ffc9a7'), true) // IInterfaceResolver assert.equal(await resolver.supportsInterface('0x4fbf0433'), true) // IMulticallable - assert.equal(await resolver.supportsInterface('0xdd48591c'), true) // IDelegatable + assert.equal(await resolver.supportsInterface('0x8295fc20'), true) // IDelegatable }) it('does not support a random interface', async () => { @@ -45,30 +63,45 @@ contract('DelegatableResolver', function (accounts) { }) }) + describe('factory', async () => { + it('predicts address', async () => { + const tx = await factory.create(operator) + const result = tx.logs[0].args.resolver + assert.equal(await factory.predictAddress.call(operator), result) + }) + + it('emits an event', async () => { + const tx = await factory.create(operator) + const log = tx.logs[0] + assert.equal(log.args.owner, operator) + }) + + it('does not allow duplicate contracts', async () => { + await expect(factory.create(owner)).to.be.revertedWith('CreateFail') + }) + }) + describe('addr', async () => { it('permits setting address by owner', async () => { - await resolver.methods['setAddr(bytes32,address)'](node, operator, { - from: owner, - }) - assert.equal(await resolver.methods['addr(bytes32)'](node), operator) + await resolver.functions['setAddr(bytes32,address)'](node, operator) + assert.equal(await resolver.functions['addr(bytes32)'](node), operator) }) it('forbids setting new address by non-owners', async () => { await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, operator, { - from: operator, - }), + operatorResolver.functions['setAddr(bytes32,address)'](node, operator), ) }) it('forbids approving wrong node', async () => { encodedname = encodeName('a.b.c.eth') const wrongnode = namehash('d.b.c.eth') - await resolver.approve(encodedname, operator, true, { from: owner }) + await resolver.approve(encodedname, operator, true) await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](wrongnode, operator, { - from: operator, - }), + operatorResolver.functions['setAddr(bytes32,address)']( + wrongnode, + operator, + ), ) }) }) @@ -92,8 +125,8 @@ contract('DelegatableResolver', function (accounts) { }) it('approves multiple users', async () => { - await resolver.approve(encodedname, operator, true, { from: owner }) - await resolver.approve(encodedname, operator2, true, { from: owner }) + await resolver.approve(encodedname, operator, true) + await resolver.approve(encodedname, operator2, true) const result = await resolver.getAuthorisedNode(encodedname, 0, operator) assert.equal(result.node, node) assert.equal(result.authorized, true) @@ -106,24 +139,17 @@ contract('DelegatableResolver', function (accounts) { it('approves subnames', async () => { const subname = 'a.b.c.eth' - await resolver.approve(encodeName(subname), operator, true, { - from: owner, - }) - await resolver.methods['setAddr(bytes32,address)']( + await resolver.approve(encodeName(subname), operator, true) + await operatorResolver.functions['setAddr(bytes32,address)']( namehash(subname), operator, - { - from: operator, - }, ) }) it('only approves the subname and not its parent', async () => { const subname = '1234.123' const parentname = 'b.c.eth' - await resolver.approve(encodeName(subname), operator, true, { - from: owner, - }) + await resolver.approve(encodeName(subname), operator, true) const result = await resolver.getAuthorisedNode( encodeName(subname), 0, @@ -141,42 +167,41 @@ contract('DelegatableResolver', function (accounts) { }) it('approves users to make changes', async () => { - await resolver.approve(encodedname, operator, true, { from: owner }) - await resolver.methods['setAddr(bytes32,address)'](node, operator, { - from: operator, - }) - assert.equal(await resolver.addr(node), operator) + await resolver.approve(encodedname, operator, true) + await operatorResolver.functions['setAddr(bytes32,address)']( + node, + operator, + ) + console.log('resolver.functions', resolver.functions['addr(bytes32)']) + assert.equal(await resolver.functions['addr(bytes32)'](node), operator) }) it('approves to be revoked', async () => { - await resolver.approve(encodedname, operator, true, { from: owner }) - resolver.methods['setAddr(bytes32,address)'](node, operator2, { - from: operator, - }), - await resolver.approve(encodedname, operator, false, { from: owner }) + await resolver.approve(encodedname, operator, true) + operatorResolver.functions['setAddr(bytes32,address)'](node, operator2) + await resolver.approve(encodedname, operator, false) await exceptions.expectFailure( - resolver.methods['setAddr(bytes32,address)'](node, operator2, { - from: operator, - }), + operatorResolver.functions['setAddr(bytes32,address)'](node, operator2), ) }) it('does not allow non owner to approve', async () => { await expect( - resolver.approve(encodedname, operator, true, { from: operator }), + operatorResolver.approve(encodedname, operator, true), ).to.be.revertedWith('NotAuthorized') }) it('emits an Approval log', async () => { - var tx = await resolver.approve(encodedname, operator, true, { - from: owner, - }) - assert.equal(tx.logs.length, 1) - assert.equal(tx.logs[0].event, 'Approval') - assert.equal(tx.logs[0].args.node, node) - assert.equal(tx.logs[0].args.operator, operator) - assert.equal(tx.logs[0].args.name, encodedname) - assert.equal(tx.logs[0].args.approved, true) + var tx = await ( + await resolver.approve(encodedname, operator, true) + ).wait() + const event = tx.events[0] + const args = event.args + assert.equal(event.event, 'Approval') + assert.equal(args.node, node) + assert.equal(args.operator, operator) + assert.equal(args.name, encodedname) + assert.equal(args.approved, true) }) }) }) diff --git a/yarn.lock b/yarn.lock index 91ac4da1..166e4cca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3050,6 +3050,10 @@ clone@2.1.2, clone@^2.0.0: resolved "https://registry.yarnpkg.com/clone/-/clone-2.1.2.tgz#1b7f4b9f591f1e8f83670401600345a02887435f" integrity sha512-3Pe/CF1Nn94hyhIYpjtiLhdCoEoz0DqQ+988E9gmeEdQZlojxnOb74wctFyuwWQHzqyf9X7C7MG8juUpqBJT8w== +clones-with-immutable-args@Arachnid/clones-with-immutable-args#feature/create2: + version "1.0.0" + resolved "https://codeload.github.com/Arachnid/clones-with-immutable-args/tar.gz/802248d00be50ae2c8a860266aeca9b13fe317c8" + code-point-at@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/code-point-at/-/code-point-at-1.1.0.tgz#0d070b4d043a5bea33a2f1a40e2edb3d9a4ccf77" From e97d21f5ea4704fc46d09992ac7b4a2f33d8b530 Mon Sep 17 00:00:00 2001 From: Chomtana Date: Thu, 4 Apr 2024 16:06:35 +0700 Subject: [PATCH 19/19] Use addressOfClone2 instead of predictAddress --- contracts/resolvers/DelegatableResolverFactory.sol | 2 +- package.json | 2 +- yarn.lock | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/resolvers/DelegatableResolverFactory.sol b/contracts/resolvers/DelegatableResolverFactory.sol index 066a3997..a134691a 100644 --- a/contracts/resolvers/DelegatableResolverFactory.sol +++ b/contracts/resolvers/DelegatableResolverFactory.sol @@ -38,6 +38,6 @@ contract DelegatableResolverFactory { */ function predictAddress(address owner) external returns (address clone) { bytes memory data = abi.encodePacked(owner); - clone = address(implementation).predictAddress(data); + clone = address(implementation).addressOfClone2(data); } } diff --git a/package.json b/package.json index e2e9f2e1..50abc811 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "@ensdomains/buffer": "^0.1.1", "@ensdomains/solsha1": "0.0.3", "@openzeppelin/contracts": "^4.1.0", - "clones-with-immutable-args": "Arachnid/clones-with-immutable-args#feature/create2", + "clones-with-immutable-args": "wighawag/clones-with-immutable-args", "dns-packet": "^5.3.0" }, "directories": { diff --git a/yarn.lock b/yarn.lock index 166e4cca..1b816c1a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3050,9 +3050,9 @@ clone@2.1.2, clone@^2.0.0: resolved "https://registry.yarnpkg.com/clone/-/clone-2.1.2.tgz#1b7f4b9f591f1e8f83670401600345a02887435f" integrity sha512-3Pe/CF1Nn94hyhIYpjtiLhdCoEoz0DqQ+988E9gmeEdQZlojxnOb74wctFyuwWQHzqyf9X7C7MG8juUpqBJT8w== -clones-with-immutable-args@Arachnid/clones-with-immutable-args#feature/create2: - version "1.0.0" - resolved "https://codeload.github.com/Arachnid/clones-with-immutable-args/tar.gz/802248d00be50ae2c8a860266aeca9b13fe317c8" +clones-with-immutable-args@wighawag/clones-with-immutable-args: + version "1.1.2" + resolved "https://codeload.github.com/wighawag/clones-with-immutable-args/tar.gz/2df4bff4eef8c061b9a92a7edbbfe12dbf6bcdb0" code-point-at@^1.0.0: version "1.1.0"