Skip to content
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

Preparation for efficient consecutive burns #447

Merged
merged 7 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 35 additions & 25 deletions contracts/ERC721A.sol
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,14 @@ contract ERC721A is IERC721A {
return _unpackedOwnership(_packedOwnerships[index]);
}

/**
* @dev Returns whether the ownership slot at `index` is initialized.
* An uninitialized slot does not necessarily mean that the slot has no owner.
*/
function _ownershipIsInitialized(uint256 index) internal view virtual returns (bool) {
return _packedOwnerships[index] != 0;
}

/**
* @dev Initializes the ownership slot minted at `index` for efficiency purposes.
*/
Expand All @@ -346,33 +354,35 @@ contract ERC721A is IERC721A {
function _packedOwnershipOf(uint256 tokenId) private view returns (uint256 packed) {
if (_startTokenId() <= tokenId) {
packed = _packedOwnerships[tokenId];
// If not burned.
if (packed & _BITMASK_BURNED == 0) {
// If the data at the starting slot does not exist, start the scan.
if (packed == 0) {
if (tokenId >= _currentIndex) _revert(OwnerQueryForNonexistentToken.selector);
// Invariant:
// There will always be an initialized ownership slot
// (i.e. `ownership.addr != address(0) && ownership.burned == false`)
// before an unintialized ownership slot
// (i.e. `ownership.addr == address(0) && ownership.burned == false`)
// Hence, `tokenId` will not underflow.
//
// We can directly compare the packed value.
// If the address is zero, packed will be zero.
for (;;) {
unchecked {
packed = _packedOwnerships[--tokenId];
}
if (packed == 0) continue;
return packed;
// If the data at the starting slot does not exist, start the scan.
if (packed == 0) {
if (tokenId >= _currentIndex) _revert(OwnerQueryForNonexistentToken.selector);
// Invariant:
// There will always be an initialized ownership slot
// (i.e. `ownership.addr != address(0) && ownership.burned == false`)
// before an unintialized ownership slot
// (i.e. `ownership.addr == address(0) && ownership.burned == false`)
// Hence, `tokenId` will not underflow.
//
// We can directly compare the packed value.
// If the address is zero, packed will be zero.
for (;;) {
unchecked {
packed = _packedOwnerships[--tokenId];
}
if (packed == 0) continue;
if (packed & _BITMASK_BURNED == 0) return packed;
// Otherwise, the token is burned, and we must revert.
// This handles the case of batch burned tokens, where only the burned bit
// of the starting slot is set, and remaining slots are left uninitialized.
_revert(OwnerQueryForNonexistentToken.selector);
}
// Otherwise, the data exists and is not burned. We can skip the scan.
// This is possible because we have already achieved the target condition.
// This saves 2143 gas on transfers of initialized tokens.
return packed;
}
// Otherwise, the data exists and we can skip the scan.
// This is possible because we have already achieved the target condition.
// This saves 2143 gas on transfers of initialized tokens.
// If the token is not burned, return `packed`. Otherwise, revert.
if (packed & _BITMASK_BURNED == 0) return packed;
}
_revert(OwnerQueryForNonexistentToken.selector);
}
Expand Down Expand Up @@ -1133,4 +1143,4 @@ contract ERC721A is IERC721A {
revert(0x00, 0x04)
}
}
}
}
102 changes: 47 additions & 55 deletions contracts/extensions/ERC721AQueryable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
override
returns (TokenOwnership memory ownership)
{
if (tokenId >= _startTokenId()) {
if (tokenId < _nextTokenId()) {
ownership = _ownershipAt(tokenId);
if (!ownership.burned) {
ownership = _ownershipOf(tokenId);
unchecked {
if (tokenId >= _startTokenId()) {
if (tokenId < _nextTokenId()) {
// If the `tokenId` is within bounds,
// scan backwards for the initialized ownership slot.
while (!_ownershipIsInitialized(tokenId)) --tokenId;
return _ownershipAt(tokenId);
}
}
}
Expand Down Expand Up @@ -109,6 +111,38 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
uint256 start,
uint256 stop
) external view virtual override returns (uint256[] memory) {
return _tokensOfOwnerIn(owner, start, stop);
}

/**
* @dev Returns an array of token IDs owned by `owner`.
*
* This function scans the ownership mapping and is O(`totalSupply`) in complexity.
* It is meant to be called off-chain.
*
* See {ERC721AQueryable-tokensOfOwnerIn} for splitting the scan into
* multiple smaller scans if the collection is large enough to cause
* an out-of-gas error (10K collections should be fine).
*/
function tokensOfOwner(address owner) external view virtual override returns (uint256[] memory) {
uint256 start = _startTokenId();
uint256 stop = _nextTokenId();
uint256[] memory tokenIds;
if (start != stop) tokenIds = _tokensOfOwnerIn(owner, start, stop);
return tokenIds;
}

/**
* @dev Helper function for returning an array of token IDs owned by `owner`.
*
* Note that this function is optimized for smaller bytecode size over runtime gas,
* since it is meant to be called off-chain.
*/
function _tokensOfOwnerIn(
address owner,
uint256 start,
uint256 stop
) private view returns (uint256[] memory) {
unchecked {
if (start >= stop) _revert(InvalidQueryRange.selector);
// Set `start = max(start, _startTokenId())`.
Expand Down Expand Up @@ -157,8 +191,9 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
do {
ownership = _ownershipAt(start);
assembly {
switch mload(add(ownership, 0x40))
// if `ownership.burned == false`.
if iszero(mload(add(ownership, 0x40))) {
case 0 {
// if `ownership.addr != address(0)`.
// The `addr` already has it's upper 96 bits clearned,
// since it is written to memory with regular Solidity.
Expand All @@ -173,6 +208,12 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
mstore(add(tokenIds, shl(5, tokenIdsIdx)), start)
}
}
// Otherwise, reset `currOwnershipAddr`.
// This handles the case of batch burned tokens
// (burned bit of first slot set, remaining slots left uninitialized).
default {
currOwnershipAddr := 0
}
start := add(start, 1)
}
} while (!(start == stop || tokenIdsIdx == tokenIdsMaxLength));
Expand All @@ -184,53 +225,4 @@ abstract contract ERC721AQueryable is ERC721A, IERC721AQueryable {
return tokenIds;
}
}

/**
* @dev Returns an array of token IDs owned by `owner`.
*
* This function scans the ownership mapping and is O(`totalSupply`) in complexity.
* It is meant to be called off-chain.
*
* See {ERC721AQueryable-tokensOfOwnerIn} for splitting the scan into
* multiple smaller scans if the collection is large enough to cause
* an out-of-gas error (10K collections should be fine).
*/
function tokensOfOwner(address owner) external view virtual override returns (uint256[] memory) {
uint256 tokenIdsLength = balanceOf(owner);
uint256[] memory tokenIds;
assembly {
// Grab the free memory pointer.
tokenIds := mload(0x40)
// Allocate one word for the length, and `tokenIdsMaxLength` words
// for the data. `shl(5, x)` is equivalent to `mul(32, x)`.
mstore(0x40, add(tokenIds, shl(5, add(tokenIdsLength, 1))))
// Store the length of `tokenIds`.
mstore(tokenIds, tokenIdsLength)
}
address currOwnershipAddr;
uint256 tokenIdsIdx;
for (uint256 i = _startTokenId(); tokenIdsIdx != tokenIdsLength; ) {
TokenOwnership memory ownership = _ownershipAt(i);
assembly {
// if `ownership.burned == false`.
if iszero(mload(add(ownership, 0x40))) {
// if `ownership.addr != address(0)`.
// The `addr` already has it's upper 96 bits clearned,
// since it is written to memory with regular Solidity.
if mload(ownership) {
currOwnershipAddr := mload(ownership)
}
// if `currOwnershipAddr == owner`.
// The `shl(96, x)` is to make the comparison agnostic to any
// dirty upper 96 bits in `owner`.
if iszero(shl(96, xor(currOwnershipAddr, owner))) {
tokenIdsIdx := add(tokenIdsIdx, 1)
mstore(add(tokenIds, shl(5, tokenIdsIdx)), i)
}
}
i := add(i, 1)
}
}
return tokenIds;
}
}
28 changes: 28 additions & 0 deletions contracts/mocks/DirectBurnBitSetterHelper.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// SPDX-License-Identifier: MIT
// ERC721A Contracts v4.2.3
// Creators: Chiru Labs

pragma solidity ^0.8.4;

contract DirectBurnBitSetterHelper {
function directSetBurnBit(uint256 index) public virtual {
bytes32 erc721aDiamondStorageSlot = keccak256('openzepplin.contracts.storage.ERC721A');

// This is `_BITMASK_BURNED` from ERC721A.
uint256 bitmaskBurned = 1 << 224;
// We use assembly to directly access the private mapping.
assembly {
// The `_packedOwnerships` mapping is at slot 4.
mstore(0x20, 4)
mstore(0x00, index)
let ownershipStorageSlot := keccak256(0x00, 0x40)
sstore(ownershipStorageSlot, or(sload(ownershipStorageSlot), bitmaskBurned))

// For diamond storage, we'll simply add the offset of the layout struct.
mstore(0x20, add(erc721aDiamondStorageSlot, 4))
mstore(0x00, index)
ownershipStorageSlot := keccak256(0x00, 0x40)
sstore(ownershipStorageSlot, or(sload(ownershipStorageSlot), bitmaskBurned))
}
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/ERC721ABurnableStartTokenIdMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ contract ERC721ABurnableStartTokenIdMock is StartTokenIdHelper, ERC721ABurnableM
) StartTokenIdHelper(startTokenId_) ERC721ABurnableMock(name_, symbol_) {}

function _startTokenId() internal view override returns (uint256) {
return startTokenId;
return startTokenId();
}
}
3 changes: 2 additions & 1 deletion contracts/mocks/ERC721AMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
pragma solidity ^0.8.4;

import '../ERC721A.sol';
import './DirectBurnBitSetterHelper.sol';

contract ERC721AMock is ERC721A {
contract ERC721AMock is ERC721A, DirectBurnBitSetterHelper {
constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {}

function numberMinted(address owner) public view returns (uint256) {
Expand Down
3 changes: 2 additions & 1 deletion contracts/mocks/ERC721AQueryableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ pragma solidity ^0.8.4;

import '../extensions/ERC721AQueryable.sol';
import '../extensions/ERC721ABurnable.sol';
import './DirectBurnBitSetterHelper.sol';

contract ERC721AQueryableMock is ERC721AQueryable, ERC721ABurnable {
contract ERC721AQueryableMock is ERC721AQueryable, ERC721ABurnable, DirectBurnBitSetterHelper {
constructor(string memory name_, string memory symbol_) ERC721A(name_, symbol_) {}

function safeMint(address to, uint256 quantity) public {
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ERC721AQueryableStartTokenIdMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ contract ERC721AQueryableStartTokenIdMock is StartTokenIdHelper, ERC721AQueryabl
) StartTokenIdHelper(startTokenId_) ERC721AQueryableMock(name_, symbol_) {}

function _startTokenId() internal view override returns (uint256) {
return startTokenId;
return startTokenId();
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/ERC721AStartTokenIdMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ contract ERC721AStartTokenIdMock is StartTokenIdHelper, ERC721AMock {
) StartTokenIdHelper(startTokenId_) ERC721AMock(name_, symbol_) {}

function _startTokenId() internal view override returns (uint256) {
return startTokenId;
return startTokenId();
}
}
19 changes: 17 additions & 2 deletions contracts/mocks/StartTokenIdHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,24 @@ pragma solidity ^0.8.4;
* to be returned by the overridden `_startTokenId()` function of ERC721A in the ERC721AStartTokenId mocks.
*/
contract StartTokenIdHelper {
uint256 public startTokenId;
// `bytes4(keccak256('startTokenId'))`.
uint256 private constant _START_TOKEN_ID_STORAGE_SLOT = 0x28f75032;

constructor(uint256 startTokenId_) {
startTokenId = startTokenId_;
_initializeStartTokenId(startTokenId_);
}

function startTokenId() public view returns (uint256 result) {
assembly {
result := sload(_START_TOKEN_ID_STORAGE_SLOT)
}
}

function _initializeStartTokenId(uint256 value) private {
// We use assembly to directly set the `startTokenId` in storage so that
// inheriting this class won't affect the layout of other storage slots.
assembly {
sstore(_START_TOKEN_ID_STORAGE_SLOT, value)
}
}
}
21 changes: 21 additions & 0 deletions test/ERC721A.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,27 @@ const createTestSuite = ({ contract, constructorArgs }) =>
});
});

context('with direct set burn bit', async function () {
it('ownerOf reverts for an uninitialized burnt token', async function () {
const [owner] = await ethers.getSigners();
await this.erc721a['safeMint(address,uint256)'](owner.address, 3);
await this.erc721a['safeMint(address,uint256)'](owner.address, 2);
await this.erc721a['safeMint(address,uint256)'](owner.address, 1);
for (let i = 0; i < 3 + 2 + 1; ++i) {
expect(await this.erc721a.ownerOf(this.startTokenId + i)).to.eq(owner.address);
}
await this.erc721a.directSetBurnBit(this.startTokenId + 3);
for (let i = 0; i < 3 + 2 + 1; ++i) {
if (3 <= i && i < 3 + 2) {
await expect(this.erc721a.ownerOf(this.startTokenId + i))
.to.be.revertedWith('OwnerQueryForNonexistentToken');
} else {
expect(await this.erc721a.ownerOf(this.startTokenId + i)).to.eq(owner.address);
}
}
});
});

context('_toString', async function () {
it('returns correct value', async function () {
expect(await this.erc721a['toString(uint256)']('0')).to.eq('0');
Expand Down
30 changes: 30 additions & 0 deletions test/extensions/ERC721AQueryable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,17 @@ const createTestSuite = ({ contract, constructorArgs }) =>
// Verify the function can still read the correct token ids
expect(ownerTokens).to.eql(offsetted(6, 8));
});

it('with direct set burn bit', async function () {
await this.erc721aQueryable['safeMint(address,uint256)'](this.addr3.address, 3);
await this.erc721aQueryable['safeMint(address,uint256)'](this.addr3.address, 2);
const nextTokenId = this.owner.expected.tokens[this.owner.expected.tokens.length - 1].add(1);
expect(await this.erc721aQueryable.tokensOfOwner(this.addr3.address))
.to.eql(this.addr3.expected.tokens.concat(Array.from({length: 5}, (_, i) => nextTokenId.add(i))));
await this.erc721aQueryable.directSetBurnBit(nextTokenId);
expect(await this.erc721aQueryable.tokensOfOwner(this.addr3.address))
.to.eql(this.addr3.expected.tokens.concat(Array.from({length: 2}, (_, i) => nextTokenId.add(3 + i))));
});
});

describe('tokensOfOwnerIn', async function () {
Expand Down Expand Up @@ -218,6 +229,17 @@ const createTestSuite = ({ contract, constructorArgs }) =>
subTests('after a token burn', async function () {
await this.erc721aQueryable.burn(offsetted(7));
});

it('with direct set burn bit', async function () {
await this.erc721aQueryable['safeMint(address,uint256)'](this.addr3.address, 3);
await this.erc721aQueryable['safeMint(address,uint256)'](this.addr3.address, 2);
const nextTokenId = this.owner.expected.tokens[this.owner.expected.tokens.length - 1].add(1);
expect(await this.erc721aQueryable.tokensOfOwnerIn(this.addr3.address, 0, 99))
.to.eql(this.addr3.expected.tokens.concat(Array.from({length: 5}, (_, i) => nextTokenId.add(i))));
await this.erc721aQueryable.directSetBurnBit(nextTokenId);
expect(await this.erc721aQueryable.tokensOfOwnerIn(this.addr3.address, 0, 99))
.to.eql(this.addr3.expected.tokens.concat(Array.from({length: 2}, (_, i) => nextTokenId.add(3 + i))));
})
});

describe('explicitOwnershipOf', async function () {
Expand Down Expand Up @@ -245,6 +267,14 @@ const createTestSuite = ({ contract, constructorArgs }) =>
const explicitOwnership = await this.erc721aQueryable.explicitOwnershipOf(this.currentIndex);
expectExplicitOwnershipNotExists(explicitOwnership);
});

it('with direct set burn bit', async function () {
await this.erc721aQueryable.directSetBurnBit(this.addr3.expected.tokens[0]);
for (let i = 0; i < this.addr3.expected.tokens.length; ++i) {
const explicitOwnership = await this.erc721aQueryable.explicitOwnershipOf(this.addr3.expected.tokens[i]);
expectExplicitOwnershipBurned(explicitOwnership, this.addr3.address);
}
});
});

describe('explicitOwnershipsOf', async function () {
Expand Down