remove unused interface "commitments" from IETHRegistrarController

[mdtanrikulu]

No description provided.

[lcfr-eth]

Hi - I don't believe this should be removed... How is it determined that it is "unused" ? Interfaces should cover all public variables/functions IMO. We use this at vision in our UI to verify commitments age etc.

I believe I did the original PR to add this as I found I needed to manually create an interface that included for my own usage instead of using the official ENS interface..

for example running the forge command 'cast interface' on the current ETHRegistrarController includes this:
cast interface 0x253553366Da8546fC250F225fe3d25d0C782303b

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.4;

interface ETHRegistrarController {
    struct Price {
        uint256 base;
        uint256 premium;
    }

    error CommitmentTooNew(bytes32 commitment);
    error CommitmentTooOld(bytes32 commitment);
    error DurationTooShort(uint256 duration);
    error InsufficientValue();
    error MaxCommitmentAgeTooHigh();
    error MaxCommitmentAgeTooLow();
    error NameNotAvailable(string name);
    error ResolverRequiredWhenDataSupplied();
    error UnexpiredCommitmentExists(bytes32 commitment);

    event NameRegistered(
        string name, bytes32 indexed label, address indexed owner, uint256 baseCost, uint256 premium, uint256 expires
    );
    event NameRenewed(string name, bytes32 indexed label, uint256 cost, uint256 expires);
    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

    function MIN_REGISTRATION_DURATION() external view returns (uint256);
    function available(string memory name) external view returns (bool);
    function commit(bytes32 commitment) external;
    function commitments(bytes32) external view returns (uint256);
    function makeCommitment(
        string memory name,
        address owner,
        uint256 duration,
        bytes32 secret,
        address resolver,
        bytes[] memory data,
        bool reverseRecord,
        uint16 ownerControlledFuses
    ) external pure returns (bytes32);
    function maxCommitmentAge() external view returns (uint256);
    function minCommitmentAge() external view returns (uint256);
    function nameWrapper() external view returns (address);
    function owner() external view returns (address);
    function prices() external view returns (address);
    function recoverFunds(address _token, address _to, uint256 _amount) external;
    function register(
        string memory name,
        address owner,
        uint256 duration,
        bytes32 secret,
        address resolver,
        bytes[] memory data,
        bool reverseRecord,
        uint16 ownerControlledFuses
    ) external payable;
    function renew(string memory name, uint256 duration) external payable;
    function renounceOwnership() external;
    function rentPrice(string memory name, uint256 duration) external view returns (Price memory price);
    function reverseRegistrar() external view returns (address);
    function supportsInterface(bytes4 interfaceID) external pure returns (bool);
    function transferOwnership(address newOwner) external;
    function valid(string memory name) external pure returns (bool);
    function withdraw() external;
}

[mdtanrikulu]

Hey @lcfr-eth 👋 unfortunately adding the commitments interface manually, creates an interface mismatch in here and breaks the CI.

Just to clarify, when I mentioned "unused", I was actually pointing out that Solidity already automatically generates getters for public mappings. From what I've seen, our manual addition isn't really called anywhere in our current code. Although I see your point about why having this getter in the interface makes sense.

I'm still scratching my head over the exact cause of this mismatch. For now, my gut says we should hold off on adding it to the interface until we figure out what's going wrong. If you're up for it, I'd appreciate a fresh set of eyes on this.

You can observe the issue by running;
yarn test test/ethregistrar/TestBulkRenewal.js before and after function commitments(bytes32) external view returns (uint256); included into IETHRegistrarController.

[RafaelPvieira]

****

[RafaelPvieira]

0x34e833fF697Ad8dE8c79911D5fCa6f5bF8eda0C7

[RafaelPvieira]

Ok