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

Missing events in multiple functions #65

Open
code423n4 opened this issue Jun 16, 2021 · 1 comment
Open

Missing events in multiple functions #65

code423n4 opened this issue Jun 16, 2021 · 1 comment

Comments

@code423n4
Copy link
Contributor

Handle

a_delamo

Vulnerability details

Impact

Multiple contracts contain methods that change important owners/addresses/variables without publishing events. Publishing events will help to keep track of all important changes on the protocol

Example:

    function setNftHubAddress(IRCNftHubL2 _newAddress, uint256 _newNftMintCount)
        external
        onlyOwner
    {
        require(address(_newAddress) != address(0));
        nfthub = _newAddress;
        totalNftMintCount = _newNftMintCount;
        //FIXME: Publish Event
    }

    /// @notice set the address of the orderbook contract
    /// @param _newAddress the address to set
    function setOrderbookAddress(IRCOrderbook _newAddress) external onlyOwner {
        require(address(_newAddress) != address(0));
        orderbook = _newAddress;
    }
/// @notice minimum rental duration (1 day divisor: i.e. 24 = 1 hour, 48 = 30 mins)
    /// @param _newDivisor the divisor to set
    function setMinRental(uint256 _newDivisor) public override onlyOwner {
        minRentalDayDivisor = _newDivisor;
        //FIXME: Publish event when changed
    }

    /// @notice set max deposit balance, to minimise funds at risk
    /// @param _newBalanceLimit the max balance to set in wei
    function setMaxContractBalance(uint256 _newBalanceLimit)
        public
        override
        onlyOwner
    {
        maxContractBalance = _newBalanceLimit;
        //FIXME: Publish event when changed
    }
@code423n4 code423n4 added 0 (Non-critical) bug Something isn't working labels Jun 16, 2021
code423n4 added a commit that referenced this issue Jun 16, 2021
@Splidge
Copy link
Collaborator

Splidge commented Jun 17, 2021

I think that it's very unlikely for any of these to be changed, I'll acknowledge the issue and if time allows implement the fixes but it's a low priority right now.
I'd add that we did have to strip many events (and modifiers) from some contracts because of the contract size limit.
Also, nftHubAddress is actually sent from each market when it is created, I think this would need to be the way to do it for minRental also because this way you know the value each market has (instead of having to check timestamps/block numbers or even transaction ordering within a block).

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

No branches or pull requests

3 participants