-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix resolve
method on L1Resolver
#84
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,7 +164,7 @@ | |
function resolve(bytes calldata name, bytes calldata data) external view override returns (bytes memory) { | ||
// Check for base.eth resolution, and resolve return early if so | ||
if (keccak256(BASE_ETH_NAME) == keccak256(name)) { | ||
return IExtendedResolver(rootResolver).resolve(name, data); | ||
return _resolve(name, data); | ||
} | ||
|
||
bytes memory callData = abi.encodeWithSelector(IExtendedResolver.resolve.selector, name, data); | ||
|
@@ -207,6 +207,28 @@ | |
|| ERC165(rootResolver).supportsInterface(interfaceID); | ||
} | ||
|
||
/// @notice Internal method for completing `resolve` intended for the `rootResolver`. | ||
/// | ||
/// @dev The `PublicResolver` located at `rootResolver` does not implement the `resolve(bytes,bytes)` method. | ||
/// This method completes the resolution request by staticcalling `rootResolver` with the resolve request. | ||
/// Implementation matches the ENS `ExtendedResolver:resolve(bytes,bytes)` method with the exception that it `staticcall`s the | ||
/// the `rootResolver` instead of `address(this)`. | ||
/// | ||
/// @param data The ABI encoded data for the underlying resolution function (Eg, addr(bytes32), text(bytes32,string), etc). | ||
/// | ||
/// @return The return data, ABI encoded identically to the underlying function. | ||
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)) | ||
} | ||
} | ||
} | ||
Comment on lines
+220
to
+230
Check warning Code scanning / Slither Low-level calls Warning
Low level call in L1Resolver._resolve(bytes,bytes):
- (success,result) = rootResolver.staticcall(data) |
||
|
||
/// @notice Generic handler for requests to the `rootResolver` | ||
/// | ||
/// @dev Inspired by the passthrough logic of proxy contracts, but leveraging `call` instead of `delegatecall` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
//SPDX-License-Identifier: MIT | ||
pragma solidity ^0.8.23; | ||
|
||
import {Test, console} from "forge-std/Test.sol"; | ||
import {L1Resolver} from "src/L1/L1Resolver.sol"; | ||
import {ENS} from "ens-contracts/registry/ENS.sol"; | ||
import {IExtendedResolver} from "ens-contracts/resolvers/profiles/IExtendedResolver.sol"; | ||
import "src/util/Constants.sol"; | ||
import "ens-contracts/resolvers/profiles/IAddrResolver.sol"; | ||
|
||
contract L1ResolverMainnet is Test { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
address signer = 0x14536667Cd30e52C0b458BaACcB9faDA7046E056; | ||
ENS public ens = ENS(0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e); | ||
address rootResolver = 0x4976fb03C32e5B8cfe2b6cCB31c09Ba78EBaBa41; | ||
address addrRoot; | ||
address l1resolver; | ||
|
||
string constant URL = "TEST_URL"; | ||
|
||
function setUp() public { | ||
uint256 forkId = vm.createFork("https://eth.llamarpc.com"); | ||
vm.selectFork(forkId); | ||
|
||
address[] memory signers = new address[](1); | ||
signers[0] = signer; | ||
l1resolver = address(new L1Resolver(URL, signers, signer, rootResolver)); | ||
|
||
vm.startPrank(signer); | ||
ens.setResolver(BASE_ETH_NODE, l1resolver); | ||
assertEq(ens.resolver(BASE_ETH_NODE), l1resolver); | ||
addrRoot = IAddrResolver(rootResolver).addr(BASE_ETH_NODE); | ||
} | ||
|
||
function test_resolves_addr() public view { | ||
address resolvedAddress = IAddrResolver(l1resolver).addr(BASE_ETH_NODE); | ||
assertEq(resolvedAddress, addrRoot); | ||
} | ||
|
||
function test_resolves_resolve() public view { | ||
bytes memory data = abi.encodeWithSelector(IAddrResolver.addr.selector, BASE_ETH_NODE); | ||
bytes memory response = IExtendedResolver(l1resolver).resolve(BASE_ETH_NAME, data); | ||
(address resolvedAddress) = abi.decode(response, (address)); | ||
assertEq(resolvedAddress, addrRoot); | ||
} | ||
} |
Check warning
Code scanning / Slither
Assembly usage Warning