Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[audit2] #11 Add reverse node claim to L2 Resolver #81

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

stevieraykatz
Copy link
Collaborator

From spearbit:

Missing explicitly claiming the reverse resolution record for L2Resolver
Status: New
Severity: Informational
AkshaySrivastav
AkshaySrivastav

created on Jul 22, 2024 at 05:52
Description
The current implementation of L2Resolver contract does not claim the reverse resolution record for itself explicitly.

Ideally just like RegistrarController, EARegistrarController & ENS's PublicResolver reverse record should be explicitly claimed for L2Resolver during its construction.

Recommendation
Add the IReverseRegistrar.claim call in L2Resolver::constructor:

    constructor(ENS ens_, address registrarController_, address reverseRegistrar_, address owner_) {
        ens = ens_;
        registrarController = registrarController_;
        reverseRegistrar = reverseRegistrar_;
        _initializeOwner(owner_);
+       IReverseRegistrar(reverseRegistrar).claim(owner_);
    }

@stevieraykatz stevieraykatz merged commit 442ef11 into Spearbit-second-audit Jul 23, 2024
1 of 2 checks passed
@stevieraykatz stevieraykatz deleted the audit2#11 branch July 23, 2024 23:25
stevieraykatz added a commit that referenced this pull request Jul 24, 2024
* [audit2] #2 Typos and Documentation (#72)

* Fix typo in PaymentReceiverUpdated

* Fix typo in L2Resolver commment

* Fix typos in L2Resolver and ReverseRegistrar natspec

* Fix comment in IPriceOracle

* Fix natspec in controllers

* Remove unused event (#71)

* [audit2] #4 Cleanup GRACE_PERIOD handling for launch auction (#73)

* Cleanup GRACE_PERIOD handling for launch auction

* Add GRACE_PERIOD to EARegistrarController

* [audit2] #5 Explicitly declare state visibility (#75)

* Explicitly declare state visibility

* Style guide

* Change resolver for reverse node when changing default resolver (#76)

* Remove reverse claiming from EARegistrarController (#78)

* Add IExtendedResolver supported signal for L2Resolver (#77)

* Add reverse node claim to L2 Resolver (#81)

* [audit2] #6 Remove addr.reverse setting from ReverseRegistrar (#74)

* Remove addr.reverse setting from ReverseRegistrar

* Address PR suggestion
@akshaysrivastav
Copy link
Collaborator

It'd have been nice if a test case validated that owner record of L2Resolver returns the admin's address
@stevieraykatz

@stevieraykatz
Copy link
Collaborator Author

@akshaysrivastav I can add that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants