feat: implements setRecord function and unit tests by 0xItadaki · Pull Request #6 · defi-wonderland/L2Resolver

0xItadaki · 2025-08-19T16:12:48Z

🤖 Linear

linear · 2025-08-19T16:12:51Z

function setRecord(bytes32 _node, string calldata _key, bytes calldata _value) external /* authorized */
- Sets raw bytes for a given node and key in the records mapping.
- Authorization logic is implemented directly within this function (see below).
- Emits: RecordSet(bytes32 indexed _node, string indexed _key, bytes _value)

Authorization Model

Write operations via setRecord(bytes32 node, string calldata key, bytes calldata value) must be permissioned. The authorization logic is implemented directly within the L2Resolver contract. It should generally follow the ENS ownership model:

To set a record for a specific node (e.g., namehash("optimism.l2.eth")), msg.sender must be the beneficial owner of that node (potentially resolved via ENS.owner(node) and checking against INameWrapper.ownerOf(node) if applicable) or an operator approved by the beneficial owner.
For non-registered 3LDs (wildcard scenario where node corresponds to something like namehash("unregistered.l2.eth")), msg.sender must be the beneficial owner of the parent domain (e.g., l2.eth) or their approved operator.
To set a reverse EIP-7930 chain identifier mapping (i.e., when node == REVERSE_LOOKUP_NODE), msg.sender must be a designated administrative entity (e.g., the owner of l2.eth or a specific authorized address). This control for REVERSE_LOOKUP_NODE writes is crucial for the integrity of reverse lookups.

The L2Resolver will require a reference to the ENS Registry (and potentially INameWrapper) to implement this authorization logic.

0xItadaki · 2025-08-19T16:14:18Z

+    ├── when the caller is an ENSregistry approved operator of the parent
+    │   ├── it emits the RecordSet event
+    │   └── it sets the record
+    ├── when the caller is a NameWrapper approved operator of the parent


This branch is probably not realistic for our use case, but it is still a possible option

0xItadaki · 2025-08-19T16:18:47Z

+    assertEq(_l2Resolver.PARENT_NODE(), _parentNodeNameHash);
+  }
+
+  modifier whenTheNodeIsTheReverseLookupNode() {


this one was generated by bulloak and I don't really need it. Not sure if it is better to leave it or run it with the option that doesn't generate modifiers. What do you think?

Lets keep it as is

0xAustrian

Very good job. I want to double check the tests again, but looks good.

0xAustrian · 2025-08-19T17:42:31Z

I think we can import this repo as a package and use ENS and INameWrapper, wdyt?

is it worth it when we just need a few lines of code for the interfaces? wouldn't it make the contract bigger by importing all those additional functions?

You are right, lets do it your way

0xAustrian · 2025-08-19T17:46:33Z

+   */
+  function _getOwner(bytes32 _node) private view returns (address _owner) {
+    _owner = ENS_REGISTRY.owner(_node);
+    if (_owner == address(0)) return address(0);


You can safely remove this line, the behaviour will be the same.

I think it would revert if the NAME_WRAPPER is not set since it would try to do NAME_WRAPPER.ownerOf(uint256(_node));, right?

Maybe we should validate that none of those addresses are zero in the constructor?

I agree, do you mind adding that to another linear task?

done. I removed the validation in this PR

0xAustrian · 2025-08-19T18:05:38Z

+  function test_SetRecordWhenTheCallerIsTheReverseAdmin() external {
+    // it emits the RecordSet event
+    vm.prank(_reverseAdmin);
+    vm.expectEmit(true, true, false, true);


vm.expectEmit(); also works

but that wouldn't be testing if the arguments are indexed, right? Or it doesn't matter since I am emitting that same event that I expect?

I think vm.expectEmit() checks all the topics and data.

0xAustrian

Everything looks good, will approve once vm.expectEmit is fixed

0xItadaki added 5 commits August 19, 2025 16:39

feat: adds setRecord and RecordSet to IL2Resolver

c104d9b

feat: implements IENS and INameWrapper interfaces

dffc626

feat: implements setRecord function

1913d2b

fix: bug with approved operator logic

26003e7

test: implements setRecord unit tests

f1fc41d

0xItadaki commented Aug 19, 2025

View reviewed changes

0xItadaki self-assigned this Aug 19, 2025

0xItadaki requested a review from 0xAustrian August 19, 2025 16:14

refactor: changes internal functions to private

b43c43d

0xItadaki commented Aug 19, 2025

View reviewed changes

Comment thread src/interfaces/IENS.sol

0xItadaki commented Aug 19, 2025

View reviewed changes

Comment thread src/interfaces/INameWrapper.sol

0xItadaki commented Aug 19, 2025

View reviewed changes

0xAustrian reviewed Aug 19, 2025

View reviewed changes

0xItadaki added 3 commits August 20, 2025 10:23

fix: removes unnecessary return in _isApprovedOperatorOrOwner

0e5f17f

style: implements _mockAndExpect in unit tests

e7efabc

docs: adds comments to different branches in setRecord

12214f5

0xAustrian reviewed Aug 20, 2025

View reviewed changes

0xItadaki added 2 commits August 20, 2025 14:00

test: uses vm.expectEmit()

c2915f9

fix: removes unnecessary address(0) check

d8224cd

0xItadaki requested a review from 0xAustrian August 20, 2025 12:03

0xAustrian approved these changes Aug 20, 2025

View reviewed changes

0xItadaki merged commit 3ac3ffb into dev Aug 20, 2025
4 checks passed

0xItadaki deleted the fet/set-record-and-tests branch August 20, 2025 12:07

Uh oh!

Conversation

0xItadaki commented Aug 19, 2025

🤖 Linear

Uh oh!

linear Bot commented Aug 19, 2025

Authorization Model

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xAustrian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xItadaki Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xAustrian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

0xItadaki Aug 20, 2025 •

edited

Loading