![]() ![]() This means that the owner can deploy a new contract at any time and take every NFT and all authorized payment tokens owned by everybody that has listed NFTs for sale on the marketplace. Recommendation: determine the original intention of this statement, and a) remove contract upgradeability, b) carefully implement renouncing, or c) remove the comment. This is not implemented in the deployment script.įurthermore, if a wallet is renounced, this may prevent further contract upgrades, obviating the need to use upgradeable contracts in the first place. Make sure that ownership of deployment wallet has been renounced Recommendation: remove any backwards compatibility shims, or if this is indeed a proxy upgrade of an existing contract, clearly document this and tell me right away! □ Make sure that ownership of deployment wallet has been renounced But the deployment scripts, the Whitepaper and discussion at start of engagement indicate this is a "deployment" not an "upgrade." Specifically:īut we ultimately decided to open a second marketplace The code makes two references to backwards compatibility. □ Reference is made to "backwards compatibility" Recommendation: clearly explain the circumstances where a listing could or could not be applicable. Somebody buys at the old price which is below market value.The value of the token increases significantly.The seller believes their listing is dead. ![]() This statement is incorrect because the listing could become "revalidated" if the token is changed back, whereas the wording "invalidated" implies that the process is permanent.Ī specification scenario could result in customer harm: If the token used for buying/selling has changed since the listing was created, this effectively blocks all the old listings with the old payment tokens from being bought. □ Incorrect note about listing "blocking"Ī comment in the code advertises a condition where listings could become invalidated.Įnsure the buyer, the seller, and the collection all agree on the token to be used for the purchase. ![]() Resolved and other less-than-material findings have no emoji marking. An informational note is marked with an information symbol ( ℹ️). Critical findings that are not resolved are marked with the bomb symbol ( □) and other unresolved material findings are marked with the warning symbol ( ⚠️). FindingsĮach finding is listed and is organized by topic. Quality and correctness of automated testing approaches and deployment strategy is excluded from the scope. Although certain of these concepts are discussed below where there is a risk of customer confusion or harm. This review does not consider the applicability of this project to any specific jurisdiction's laws. Although certain notes about the contracts used are provided below.Ĭertain comments in the Trove Whitepaper discussing "money", "currency", "pricing", "capital", coordinated seller behavior ("temperature") and "", forward looking statements regarding revenue expectations, and similar are excluded from this review. OpenZeppelin Contacts Upgradable (version 4.5.2) were specifically excluded from the scope. Sensitivity was set as "anything we should fix or that you wouldn't want a million people to read". Any deviation from this standard, or deficiency against relevant best programming practices would be an in-scope review finding. The review was performed for the purpose of comparing Trove Marketplace against the plain English meaning of comments and function names, the provided documentation in the README, documentation posted in the Trove Marketplace Whitepaper (version 01.2022) (SHA-256 hash: 2788fdd66f241ac672e6d04e73b416ecff234b174841c7cbe764fc55ba129b8b09ca09d2fa53d042b7fd1617bf38c9c2) and a general understanding of NFT marketplaces. Due to limitations of the engagement, the review does not include functional testing (human using compiled program). The scope of review included code-level review (human reading source code), automated code review, (computer reading source code) and unit testing (computer running small-scope tests). This is a review of Trove Marketplace, pre-release version, "vinny/collection-level-currency" branch. Thank you for the opportunity to participate in this code review! Review scope I can turn these into bite-sized issues in this or the other repo as appropriate. I'm not quite sure if this branch is intended to merge into the master branch here or to a forked repository. ![]()
0 Comments
Leave a Reply. |
AuthorWrite something about yourself. No need to be fancy, just an overview. ArchivesCategories |