0xsequence / niftyswap

Niftyswap - decentralized swap protocol for ERC-1155 tokens, inspired by Uniswap

Niftyswap

Niftyswap is a decentralized token swap protocol for ERC-1155 tokens on Ethereum. In other words, Niftyswap is Uniswap for ERC-1155 tokens.

For detailed documentation on the Niftyswap Protocol and how to use it, please read the SPECIFICATION.

We are incredibly thankful for the work done by the Uniswap team, without which Niftyswap wouldn't exists.

Description

Niftyswap is an implementation of Uniswap, a protocol for automated token exchange on Ethereum. While Uniswap is for trading ERC-20 tokens, Niftyswap is a protocol for ERC-1155 tokens. Both are designed to favor ease of use and provide guaranteed access to liquidity on-chain.

Most exchanges maintain an order book and facilitate matches between buyers and sellers. Niftyswap smart contracts hold liquidity reserves of various tokens, and trades are executed directly against these reserves. Prices are set automatically using the constant product $x*y = K$ market maker mechanism, which keeps overall reserves in relative equilibrium. Reserves are pooled between a network of liquidity providers who supply the system with tokens in exchange for a proportional share of transaction fees.

An important feature of Niftyswap is the utilization of a factory/registry contract that deploys a separate exchange contract for each ERC-1155 token contract. These exchange contracts each hold independent reserves of a single fungible ERC-1155 currency and their associated ERC-1155 token id. This allows trades between the Currency and the ERC-1155 tokens based on the relative supplies.

For more details, see Specification

Getting started

Install

yarn add @0xsequence/niftyswap or npm install @0xsequence/niftyswap

Differences with Uniswap

There are some differences compared to the original Uniswap that we would like to outline below:

1. For ERC-1155 tokens, not ERC-20s
2. Base currency is not ETH, but needs to be an ERC-1155
3. Liquidity fee is 0.5% instead of 0.3%
4. All fees are taken from base currency (Uniswap takes trading fees on both sides). This will lead to some small inneficiencies which will be corrected via arbitrage.
5. Users do not need to set approvals before their first trade
6. 100% native meta-tx friendly for ERC-1155 implementations with native meta-tx functionalities
7. Front-end implementations can add arbitrary fee (in addition to the 0.5%) for tokens with native meta-transactions.
8. Less functions than Uniswap

There are pros and cons to these differences and we welcome you to discuss these by openning issues in this repository.

Contracts

• NiftyswapExchange.sol: The exchange contract that handles the logic for exchanging assets for a given base token.
• NiftyswapFactory.sol: The exchange factory that allows the creation of nifyswap exchanges for the tokens of a given ERC-1155 token conract and an ERC-1155 base currency.

Security Review

Niftyswap has been audited by two independant parties and all issues discovered were addressed.

• Agustín Aguilar**
• Consensys Diligence

** Agustín was hired as a full-time employee at Horizon after the audit was completed. Agustín did not take part in the writing of Niftyswap contracts.

Dev env & release

This repository is configured as a yarn workspace, and has multiple pacakge.json files. Specifically, we have the root ./package.json for the development environment, contract compilation and testing. Contract source code and distribution files are packaged in "src/package.json".

To release a new version, make sure to bump the version, tag it, and run yarn release. The release command will publish the @0xsequence/niftyswap package in the "src/" folder, separate from the root package. The advantage here is that application developers who consume @0xsequence/niftyswap aren't required to install any of the devDependencies in their toolchains as our build and contract packages are separated.

LICENSE

Copyright (c) 2017-present Horizon Blockchain Games Inc.

Licensed under Apache-2.0

niftyswap's Issues

L2 - An error on `base` or `token` could leave the whole pool frozen

If any of the two contracts of the tokens encounters an error and it stops being able to perform transfers the whole pool it's affected, making the associated funds on the other token impossible to withdraw.

Consider adding an aditional removeLiquidity parameter that allows the withdrawal of only base or only token.

Similar issuees had happened on Uniswap before (https://twitter.com/UniswapExchange/status/1072286773554876416) and (https://blog.synthetix.io/guide-on-migrating-seth-liquidity/)

By @Agusx1211

L4 - `addLiquidity` can round in favor of new liquidity provider

by @Agusx1211

Support for ERC20::ERC1155 Niftyswap pool

Presently, NiftyswapExchange.sol is implemented to pool/swap 1155:1155 pairs, which is very cool and useful. As discussed, it's convenient to interface with ERC20's such as having a DAI:1155 niftyswap pool or USDC:1155 pool, etc.

I propose to leave NiftyswapExchange.sol as it is (1155:1155), and write a variant as NiftyswapExchange20.sol for 20:1155 pairs, where the base currency is an ERC20 token.

Adding creatorFee to Niftyswap

Almost all NFT exchanges allow for the creators to receive part of the trading fee, but Niftyswap currently doesn't.

As a V1, we could add two parameters in an exchange contract ; creatorFee and creatorAddress. The former is the % of fee that would go to the creator and the latter would be the address where these fees would be sent to. In order to prevent capture by front runners and such, these two parameters should be set by an operator.

Specifications:

• Only do this for the ERC20:ERC1155 contract
• Each exchange contract now need two new variables: creatorAddress and creatorFee. Both default to 0x0 on exchange creation
• These two variables should only be set by the owner. The owner should be the owner of the factory, passed down to the exchange contract on creation.
• Only the owner should be able to change the creatorAddress and creatorFee.
• When doing any trade (buy or sell), if creatorFee is > 0, an additional fee is charged in the ERC-20 currency and sent to the creatorAddress
• Adding or removing liquidity should NOT trigger a fee to be paid obviously.

Bug Bounty: Pre-audit

We are currently partnering with Gitcoin to provide bounties for vulnerabilities in our contracts. The rewards are small as these contracts have not yet been audited. The bounties size will be reviewed upward once the audits have been performed.

Contract eligible for bounty

• NiftyswapExchange.sol
• NiftyswapFactory.sol

What vulnerabilities should you look for?

We of course want to know every vulnerability, but in particular:

• Risk of funds being stolen
• Risk of funds being frozen or lost
• Risk of invalid balance changes
• Invalid price changes

We follow many of the bug bounty rules that the Ethereum Foundation does:

• Decisions on the eligibility and size of a reward are the sole discretion of Horizon Games.

• Public disclosure of a vulnerability makes it ineligible for a bounty. Instead issues must be submitted to [email protected].

• Issues must be new to the team. They can’t have already been identified by another user or by an audit.

• No employees, contractors or others with current or prior commercial relationships with Horizon Games are eligible for rewards.

• Provide the steps required to demonstrate an issue. If we cannot reproduce an issue we will not be able to reward it.

Bounty size

The size of the bounty will vary depending on the severity of the issue discovered. The severity is calculated according to the OWASP risk rating model based on Impact and Likelihood.

Decisions on the eligibility and size of a reward are guided by the rules above, but are, in the end, determined at the sole discretion of Horizon Games

Critical: up to $3,000
High: up to $1,500
Medium: up to $500
Low: up to $250

Other considerations

In addition to severity, other variables are also considered when Horizon evaluates the eligibility and size of a bounty, including (but not limited to):

• Quality of description
  Higher rewards are paid for clear, well-written submissions.

• Quality of reproducibility
  Please include test code, scripts and detailed instructions. The easier it is for us to reproduce and verify the vulnerability, the higher the reward.

• Quality of fix, if included
  Higher rewards are paid for submissions with clear description of how to fix the issue.

Updating solidity compiler version to ^0.8.0

Currently the contracts (For example https://github.com/0xsequence/niftyswap/blob/master/src/contracts/exchange/NiftyswapExchange.sol) sepecifies the compier to be
pragma solidity 0.7.4;
I was wandering If it can be upgraded to ^0.8.0 (Because Iam using this version in my project).

Thanks :)

N8 - Minted liquidity tokens can round to zero

N7 - `refundBase` isn’t sent to the seller

By @Agusx1211

N2 - Rounding up during while buying tokens or adding liquidity can be avoided

The NiftyswapExchange contracts always round transactions in a way that's favorable to the existing liquidity providers; this avoids them paying for inefficiencies when trading or adding liquidity, further aligning all incentives. This rounding it's always performed by increasing the resulting amount by 1, although sufficient, it may lead to rounding calculations that don't require it.

Consider replacing it with a more precise divRound method a / b + a % b == 0 ? 0 : 1

By @Agusx1211

N1 - Token trades are not easily composable

The contracts don't allow users to perform trades using multiple pools with a single transaction; this could be partionally fixed by enabling the users to customize the _data field when transferring the newly acquired tokens, it could contain an aditional trade to be performed on another exchange.

It has to be noted that such a solution only works to enable trades between different exchange contracts. However, trades of different IDs on the same exchange contract would still not be possible, because of the usage of the reentrancy-lock.

By @Agusx1211

N5 - Definition, and assignment of arrays can be optimized

by @Agusx1211

Orderbook: Collectible Detail functionality integrations

• buy button (use getTopOrder -> add to cart)
• sell buttton (use getTopOrder -> add to cart)
• accept offers
• cancel listings / offers

L3 - Minimum base provided can be circumvent

By @Agusx1211

N3 - Assembly code for `functionSignature` retrieval can be removed

The onERC1155BatchReceived method on the exchange uses assembly code to retrieve the first 4 bytes of the _data; this code can be simplified by using abi.decode(_data, (bytes4)) although less efficient (29 GAS approx), it's simpler to review, and audit.

By @Agusx1211

N6 - Functions mutability can be restricted to pure

By @Agusx1211

Export method signatures and data types for npm package consumers

it would be great if we can export these constants so that consumers can directly import them from the released package

niftyswap/tests/utils/helpers.ts

Lines 40 to 84 in f0aba0b

	export const methodsSignature = {
	BUYTOKENS: '0xb2d81047',
	SELLTOKENS: '0xdb08ec97',
	ADDLIQUIDITY: '0x82da2b73',
	REMOVELIQUIDITY: '0x5c0bf259'
	}
	export const methodsSignature20 = {
	BUYTOKENS: '0xb2d81047',
	SELLTOKENS: '0xade79c7a',
	ADDLIQUIDITY: '0x82da2b73',
	REMOVELIQUIDITY: '0x5c0bf259'
	}
	export const BuyTokensType = `tuple(
	address recipient,
	uint256[] tokensBoughtIDs,
	uint256[] tokensBoughtAmounts,
	uint256 deadline
	)`
	export const SellTokensType = `tuple(
	address recipient,
	uint256 minBaseTokens,
	uint256 deadline
	)`
	export const SellTokens20Type = `tuple(
	address recipient,
	uint256 minCurrency,
	address[] extraFeeRecipients,
	uint256[] extraFeeAmounts,
	uint256 deadline
	)`
	export const AddLiquidityType = `tuple(
	uint256[] maxCurrency,
	uint256 deadline
	)`
	export const RemoveLiquidityType = `tuple(
	uint256[] minCurrency,
	uint256[] minTokens,
	uint256 deadline
	)`

TypeError: Member "buyTokens" not found or not visible after argument-dependent lookup in address

Hi this is on the sol-update branch

Heres some code:

NiftyswapFactory20 internal immutable swapFactory;
...
//
// in constructor
//
swapFactory = new NiftyswapFactory20(address(this));
swapFactory.createExchange(address(this), WETH9, 10, 0);

uint256[] memory amount = new uint256[](1);
amount[0] = 10000000000;

bytes memory data = abi.encode(
    ADDLIQUIDITY_SIG,
    AddLiquidityObj(amount, block.timestamp + SECONDS_DEADLINE)
);

safeBatchTransferFrom(
    address(this),
    address(swapFactory),
    tokenID,
    amount,
    data
);

...
// in a function for buying the token
swapFactory.getPairExchanges(address(this), WETH9)[0].buyTokens(...);

This last line errors with this:

TypeError: Member "buyTokens" not found or not visible after argument-dependent lookup in address.

I'd just like to know how to fix it.

N4 - self-deposit isn't enforced

The exchange defines a function signature for self-deposit that's used during an addLiquidty call, to accept the base tokens.

The exchange doesn't check that the operator of the self-deposit transfer it's, in fact, itself, neither that the self-deposit token is the base token.

This allows depositing any ERC1155 token on the exchange, as long as the data contains the self-deposit signature.

By @Agusx1211

C1 - Pool can be drained if `base` matches `token`

The issue occurs when an exchange is created with the same contract for baseToken and token; the system allows anyone to create a pool with base[baseTokenID]/token[ID], and allows ID to be equal to baseTokenID, hence the creation of a pool of base/base.

When the exchange rate of a pool gets calculated, the contract assumes that all its token balance corresponds to the pool, but this is not the case if the pair is base/base, in this case, the balance of the contract includes the provided liquidity for all the other pairs.

This miscalculation of _getTokenReserves opens the possibility of executing two different exploits, one of such exploits it's performing trades against the base/base pair, and the other is the use of removeLiquidity to take the liquidity of all the pools at once.

By @Agusx1211

Front Running Exchange Creation

As per the comment at https://github.com/arcadeum/niftyswap/blob/c4c94e718e4195b4b29b2e69f9b65132dfa6456c/contracts/exchange/NiftyswapFactory.sol#L20 the current design permits for only one niftyswap exchange per ERC-1155 token, which could lead to some ERC-1155 tokens being "blocked".

As per @Agusx1211 :

The contract factory uses the mapping tokenToExchange to avoid the creation of duplicated pools for the same token; however, the _baseTokenAddr token of each exchange is not defined on the factory and instead is provided as one of the input parameters of the createExchange function.

Because the calling of createExchange is not permissioned, an attacker could create exchanges with random or broken base addresses, blocking the genuine creation of the exchanges.

Consider adding an aditional dimension to the tokenToExchange mapping and only enforce unique base/token combinations.