0xacme/erc404

erc404's Issues

Usage of for loops might lead to large amounts of tokens not beeing transferable.

Usage of for loops might lead to large amounts of tokens not beeing transferable.
In the code bellow, if the variables tokens_to_burn or tokens_to_mint are big enough, that would lead to an Out of Gas error and prevent large token transfers.

ERC404/src/ERC404.sol

Lines 324 to 340 in 14c1362

	// Skip burn for certain addresses to save gas
	if (!whitelist[from]) {
	uint256 tokens_to_burn = (balanceBeforeSender / unit) -
	(balanceOf[from] / unit);
	for (uint256 i = 0; i < tokens_to_burn; i++) {
	_burn(from);
	}
	}
	// Skip minting for certain addresses to save gas
	if (!whitelist[to]) {
	uint256 tokens_to_mint = (balanceOf[to] / unit) -
	(balanceBeforeReceiver / unit);
	for (uint256 i = 0; i < tokens_to_mint; i++) {
	_mint(to);
	}
	}

ERC-20 transfer events not being handled correctly

Resulting in most of the ERC-404 tokens being unindexed by etherscan.

As a workout, we have used libraries to separate events and correctly emit ERC20 transfer events here

#11

No obvious correlation to official ERCs

Hi there,

I checked the following:

• https://github.com/ethereum/ERCs
• https://github.com/ethereum/eips
• https://eips.ethereum.org/

and I couldn't see any mention of this ERC on any of them, so I'm confused how this ended up as ERC-404. Am I missing something?

UNLICENSED is used for non-open source projects

UNLICENSED is used for non-open source projects. Since README.md mentions that a project is free for anyone to use, I suggest changing the licence to MIT to make it true open source.

If you do not want to specify a license or if the source code is not open-source, please use the special value UNLICENSED. Note that UNLICENSED (no usage allowed, not present in SPDX license list) is different from UNLICENSE (grants all rights to everyone).

Source: https://docs.soliditylang.org/en/latest/layout-of-source-files.html

MEV protection

What would happen in the scenario where:

• Malicious searcher acquire an NFT
• Sells the NFT on a marketplace
• As soon as buy tx lands in the mempool, search insert a transfer tx where they remove 99% of the tokens

ERC1155 Alternative?

I’ve decided to try to rewrite the protocol as an ERC1155 protocol for several reasons:

No mixed function/signature pairs in ERC1155/ERC20.
Use of batch _update allows users to specify the NFT/ERC20 pairings specifically.

What I did:

Made a mapping _totalSupply[erc20Id] to override ERC20 totalSupply() using erc20Id of 0
Made ERC1155 ids above erc20Id non-fungible using _totalSupply[id];
Overrode ERC20 and ERC1155 _update functions ensuring all balance mappings were pointing to the ERC1155 contract using _balances[erc20Id] and that _totalSupply was correctly calculated
Ensured that any mismatch of ERC20 and NF ERC1155 tokens (incorrectly called erc721 in the code) is brought up and checked against balance.

Thanks to this approach all native ERC20 and ERC1155 minting, burning and transfer functions behave as expected.

See my repo, it’s a vastly different approach to the current standard but achieves similar effects to what “ERC404” does.

github.com/TechnicallyWeb3/TW3404

Things I haven’t had time to do to make it complete:
Add formal “ERC420” errors
Add in function balanceOfNft(address) to mimic old ERC721 balanceOf using ownedTokens.length (optional - not necessary for ERC1155 platforms)
Add ownerOf mapping/function to mimic ERC721 function (also optional for same reason)
Add ERC1155 and ERC20 emits where needed
Full testing scripts
Check how brutal gas is😅

If amount is smaller then minted how to transfer?

With this code here

ERC404/src/ERC404.sol

Line 215 in 14c1362

if (amountOrId <= minted) {

you enforce that if amountOrId in transferFrom is smaller than minted it will try to transfer NFT.
But what if I want to transfer less than 10k amount, for example I want to transfer 1000 amount and transferFrom is called, how will I transfer that tokens when it will keep going to the first part of condition and fail there?