Vectorized / dn404

Implementation of a co-joined ERC20 and ERC721 pair.
MIT License
476 stars 158 forks source link

DoS Vector: Transfering relatively large amounts of DN404 always `oog`s #72

Open akshatmittal opened 9 months ago

akshatmittal commented 9 months ago

This shouldn't be a surprise, but if you create a DN404 with higher than 2.7k supply (in units, aka 18dec) and attempt to transfer ~2.7k tokens to another address, the gas consumption to mint the NFTs is over 30m reverting with oog on mainnet.

There's probably no workaround for this, but it's at least worth documenting somewhere.

Fairly easy to verify:

contract RevertTest is SoladyTest {
    SimpleDN404 dn;

    function setUp() public {
        dn = new SimpleDN404("DN404", "DN", 2700 * 10 ** 18, address(this));
    }

    function testTransfer() public { // gas: 30715623
        dn.transfer(address(1), 2_700 * 10 ** 18);
    }
}
Vectorized commented 9 months ago

Ok, we should add a cautionary comment. Nice suggestion.

ghost commented 9 months ago

@akshatmittal This is intentional and I have degen'd this feature to build something new on DN404 😈

Vectorized commented 9 months ago

WTF. kekw. @zerotwodao

ghost commented 9 months ago

@Vectorized Although I think if there is a function that could disable NFT transfers on behalf of other EOAs would be super cool and would increase adoption of DN404 tokens from centralized exchanges, I think it would be great if we have a simple oracle contract of merkle root to verify inclusion proofs that contains deposit wallets

akshatmittal commented 9 months ago

@zerotwodao If it's another PoG token I'll lose my mind lol. We have enough XEN in this world. 😹

ghost commented 9 months ago

@akshatmittal

8g0r4d