Skip to content

code-423n4/2022-08-foundation

Repository files navigation

Foundation Drop contest details

Overview

We're releasing four new contracts that are in scope for the audit:

  • NFT Collection Factory: An upgradable factory that creates NFT collections. Supports creating and initializing ERC-1165 minimal proxies pointing to NFT collection contract templates.
  • NFT Drop Collection: A contract to batch mint and reveal a collection of NFTs. A creator deploys this collection with the intent of creating a mint drop with a reveal step where all NFTs in the collection have the same baseURI and are differentiated through their token id. In addition to the creator, to facilitate sales by marketplace contracts, any address can be granted permissions to mint from this contract.
  • NFT Collection: A collection of NFTs by a single creator. The NFTs produced by this collection are 1-1 with support for unique baseURI. All NFTs from this contract are minted by the same creator. Note that, this contract exists on mainnet and has been in active use for the past year; however, we've refactored/cleaned it up to be more maintainable, extensible and gas efficient and will be deploying the new version in our upcoming release.
  • NFT Drop Market: A Foundation market contract which enables creators to drop NFT collections. Creators can set sale terms on their deployed NFT collections and collectors can mint from them using the NFT drop market. Currently only supports fixed price sales of NFT Drop Collections, does not support any other mechanic or collection type.

For more detailed docs on drops please navigate to FoundationOS.

Scope

In scope

Contract Documentation Goerli Deployment
NFTCollectionFactory.sol NFTCollectionFactory 0xf06eeD0514346511Ffe61efD6F6F0C66bBa284Db
NFTDropCollection.sol NFTDropCollection 0xce844d8b74ad9969bc6d17261dd52693b51320d5
NFTCollection.sol NFTCollection 0x0d8f20128f93dde8e367adfbd78b2a7ca84a40cd
NFTDropMarket.sol NFTDropMarket 0xe43562f11737443F760dBf885fa0D30c45C6927B

& all their inherited / library contracts.

We would like to call out extra attention to NFTDropMarketFixedPriceSale and MarketFees mixins as both of them are highly critical paths that perform accounting, fund distribution and transfers.

Sizes

This is the complete list of what's in scope for this contest:

Contract Name SLOC Purpose
NFTDropMarket 61 The main / top-level contract for all drop market tools on Foundation.
Constants 10 Shared constant values used by various mixins.
FETHNode 36 A wrapper for communicating with the FETH contract.
FoundationTreasuryNode 34 A wrapper for communicating with the treasury contract which collects Foundation fees and defines the admin & operator roles.
Gap10000 4 A placeholder contract leaving room for new mixins to be added to the future.
MarketFees 285 Distributes revenue from sales.
BytesLibrary 30 A library for manipulation of byte arrays.
ArrayLibrary 17 Helper functions for arrays.
MarketSharedCore 8 A base class for Foundation market contracts to define functions that other market contract may implement or extend.
NFTDropMarketCore 5 A base class for the drop specific market contract to define functions that other mixins may implement or extend.
NFTDropMarketFixedPriceSale 137 Allows creators to list a drop collection for sale at a fixed price point.
NFTCollectionFactory 169 A factory to create NFT collections.
AddressLibrary 12 A library for address helpers not already covered by the OZ library library.
NFTDropCollection 129 A contract to batch mint and reveal a collection of NFTs.
ContractFactory 15 Stores a reference to the factory which is used to create contract proxies. (shared across all collection types)
AdminRole 22 Defines a role for admin accounts.
MinterRole 23 Defines a role for minter accounts.
CollectionRoyalties 41 Defines various royalty APIs for broad marketplace support. (shared across all collection types)
SequentialMintCollection 47 Extends the OZ ERC721 implementation for collections which mint sequential token IDs. (shared across all collection types)
NFTCollection 133 A collection of NFTs by a single creator.
Total 1228

Out of scope

Any issues or improvements on how we integrate with the contracts above is in scope.

Additional Context

Mixins

In order to maintain readability as our contracts grow in complexity, we separate responsibilities into different abstract contracts which we call 'mixins'. We try not to create too many interdependencies between mixins, shared logic may be defined in NFTDropMarketCore so mixins do not need to call each other directly.

Upgrades

Our new factory and NFT drop market contracts will be upgradeable proxies allowing us to add more features overtime. For an overview of the upgrade pattern, refer to the OpenZeppelin documentation.

FETH ERC-20 token

This contract is not in scope for this contest.

FETH is an ERC-20 token modeled after WETH9. It has the ability to lockup tokens for 24-25 hours - during this time they may not be transferred or withdrawn, except by our market contract which requested the lockup in the first place.

Since after lockups expire, FETH is just another wrapped ETH token contract - we allow using your available FETH balance to mint from a NFT drop collection.

Note that both FETH have been audited in a previous C4 contest and have been live on mainnet for ~4 months.

Tests

Hardhat tests:

Setup:

yarn
yarn build

Test:

yarn test

(run yarn build again if ABIs have changed)

The tests in this repo are not our full test suite. We have simplified what was included here to clearly demonstrate core features and interactions; however, they still do a good job in covering most cases.

Troubleshooting:

  • If you get Error: Lock file is already being held you can try again, deleting ./cache sometimes helps as well. Finding a fix for this common occurrence would be a high value QA issue :)
  • Hardhat Tracer is included to help with debugging, e.g. run yarn test --logs (or --trace, --fulltrace).
  • The test suite takes awhile to run, when testing for something specific execute just that file with yarn test path/to/test.ts e.g. yarn test test/NFTDropMarket/fixedPrice/drop.ts --logs. And/or update an it test to it.only(....

Gas Testing

The Hardhat gas reporter will print gas usage when running tests. We also have included a custom report we use to measure gas costs for the core scenarios we are interested in. When submitting optimizations, it would be most helpful to us if you communicate the impact in terms of the diff to the gas-stories.txt file your recommendation would create.

yarn test-gas-stories

This writes changes to ./gas-stories.txt locally.

Coverage

Most, but not all, of our tests have been added to this repo. If you run the coverage report you'll see pretty good coverage. Note that our private repo has 100% coverage, what's included here has been simplified a bit to remove some out of scope contracts and as a result coverage is not perfect here.

yarn coverage

After running this command, you'll need to re-run yarn build.

To view the report, open coverage/index.html in your browser.

Forge tests:

See test/foundry/FixedPriceDrop.sol for an example you can build on to probe for issues.

Setup:

git submodule update --init --recursive --remote
yarn
curl -L https://foundry.paradigm.xyz | bash
foundryup

Test:

forge test

If forge fails to run, please confirm you are running the latest version. This was tested with forge 0.2.0.

Note: After running forge you need to run yarn build again for the Hardhat tests to work.

Slither

We have run the default detectors with Slither, posted the output along with our responses to each. Please do not submit these findings unless you have reason to believe our responses here are not valid.

Responses use quote replies like this.

Note: Slither currently crashes on the try/catch blocks in our code. In order to generate these results, we temporarily removed the try/catch blocks.

You can use the slither branch in this repo to run Slither locally. This code has been modified to allow Slither to run locally with minimal changes, but the changes are not part of this contest and we removed mocks so tests will not run on that branch -- please validate your findings against the main branch before submitting!

You may need to upgrade Slither, these results were run with 0.8.3.