You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is the top-ranked automated findings report, from vuln-detector bot. All findings in this report will be considered known issues for the purposes of your C4 audit.
The use of a logical AND in place of double if is slightly less gas efficient in instances where there isn't a corresponding else statement for the given if statement
[M-02] Transferred ERC721 can be stuck permanently
If the recipient is not a EOA, safeTransferFrom ensures that the contract is able to safely receive the token. In the worst-case scenario, it may result in tokens frozen permanently, as the following code uses transferFrom, which doesn't check if the recipient can handle the NFT.
[L-02] For loops in public or external functions should be avoided due to high gas costs and possible DOS
In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly
There are 5 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
209: function createPiece(
210: ArtPieceMetadata calldatametadata,
211: CreatorBps[] calldatacreatorArray212: ) publicreturns (uint256) {
213: uint256 creatorArrayLength =validateCreatorsArray(creatorArray);
214:
215: // Validate the media type and associated data216: validateMediaType(metadata);
217:
218: uint256 pieceId = _currentPieceId++;
219:
220: /// @dev Insert the new piece into the max heap221: maxHeap.insert(pieceId, 0);
222:
223: ArtPiece storage newPiece = pieces[pieceId];
224:
225: newPiece.pieceId = pieceId;
226: newPiece.totalVotesSupply =_calculateVoteWeight(
227: erc20VotingToken.totalSupply(),
228: erc721VotingToken.totalSupply()
229: );
230: newPiece.totalERC20Supply = erc20VotingToken.totalSupply();
231: newPiece.metadata = metadata;
232: newPiece.sponsor =msg.sender;
233: newPiece.creationBlock =block.number;
234: newPiece.quorumVotes = (quorumVotesBPS * newPiece.totalVotesSupply) /10_000;
235:
236: for (uint i; i < creatorArrayLength; i++) {
There are some missing checks in these functions, and this could lead to unexpected scenarios. Consider always adding a sanity check for state variables.
There are 1 instance(s) of this issue:
File: packages/revolution/src/libs/VRGDAC.sol
//@audit _targetPrice, _perTimeUnit, are not checked28: constructor(int256_targetPrice, int256_priceDecayPercent, int256_perTimeUnit) {
29: targetPrice = _targetPrice;
The use of fixed decimal values such as 1e18 or 1e8 in Solidity contracts can lead to inaccuracies, bugs, and vulnerabilities, particularly when interacting with tokens having different decimal configurations.Not all ERC20 tokens follow the standard 18 decimal places, and assumptions about decimal places can lead to miscalculations.
Always retrieve and use the decimals() function from the token contract itself when performing calculations involving token amounts.This ensures that your contract correctly handles tokens with any number of decimal places, mitigating the risk of numerical errors or under / overflows that could jeopardize contract integrity and user funds.
The initialize() functions are not protected by a modifier, which allow attackers to call this function once the contract is deployed through the proxy. Consider adding modifiers to protect this function or create a contract that both deploy the project and initialize it on the same transaction.
There are 6 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
113: function initialize(
[L-06] Lack of disableinitializers call to prevent uninitialized contracts
Multiple contracts are using the Initializable module from OpenZeppelin. For this reason and in order to prevent leaving that contract uninitialized OpenZeppelin's documentation recommends adding the _disableInitializers function in the constructor to automatically lock the contracts when they are deployed. this will protect the contract that holds the logic business from beeing initialized by an attack.
There are 6 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
113: function initialize(
Making function calls or external calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.
When there are hard forks, users often have to go through many hoops to ensure that they control ownership on every fork. Consider adding require(1 == chain.chainId), or the chain ID of whichever chain you prefer, to the functions below, or at least include the chain ID in the URI, so that there is no confusion about which chain is the owner of the NFT.
There are 1 instance(s) of this issue:
File: packages/revolution/src/VerbsToken.sol
193: function tokenURI(uint256tokenId) publicviewoverridereturns (stringmemory) {
[L-09]onlyOwner functions not accessible if owner renounces ownership
The owner is able to perform certain privileged activities, but it's possible to set the owner to address(0). This can represent a certain risk if the ownership is renounced for any other reason than by design.
Renouncing ownership will leave the contract without an owner, therefore limiting any functionality that needs authority.
There are 26 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
208: function pause() externaloverride onlyOwner {
[L-10] Governance operations should be behind a timelock
All critical and governance operations should be protected by a timelock. For example from the point of view of a user, the changing of the owner of a contract is a high risk operation that may have outcomes ranging from an attacker gaining control over the protocol, to the function no longer functioning due to a typo in the destination address. To give users plenty of warning so that they can validate any ownership changes, changes of ownership should be behind a timelock.
There are 3 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
119: function insert(uint256itemId, uint256value) public onlyAdmin {
120: heap[size] = itemId;
121: valueMapping[itemId] = value; // Update the value mapping122: positionMapping[itemId] = size; // Update the position mapping123:
124: uint256 current = size;
125: while (current !=0&& valueMapping[heap[current]] > valueMapping[heap[parent(current)]]) {
126: swap(current, parent(current));
127: current =parent(current);
128: }
129: size++;
130: }
[L-11] Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows when casting from various type int/uint values
There are 6 instance(s) of this issue:
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `totalTokensForCreators` is getting converted from `int256` to `uint256`202: _mint(creatorsAddress, uint256(totalTokensForCreators));
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `totalTokensForBuyers` is getting converted from `int256` to `uint256`224: uint256(totalTokensForBuyers),
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `totalTokensForCreators` is getting converted from `int256` to `uint256`225: uint256(totalTokensForCreators),
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `totalTokensForBuyers` is getting converted from `int256` to `uint256`229: returnuint256(totalTokensForBuyers);
Setters should have initial value check to prevent assigning wrong value to the variable. Assginment of wrong value can lead to unexpected behavior of the contract.
There are 6 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
277: function setTimeBuffer(uint256_timeBuffer) externaloverride onlyOwner {
[L-13] Consider implementing two-step procedure for updating protocol addresses
A copy-paste error or a typo may end up bricking protocol functionality, or sending tokens to an address with no known private key. Consider implementing a two-step procedure for updating protocol addresses, where the recipient is set as pending, and must 'accept' the assignment by making an affirmative call. A straight forward way of doing this would be to have the target contracts implement EIP-165, and to have the 'set' functions ensure that the recipient is of the right interface type.
[L-15] Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions
See this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
[L-16] Consider using descriptive constants when passing zero as a function argument
Passing zero as a function argument can sometimes result in a security issue (e.g. passing zero as the slippage parameter). Consider using a constant variable with a descriptive name, so it's clear that the argument is intentionally being used, and for the right reasons.
There are 1 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit parameter number 2 starting from left221: maxHeap.insert(pieceId, 0);
[L-17] Functions calling contracts/addresses with transfer hooks are missing reentrancy guards
Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open the users of this protocol up to read-only reentrancies with no way to protect against it, except by block-listing the whole protocol.
There are 2 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit function `_settleAuction()` is not protected against reentrancy361: else verbs.transferFrom(address(this), _auction.bidder, _auction.verbId);
File: packages/revolution/src/AuctionHouse.sol
//@audit function `_safeTransferETHWithFallback()` is not protected against reentrancy438: bool wethSuccess =IWETH(WETH).transfer(_to, _amount);
[L-19] prevent re-setting a state variable with the same value
Not only is wasteful in terms of gas, but this is especially problematic when an event is emitted and the old and new values set are the same, as listeners might not expect this kind of scenario.
There are 17 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
152: function settleCurrentAndCreateNewAuction() externaloverride nonReentrant whenNotPaused {
### [G-01]<a name="g-01"></a> State variable read in a loop
The state variable should be cached in a local variable rather than reading it on every iteration of the for-loop, which will replace each Gwarmaccess (100 gas) with a much cheaper stack read.
There are 2 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit `verbs` is read in this loop384: for (uint256 i =0; i < numCreators; i++) {
385: ICultureIndex.CreatorBps memory creator = verbs.getArtPieceById(_auction.verbId).creators[i];
File: packages/revolution/src/AuctionHouse.sol
//@audit `entropyRateBps` is read in this loop384: for (uint256 i =0; i < numCreators; i++) {
385: ICultureIndex.CreatorBps memory creator = verbs.getArtPieceById(_auction.verbId).creators[i];
386: vrgdaReceivers[i] = creator.creator;
387: vrgdaSplits[i] = creator.bps;
388:
389: //Calculate paymentAmount for specific creator based on BPS splits - same as multiplying by creatorDirectPayment390: uint256 paymentAmount = (creatorsShare * entropyRateBps * creator.bps) / (10_000*10_000);
[G-02] Multiple accesses of a mapping/array should use a local variable cache
The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata
We can use assembly to emit events efficiently by utilizing scratch space and the free memory pointer. This will allow us to potentially avoid memory expansion costs. Note: In order to do this optimization safely, we will need to cache and restore the free memory pointer.
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past. See source.
There are 3 instance(s) of this issue:
File: packages/revolution/src/VerbsToken.sol
//@audit avoid using `bool` type for isMinterLocked51: boolpublic isMinterLocked;
If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).
There are 2 instance(s) of this issue:
File: packages/revolution/src/ERC20TokenEmitter.sol
209: for (uint256 i =0; i < addresses.length; i++) {
[G-10] State variables should be cached in stack variables rather than re-reading them from storage
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
[G-11] Use calldata instead of memory for function arguments that do not get mutated
Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.
There are 8 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit Make `_cultureIndexParams` as a calldata115: IRevolutionBuilder.CultureIndexParams memory_cultureIndexParams
File: packages/revolution/src/VerbsToken.sol
//@audit Make `_erc721TokenParams` as a calldata135: IRevolutionBuilder.ERC721TokenParamsmemory_erc721TokenParams
File: packages/revolution/src/VerbsToken.sol
//@audit Make `newContractURIHash` as a calldata169: function setContractURIHash(stringmemorynewContractURIHash) external onlyOwner {
[G-12] With assembly, .call (bool success) transfer can be done gas-optimized
return data (bool success,) has to be stored due to EVM architecture, but in a usage like below, out and outsize values are given (0,0), this storage disappears and gas optimization is provided.
File: packages/revolution/src/AuctionHouse.sol
129: require(
130: _auctionParams.creatorRateBps >= _auctionParams.minCreatorRateBps,
131: "Creator rate must be greater than or equal to the creator rate"132: );
File: packages/revolution/src/AuctionHouse.sol
218: require(
219: _creatorRateBps >= minCreatorRateBps,
220: "Creator rate must be greater than or equal to minCreatorRateBps"221: );
File: packages/revolution/src/AuctionHouse.sol
234: require(_minCreatorRateBps <= creatorRateBps, "Min creator rate must be less than or equal to creator rate");
File: packages/revolution/src/CultureIndex.sol
398: require(
399: len == pieceIds.length&& len == deadline.length&& len == v.length&& len == r.length&& len == s.length,
400: "Array lengths must match"401: );
File: packages/revolution/src/CultureIndex.sol
523: require(totalVoteWeights[piece.pieceId] >= piece.quorumVotes, "Does not meet quorum votes to be dropped.");
[G-17] Stack variable cost less while used in emiting event
Even if the variable is going to be used only one time, caching a state variable and use its cache in an emit would help you reduce the cost by at least 9 gas
There are 6 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
// @audit `startTime` is a state variable326: emitAuctionCreated(verbId, startTime, endTime);
File: packages/revolution/src/CultureIndex.sol
// @audit `quorumVotesBPS` is a state variable141: emitQuorumVotesBPSSet(quorumVotesBPS, _cultureIndexParams.quorumVotesBPS);
File: packages/revolution/src/CultureIndex.sol
// @audit `quorumVotesBPS` is a state variable500: emitQuorumVotesBPSSet(quorumVotesBPS, newQuorumVotesBPS);
File: packages/revolution/src/ERC20TokenEmitter.sol
// @audit `entropyRateBps` is a state variable291: emitEntropyRateBpsUpdated(entropyRateBps = _entropyRateBps);
File: packages/revolution/src/ERC20TokenEmitter.sol
// @audit `creatorRateBps` is a state variable302: emitCreatorRateBpsUpdated(creatorRateBps = _creatorRateBps);
File: packages/revolution/src/ERC20TokenEmitter.sol
// @audit `creatorsAddress` is a state variable312: emitCreatorsAddressUpdated(creatorsAddress = _creatorsAddress);
Emitting an event has an overhead of 375 gas, which will be incurred on every iteration of the loop. It is cheaper to emit only once after the loop has finished.
There are 1 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit PieceCreatorAdded is emited inside this loop243: for (uint i; i < creatorArrayLength; i++) {
244: emitPieceCreatorAdded(pieceId, creatorArray[i].creator, msg.sender, creatorArray[i].bps);
[G-22]require()/revert() strings longer than 32 bytes cost extra gas
Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas
There are 21 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
129: require(
130: _auctionParams.creatorRateBps >= _auctionParams.minCreatorRateBps,
131: "Creator rate must be greater than or equal to the creator rate"132: );
File: packages/revolution/src/AuctionHouse.sol
218: require(
219: _creatorRateBps >= minCreatorRateBps,
220: "Creator rate must be greater than or equal to minCreatorRateBps"221: );
File: packages/revolution/src/AuctionHouse.sol
222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
223: creatorRateBps = _creatorRateBps;
File: packages/revolution/src/AuctionHouse.sol
234: require(_minCreatorRateBps <= creatorRateBps, "Min creator rate must be less than or equal to creator rate");
235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
236:
File: packages/revolution/src/CultureIndex.sol
182: require(creatorArrayLength <= MAX_NUM_CREATORS, "Creator array must not be > MAX_NUM_CREATORS");
183:
File: packages/revolution/src/CultureIndex.sol
523: require(totalVoteWeights[piece.pieceId] >= piece.quorumVotes, "Does not meet quorum votes to be dropped.");
524:
File: packages/revolution/src/ERC20TokenEmitter.sol
255: require(etherAmount >0, "Ether amount must be greater than 0");
256: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
File: packages/revolution/src/ERC20TokenEmitter.sol
272: require(paymentAmount >0, "Payment amount must be greater than 0");
273: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
Merging multiple for loops within a function in Solidity can enhance efficiency and reduce gas costs, especially when they share a common iterating variable or perform related operations. By minimizing redundant iterations over the same data set, execution becomes more cost-effective. However, while merging can optimize gas usage and simplify logic, it may also increase code complexity. Therefore, careful balance between optimization and maintainability is essential, along with thorough testing to ensure the refactored code behaves as expected.
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
236: for (uint i; i < creatorArrayLength; i++) {
237: newPiece.creators.push(creatorArray[i]);
238: }
239:
240: emitPieceCreated(pieceId, msg.sender, metadata, newPiece.quorumVotes, newPiece.totalVotesSupply);
241:
242: // Emit an event for each creator243: for (uint i; i < creatorArrayLength; i++) {
[G-24] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
33: mapping(address=>uint256) public nonces;
public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted
[G-26] Not using the named return variables anywhere in the function is confusing
Consider changing the variable to be an unnamed one, since the variable is never assigned, nor is it returned by name. If the optimizer is not turned on, leaving the code as it is will also waste gas for the stack variable.
There are 5 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
// @audit success419: function _verifyVoteSignature(
Payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided.A constructor can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.
[G-28] Using private rather than public for constants, saves gas
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
[G-29] Functions guaranteed to revert when called by normal users can be marked payable
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2), which costs an average of about ** 21 gas per call ** to the function, in addition to the extra deployment cost
There are 32 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
208: function pause() externaloverride onlyOwner {
[G-30] Avoid updating storage when the value hasn't changed to save gas
If the old value is equal to the new value, not re-storing the value will avoid a Gsreset (2900 gas), potentially at the expense of a Gcoldsload (2100 gas) or a Gwarmaccess (100 gas)
There are 17 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
152: function settleCurrentAndCreateNewAuction() externaloverride nonReentrant whenNotPaused {
[G-33] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead
When using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Each operation involving a uint8 costs an extra ** 22 - 28 gas ** (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed
There are 3 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit `_minBidIncrementPercentage` is `uint8`297: function setMinBidIncrementPercentage(uint8_minBidIncrementPercentage) externaloverride onlyOwner {
[G-34] The use of a logical AND in place of double if is slightly less gas efficient in instances where there isn't a corresponding else statement for the given if statement
Using a double if statement instead of logical AND (&&) can provide similar short-circuiting behavior whereas double if is slightly more efficient.
There are 3 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
383: if (creatorsShare >0&& entropyRateBps >0) {
384: for (uint256 i =0; i < numCreators; i++) {
[G-35] Splitting require() statements that use && saves gas
See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper by 3 gas
There are 3 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
160: require(uint8(metadata.mediaType) >0&&uint8(metadata.mediaType) <=5, "Invalid media type");
161:
File: packages/revolution/src/CultureIndex.sol
398: require(
399: len == pieceIds.length&& len == deadline.length&& len == v.length&& len == r.length&& len == s.length,
400: "Array lengths must match"401: );
[G-36] Cache state variables outside of loop to avoid reading storage on every iteration
Reading from storage should always try to be avoided within loops.In the following instances, we are able to cache state variables outside of the loop to save a Gwarmaccess(100 gas) per loop iteration.
Note: Due to stack too deep errors, we will not be able to cache all the state variables read within the loops.
There are 1 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit `entropyRateBps` is a state variable, try to cache it outside the loop390: uint256 paymentAmount = (creatorsShare * entropyRateBps * creator.bps) / (10_000*10_000);
[G-39] Can make the variable outside the loop to save gas
Creating variables inside the loop consum more gas compared to declaring them outside and just reaffecting values to them inside the loop.
There are 2 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit variable `creator` is created inside a loop.385: ICultureIndex.CreatorBps memory creator = verbs.getArtPieceById(_auction.verbId).creators[i];
The Solidity compiler's Intermediate Representation (IR) based code generator, which can be activated using --via-ir on the command line or {""viaIR"": true} in the options, serves a dual purpose. Firstly, it boosts the transparency and audibility of code generation, which enhances developers' comprehension and control over the contract's final bytecode. Secondly, it enables more sophisticated optimization passes that span multiple functions, thereby potentially leading to more efficient bytecode.
It's important to note that using the IR- based code generator may lengthen compile times due to the extra optimization steps.Therefore, it's advised to test your contract with and without this option enabled to measure the performance and gas cost implications.If the IR- based code generator significantly enhances your contract's performance or reduces gas costs, consider using the --via-ir flag during deployment.This way, you can leverage more advanced compiler optimizations without hindering your development workflow.
[G-43] Stat variables can be packed into fewer storage slots by truncating timestamp bytes
By using a uint32 rather than a larger type for variables that track timestamps, one can save gas by using fewer storage slots per struct, at the expense of the protocol breaking after the year 2106 (when uint32 wraps). If this is an acceptable tradeoff, each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the stat variable. Subsequent reads as well as writes have smaller gas savings
There are 2 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
//@audit the following variables could be packed: uint256public timeBuffer;
uint256public duration;
39: contractAuctionHouseis
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit the following variables could be packed: uint256public startTime;
17: contractERC20TokenEmitteris
[G-44] State variables can be packed into fewer storage slots
If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
There are 2 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
// @audit from 9 to 7 you need to change the structure elements order to: , uint256, uint256, uint256, uint256, uint256, uint256, address, uint8, IVerbsToken, IERC20TokenEmitter, IAuctionHouse.Auction, IRevolutionBuilder039: contractAuctionHouseis040: IAuctionHouse,
041: VersionedContract,
042: UUPS,
043: PausableUpgradeable,
044: ReentrancyGuardUpgradeable,
045: Ownable2StepUpgradeable046: {
047: // The Verbs ERC721 token contract048: IVerbsToken public verbs;
049:
050: // The ERC20 governance token051: IERC20TokenEmitterpublic erc20TokenEmitter;
052:
053: // The address of the WETH contract054: addresspublic WETH;
055:
056: // The minimum amount of time left in an auction after a new bid is created057: uint256public timeBuffer;
058:
059: // The minimum price accepted in an auction060: uint256public reservePrice;
061:
062: // The minimum percentage difference between the last bid amount and the current bid063: uint8public minBidIncrementPercentage;
064:
065: // The split of the winning bid that is reserved for the creator of the Verb in basis points066: uint256public creatorRateBps;
067:
068: // The all time minimum split of the winning bid that is reserved for the creator of the Verb in basis points069: uint256public minCreatorRateBps;
070:
071: // The split of (auction proceeds * creatorRate) that is sent to the creator as ether in basis points072: uint256public entropyRateBps;
073:
074: // The duration of a single auction075: uint256public duration;
076:
077: // The active auction078: IAuctionHouse.Auction public auction;
079:
080: /// ///081: /// IMMUTABLES ///082: /// ///083:
084: /// @notice The contract upgrade manager085: IRevolutionBuilder publicimmutable manager;
086:
087: // TODO investigate this - The minimum gas threshold for creating an auction (minting VerbsToken)088: uint32public constant MIN_TOKEN_MINT_GAS_THRESHOLD =750_000;
089:
090: /// ///091: /// CONSTRUCTOR ///092: /// ///093:
094: /// @param _manager The contract upgrade manager address095: constructor(address_manager) payable initializer {
096: manager =IRevolutionBuilder(_manager);
097: }
098:
099: /// ///100: /// INITIALIZER ///101: /// ///102:
103: /**104: * @notice Initialize the auction house and base contracts,105: * populate configuration values, and pause the contract.106: * @dev This function can only be called once.107: * @param _erc721Token The address of the Verbs ERC721 token contract.108: * @param _erc20TokenEmitter The address of the ERC-20 token emitter contract.109: * @param _initialOwner The address of the owner.110: * @param _weth The address of the WETH contract111: * @param _auctionParams The auction params for auctions.112: */113: function initialize(
114: address_erc721Token,
115: address_erc20TokenEmitter,
116: address_initialOwner,
117: address_weth,
118: IRevolutionBuilder.AuctionParams calldata_auctionParams119: ) external initializer {
120: require(msg.sender==address(manager), "Only manager can initialize");
121: require(_weth !=address(0), "WETH cannot be zero address");
122:
123: __Pausable_init();
124: __ReentrancyGuard_init();
125: __Ownable_init(_initialOwner);
126:
127: _pause();
128:
129: require(
130: _auctionParams.creatorRateBps >= _auctionParams.minCreatorRateBps,
131: "Creator rate must be greater than or equal to the creator rate"132: );
133:
134: verbs =IVerbsToken(_erc721Token);
135: erc20TokenEmitter =IERC20TokenEmitter(_erc20TokenEmitter);
136: timeBuffer = _auctionParams.timeBuffer;
137: reservePrice = _auctionParams.reservePrice;
138: minBidIncrementPercentage = _auctionParams.minBidIncrementPercentage;
139: duration = _auctionParams.duration;
140: creatorRateBps = _auctionParams.creatorRateBps;
141: entropyRateBps = _auctionParams.entropyRateBps;
142: minCreatorRateBps = _auctionParams.minCreatorRateBps;
143: WETH = _weth;
144: }
145:
146: /**147: * @notice Settle the current auction, mint a new Verb, and put it up for auction.148: */149: // Can technically reenter via cross function reentrancies in _createAuction, auction, and pause, but those are only callable by the owner.150: // @wardens if you can find an exploit here go for it - we might be wrong.151: // slither-disable-next-line reentrancy-eth152: function settleCurrentAndCreateNewAuction() externaloverride nonReentrant whenNotPaused {
153: _settleAuction();
154: _createAuction();
155: }
156:
157: /**158: * @notice Settle the current auction.159: * @dev This function can only be called when the contract is paused.160: */161: function settleAuction() externaloverride whenPaused nonReentrant {
162: _settleAuction();
163: }
164:
165: /**166: * @notice Create a bid for a Verb, with a given amount.167: * @dev This contract only accepts payment in ETH.168: * @param verbId The ID of the Verb to bid on.169: * @param bidder The address of the bidder.170: */171: function createBid(uint256verbId, addressbidder) externalpayableoverride nonReentrant {
172: IAuctionHouse.Auction memory _auction = auction;
173:
174: //require bidder is valid address175: require(bidder !=address(0), "Bidder cannot be zero address");
176: require(_auction.verbId == verbId, "Verb not up for auction");
177: //slither-disable-next-line timestamp178: require(block.timestamp< _auction.endTime, "Auction expired");
179: require(msg.value>= reservePrice, "Must send at least reservePrice");
180: require(
181: msg.value>= _auction.amount + ((_auction.amount * minBidIncrementPercentage) /100),
182: "Must send more than last bid by minBidIncrementPercentage amount"183: );
184:
185: address payable lastBidder = _auction.bidder;
186:
187: auction.amount =msg.value;
188: auction.bidder =payable(bidder);
189:
190: // Extend the auction if the bid was received within `timeBuffer` of the auction end time191: bool extended = _auction.endTime -block.timestamp< timeBuffer;
192: if (extended) auction.endTime = _auction.endTime =block.timestamp+ timeBuffer;
193:
194: // Refund the last bidder, if applicable195: if (lastBidder !=address(0)) _safeTransferETHWithFallback(lastBidder, _auction.amount);
196:
197: emitAuctionBid(_auction.verbId, bidder, msg.sender, msg.value, extended);
198:
199: if (extended) emitAuctionExtended(_auction.verbId, _auction.endTime);
200: }
201:
202: /**203: * @notice Pause the Verbs auction house.204: * @dev This function can only be called by the owner when the205: * contract is unpaused. While no new auctions can be started when paused,206: * anyone can settle an ongoing auction.207: */208: function pause() externaloverride onlyOwner {
209: _pause();
210: }
211:
212: /**213: * @notice Set the split of the winning bid that is reserved for the creator of the Verb in basis points.214: * @dev Only callable by the owner.215: * @param _creatorRateBps New creator rate in basis points.216: */217: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
218: require(
219: _creatorRateBps >= minCreatorRateBps,
220: "Creator rate must be greater than or equal to minCreatorRateBps"221: );
222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
223: creatorRateBps = _creatorRateBps;
224:
225: emitCreatorRateBpsUpdated(_creatorRateBps);
226: }
227:
228: /**229: * @notice Set the minimum split of the winning bid that is reserved for the creator of the Verb in basis points.230: * @dev Only callable by the owner.231: * @param _minCreatorRateBps New minimum creator rate in basis points.232: */233: function setMinCreatorRateBps(uint256_minCreatorRateBps) external onlyOwner {
234: require(_minCreatorRateBps <= creatorRateBps, "Min creator rate must be less than or equal to creator rate");
235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
236:
237: //ensure new min rate cannot be lower than previous min rate238: require(
239: _minCreatorRateBps > minCreatorRateBps,
240: "Min creator rate must be greater than previous minCreatorRateBps"241: );
242:
243: minCreatorRateBps = _minCreatorRateBps;
244:
245: emitMinCreatorRateBpsUpdated(_minCreatorRateBps);
246: }
247:
248: /**249: * @notice Set the split of (auction proceeds * creatorRate) that is sent to the creator as ether in basis points.250: * @dev Only callable by the owner.251: * @param _entropyRateBps New entropy rate in basis points.252: */253: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
255:
256: entropyRateBps = _entropyRateBps;
257: emitEntropyRateBpsUpdated(_entropyRateBps);
258: }
259:
260: /**261: * @notice Unpause the Verbs auction house.262: * @dev This function can only be called by the owner when the263: * contract is paused. If required, this function will start a new auction.264: */265: function unpause() externaloverride onlyOwner {
266: _unpause();
267:
268: if (auction.startTime ==0|| auction.settled) {
269: _createAuction();
270: }
271: }
272:
273: /**274: * @notice Set the auction time buffer.275: * @dev Only callable by the owner.276: */277: function setTimeBuffer(uint256_timeBuffer) externaloverride onlyOwner {
278: timeBuffer = _timeBuffer;
279:
280: emitAuctionTimeBufferUpdated(_timeBuffer);
281: }
282:
283: /**284: * @notice Set the auction reserve price.285: * @dev Only callable by the owner.286: */287: function setReservePrice(uint256_reservePrice) externaloverride onlyOwner {
288: reservePrice = _reservePrice;
289:
290: emitAuctionReservePriceUpdated(_reservePrice);
291: }
292:
293: /**294: * @notice Set the auction minimum bid increment percentage.295: * @dev Only callable by the owner.296: */297: function setMinBidIncrementPercentage(uint8_minBidIncrementPercentage) externaloverride onlyOwner {
298: minBidIncrementPercentage = _minBidIncrementPercentage;
299:
300: emitAuctionMinBidIncrementPercentageUpdated(_minBidIncrementPercentage);
301: }
302:
303: /**304: * @notice Create an auction.305: * @dev Store the auction details in the `auction` state variable and emit an AuctionCreated event.306: * If the mint reverts, the minter was updated without pausing this contract first. To remedy this,307: * catch the revert and pause this contract.308: */309: function _createAuction() internal {
310: // Check if there's enough gas to safely execute token.mint() and subsequent operations311: require(gasleft() >= MIN_TOKEN_MINT_GAS_THRESHOLD, "Insufficient gas for creating auction");
312:
313: try verbs.mint() returns (uint256verbId) {
314: uint256 startTime =block.timestamp;
315: uint256 endTime = startTime + duration;
316:
317: auction =Auction({
318: verbId: verbId,
319: amount: 0,
320: startTime: startTime,
321: endTime: endTime,
322: bidder: payable(0),
323: settled: false324: });
325:
326: emitAuctionCreated(verbId, startTime, endTime);
327: } catch {
328: _pause();
329: }
330: }
331:
332: /**333: * @notice Settle an auction, finalizing the bid and paying out to the owner. Pays out to the creator and the owner based on the creatorRateBps and entropyRateBps.334: * @dev If there are no bids, the Verb is burned.335: */336: function _settleAuction() internal {
337: IAuctionHouse.Auction memory _auction = auction;
338:
339: require(_auction.startTime !=0, "Auction hasn't begun");
340: require(!_auction.settled, "Auction has already been settled");
341: //slither-disable-next-line timestamp342: require(block.timestamp>= _auction.endTime, "Auction hasn't completed");
343:
344: auction.settled =true;
345:
346: uint256 creatorTokensEmitted =0;
347: // Check if contract balance is greater than reserve price348: if (address(this).balance < reservePrice) {
349: // If contract balance is less than reserve price, refund to the last bidder350: if (_auction.bidder !=address(0)) {
351: _safeTransferETHWithFallback(_auction.bidder, _auction.amount);
352: }
353:
354: // And then burn the Noun355: verbs.burn(_auction.verbId);
356: } else {
357: //If no one has bid, burn the Verb358: if (_auction.bidder ==address(0))
359: verbs.burn(_auction.verbId);
360: //If someone has bid, transfer the Verb to the winning bidder361: else verbs.transferFrom(address(this), _auction.bidder, _auction.verbId);
362:
363: if (_auction.amount >0) {
364: // Ether going to owner of the auction365: uint256 auctioneerPayment = (_auction.amount * (10_000- creatorRateBps)) /10_000;
366:
367: //Total amount of ether going to creator368: uint256 creatorsShare = _auction.amount - auctioneerPayment;
369:
370: uint256 numCreators = verbs.getArtPieceById(_auction.verbId).creators.length;
371: address deployer = verbs.getArtPieceById(_auction.verbId).sponsor;
372:
373: //Build arrays for erc20TokenEmitter.buyToken374: uint256[] memory vrgdaSplits =newuint256[](numCreators);
375: address[] memory vrgdaReceivers =newaddress[](numCreators);
376:
377: //Transfer auction amount to the DAO treasury378: _safeTransferETHWithFallback(owner(), auctioneerPayment);
379:
380: uint256 ethPaidToCreators =0;
381:
382: //Transfer creator's share to the creator, for each creator, and build arrays for erc20TokenEmitter.buyToken383: if (creatorsShare >0&& entropyRateBps >0) {
384: for (uint256 i =0; i < numCreators; i++) {
385: ICultureIndex.CreatorBps memory creator = verbs.getArtPieceById(_auction.verbId).creators[i];
386: vrgdaReceivers[i] = creator.creator;
387: vrgdaSplits[i] = creator.bps;
388:
389: //Calculate paymentAmount for specific creator based on BPS splits - same as multiplying by creatorDirectPayment390: uint256 paymentAmount = (creatorsShare * entropyRateBps * creator.bps) / (10_000*10_000);
391: ethPaidToCreators += paymentAmount;
392:
393: //Transfer creator's share to the creator394: _safeTransferETHWithFallback(creator.creator, paymentAmount);
395: }
396: }
397:
398: //Buy token from ERC20TokenEmitter for all the creators399: if (creatorsShare > ethPaidToCreators) {
400: creatorTokensEmitted = erc20TokenEmitter.buyToken{ value: creatorsShare - ethPaidToCreators }(
401: vrgdaReceivers,
402: vrgdaSplits,
403: IERC20TokenEmitter.ProtocolRewardAddresses({
404: builder: address(0),
405: purchaseReferral: address(0),
406: deployer: deployer
407: })
408: );
409: }
410: }
411: }
412:
413: emitAuctionSettled(_auction.verbId, _auction.bidder, _auction.amount, creatorTokensEmitted);
414: }
415:
416: /// @notice Transfer ETH/WETH from the contract417: /// @param _to The recipient address418: /// @param _amount The amount transferring419: function _safeTransferETHWithFallback(address_to, uint256_amount) private {
420: // Ensure the contract has enough ETH to transfer421: if (address(this).balance < _amount) revert("Insufficient balance");
422:
423: // Used to store if the transfer succeeded424: bool success;
425:
426: assembly {
427: // Transfer ETH to the recipient428: // Limit the call to 50,000 gas429: success :=call(50000, _to, _amount, 0, 0, 0, 0)
430: }
431:
432: // If the transfer failed:433: if (!success) {
434: // Wrap as WETH435: IWETH(WETH).deposit{ value: _amount }();
436:
437: // Transfer WETH instead438: bool wethSuccess =IWETH(WETH).transfer(_to, _amount);
439:
440: // Ensure successful transfer441: if (!wethSuccess) revert("WETH transfer failed");
442: }
443: }
444:
445: /// ///446: /// AUCTION UPGRADE ///447: /// ///448:
449: /// @notice Ensures the caller is authorized to upgrade the contract and the new implementation is valid450: /// @dev This function is called in `upgradeTo` & `upgradeToAndCall`451: /// @param _newImpl The new implementation address452: function _authorizeUpgrade(address_newImpl) internalviewoverride onlyOwner whenPaused {
453: // Ensure the new implementation is registered by the Builder DAO454: if (!manager.isRegisteredUpgrade(_getImplementation(), _newImpl)) revertINVALID_UPGRADE(_newImpl);
455: }
456: }
File: packages/revolution/src/VerbsToken.sol
// @audit from 5 to 4 you need to change the structure elements order to: , uint256, string, mapping, address, bool, bool, bool, IDescriptorMinimal, ICultureIndex, IRevolutionBuilder033: contractVerbsTokenis034: IVerbsToken,
035: VersionedContract,
036: UUPS,
037: Ownable2StepUpgradeable,
038: ReentrancyGuardUpgradeable,
039: ERC721CheckpointableUpgradeable040: {
041: // An address who has permissions to mint Verbs042: addresspublic minter;
043:
044: // The Verbs token URI descriptor045: IDescriptorMinimal public descriptor;
046:
047: // The CultureIndex contract048: ICultureIndex public cultureIndex;
049:
050: // Whether the minter can be updated051: boolpublic isMinterLocked;
052:
053: // Whether the CultureIndex can be updated054: boolpublic isCultureIndexLocked;
055:
056: // Whether the descriptor can be updated057: boolpublic isDescriptorLocked;
058:
059: // The internal verb ID tracker060: uint256private _currentVerbId;
061:
062: // IPFS content hash of contract-level metadata063: stringprivate _contractURIHash ="QmQzDwaZ7yQxHHs7sQQenJVB89riTSacSGcJRv9jtHPuz5";
064:
065: // The Verb art pieces066: mapping(uint256=> ICultureIndex.ArtPiece) public artPieces;
067:
068: /// ///069: /// MODIFIERS ///070: /// ///071:
072: /**073: * @notice Require that the minter has not been locked.074: */075: modifier whenMinterNotLocked() {
076: require(!isMinterLocked, "Minter is locked");
077: _;078: }
079:
080: /**081: * @notice Require that the CultureIndex has not been locked.082: */083: modifier whenCultureIndexNotLocked() {
084: require(!isCultureIndexLocked, "CultureIndex is locked");
085: _;086: }
087:
088: /**089: * @notice Require that the descriptor has not been locked.090: */091: modifier whenDescriptorNotLocked() {
092: require(!isDescriptorLocked, "Descriptor is locked");
093: _;094: }
095:
096: /**097: * @notice Require that the sender is the minter.098: */099: modifier onlyMinter() {
100: require(msg.sender== minter, "Sender is not the minter");
101: _;102: }
103:
104: /// ///105: /// IMMUTABLES ///106: /// ///107:
108: /// @notice The contract upgrade manager109: IRevolutionBuilder privateimmutable manager;
110:
111: /// ///112: /// CONSTRUCTOR ///113: /// ///114:
115: /// @param _manager The contract upgrade manager address116: constructor(address_manager) payable initializer {
117: manager =IRevolutionBuilder(_manager);
118: }
119:
120: /// ///121: /// INITIALIZER ///122: /// ///123:
124: /// @notice Initializes a DAO's ERC-721 token contract125: /// @param _minter The address of the minter126: /// @param _initialOwner The address of the initial owner127: /// @param _descriptor The address of the token URI descriptor128: /// @param _cultureIndex The address of the CultureIndex contract129: /// @param _erc721TokenParams The name, symbol, and contract metadata of the token130: function initialize(
131: address_minter,
132: address_initialOwner,
133: address_descriptor,
134: address_cultureIndex,
135: IRevolutionBuilder.ERC721TokenParamsmemory_erc721TokenParams136: ) external initializer {
137: require(msg.sender==address(manager), "Only manager can initialize");
138:
139: require(_minter !=address(0), "Minter cannot be zero address");
140: require(_initialOwner !=address(0), "Initial owner cannot be zero address");
141:
142: // Initialize the reentrancy guard143: __ReentrancyGuard_init();
144:
145: // Setup ownable146: __Ownable_init(_initialOwner);
147:
148: // Initialize the ERC-721 token149: __ERC721_init(_erc721TokenParams.name, _erc721TokenParams.symbol);
150: _contractURIHash = _erc721TokenParams.contractURIHash;
151:
152: // Set the contracts153: minter = _minter;
154: descriptor =IDescriptorMinimal(_descriptor);
155: cultureIndex =ICultureIndex(_cultureIndex);
156: }
157:
158: /**159: * @notice The IPFS URI of contract-level metadata.160: */161: function contractURI() publicviewreturns (stringmemory) {
162: returnstring(abi.encodePacked("ipfs://", _contractURIHash));
163: }
164:
165: /**166: * @notice Set the _contractURIHash.167: * @dev Only callable by the owner.168: */169: function setContractURIHash(stringmemorynewContractURIHash) external onlyOwner {
170: _contractURIHash = newContractURIHash;
171: }
172:
173: /**174: * @notice Mint a Verb to the minter.175: * @dev Call _mintTo with the to address(es).176: */177: function mint() publicoverride onlyMinter nonReentrant returns (uint256) {
178: return_mintTo(minter);
179: }
180:
181: /**182: * @notice Burn a verb.183: */184: function burn(uint256verbId) publicoverride onlyMinter nonReentrant {
185: _burn(verbId);
186: emitVerbBurned(verbId);
187: }
188:
189: /**190: * @notice A distinct Uniform Resource Identifier (URI) for a given asset.191: * @dev See {IERC721Metadata-tokenURI}.192: */193: function tokenURI(uint256tokenId) publicviewoverridereturns (stringmemory) {
194: return descriptor.tokenURI(tokenId, artPieces[tokenId].metadata);
195: }
196:
197: /**198: * @notice Similar to `tokenURI`, but always serves a base64 encoded data URI199: * with the JSON contents directly inlined.200: */201: function dataURI(uint256tokenId) publicviewoverridereturns (stringmemory) {
202: return descriptor.dataURI(tokenId, artPieces[tokenId].metadata);
203: }
204:
205: /**206: * @notice Set the token minter.207: * @dev Only callable by the owner when not locked.208: */209: function setMinter(address_minter) externaloverride onlyOwner nonReentrant whenMinterNotLocked {
210: require(_minter !=address(0), "Minter cannot be zero address");
211: minter = _minter;
212:
213: emitMinterUpdated(_minter);
214: }
215:
216: /**217: * @notice Lock the minter.218: * @dev This cannot be reversed and is only callable by the owner when not locked.219: */220: function lockMinter() externaloverride onlyOwner whenMinterNotLocked {
221: isMinterLocked =true;
222:
223: emitMinterLocked();
224: }
225:
226: /**227: * @notice Set the token URI descriptor.228: * @dev Only callable by the owner when not locked.229: */230: function setDescriptor(
231: IDescriptorMinimal _descriptor232: ) externaloverride onlyOwner nonReentrant whenDescriptorNotLocked {
233: descriptor = _descriptor;
234:
235: emitDescriptorUpdated(_descriptor);
236: }
237:
238: /**239: * @notice Lock the descriptor.240: * @dev This cannot be reversed and is only callable by the owner when not locked.241: */242: function lockDescriptor() externaloverride onlyOwner whenDescriptorNotLocked {
243: isDescriptorLocked =true;
244:
245: emitDescriptorLocked();
246: }
247:
248: /**249: * @notice Set the token CultureIndex.250: * @dev Only callable by the owner when not locked.251: */252: function setCultureIndex(ICultureIndex _cultureIndex) external onlyOwner whenCultureIndexNotLocked nonReentrant {
253: cultureIndex = _cultureIndex;
254:
255: emitCultureIndexUpdated(_cultureIndex);
256: }
257:
258: /**259: * @notice Lock the CultureIndex260: * @dev This cannot be reversed and is only callable by the owner when not locked.261: */262: function lockCultureIndex() externaloverride onlyOwner whenCultureIndexNotLocked {
263: isCultureIndexLocked =true;
264:
265: emitCultureIndexLocked();
266: }
267:
268: /**269: * @notice Fetch an art piece by its ID.270: * @param verbId The ID of the art piece.271: * @return The ArtPiece struct associated with the given ID.272: */273: function getArtPieceById(uint256verbId) publicviewreturns (ICultureIndex.ArtPiece memory) {
274: require(verbId <= _currentVerbId, "Invalid piece ID");
275: return artPieces[verbId];
276: }
277:
278: /**279: * @notice Mint a Verb with `verbId` to the provided `to` address. Pulls the top voted art piece from the CultureIndex.280: */281: function _mintTo(addressto) internalreturns (uint256) {
282: ICultureIndex.ArtPiece memory artPiece = cultureIndex.getTopVotedPiece();
283:
284: // Check-Effects-Interactions Pattern285: // Perform all checks286: require(
287: artPiece.creators.length<= cultureIndex.MAX_NUM_CREATORS(),
288: "Creator array must not be > MAX_NUM_CREATORS"289: );
290:
291: // Use try/catch to handle potential failure292: try cultureIndex.dropTopVotedPiece() returns (ICultureIndex.ArtPiece memory_artPiece) {
293: artPiece = _artPiece;
294: uint256 verbId = _currentVerbId++;
295:
296: ICultureIndex.ArtPiece storage newPiece = artPieces[verbId];
297:
298: newPiece.pieceId = artPiece.pieceId;
299: newPiece.metadata = artPiece.metadata;
300: newPiece.isDropped = artPiece.isDropped;
301: newPiece.sponsor = artPiece.sponsor;
302: newPiece.totalERC20Supply = artPiece.totalERC20Supply;
303: newPiece.quorumVotes = artPiece.quorumVotes;
304: newPiece.totalVotesSupply = artPiece.totalVotesSupply;
305:
306: for (uint i =0; i < artPiece.creators.length; i++) {
307: newPiece.creators.push(artPiece.creators[i]);
308: }
309:
310: _mint(to, verbId);
311:
312: emitVerbCreated(verbId, artPiece);
313:
314: return verbId;
315: } catch {
316: // Handle failure (e.g., revert, emit an event, set a flag, etc.)317: revert("dropTopVotedPiece failed");
318: }
319: }
320:
321: /// ///322: /// TOKEN UPGRADE ///323: /// ///324:
325: // /// @notice Ensures the caller is authorized to upgrade the contract and that the new implementation is valid326: // /// @dev This function is called in `upgradeTo` & `upgradeToAndCall`327: // /// @param _newImpl The new implementation address328: function _authorizeUpgrade(address_newImpl) internalviewoverride onlyOwner {
329: // Ensure the implementation is valid330: require(manager.isRegisteredUpgrade(_getImplementation(), _newImpl), "Invalid upgrade");
331: }
332: }
A do while loop will cost less gas since the condition is not being checked for the first iteration, Check my example on github. Actually, do while alwayse cast less gas compared to For check my second example github
There are 9 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
384: for (uint256 i =0; i < numCreators; i++) {
[G-48]++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops
The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop
There are 9 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
384: for (uint256 i =0; i < numCreators; i++) {
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
Payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided.An Initializer can safely be marked as payable, since only the allowed user would be able to pass funds, and the project itself would not pass any funds.
File: packages/revolution/src/MaxHeap.sol
55: function initialize(address_initialOwner, address_admin) public initializer {
56: require(msg.sender==address(manager), "Only manager can initialize");
Cacheing saves gas when compared to repeating the calculation at each point it is used in the contract.The instance below represents the second+ time of calling address(this) in a specific function
[N-02] Consider using modifiers for address control
Modifiers in Solidity can improve code readability and modularity by encapsulating repetitive checks, such as address validity checks, into a reusable construct. For example, an onlyOwner modifier can be used to replace repetitive require(msg.sender == owner) checks across several functions, reducing code redundancy and enhancing maintainability. To implement, define a modifier with the check, then apply the modifier to relevant functions.
There are 9 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
56: require(msg.sender==address(manager), "Only manager can initialize");
[N-03] Large or complicated code bases should implement invariant tests
Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement invariant fuzzing tests. Invariant fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and invariant fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.
There are 1 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
@audit Should implement invariant tests1:
[N-04] Assembly blocks should have extensive comments
Assembly blocks are taking a lot more time to audit than normal Solidity code, and often have gotchas and side-effects that the Solidity versions of the same code do not. Consider adding more comments explaining what is being done in every step of the assembly code
[N-07] Common functions should be refactored to a common base contract
The functions below have the same implementation as is seen in other files. The functions should be refactored into functions of a common base contract
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit this function is already seen in `packages/revolution/src/CultureIndex.sol`543: function _authorizeUpgrade(address_newImpl) internalviewoverride onlyOwner {
544: // Ensure the new implementation is a registered upgrade545: if (!manager.isRegisteredUpgrade(_getImplementation(), _newImpl)) revertINVALID_UPGRADE(_newImpl);
546: }
File: packages/revolution/src/AuctionHouse.sol
//@audit this function is already seen in `packages/revolution/src/AuctionHouse.sol`208: function pause() externaloverride onlyOwner {
209: _pause();
210: }
To maintain readability in code, particularly in Solidity which can involve complex mathematical operations, it is often recommended to limit the number of arithmetic operations to a maximum of 2-3 per line. Too many operations in a single line can make the code difficult to read and understand, increase the likelihood of mistakes, and complicate the process of debugging and reviewing the code. Consider splitting such operations over more than one line, take special care when dealing with division however. Try to limit the number of arithmetic operations to a maximum of 3 per line.
Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.
There are 5 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit The same constant is already defined on file : packages/revolution/src/MaxHeap.sol85: IRevolutionBuilder privateimmutable manager;
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit The same constant is already defined on file : packages/revolution/src/MaxHeap.sol37: IRevolutionBuilder privateimmutable manager;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit The same constant is already defined on file : packages/revolution/src/MaxHeap.sol55: IRevolutionBuilder privateimmutable manager;
File: packages/revolution/src/AuctionHouse.sol
//@audit The same constant is already defined on file : packages/revolution/src/MaxHeap.sol85: IRevolutionBuilder publicimmutable manager;
File: packages/revolution/src/VerbsToken.sol
//@audit The same constant is already defined on file : packages/revolution/src/MaxHeap.sol109: IRevolutionBuilder privateimmutable manager;
File: packages/revolution/src/CultureIndex.sol
//@audit `MAX_NUM_CREATORS`182: require(creatorArrayLength <= MAX_NUM_CREATORS, "Creator array must not be > MAX_NUM_CREATORS");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `10_000`289: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `10_000`300: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
[N-12] NatSpec documentation for contract is missing
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.source
[N-13] Contract does not follow the Solidity style guide's suggested layout ordering
The style guide says that, within a contract, the ordering should be 1) Type declarations, 2) State variables, 3) Events, 4) Modifiers, and 5) Functions, but the contract(s) below do not follow this ordering
There are 2 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
//@audit the variable definition is misplaced65: mapping(uint256=>uint256) public heap;
When developing smart contracts in Solidity, it's crucial to validate the inputs of your functions. This includes ensuring that the bytes parameters are not empty, especially when they represent crucial data such as addresses, identifiers, or raw data that the contract needs to process.
Missing empty bytes checks can lead to unexpected behaviour in your contract.For instance, certain operations might fail, produce incorrect results, or consume unnecessary gas when performed with empty bytes.Moreover, missing input validation can potentially expose your contract to malicious activity, including exploitation of unhandled edge cases.
To mitigate these issues, always validate that bytes parameters are not empty when the logic of your contract requires it.
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit ,r ,s are not checked367: function voteForManyWithSig(
368: addressfrom,
369: uint256[] calldatapieceIds,
370: uint256deadline,
371: uint8v,
372: bytes32r,
373: bytes32s374: ) external nonReentrant {
When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.
[N-19] Defining All External/Public Functions in Contract Interfaces
It is preferable to have all the external and public function in an interface to make using them easier by developers. This helps ensure the whole API is extracted in a interface.
There are 15 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
119: function insert(uint256itemId, uint256value) public onlyAdmin {
120: heap[size] = itemId;
File: packages/revolution/src/MaxHeap.sol
136: function updateValue(uint256itemId, uint256newValue) public onlyAdmin {
137: uint256 position = positionMapping[itemId];
File: packages/revolution/src/ERC20TokenEmitter.sol
237: function buyTokenQuote(uint256amount) publicviewreturns (intspentY) {
238: require(amount >0, "Amount must be greater than 0");
File: packages/revolution/src/ERC20TokenEmitter.sol
254: function getTokenQuoteForEther(uint256etherAmount) publicviewreturns (intgainedX) {
255: require(etherAmount >0, "Ether amount must be greater than 0");
If you leave a floating pragma in your code (pragma solidity 0.4>=0.6. 0. ), you won't know which version was deployed to compile your code, leading to unexpected behavior
[N-22] NatSpec documentation for function is missing
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.source
There are 16 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
288: function _getVotes(addressaccount) internalviewreturns (uint256) {
[N-23] Function ordering does not follow the Solidity style guide
According to the Solidity style guide, functions should be laid out in the following order :constructor(), receive(), fallback(), external, public, internal, private, but the cases below do not follow this pattern
There are 6 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
94: function maxHeapify(uint256pos) internal {
[N-25] Hardcoded string that is repeatedly used can be replaced with a constant
For better maintainability, please consider creating and using a constant for those strings instead of hardcoding
There are 4 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
//@audit `Heap is empty` is used multiple time consider using a constant for it.157: require(size >0, "Heap is empty");
File: packages/revolution/src/CultureIndex.sol
//@audit `Invalid piece ID` is used multiple time consider using a constant for it.308: require(pieceId < _currentPieceId, "Invalid piece ID");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `Transfer failed.` is used multiple time consider using a constant for it.192: require(success, "Transfer failed.");
File: packages/revolution/src/VerbsToken.sol
//@audit `Minter cannot be zero address` is used multiple time consider using a constant for it.139: require(_minter !=address(0), "Minter cannot be zero address");
[N-27] Some if-statement can be converted to a ternary
Improving code readability and compactness is an integral part of optimal programming practices. The use of ternary operators in place of if-else conditions is one such measure. Ternary operators allow us to write conditional statements in a more concise manner, thereby enhancing readability and simplicity. They follow the syntax condition ? exprIfTrue : exprIfFalse, which interprets as "if the condition is true, evaluate to exprIfTrue, else evaluate to exprIfFalse". By adopting this approach, we make our code more streamlined and intuitive, which could potentially aid in better understanding and maintenance of the codebase.
File: packages/revolution/src/AuctionHouse.sol
382: //Transfer creator's share to the creator, for each creator, and build arrays for erc20TokenEmitter.buyToken
File: packages/revolution/src/AuctionHouse.sol
389: //Calculate paymentAmount for specific creator based on BPS splits - same as multiplying by creatorDirectPayment
Some parts of the codebase use require statements, while others use custom errors. Consider refactoring the code to use the same approach: the following findings represent the minority of require vs error, and they show the first occurance in each file, for brevity.
There are 17 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
183: if (!manager.isRegisteredUpgrade(_getImplementation(), _newImpl)) revertINVALID_UPGRADE(_newImpl);
In Solidity, just like in most other programming languages, regular comments serve to make code more understandable for developers. These are usually denoted by // for single line comments, or /* ... */ for multi-line comments, and are ignored by the compiler.
On the other hand, NatSpec comments in Solidity, denoted by /// for single - line comments, or/** ... */ for multi - line comments, serve a different purpose.Besides aiding developer comprehension, they also form a part of the contract's interface, as they can be parsed and used by tools such as automated documentation generators or IDEs to provide users with details about the contract's functions, parameters and behavior.NatSpec comments can also be retrieved via JSON interfaces, and as such, they're included in the contract's ABI.
Thus, using/// and /** ... */ appropriately ensures not only proper documentation for developers, but also helps create a richer and more informative interface for users and external tools interacting with your contract.
There are 3 instance(s) of this issue:
File: packages/revolution/src/VerbsToken.sol
325: // /// @notice Ensures the caller is authorized to upgrade the contract and that the new implementation is valid
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. The solidity style guide recommends a maximumum line length of 120 characters, so the lines below should be split when they reach that length.
There are 20 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
53: /// @notice The basis point number of votes in support of a art piece required in order for a quorum to be reached and for an art piece to be dropped.
File: packages/revolution/src/CultureIndex.sol
197: * @param metadata The metadata associated with the art piece, including name, description, image, and optional animation URL.
File: packages/revolution/src/CultureIndex.sol
198: * @param creatorArray An array of creators who contributed to the piece, along with their respective basis points that must sum up to 10,000.
File: packages/revolution/src/CultureIndex.sol
304: * @dev Requires that the pieceId is valid, the voter has not already voted on this piece, and the weight is greater than the minimum vote weight.
File: packages/revolution/src/CultureIndex.sol
329: * @dev Requires that the pieceId is valid, the voter has not already voted on this piece, and the weight is greater than the minimum vote weight.
File: packages/revolution/src/CultureIndex.sol
339: * @dev Requires that the pieceIds are valid, the voter has not already voted on this piece, and the weight is greater than the minimum vote weight.
File: packages/revolution/src/CultureIndex.sol
350: * @dev Requires that the pieceIds are valid, the voter has not already voted on this piece, and the weight is greater than the minimum vote weight.
File: packages/revolution/src/NontransferableERC20Votes.sol
7: * @dev Extension of ERC-20 to support Compound-like voting and delegation. This version is more generic than Compound's,
File: packages/revolution/src/ERC20TokenEmitter.sol
146: * @notice A payablefunction that allows a user to buy tokens for a list of addresses and a list of basis points to split the token purchase between.
File: packages/revolution/src/ERC20TokenEmitter.sol
233: * @notice Returns the amount of wei that would be spent to buy an amount of tokens. Does not take into account the protocol rewards.
File: packages/revolution/src/ERC20TokenEmitter.sol
239: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
File: packages/revolution/src/ERC20TokenEmitter.sol
250: * @notice Returns the amount of tokens that would be emitted for an amount of wei. Does not take into account the protocol rewards.
File: packages/revolution/src/ERC20TokenEmitter.sol
256: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
File: packages/revolution/src/ERC20TokenEmitter.sol
267: * @notice Returns the amount of tokens that would be emitted for the payment amount, taking into account the protocol rewards.
File: packages/revolution/src/ERC20TokenEmitter.sol
273: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.
File: packages/revolution/src/AuctionHouse.sol
149: // Can technically reenter via cross function reentrancies in _createAuction, auction, and pause, but those are only callable by the owner.
File: packages/revolution/src/AuctionHouse.sol
333: * @notice Settle an auction, finalizing the bid and paying out to the owner. Pays out to the creator and the owner based on the creatorRateBps and entropyRateBps.
File: packages/revolution/src/AuctionHouse.sol
382: //Transfer creator's share to the creator, for each creator, and build arrays for erc20TokenEmitter.buyToken
File: packages/revolution/src/AuctionHouse.sol
389: //Calculate paymentAmount for specific creator based on BPS splits - same as multiplying by creatorDirectPayment
File: packages/revolution/src/VerbsToken.sol
279: * @notice Mint a Verb with `verbId` to the provided `to` address. Pulls the top voted art piece from the CultureIndex.
File: packages/revolution/src/CultureIndex.sol
//@audit This message need more details : Already voted311: require(!(votes[pieceId][voter].voterAddress !=address(0)), "Already voted");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit This message need more details : Must send ether160: require(msg.value>0, "Must send ether");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit This message need more details : Invalid address310: require(_creatorsAddress !=address(0), "Invalid address");
File: packages/revolution/src/AuctionHouse.sol
//@audit This message need more details : Auction expired178: require(block.timestamp< _auction.endTime, "Auction expired");
File: packages/revolution/src/VerbsToken.sol
//@audit This message need more details : Invalid upgrade330: require(manager.isRegisteredUpgrade(_getImplementation(), _newImpl), "Invalid upgrade");
[N-42]override function arguments that are unused should have the variable name removed or commented out to avoid compiler warnings
There are 4 instance(s) of this issue:
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit from is not used//@audit to is not used//@audit value is not used101: function _transfer(addressfrom, addressto, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit owner is not used//@audit spender is not used//@audit value is not used141: function _approve(addressowner, addressspender, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit owner is not used//@audit spender is not used//@audit value is not used//@audit emitEvent is not used148: function _approve(addressowner, addressspender, uint256value, boolemitEvent) internalvirtualoverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit owner is not used//@audit spender is not used//@audit value is not used155: function _spendAllowance(addressowner, addressspender, uint256value) internalvirtualoverride {
Starting with Solidity version 0.8.8, using the override keyword when the function solely overrides an interface function, and the function doesn't exist in multiple base contracts, is unnecessary.
There are 30 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
181: function _authorizeUpgrade(address_newImpl) internalviewoverride onlyOwner {
File: packages/revolution/src/CultureIndex.sol
// @audit the @param blockNumber is missing
@notice Returns the voting power of a voter at the current block.
@param account The address of the voter.
@return The voting power of the voter.
File: packages/revolution/src/CultureIndex.sol
// @audit the @param voter is missing
@notice Fetch the list of votes for a given art piece.
@param pieceId The ID of the art piece.
@return An array of Vote structs for the given art piece ID.
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param _initialOwner is missing// @audit the @param _name is missing// @audit the @param _symbol is missing///
INITIALIZER //////
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param from is missing// @audit the @param to is missing// @audit the @param value is missing
@dev Not allowed
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param is missing// @audit the @param is missing// @audit the @param is missing
@dev Not allowed
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param account is missing// @audit the @param value is missing
@dev Creates a `value` amount of tokens and assigns them to `account`, by transferring it from address(0).
Relies on the `_update` mechanism
Emits a {Transfer} event with `from` set to the zero address.
NOTE: This function is not virtual, {_update} should be overridden instead.
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param owner is missing// @audit the @param spender is missing// @audit the @param value is missing
@dev Not allowed
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param owner is missing// @audit the @param spender is missing// @audit the @param value is missing// @audit the @param emitEvent is missing
@dev Not allowed
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @param owner is missing// @audit the @param spender is missing// @audit the @param value is missing
@dev Not allowed
File: packages/revolution/src/ERC20TokenEmitter.sol
// @audit the @param is missing
@notice Set the creators address to pay the creatorRate to. Can be a contract.
@dev Only callable by the owner.
File: packages/revolution/src/AuctionHouse.sol
// @audit the @param _timeBuffer is missing
@notice Set the auction time buffer.
@dev Only callable by the owner.
File: packages/revolution/src/AuctionHouse.sol
// @audit the @param _reservePrice is missing
@notice Set the auction reserve price.
@dev Only callable by the owner.
File: packages/revolution/src/AuctionHouse.sol
// @audit the @param _minBidIncrementPercentage is missing
@notice Set the auction minimum bid increment percentage.
@dev Only callable by the owner.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param newContractURIHash is missing
@notice Set the _contractURIHash.
@dev Only callable by the owner.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param tokenId is missing
@notice A distinct Uniform Resource Identifier (URI) for a given asset.
@dev See {IERC721Metadata-tokenURI}.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param tokenId is missing
@notice Similar to `tokenURI`, but always serves a base64 encoded data URI
with the JSON contents directly inlined.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param is missing
@notice Set the token minter.
@dev Only callable by the owner when not locked.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param _descriptor is missing
@notice Set the token URI descriptor.
@dev Only callable by the owner when not locked.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param _cultureIndex is missing
@notice Set the token CultureIndex.
@dev Only callable by the owner when not locked.
File: packages/revolution/src/VerbsToken.sol
// @audit the @param to is missing
@notice Mint a Verb with `verbId` to the provided `to` address. Pulls the top voted art piece from the CultureIndex.
[N-49] Setters should prevent re-setting of the same value
This especially problematic when the setter also emits the same value, which may be confusing to offline parsers
There are 13 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit `quorumVotesBPS` and `newQuorumVotesBPS` are never checked for the same value setting498: function _setQuorumVotesBPS(uint256newQuorumVotesBPS) external onlyOwner {
499: require(newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, "CultureIndex::_setQuorumVotesBPS: invalid quorum bps");
500: emitQuorumVotesBPSSet(quorumVotesBPS, newQuorumVotesBPS);
501:
502: quorumVotesBPS = newQuorumVotesBPS;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `entropyRateBps` and `_entropyRateBps` are never checked for the same value setting288: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
289: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
290:
291: emitEntropyRateBpsUpdated(entropyRateBps = _entropyRateBps);
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `creatorRateBps` and `_creatorRateBps` are never checked for the same value setting299: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
300: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
301:
302: emitCreatorRateBpsUpdated(creatorRateBps = _creatorRateBps);
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `creatorsAddress` and `_creatorsAddress` are never checked for the same value setting309: function setCreatorsAddress(address_creatorsAddress) externaloverride onlyOwner nonReentrant {
310: require(_creatorsAddress !=address(0), "Invalid address");
311:
312: emitCreatorsAddressUpdated(creatorsAddress = _creatorsAddress);
File: packages/revolution/src/AuctionHouse.sol
//@audit `creatorRateBps` and `_creatorRateBps` are never checked for the same value setting217: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
218: require(
219: _creatorRateBps >= minCreatorRateBps,
220: "Creator rate must be greater than or equal to minCreatorRateBps"221: );
222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
223: creatorRateBps = _creatorRateBps;
File: packages/revolution/src/AuctionHouse.sol
//@audit `entropyRateBps` and `_entropyRateBps` are never checked for the same value setting253: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
255:
256: entropyRateBps = _entropyRateBps;
File: packages/revolution/src/AuctionHouse.sol
//@audit `timeBuffer` and `_timeBuffer` are never checked for the same value setting277: function setTimeBuffer(uint256_timeBuffer) externaloverride onlyOwner {
278: timeBuffer = _timeBuffer;
File: packages/revolution/src/AuctionHouse.sol
//@audit `reservePrice` and `_reservePrice` are never checked for the same value setting287: function setReservePrice(uint256_reservePrice) externaloverride onlyOwner {
288: reservePrice = _reservePrice;
File: packages/revolution/src/AuctionHouse.sol
//@audit `minBidIncrementPercentage` and `_minBidIncrementPercentage` are never checked for the same value setting297: function setMinBidIncrementPercentage(uint8_minBidIncrementPercentage) externaloverride onlyOwner {
298: minBidIncrementPercentage = _minBidIncrementPercentage;
File: packages/revolution/src/VerbsToken.sol
//@audit `_contractURIHash` and `newContractURIHash` are never checked for the same value setting169: function setContractURIHash(stringmemorynewContractURIHash) external onlyOwner {
170: _contractURIHash = newContractURIHash;
File: packages/revolution/src/VerbsToken.sol
//@audit `minter` and `_minter` are never checked for the same value setting209: function setMinter(address_minter) externaloverride onlyOwner nonReentrant whenMinterNotLocked {
210: require(_minter !=address(0), "Minter cannot be zero address");
211: minter = _minter;
File: packages/revolution/src/VerbsToken.sol
//@audit `descriptor` and `_descriptor` are never checked for the same value setting230: function setDescriptor(
231: IDescriptorMinimal _descriptor232: ) externaloverride onlyOwner nonReentrant whenDescriptorNotLocked {
233: descriptor = _descriptor;
File: packages/revolution/src/VerbsToken.sol
//@audit `cultureIndex` and `_cultureIndex` are never checked for the same value setting252: function setCultureIndex(ICultureIndex _cultureIndex) external onlyOwner whenCultureIndexNotLocked nonReentrant {
253: cultureIndex = _cultureIndex;
File: packages/revolution/src/CultureIndex.sol
// @audit the @return is missing
@notice Utility function to verify a signature for a specific vote
@param from Vote from thisaddress
@param pieceIds Vote on this pieceId
@param deadline Deadline for the signature to be valid
@param v V component of signature
@param r R component of signature
@param s S component of signature
File: packages/revolution/src/CultureIndex.sol
// @audit the @return is missing
@notice Current quorum votes using ERC721 Total Supply, ERC721 Vote Weight, and ERC20 Total Supply
Differs from `GovernerBravo` which uses fixed amount
File: packages/revolution/src/NontransferableERC20Votes.sol
// @audit the @return is missing
@dev Returns the number of decimals used to get its user representation.
For example, if `decimals` equals `2`, a balance of `505` tokens should
be displayed to a user as `5.05` (`505/10**2`).
Tokens usually opt for a value of 18, imitating the relationship between
Ether and Wei. This is the default value returned by thisfunction, unless
it's overridden.NOTE: This information is only used for _display_ purposes: it inno way affects any of the arithmetic of the contract, including{IERC20-balanceOf} and {IERC20-transfer}.
File: packages/revolution/src/VerbsToken.sol
// @audit the @return is missing
@notice Mint a Verb to the minter.
@dev Call _mintTo with the to address(es).
File: packages/revolution/src/VerbsToken.sol
// @audit the @return is missing
@notice A distinct Uniform Resource Identifier (URI) for a given asset.
@dev See {IERC721Metadata-tokenURI}.
File: packages/revolution/src/VerbsToken.sol
// @audit the @return is missing
@notice Similar to `tokenURI`, but always serves a base64 encoded data URI
with the JSON contents directly inlined.
File: packages/revolution/src/VerbsToken.sol
// @audit the @return is missing
@notice Mint a Verb with `verbId` to the provided `to` address. Pulls the top voted art piece from the CultureIndex.
[N-52] Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability
While the compiler knows to optimize away the exponentiation, it's still better coding practice to use idioms that do not require compiler optimization, if they exist
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `10_000`289: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit `10_000`300: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit `10_000`254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.
[N-59] Critical functions should be a two step procedure
Critical functions in Solidity contracts should follow a two-step procedure to enhance security, minimize human error, and ensure proper access control. By dividing sensitive operations into distinct phases, such as initiation and confirmation, developers can introduce a safeguard against unintended actions or unauthorized access.
There are 15 instance(s) of this issue:
File: packages/revolution/src/AuctionHouse.sol
217: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `from` is not used101: function _transfer(addressfrom, addressto, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `to` is not used101: function _transfer(addressfrom, addressto, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `value` is not used101: function _transfer(addressfrom, addressto, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `owner` is not used141: function _approve(addressowner, addressspender, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `spender` is not used141: function _approve(addressowner, addressspender, uint256value) internaloverride {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit `value` is not used141: function _approve(addressowner, addressspender, uint256value) internaloverride {
[N-64] Use string.concat() on strings instead of abi.encodePacked() for clearer semantic meaning
Starting with version 0.8.12, Solidity has the string.concat() function, which allows one to concatenate a list of strings, without extra padding. Using this function rather than abi.encodePacked() makes the intended operation more clear, leading to less reviewer confusion.
[N-65] Constants should be defined rather than using magic numbers
Even assembly can benefit from using readable constants instead of hex/numeric literals
There are 30 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit Try to make a `constant` with `5` value160: require(uint8(metadata.mediaType) >0&&uint8(metadata.mediaType) <=5, "Invalid media type");
File: packages/revolution/src/CultureIndex.sol
//@audit Try to make a `constant` with `10_000` value190: require(totalBps ==10_000, "Total BPS must sum up to 10,000");
File: packages/revolution/src/CultureIndex.sol
//@audit Try to make a `constant` with `10_000` value234: newPiece.quorumVotes = (quorumVotesBPS * newPiece.totalVotesSupply) /10_000;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value173: uint256 toPayTreasury = (msgValueRemaining * (10_000- creatorRateBps)) /10_000;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value177: uint256 creatorDirectPayment = ((msgValueRemaining - toPayTreasury) * entropyRateBps) /10_000;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value217: require(bpsSum ==10_000, "bps must add up to 10_000");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value173: uint256 toPayTreasury = (msgValueRemaining * (10_000- creatorRateBps)) /10_000;
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value212: _mint(addresses[i], uint256((totalTokensForBuyers *int(basisPointSplits[i])) /10_000));
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value279: amount: int(((paymentAmount -computeTotalReward(paymentAmount)) * (10_000- creatorRateBps)) /10_000)
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value279: amount: int(((paymentAmount -computeTotalReward(paymentAmount)) * (10_000- creatorRateBps)) /10_000)
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value289: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit Try to make a `constant` with `10_000` value300: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `100` value181: msg.value>= _auction.amount + ((_auction.amount * minBidIncrementPercentage) /100),
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `10_000` value222: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `10_000` value235: require(_minCreatorRateBps <=10_000, "Min creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `10_000` value254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `10_000` value365: uint256 auctioneerPayment = (_auction.amount * (10_000- creatorRateBps)) /10_000;
File: packages/revolution/src/AuctionHouse.sol
//@audit Try to make a `constant` with `10_000` value365: uint256 auctioneerPayment = (_auction.amount * (10_000- creatorRateBps)) /10_000;
File: packages/protocol-rewards/src/abstract/RewardSplits.sol
//@audit Try to make a `constant` with `10_000` value57: builderReferralReward: (paymentAmountWei * BUILDER_REWARD_BPS) /10_000,
File: packages/protocol-rewards/src/abstract/RewardSplits.sol
//@audit Try to make a `constant` with `10_000` value58: purchaseReferralReward: (paymentAmountWei * PURCHASE_REFERRAL_BPS) /10_000,
File: packages/protocol-rewards/src/abstract/RewardSplits.sol
//@audit Try to make a `constant` with `10_000` value59: deployerReward: (paymentAmountWei * DEPLOYER_REWARD_BPS) /10_000,
File: packages/protocol-rewards/src/abstract/RewardSplits.sol
//@audit Try to make a `constant` with `10_000` value60: revolutionReward: (paymentAmountWei * REVOLUTION_REWARD_BPS) /10_000
The SMTChecker is a valuable tool for Solidity developers as it helps detect potential vulnerabilities and logical errors in the contract's code. By utilizing Satisfiability Modulo Theories (SMT) solvers, it can reason about the potential states a contract can be in, and therefore, identify conditions that could lead to undesirable behavior. This automatic formal verification can catch issues that might otherwise be missed in manual code reviews or standard testing, enhancing the overall contract's security and reliability.
Due to multiple if, loop and conditions the following functions has a very complex controle flow that could make auditing very difficult to cover all possible path
Therefore, consider breaking down these blocks into more manageable units, by splitting things into utility functions, by reducing nesting, and by using early returns
There are 2 instance(s) of this issue:
File: packages/revolution/src/ERC20TokenEmitter.sol
152: function buyToken(
153: address[] calldataaddresses,
154: uint[] calldatabasisPointSplits,
155: ProtocolRewardAddresses calldataprotocolRewardsRecipients156: ) publicpayable nonReentrant whenNotPaused returns (uint256tokensSoldWad) {
157: //prevent treasury from paying itself158: require(msg.sender!= treasury &&msg.sender!= creatorsAddress, "Funds recipient cannot buy tokens");
159:
160: require(msg.value>0, "Must send ether");
161: // ensure the same number of addresses and bps162: require(addresses.length== basisPointSplits.length, "Parallel arrays required");
163:
164: // Get value left after protocol rewards165: uint256 msgValueRemaining =_handleRewardsAndGetValueToSend(
166: msg.value,
167: protocolRewardsRecipients.builder,
168: protocolRewardsRecipients.purchaseReferral,
169: protocolRewardsRecipients.deployer
170: );
171:
172: //Share of purchase amount to send to treasury173: uint256 toPayTreasury = (msgValueRemaining * (10_000- creatorRateBps)) /10_000;
174:
175: //Share of purchase amount to reserve for creators176: //Ether directly sent to creators177: uint256 creatorDirectPayment = ((msgValueRemaining - toPayTreasury) * entropyRateBps) /10_000;
178: //Tokens to emit to creators179: int totalTokensForCreators = ((msgValueRemaining - toPayTreasury) - creatorDirectPayment) >0180: ?getTokenQuoteForEther((msgValueRemaining - toPayTreasury) - creatorDirectPayment)
181: : int(0);
182:
183: // Tokens to emit to buyers184: int totalTokensForBuyers = toPayTreasury >0?getTokenQuoteForEther(toPayTreasury) : int(0);
185:
186: //Transfer ETH to treasury and update emitted187: emittedTokenWad += totalTokensForBuyers;
188: if (totalTokensForCreators >0) emittedTokenWad += totalTokensForCreators;
189:
190: //Deposit funds to treasury191: (boolsuccess, ) = treasury.call{ value: toPayTreasury }(newbytes(0));
192: require(success, "Transfer failed.");
193:
194: //Transfer ETH to creators195: if (creatorDirectPayment >0) {
196: (success, ) = creatorsAddress.call{ value: creatorDirectPayment }(newbytes(0));
197: require(success, "Transfer failed.");
198: }
199:
200: //Mint tokens for creators201: if (totalTokensForCreators >0&& creatorsAddress !=address(0)) {
202: _mint(creatorsAddress, uint256(totalTokensForCreators));
203: }
204:
205: uint256 bpsSum =0;
206:
207: //Mint tokens to buyers208:
209: for (uint256 i =0; i < addresses.length; i++) {
210: if (totalTokensForBuyers >0) {
211: // transfer tokens to address212: _mint(addresses[i], uint256((totalTokensForBuyers *int(basisPointSplits[i])) /10_000));
213: }
214: bpsSum += basisPointSplits[i];
215: }
216:
217: require(bpsSum ==10_000, "bps must add up to 10_000");
218:
219: emitPurchaseFinalized(
220: msg.sender,
221: msg.value,
222: toPayTreasury,
223: msg.value- msgValueRemaining,
224: uint256(totalTokensForBuyers),
225: uint256(totalTokensForCreators),
226: creatorDirectPayment
227: );
228:
229: returnuint256(totalTokensForBuyers);
230: }
The functions below take in an unbounded array, and make function calls for entries in the array. While the function will revert if it eventually runs out of gas, it may be a nicer user experience to require() that the length of the array is below some reasonable maximum, so that the user doesn't have to use up a full transaction's gas only to see that the transaction reverts.
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
//@audit array name creatorArray209: function createPiece(
210: ArtPieceMetadata calldatametadata,
211: CreatorBps[] calldatacreatorArray212: ) publicreturns (uint256) {
213: uint256 creatorArrayLength =validateCreatorsArray(creatorArray);
214:
215: // Validate the media type and associated data216: validateMediaType(metadata);
217:
218: uint256 pieceId = _currentPieceId++;
219:
220: /// @dev Insert the new piece into the max heap221: maxHeap.insert(pieceId, 0);
222:
223: ArtPiece storage newPiece = pieces[pieceId];
224:
225: newPiece.pieceId = pieceId;
226: newPiece.totalVotesSupply =_calculateVoteWeight(
227: erc20VotingToken.totalSupply(),
228: erc721VotingToken.totalSupply()
229: );
230: newPiece.totalERC20Supply = erc20VotingToken.totalSupply();
231: newPiece.metadata = metadata;
232: newPiece.sponsor =msg.sender;
233: newPiece.creationBlock =block.number;
234: newPiece.quorumVotes = (quorumVotesBPS * newPiece.totalVotesSupply) /10_000;
235:
236: for (uint i; i < creatorArrayLength; i++) {
237: newPiece.creators.push(creatorArray[i]);
238: }
239:
240: emitPieceCreated(pieceId, msg.sender, metadata, newPiece.quorumVotes, newPiece.totalVotesSupply);
241:
242: // Emit an event for each creator243: for (uint i; i < creatorArrayLength; i++) {
244: emitPieceCreatorAdded(pieceId, creatorArray[i].creator, msg.sender, creatorArray[i].bps);
245: }
246:
247: return newPiece.pieceId;
248: }
File: packages/revolution/src/ERC20TokenEmitter.sol
237: function buyTokenQuote(uint256amount) publicviewreturns (intspentY) {
238: require(amount >0, "Amount must be greater than 0");
239: // Note: By using toDaysWadUnsafe(block.timestamp - startTime) we are establishing that 1 "unit of time" is 1 day.240: // solhint-disable-next-line not-rely-on-time241: return242: vrgdac.xToY({
243: timeSinceStart: toDaysWadUnsafe(block.timestamp- startTime),
244: sold: emittedTokenWad,
245: amount: int(amount)
246: });
247: }
The contracts should expose an interface so that other projects can more easily integrate with it, without having to develop their own non-standard variants.
There are 15 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
119: function insert(uint256itemId, uint256value) public onlyAdmin {
[N-76] Contract declarations should have NatSpec @notice annotations
There are 7 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
11: /// @title MaxHeap implementation in Solidity12: /// @dev This contract implements a Max Heap data structure with basic operations13: /// @author Written by rocketman and gpt414: contractMaxHeapisVersionedContract, UUPS, Ownable2StepUpgradeable, ReentrancyGuardUpgradeable {
Here is an example of camelCase/lowerCamelCase and other types:
'helloWorld' is a CamelCase
'HelloWorld' is Not CamelCase (PascalCase)
'hello_world' is Not CamelCase (snake_case)
For more details
There are 8 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
//@audit `` is not in CamelCase30: constructor(address_manager) payable initializer {
File: packages/revolution/src/CultureIndex.sol
//@audit `_setQuorumVotesBPS` is not in CamelCase498: function _setQuorumVotesBPS(uint256newQuorumVotesBPS) external onlyOwner {
File: packages/revolution/src/libs/VRGDAC.sol
//@audit `` is not in CamelCase28: constructor(int256_targetPrice, int256_priceDecayPercent, int256_perTimeUnit) {
[N-78] Expressions for constant values should use immutable rather than constant
While it does not save gas for some simple binary expressions because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand. There is a difference between constant variables and immutable variables, and they should each be used in their appropriate contexts. constants should be used for literal values written into the code, and immutable variables should be used for expressions, or values calculated in, or passed into the constructor.
File: packages/revolution/src/CultureIndex.sol
//@audit `validateMediaType` is not in CamelCase159: function validateMediaType(ArtPieceMetadata calldatametadata) internalpure {
File: packages/revolution/src/CultureIndex.sol
//@audit `validateCreatorsArray` is not in CamelCase179: function validateCreatorsArray(CreatorBps[] calldatacreatorArray) internalpurereturns (uint256) {
File: packages/revolution/src/libs/VRGDAC.sol
//@audit `pIntegral` is not in CamelCase86: function pIntegral(int256timeSinceStart, int256sold) internalviewreturns (int256) {
File: packages/protocol-rewards/src/abstract/RewardSplits.sol
//@audit `` is not in CamelCase29: constructor(address_protocolRewards, address_revolutionRewardRecipient) payable {
function func(address a, address) -> function func(address a, address /* b */)
There are 23 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
//@audit parameter number 0 starting from left need inline comment78: function parent(uint256pos) privatepurereturns (uint256) {
File: packages/revolution/src/CultureIndex.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment307: function _vote(uint256pieceId, addressvoter) internal {
File: packages/revolution/src/CultureIndex.sol
//@audit parameter number 0 starting from left need inline comment451: function getPieceById(uint256pieceId) publicviewreturns (ArtPiece memory) {
File: packages/revolution/src/CultureIndex.sol
//@audit parameter number 0 starting from left need inline comment461: function getVote(uint256pieceId, addressvoter) publicviewreturns (Vote memory) {
File: packages/revolution/src/CultureIndex.sol
//@audit parameter number 0 starting from left need inline comment498: function _setQuorumVotesBPS(uint256newQuorumVotesBPS) external onlyOwner {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment94: function transfer(address, uint256) publicvirtualoverridereturns (bool) {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment//@audit parameter number 2 starting from left need inline comment108: function transferFrom(address, address, uint256) publicvirtualoverridereturns (bool) {
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment115: function approve(address, uint256) publicvirtualoverridereturns (bool) {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment237: function buyTokenQuote(uint256amount) publicviewreturns (intspentY) {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment254: function getTokenQuoteForEther(uint256etherAmount) publicviewreturns (intgainedX) {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment271: function getTokenQuoteForPayment(uint256paymentAmount) externalviewreturns (intgainedX) {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment288: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment299: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit parameter number 0 starting from left need inline comment309: function setCreatorsAddress(address_creatorsAddress) externaloverride onlyOwner nonReentrant {
File: packages/revolution/src/AuctionHouse.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment171: function createBid(uint256verbId, addressbidder) externalpayableoverride nonReentrant {
File: packages/revolution/src/AuctionHouse.sol
//@audit parameter number 0 starting from left need inline comment217: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
File: packages/revolution/src/AuctionHouse.sol
//@audit parameter number 0 starting from left need inline comment233: function setMinCreatorRateBps(uint256_minCreatorRateBps) external onlyOwner {
File: packages/revolution/src/AuctionHouse.sol
//@audit parameter number 0 starting from left need inline comment253: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
File: packages/revolution/src/VerbsToken.sol
//@audit parameter number 0 starting from left need inline comment//@audit parameter number 1 starting from left need inline comment130: function initialize(
File: packages/revolution/src/VerbsToken.sol
//@audit parameter number 0 starting from left need inline comment209: function setMinter(address_minter) externaloverride onlyOwner nonReentrant whenMinterNotLocked {
File: packages/revolution/src/VerbsToken.sol
//@audit parameter number 0 starting from left need inline comment273: function getArtPieceById(uint256verbId) publicviewreturns (ICultureIndex.ArtPiece memory) {
Consider using formal verification to mathematically prove that your code does what is intended, and does not have any edge cases with unexpected behavior. The solidity compiler itself has this functionality built in
There are 1 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
@audit Should implement invariant tests1:
[N-84] Missing zero address check in functions with address parameters
Adding a zero address check for each address type parameter can prevent errors.
There are 28 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
//@audit , are not checked78: function parent(uint256pos) privatepurereturns (uint256) {
79: require(pos !=0, "Position should not be zero");
File: packages/revolution/src/MaxHeap.sol
//@audit itemId, value, are not checked119: function insert(uint256itemId, uint256value) public onlyAdmin {
120: heap[size] = itemId;
File: packages/revolution/src/MaxHeap.sol
//@audit itemId, are not checked136: function updateValue(uint256itemId, uint256newValue) public onlyAdmin {
137: uint256 position = positionMapping[itemId];
File: packages/revolution/src/CultureIndex.sol
//@audit , are not checked498: function _setQuorumVotesBPS(uint256newQuorumVotesBPS) external onlyOwner {
499: require(newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, "CultureIndex::_setQuorumVotesBPS: invalid quorum bps");
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit , are not checked94: function transfer(address, uint256) publicvirtualoverridereturns (bool) {
95: revertTRANSFER_NOT_ALLOWED();
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit value, are not checked101: function _transfer(addressfrom, addressto, uint256value) internaloverride {
102: revertTRANSFER_NOT_ALLOWED();
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit , are not checked115: function approve(address, uint256) publicvirtualoverridereturns (bool) {
116: revertTRANSFER_NOT_ALLOWED();
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit value, are not checked141: function _approve(addressowner, addressspender, uint256value) internaloverride {
142: revertTRANSFER_NOT_ALLOWED();
File: packages/revolution/src/NontransferableERC20Votes.sol
//@audit value, are not checked155: function _spendAllowance(addressowner, addressspender, uint256value) internalvirtualoverride {
156: revertTRANSFER_NOT_ALLOWED();
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit , are not checked237: function buyTokenQuote(uint256amount) publicviewreturns (intspentY) {
238: require(amount >0, "Amount must be greater than 0");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit , are not checked254: function getTokenQuoteForEther(uint256etherAmount) publicviewreturns (intgainedX) {
255: require(etherAmount >0, "Ether amount must be greater than 0");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit , are not checked271: function getTokenQuoteForPayment(uint256paymentAmount) externalviewreturns (intgainedX) {
272: require(paymentAmount >0, "Payment amount must be greater than 0");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit , are not checked288: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
289: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
File: packages/revolution/src/ERC20TokenEmitter.sol
//@audit , are not checked299: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
300: require(_creatorRateBps <=10_000, "Creator rate must be less than or equal to 10_000");
File: packages/revolution/src/AuctionHouse.sol
//@audit , are not checked217: function setCreatorRateBps(uint256_creatorRateBps) external onlyOwner {
218: require(
File: packages/revolution/src/AuctionHouse.sol
//@audit , are not checked233: function setMinCreatorRateBps(uint256_minCreatorRateBps) external onlyOwner {
234: require(_minCreatorRateBps <= creatorRateBps, "Min creator rate must be less than or equal to creator rate");
File: packages/revolution/src/AuctionHouse.sol
//@audit , are not checked253: function setEntropyRateBps(uint256_entropyRateBps) external onlyOwner {
254: require(_entropyRateBps <=10_000, "Entropy rate must be less than or equal to 10_000");
[N-85] Use a struct to encapsulate multiple function parameters
If a function has too many parameters, replacing them with a struct can improve code readability and maintainability, increase reusability, and reduce the likelihood of errors when passing the parameters.
File: packages/revolution/src/CultureIndex.sol
151: /**152: * Validates the media type and associated data.153: * @param metadata The metadata associated with the art piece.154: *155: * Requirements:156: * - The media type must be one of the defined types in the MediaType enum.157: * - The corresponding media data must not be empty.158: */
File: packages/revolution/src/NontransferableERC20Votes.sol
74: /**75: * @dev Returns the number of decimals used to get its user representation.76: * For example, if `decimals` equals `2`, a balance of `505` tokens should77: * be displayed to a user as `5.05` (`505 / 10 ** 2`).78: *79: * Tokens usually opt for a value of 18, imitating the relationship between80: * Ether and Wei. This is the default value returned by this function, unless81: * it's overridden.82: *83: * NOTE: This information is only used for _display_ purposes: it in84: * no way affects any of the arithmetic of the contract, including85: * {IERC20-balanceOf} and {IERC20-transfer}.86: */
File: packages/revolution/src/NontransferableERC20Votes.sol
119: /**120: * @dev Creates a `value` amount of tokens and assigns them to `account`, by transferring it from address(0).121: * Relies on the `_update` mechanism122: *123: * Emits a {Transfer} event with `from` set to the zero address.124: *125: * NOTE: This function is not virtual, {_update} should be overridden instead.126: */
File: packages/revolution/src/MaxHeap.sol
50: /**51: * @notice Initializes the maxheap contract52: * @param _initialOwner The initial owner of the contract53: * @param _admin The contract that is allowed to update the data store54: */
File: packages/revolution/src/MaxHeap.sol
75: /// @notice Get the parent index of a given position76: /// @param pos The position for which to find the parent77: /// @return The index of the parent node
File: packages/revolution/src/MaxHeap.sol
83: /// @notice Swap two nodes in the heap84: /// @param fpos The position of the first node85: /// @param spos The position of the second node
File: packages/revolution/src/CultureIndex.sol
100: /**101: * @notice Initializes a token's metadata descriptor102: * @param _erc20VotingToken The address of the ERC20 voting token, commonly referred to as "points"103: * @param _erc721VotingToken The address of the ERC721 voting token, commonly the dropped art pieces104: * @param _initialOwner The owner of the contract, allowed to drop pieces. Commonly updated to the AuctionHouse105: * @param _maxHeap The address of the max heap contract106: * @param _dropperAdmin The address that can drop new art pieces107: * @param _cultureIndexParams The CultureIndex settings108: */
File: packages/revolution/src/CultureIndex.sol
151: /**152: * Validates the media type and associated data.153: * @param metadata The metadata associated with the art piece.154: *155: * Requirements:156: * - The media type must be one of the defined types in the MediaType enum.157: * - The corresponding media data must not be empty.158: */
File: packages/revolution/src/CultureIndex.sol
170: /**171: * @notice Checks the total basis points from an array of creators and returns the length172: * @param creatorArray An array of Creator structs containing address and basis points.173: * @return Returns the total basis points calculated from the array of creators.174: *175: * Requirements:176: * - The `creatorArray` must not contain any zero addresses.177: * - The function will return the length of the `creatorArray`.178: */
File: packages/revolution/src/CultureIndex.sol
195: /**196: * @notice Creates a new piece of art with associated metadata and creators.197: * @param metadata The metadata associated with the art piece, including name, description, image, and optional animation URL.198: * @param creatorArray An array of creators who contributed to the piece, along with their respective basis points that must sum up to 10,000.199: * @return Returns the unique ID of the newly created art piece.200: *201: * Emits a {PieceCreated} event for the newly created piece.202: * Emits a {PieceCreatorAdded} event for each creator added to the piece.203: *204: * Requirements:205: * - `metadata` must include name, description, and image. Animation URL is optional.206: * - `creatorArray` must not contain any zero addresses.207: * - The sum of basis points in `creatorArray` must be exactly 10,000.208: */
File: packages/revolution/src/CultureIndex.sol
250: /**251: * @notice Checks if a specific voter has already voted for a given art piece.252: * @param pieceId The ID of the art piece.253: * @param voter The address of the voter.254: * @return A boolean indicating if the voter has voted for the art piece.255: */
File: packages/revolution/src/CultureIndex.sol
260: /**261: * @notice Returns the voting power of a voter at the current block.262: * @param account The address of the voter.263: * @return The voting power of the voter.264: */
File: packages/revolution/src/CultureIndex.sol
269: /**270: * @notice Returns the voting power of a voter at the current block.271: * @param account The address of the voter.272: * @return The voting power of the voter.273: */
File: packages/revolution/src/CultureIndex.sol
278: /**279: * @notice Calculates the vote weight of a voter.280: * @param erc20Balance The ERC20 balance of the voter.281: * @param erc721Balance The ERC721 balance of the voter.282: * @return The vote weight of the voter.283: */
File: packages/revolution/src/CultureIndex.sol
360: /// @notice Execute a vote via signature361: /// @param from Vote from this address362: /// @param pieceIds Vote on this list of pieceIds363: /// @param deadline Deadline for the signature to be valid364: /// @param v V component of signature365: /// @param r R component of signature366: /// @param s S component of signature
File: packages/revolution/src/CultureIndex.sol
382: /// @notice Execute a batch of votes via signature, each with their own signature383: /// @param from Vote from these addresses384: /// @param pieceIds Vote on these lists of pieceIds385: /// @param deadline Deadlines for the signature to be valid386: /// @param v V component of signatures387: /// @param r R component of signatures388: /// @param s S component of signatures
File: packages/revolution/src/CultureIndex.sol
412: /// @notice Utility function to verify a signature for a specific vote413: /// @param from Vote from this address414: /// @param pieceIds Vote on this pieceId415: /// @param deadline Deadline for the signature to be valid416: /// @param v V component of signature417: /// @param r R component of signature418: /// @param s S component of signature
File: packages/revolution/src/CultureIndex.sol
446: /**447: * @notice Fetch an art piece by its ID.448: * @param pieceId The ID of the art piece.449: * @return The ArtPiece struct associated with the given ID.450: */
File: packages/revolution/src/CultureIndex.sol
456: /**457: * @notice Fetch the list of votes for a given art piece.458: * @param pieceId The ID of the art piece.459: * @return An array of Vote structs for the given art piece ID.460: */
File: packages/revolution/src/CultureIndex.sol
466: /**467: * @notice Fetch the top-voted art piece.468: * @return The ArtPiece struct of the top-voted art piece.469: */
File: packages/revolution/src/CultureIndex.sol
505: /**506: * @notice Current quorum votes using ERC721 Total Supply, ERC721 Vote Weight, and ERC20 Total Supply507: * Differs from `GovernerBravo` which uses fixed amount508: */
File: packages/revolution/src/NontransferableERC20Votes.sol
62: /// @notice Initializes a DAO's ERC-20 governance token contract63: /// @param _initialOwner The address of the initial owner64: /// @param _erc20TokenParams The params of the token
File: packages/revolution/src/ERC20TokenEmitter.sol
145: /**146: * @notice A payable function that allows a user to buy tokens for a list of addresses and a list of basis points to split the token purchase between.147: * @param addresses The addresses to send purchased tokens to.148: * @param basisPointSplits The basis points of the purchase to send to each address.149: * @param protocolRewardsRecipients The addresses to pay the builder, purchaseRefferal, and deployer rewards to150: * @return tokensSoldWad The amount of tokens sold in wad units.151: */
File: packages/revolution/src/ERC20TokenEmitter.sol
232: /**233: * @notice Returns the amount of wei that would be spent to buy an amount of tokens. Does not take into account the protocol rewards.234: * @param amount the amount of tokens to buy.235: * @return spentY The cost in wei of the token purchase.236: */
File: packages/revolution/src/ERC20TokenEmitter.sol
249: /**250: * @notice Returns the amount of tokens that would be emitted for an amount of wei. Does not take into account the protocol rewards.251: * @param etherAmount the payment amount in wei.252: * @return gainedX The amount of tokens that would be emitted for the payment amount.253: */
File: packages/revolution/src/ERC20TokenEmitter.sol
266: /**267: * @notice Returns the amount of tokens that would be emitted for the payment amount, taking into account the protocol rewards.268: * @param paymentAmount the payment amount in wei.269: * @return gainedX The amount of tokens that would be emitted for the payment amount.270: */
File: packages/revolution/src/AuctionHouse.sol
416: /// @notice Transfer ETH/WETH from the contract417: /// @param _to The recipient address418: /// @param _amount The amount transferring
File: packages/revolution/src/VerbsToken.sol
124: /// @notice Initializes a DAO's ERC-721 token contract125: /// @param _minter The address of the minter126: /// @param _initialOwner The address of the initial owner127: /// @param _descriptor The address of the token URI descriptor128: /// @param _cultureIndex The address of the CultureIndex contract129: /// @param _erc721TokenParams The name, symbol, and contract metadata of the token
File: packages/revolution/src/VerbsToken.sol
197: /**198: * @notice Similar to `tokenURI`, but always serves a base64 encoded data URI199: * with the JSON contents directly inlined.200: */
File: packages/revolution/src/VerbsToken.sol
268: /**269: * @notice Fetch an art piece by its ID.270: * @param verbId The ID of the art piece.271: * @return The ArtPiece struct associated with the given ID.272: */
File: packages/revolution/src/VerbsToken.sol
278: /**279: * @notice Mint a Verb with `verbId` to the provided `to` address. Pulls the top voted art piece from the CultureIndex.280: */
File: packages/revolution/src/libs/VRGDAC.sol
24: /// @notice Sets target price and per time unit price decay for the VRGDA.25: /// @param _targetPrice The target price for a token if sold on pace, scaled by 1e18.26: /// @param _priceDecayPercent The percent price decays per unit of time with no sales, scaled by 1e18.27: /// @param _perTimeUnit The number of tokens to target selling in 1 full unit of time, scaled by 1e18.
[N-89] Use custom errors rather than revert()/require() strings for better readability
Custom errors are available from solidity version 0.8.4. Custom errors are more easily processed in try-catch blocks, and are easier to re-use and maintain.
There are 75 instance(s) of this issue:
File: packages/revolution/src/MaxHeap.sol
42: require(msg.sender== admin, "Sender is not the admin");
File: packages/revolution/src/CultureIndex.sol
398: require(
399: len == pieceIds.length&& len == deadline.length&& len == v.length&& len == r.length&& len == s.length,
400: "Array lengths must match"401: );
File: packages/revolution/src/CultureIndex.sol
523: require(totalVoteWeights[piece.pieceId] >= piece.quorumVotes, "Does not meet quorum votes to be dropped.");
File: packages/revolution/src/AuctionHouse.sol
129: require(
130: _auctionParams.creatorRateBps >= _auctionParams.minCreatorRateBps,
131: "Creator rate must be greater than or equal to the creator rate"132: );
File: packages/revolution/src/AuctionHouse.sol
218: require(
219: _creatorRateBps >= minCreatorRateBps,
220: "Creator rate must be greater than or equal to minCreatorRateBps"221: );
File: packages/revolution/src/AuctionHouse.sol
234: require(_minCreatorRateBps <= creatorRateBps, "Min creator rate must be less than or equal to creator rate");
File: packages/revolution/src/MaxHeap.sol
178: /// @notice Ensures the caller is authorized to upgrade the contract and that the new implementation is valid179: /// @dev This function is called in `upgradeTo` & `upgradeToAndCall`180: /// @param _newImpl The new implementation address
File: packages/revolution/src/CultureIndex.sol
260: /**261: * @notice Returns the voting power of a voter at the current block.262: * @param account The address of the voter.263: * @return The voting power of the voter.264: */
File: packages/revolution/src/CultureIndex.sol
269: /**270: * @notice Returns the voting power of a voter at the current block.271: * @param account The address of the voter.272: * @return The voting power of the voter.273: */
File: packages/revolution/src/CultureIndex.sol
540: /// @notice Ensures the caller is authorized to upgrade the contract and that the new implementation is valid541: /// @dev This function is called in `upgradeTo` & `upgradeToAndCall`542: /// @param _newImpl The new implementation address
File: packages/revolution/src/NontransferableERC20Votes.sol
74: /**75: * @dev Returns the number of decimals used to get its user representation.76: * For example, if `decimals` equals `2`, a balance of `505` tokens should77: * be displayed to a user as `5.05` (`505 / 10 ** 2`).78: *79: * Tokens usually opt for a value of 18, imitating the relationship between80: * Ether and Wei. This is the default value returned by this function, unless81: * it's overridden.82: *83: * NOTE: This information is only used for _display_ purposes: it in84: * no way affects any of the arithmetic of the contract, including85: * {IERC20-balanceOf} and {IERC20-transfer}.86: */
File: packages/revolution/src/NontransferableERC20Votes.sol
119: /**120: * @dev Creates a `value` amount of tokens and assigns them to `account`, by transferring it from address(0).121: * Relies on the `_update` mechanism122: *123: * Emits a {Transfer} event with `from` set to the zero address.124: *125: * NOTE: This function is not virtual, {_update} should be overridden instead.126: */
File: packages/revolution/src/ERC20TokenEmitter.sol
127: /**128: * @notice Pause the contract.129: * @dev This function can only be called by the owner when the130: * contract is unpaused.131: */
File: packages/revolution/src/ERC20TokenEmitter.sol
136: /**137: * @notice Unpause the token emitter.138: * @dev This function can only be called by the owner when the139: * contract is paused.140: */
File: packages/revolution/src/ERC20TokenEmitter.sol
305: /**306: * @notice Set the creators address to pay the creatorRate to. Can be a contract.307: * @dev Only callable by the owner.308: */
File: packages/revolution/src/AuctionHouse.sol
157: /**158: * @notice Settle the current auction.159: * @dev This function can only be called when the contract is paused.160: */
File: packages/revolution/src/AuctionHouse.sol
165: /**166: * @notice Create a bid for a Verb, with a given amount.167: * @dev This contract only accepts payment in ETH.168: * @param verbId The ID of the Verb to bid on.169: * @param bidder The address of the bidder.170: */
File: packages/revolution/src/AuctionHouse.sol
202: /**203: * @notice Pause the Verbs auction house.204: * @dev This function can only be called by the owner when the205: * contract is unpaused. While no new auctions can be started when paused,206: * anyone can settle an ongoing auction.207: */
File: packages/revolution/src/AuctionHouse.sol
260: /**261: * @notice Unpause the Verbs auction house.262: * @dev This function can only be called by the owner when the263: * contract is paused. If required, this function will start a new auction.264: */
File: packages/revolution/src/AuctionHouse.sol
293: /**294: * @notice Set the auction minimum bid increment percentage.295: * @dev Only callable by the owner.296: */
File: packages/revolution/src/AuctionHouse.sol
449: /// @notice Ensures the caller is authorized to upgrade the contract and the new implementation is valid450: /// @dev This function is called in `upgradeTo` & `upgradeToAndCall`451: /// @param _newImpl The new implementation address
File: packages/revolution/src/VerbsToken.sol
189: /**190: * @notice A distinct Uniform Resource Identifier (URI) for a given asset.191: * @dev See {IERC721Metadata-tokenURI}.192: */
File: packages/revolution/src/VerbsToken.sol
197: /**198: * @notice Similar to `tokenURI`, but always serves a base64 encoded data URI199: * with the JSON contents directly inlined.200: */
File: packages/revolution/src/VerbsToken.sol
205: /**206: * @notice Set the token minter.207: * @dev Only callable by the owner when not locked.208: */
File: packages/revolution/src/VerbsToken.sol
216: /**217: * @notice Lock the minter.218: * @dev This cannot be reversed and is only callable by the owner when not locked.219: */
File: packages/revolution/src/VerbsToken.sol
226: /**227: * @notice Set the token URI descriptor.228: * @dev Only callable by the owner when not locked.229: */
File: packages/revolution/src/VerbsToken.sol
238: /**239: * @notice Lock the descriptor.240: * @dev This cannot be reversed and is only callable by the owner when not locked.241: */
File: packages/revolution/src/VerbsToken.sol
258: /**259: * @notice Lock the CultureIndex260: * @dev This cannot be reversed and is only callable by the owner when not locked.261: */
[N-91] Multiple mappings with same keys can be combined into a single struct mapping for readability
Well-organized data structures make code reviews easier, which may lead to fewer bugs. Consider combining related mappings into mappings to structs, so it's clear what data is related.
There are 2 instance(s) of this issue:
File: packages/revolution/src/CultureIndex.sol
33: mapping(address=>uint256) public nonces;
The directive using A for B can be used to attach functions (A) as operators to user-defined value types or as member functions to any type (B). The member functions receive the object they are called on as their first parameter (like the self variable in Python). The operator functions receive operands as parameters. Doing so improves readability, makes debugging easier, and promotes modularity and reusability in the code.
[N-96] [Solidity]: All verbatim blocks are considered identical by deduplicator and can incorrectly be unified
The block deduplicator is a step of the opcode-based optimizer which identifies equivalent assembly blocks and merges them into a single one. However, when blocks contained verbatim, their comparison was performed incorrectly, leading to the collapse of assembly blocks which are identical except for the contents of the verbatim items. Since verbatim is only available in Yul, compilation of Solidity sources is not affected. For more details check the following link