-
Notifications
You must be signed in to change notification settings - Fork 842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added burn functionality #61
Merged
+446
−38
Merged
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
d92c3d0
Added burn functionality.
Vectorized 4a8bc00
Changed _initOneIndexed
Vectorized 00fe576
Moved burn function into ERC721ABurnable
Vectorized 2052461
Moved burn function into ERC721ABurnable
Vectorized 33d4b41
Remove redundant burn check in ownershipOf
Vectorized 5a6c851
Optimized ownershipOf
Vectorized a2ef813
Removed aux from AddressData for future PR
Vectorized 07573e9
Packed currentIndex and totalBurned for gas savings
Vectorized bfd6d97
Added gas optimizations
Vectorized 8fc600f
Added gas optimizations
Vectorized e23e803
Merge branch 'main' into feature/burnable
Vectorized 8ff5722
Added requested changes
Vectorized a66b24b
Merge branch 'chiru-labs:main' into feature/burnable
Vectorized e1df945
Edited comments.
Vectorized 9820bcb
Renamed totalBurned to burnedCounter
Vectorized 7a02958
Renamed to burnCounter
Vectorized 1cf3bc1
Updated comments.
Vectorized f25c639
Mark transferFrom/safeTransferFrom virtual
Vectorized 92c82e1
Mark transferFrom/safeTransferFrom virtual
Vectorized 66b488f
Updated comments.
Vectorized 39ff829
Tidy up tests
Vectorized 212912c
Inlined _exists for _burn and _transfer.
Vectorized bcbfb94
Merged custom errors
Vectorized 1f3bc9e
Merged custom errors
Vectorized a06b5bc
Merge branch 'main' into feature/burnable
Vectorized 64bc7e7
Fixed missing change from #59
Vectorized 01e7ade
Gas optimization
Vectorized 2e4857d
update specs for _beforeTokenTransfers and _afterTokenTransfers hooks
ahbanavi 5ac0204
Added #84
Vectorized 53ca717
Added #87
Vectorized d08c926
Merge branch 'main' into feature/burnable
Vectorized c493bf1
Added #85
Vectorized 492ec57
Merge branch 'main' into feature/burnable
Vectorized 7cf1fca
Merge branch 'feature/burnable' into feature/burnable
Vectorized 1aeb1b8
Merge pull request #1 from ahbanavi/feature/burnable
Vectorized 1bc34da
Added #89
Vectorized 6e1ab7a
Added comments on packing _currentIndex and _burnCounter
Vectorized 9ee026c
Removed overflow check for updatedIndex
Vectorized 314faba
Added requested test changes
Vectorized c539195
Removed unused variable in burn test
Vectorized ca9bb69
Removed unused variable in burn test
Vectorized File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// SPDX-License-Identifier: MIT | ||
// Creator: Chiru Labs | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import '../ERC721A.sol'; | ||
import '@openzeppelin/contracts/utils/Context.sol'; | ||
|
||
/** | ||
* @title ERC721A Burnable Token | ||
* @dev ERC721A Token that can be irreversibly burned (destroyed). | ||
*/ | ||
abstract contract ERC721ABurnable is Context, ERC721A { | ||
|
||
/** | ||
* @dev Burns `tokenId`. See {ERC721A-_burn}. | ||
* | ||
* Requirements: | ||
* | ||
* - The caller must own `tokenId` or be an approved operator. | ||
*/ | ||
function burn(uint256 tokenId) public virtual { | ||
TokenOwnership memory prevOwnership = ownershipOf(tokenId); | ||
|
||
bool isApprovedOrOwner = (_msgSender() == prevOwnership.addr || | ||
getApproved(tokenId) == _msgSender() || | ||
isApprovedForAll(prevOwnership.addr, _msgSender())); | ||
|
||
require(isApprovedOrOwner, 'ERC721A: caller is not owner nor approved'); | ||
|
||
_burn(tokenId); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// SPDX-License-Identifier: MIT | ||
// Creators: Chiru Labs | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import '../extensions/ERC721ABurnable.sol'; | ||
|
||
contract ERC721ABurnableMock is ERC721A, ERC721ABurnable { | ||
constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} | ||
|
||
function exists(uint256 tokenId) public view returns (bool) { | ||
return _exists(tokenId); | ||
} | ||
|
||
function safeMint(address to, uint256 quantity) public { | ||
_safeMint(to, quantity); | ||
} | ||
|
||
function initOneIndexed() public { | ||
_initOneIndexed(); | ||
} | ||
|
||
function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) { | ||
return _ownerships[index]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// SPDX-License-Identifier: MIT | ||
// Creators: Chiru Labs | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import '../extensions/ERC721ABurnable.sol'; | ||
import '../extensions/ERC721AOwnersExplicit.sol'; | ||
|
||
contract ERC721ABurnableOwnersExplicitMock is ERC721A, ERC721ABurnable, ERC721AOwnersExplicit { | ||
constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {} | ||
|
||
function exists(uint256 tokenId) public view returns (bool) { | ||
return _exists(tokenId); | ||
} | ||
|
||
function safeMint(address to, uint256 quantity) public { | ||
_safeMint(to, quantity); | ||
} | ||
|
||
function initOneIndexed() public { | ||
_initOneIndexed(); | ||
} | ||
|
||
function setOwnersExplicit(uint256 quantity) public { | ||
_setOwnersExplicit(quantity); | ||
} | ||
|
||
function getOwnershipAt(uint256 index) public view returns (TokenOwnership memory) { | ||
return _ownerships[index]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why you decided to include this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping track of numberMinted and the block timestamps for ownerships are not in the spec, but are useful to squeeze the most out of each SSTORE. Going by the same spirit, numberBurned is included.
If you prefer, it can be removed or moved to a future PR. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep this in for now - was just curious. We're still going through this PR, just been a super busy week. Will try to get this done soon!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of the currently opened feature PRs, which ones likely to be merged before this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged the custom errors from #73.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the next PR I'm reviewing