Liquid Staking

Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

contracts/governance/StakedotlinkCouncil.sol

Your Solidity contract StakedotlinkCouncil looks well-structured, but there are a few potential vulnerabilities and areas for improvement that you should consider:

1. Reentrancy Attacks

Although your contract doesn't seem to have external calls that could lead to reentrancy, it’s always good to be cautious when state changes occur before external calls. Consider using the Checks-Effects-Interactions pattern, ensuring you perform all checks and state updates before calling external functions.

2. Token Ownership Management

The tokenOwned mapping currently allows an address to own only one token at a time. This could lead to issues if someone wants to transfer a token without minting a new one first. You might want to consider a more typical ERC721 implementation where each address can own multiple tokens. This could involve changing tokenOwned to track the number of tokens owned and utilizing an array of token IDs for each owner.

3. Inefficient Token Burn Implementation

The current burn function iterates through the entire tokens array to remove a token ID, which could lead to inefficient gas costs, especially as the number of tokens increases. You can optimize this by maintaining a mapping of token IDs to their index in the tokens array, allowing for O(1) removals. Additionally, consider implementing a more standard way of managing the tokens, similar to ERC721, to simplify logic and increase compatibility.

4. Lack of Access Control for Transfer Functions

The transferFrom function is restricted to the contract owner, which may not be the intended behavior for a council token. Typically, transfer functions should allow token holders to transfer their tokens without requiring the owner to facilitate every transfer. Consider removing onlyOwner from this function if you intend for users to manage their tokens independently.

5. Gas Limit and Transaction Failures

The tokens array management can lead to out-of-gas errors as the number of tokens increases. If a large number of tokens are held, the loop in the burn function could cause gas limits to be exceeded. Using an index mapping can alleviate this issue.

6. Potential Misuse of the Mint Function

The mint function allows the owner to mint tokens without checking if the token ID is unique (beyond checking if it is currently owned). Implement a mapping to track minted token IDs to avoid minting the same token ID multiple times.

7. Token URI Management

There is a risk of someone calling setTokenURI to change the URI of a token to a malicious link if not handled properly. Ensure that the input URI is sanitized if it could be exposed to external users.

8. Events Consistency

Make sure to emit events in all relevant functions. For example, when a token is burned, you already emit a Burn event, but ensure that all state changes that modify ownership or important attributes emit corresponding events.

9. Constructor Visibility

Although the default visibility of the constructor is public, explicitly marking it as public could improve code clarity.

10. Owner Access Control

While you use onlyOwner for critical functions, consider potential scenarios where the owner might be compromised. You might want to implement a way to transfer ownership securely or implement a multi-signature wallet for more critical operations.

Conclusion

Overall, while the contract has a solid foundation, addressing these points could enhance its security, performance, and adherence to standard practices in the ERC721 implementation. It’s advisable to conduct thorough testing and consider an audit before deploying any contract on the mainnet.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.