Fix resolve method on L1Resolver
#84
Conversation
🟡 Heimdall Review Status
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import "src/util/Constants.sol"; | ||
| import "ens-contracts/resolvers/profiles/IAddrResolver.sol"; | ||
|
|
||
| contract L1ResolverMainnet is Test { |
There was a problem hiding this comment.
Good to have. Are there other forking tests that we could add for similar type? Maybe all the base.eth sub names behaving as expected?
There was a problem hiding this comment.
Resolution requests for L1 base.eth subdomains (i.e. mint.base.eth) shouldn't ever hit the base.eth resolver since they have resolvers set on the Registry. Generally, I think that expanding the fork test infra is a good idea. Happy to work on a separate PR with expanded test files.
cb-heimdall
dismissed
cb-elileers’s stale review
Approved review 2205674558 from cb-elileers is now dismissed due to new commit. Re-request for approval.
stevieraykatz
changed the title
resolver method on L1Resolverresolve method on L1Resolver
| function _resolve(bytes memory, bytes memory data) internal view returns (bytes memory) { | ||
| (bool success, bytes memory result) = rootResolver.staticcall(data); | ||
| if (success) { | ||
| return result; | ||
| } else { | ||
| // Revert with the reason provided by the call | ||
| assembly { | ||
| revert(add(result, 0x20), mload(result)) | ||
| } | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Assembly usage Warning
| function _resolve(bytes memory, bytes memory data) internal view returns (bytes memory) { | ||
| (bool success, bytes memory result) = rootResolver.staticcall(data); | ||
| if (success) { | ||
| return result; | ||
| } else { | ||
| // Revert with the reason provided by the call | ||
| assembly { | ||
| revert(add(result, 0x20), mload(result)) | ||
| } | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Low-level calls Warning
The ENS team correctly identified that the deployed L1 Resolver at 0x480F8F2FfE823Dc70F499Cc2542C42a3a6aD3f20 expects that the
rootResolverimplements theresolvemethod. However, the PublicResolver at 0x4976fb03C32e5B8cfe2b6cCB31c09Ba78EBaBa41 (ourrootResolver) does not implement theresolvemethod. Thus, calling:will always revert.
This fix implements a new internal method
_resolve(bytes,bytes)to correctly handle resolve requests inteded for thebase.ethname. The implementation is identical to the ENS ExtendedResolver:resolve(bytes,bytes) with the exception thataddress(this)has been replaced withrootResolver.A new forking test file has been added to validate that this resolves the issue.