chiru-labs / ERC721A

https://ERC721A.org
MIT License
2.5k stars 841 forks source link

De-duplicate the `Transfer` events in the `_mint` function #402

Closed KamaDeFi closed 2 years ago

Vectorized commented 2 years ago

The events are duplicated on purpose to save gas.

cygaar commented 2 years ago

Perhaps we can add a short comment in the code on why we're duplicating @Vectorized

KamaDeFi commented 2 years ago

I've thought about this some more, and if I understand correctly, the only saving in gas here is to avoid only one extra call of tokenId := add(tokenId, 1) and iszero(eq(tokenId, end)). That doesn't seem to have the massive gas-saving benefit that is worth duplicating code in my opinion.

Examples:

Scenario 1, quantity = 1

Existing code would call:

  1. log4
  2. let tokenId := add(startTokenId, 1)
  3. iszero(eq(tokenId, end))

But the new code would call:

  1. let tokenId := startTokenId
  2. iszero(eq(tokenId, end))
  3. log4
  4. tokenId := add(tokenId, 1)
  5. iszero(eq(tokenId, end))

Scenario 2, quantity = 2

Existing code would call:

  1. log4
  2. let tokenId := add(startTokenId, 1)
  3. iszero(eq(tokenId, end))
  4. log4
  5. tokenId := add(tokenId, 1)
  6. iszero(eq(tokenId, end))

But the new code would call:

  1. let tokenId := startTokenId
  2. iszero(eq(tokenId, end))
  3. log4
  4. tokenId := add(tokenId, 1)
  5. iszero(eq(tokenId, end))
  6. log4
  7. tokenId := add(tokenId, 1)
  8. iszero(eq(tokenId, end))