The team of AsyncArt contracted byterocket to conduct a smart contract audit of the NFT Auction contract (“Unstoppable Auctions”) for AsyncArt. AsyncArt is developing and providing a novel platform, allowing artists and users to experience art uniquely through modifiable NFTs. Art pieces are separated into master layers and controller layers, which users may control to alter the appearance of the NFT. This newest contract will allow users and artists to sell their NFTs in auctions or sales with ETH or arbitrary ERC20 tokens.
The functionality is contained in the NFTAuction.sol file, with ERC721 (NFT) and ERC20 related imports.
The team of byterocket reviewed and audited the above smart contracts in the course of this audit. We started on the 8th of September and finished on the 17th of September.
The audit included the following services:
byterocket gained access to the code via their public GitHub repository. We initially based the audit on the master branch’s state on September 16th, 2021 (commit hash d68bd0acf80ff45f322b251c2eef500f3679ab8d).
We conducted a manual multi-pass code review of the smart contracts mentioned in section (1). Three different people went through the smart contract independently and compared their results in multiple concluding discussions.
These contracts are written according to the latest standards used within the Ethereum community and the Solidity community’s best practices. The naming of variables is very logical and understandable, which results in the contract being useful to understand. The code is very well documented and up to the latest standards.
On the code level, we found no bugs or flaws. A further check with multiple automated reviewing tools (MythX, Slither, Manticore, and different fuzzing tools) did not find any additional bugs besides some common false positives.
This is more of a discussion point, not a bug or vulnerability, hence it is listed under miscellaneous. In the payout function starting in line 1212, an ETH-based payment is tried to be settled with 20.000 gas. If it fails, the ETH amount will be added to the failedTransferCredits variable, which is a very good mechanism.
However, if this fails, the 20.000 gas has not been enough, hence we doubt that the recovery function, which also provides 20.000 gas, would work in that case. We do think that 20.000 gas is plenty for any transfer to be successfully executed, hence we do not see this as a really crucial point. We just want to point it out. We think that an increase of the gas limit for this function might “solve” this, if it’s desired.
Update on October 6th, 2021
We have discussed this point further with the AsyncArt team. Since there is no risk whatsoever from this code, we leave it entirely up to the team to decide if they feel a change is necessary.
Part of our audits are also analyses of the protocol and its logic. A team of three auditors went through the implementation and documentation of the implemented protocol.
We went through all of the provided documentation, tests and contracts in a very detailed manner. The general description of the protocol is very well made, it’s very easy to understand how each function is supposed to work and what it implements.
We were not able to discover any problems in the protocol implemented in the smart contract.
As per our testing strategy, we deploy audited smart contracts (if requested by the client) onto a testnet to verify the implementation’s functionality. We usually deploy simple smart contracts to a local Ganache instance, which is sufficient for most cases. In this case, given the contracts’ complexity, we wanted to ensure no Ganache-related coverups of the contracts’ misbehavior. We created two testnets: a geth-based one and a parity/openethereum-based one. All of our tests have been executed on both testnets without any difference in the results. We were able to use the contracts as intended and could not maliciously game the protocol in practice.
We used fuzzing tools that generate random and semi-random inputs and interact with the contracts, trying to produce any unforeseen states within the contracts, especially the treasury contract. Throughout our tests, we were not able to create or observe any issues or problems. The contract behaved as expected and reverted correctly, given wrong inputs. No unforeseen states occurred during our fuzz-tests.
During our code review (which was done manually and automated), we found no bugs or flaws. Our automated systems and review tools also did not find any additional ones.
The protocol review and analysis did neither uncover any game-theoretical nature problems nor any other functions prone to abuse.
During our multiple deployments to various local testnets, we haven’t been able to find any additional problems or unforeseen issues.
In general, we are delighted with the overall quality of the code and its documentation.
We store our public audit reports on IPFS; a peer-to-peer network called the "Inter Planetary File System". This allows us to store our reports in a distributed network instead of just a single server, so even if our website is down, every report is still available.
The IPFS Hash, a unique identifier of the report, is signed on-chain by both the client and us to prove that both sides have approved this audit report. This signing mechanism allows users to verify that neither side has faked or tampered with the audit.