Skip to content

Commit

Permalink
Remove unnecessary internal functions, max out test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
Vectorized committed Aug 27, 2024
1 parent c48aa14 commit ab31bd7
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 80 deletions.
42 changes: 0 additions & 42 deletions contracts/ERC721A.sol
Original file line number Diff line number Diff line change
Expand Up @@ -820,41 +820,6 @@ contract ERC721A is IERC721A {
}
}

/**
* @dev Equivalent to `_safeBatchTransferFrom(from, to, tokenIds)`.
*/
function _safeBatchTransferFrom(
address from,
address to,
uint256[] memory tokenIds
) internal virtual {
_safeBatchTransferFrom(address(0), from, to, tokenIds);
}

/**
* @dev Equivalent to `_safeBatchTransferFrom(by, from, to, tokenIds, '')`.
*/
function _safeBatchTransferFrom(
address by,
address from,
address to,
uint256[] memory tokenIds
) internal virtual {
_safeBatchTransferFrom(by, from, to, tokenIds, '');
}

/**
* @dev Equivalent to `_safeBatchTransferFrom(address(0), from, to, tokenIds, _data)`.
*/
function _safeBatchTransferFrom(
address from,
address to,
uint256[] memory tokenIds,
bytes memory _data
) internal virtual {
_safeBatchTransferFrom(address(0), from, to, tokenIds, _data);
}

/**
* @dev Safely transfers `tokenIds` in batch from `from` to `to`.
*
Expand Down Expand Up @@ -1375,13 +1340,6 @@ contract ERC721A is IERC721A {
}
}

/**
* @dev Equivalent to `_batchBurn(address(0), tokenIds)`.
*/
function _batchBurn(uint256[] memory tokenIds) internal virtual {
_batchBurn(address(0), tokenIds);
}

/**
* @dev Destroys `tokenIds`.
* Approvals are not cleared when tokenIds are burned.
Expand Down
10 changes: 0 additions & 10 deletions contracts/mocks/ERC721ABatchBurnableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,4 @@ contract ERC721ABatchBurnableMock is ERC721ABatchBurnable, DirectBurnBitSetterHe
function initializeOwnershipAt(uint256 index) public {
_initializeOwnershipAt(index);
}

function batchBurnUnoptimized(uint256[] memory tokenIds) public {
unchecked {
uint256 tokenId;
for (uint256 i; i < tokenIds.length; ++i) {
tokenId = tokenIds[i];
_burn(tokenId);
}
}
}
}
34 changes: 10 additions & 24 deletions contracts/mocks/ERC721ABatchTransferableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import '../extensions/ERC721ABatchTransferable.sol';
contract ERC721ABatchTransferableMock is ERC721ABatchTransferable {
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);
}
Expand All @@ -21,22 +17,6 @@ contract ERC721ABatchTransferableMock is ERC721ABatchTransferable {
return _ownershipAt(index);
}

function totalMinted() public view returns (uint256) {
return _totalMinted();
}

function totalBurned() public view returns (uint256) {
return _totalBurned();
}

function numberBurned(address owner) public view returns (uint256) {
return _numberBurned(owner);
}

function burn(uint256 tokenId) public {
_burn(tokenId, true);
}

function initializeOwnershipAt(uint256 index) public {
_initializeOwnershipAt(index);
}
Expand All @@ -59,10 +39,8 @@ contract ERC721ABatchTransferableMock is ERC721ABatchTransferable {
uint256[] memory tokenIds
) public {
unchecked {
uint256 tokenId;
for (uint256 i; i < tokenIds.length; ++i) {
tokenId = tokenIds[i];
transferFrom(from, to, tokenId);
for (uint256 i; i != tokenIds.length; ++i) {
transferFrom(from, to, tokenIds[i]);
}
}
}
Expand All @@ -75,4 +53,12 @@ contract ERC721ABatchTransferableMock is ERC721ABatchTransferable {
) public {
_batchTransferFrom(by, from, to, tokenIds);
}

function directBatchTransferFrom(
address from,
address to,
uint256[] memory tokenIds
) public {
_batchTransferFrom(from, to, tokenIds);
}
}
16 changes: 16 additions & 0 deletions test/GasUsage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ describe('ERC721A Gas Usage', function () {
});
});

context('mintHundred', function () {
it('runs mintHundred 2 times', async function () {
for (let i = 0; i < 2; i++) {
await this.erc721a.mintHundred(this.addr1.address);
}
});
});

context('safeMintHundred', function () {
it('runs safeMintHundred 2 times', async function () {
for (let i = 0; i < 2; i++) {
await this.erc721a.safeMintHundred(this.addr1.address);
}
});
});

context('transferFrom', function () {
beforeEach(async function () {
await this.erc721a.mintTen(this.owner.address);
Expand Down
4 changes: 4 additions & 0 deletions test/extensions/ERC721ABatchBurnable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ const createTestSuite = ({ contract, constructorArgs }) =>
});
});

it('batch burn nothing', async function () {
await this.erc721aBatchBurnable.connect(this.addr1).batchBurn([]);
});

it('changes numberBurned', async function () {
expect(await this.erc721aBatchBurnable.numberBurned(this.addr1.address)).to.equal(this.totalBurned);
await this.erc721aBatchBurnable.connect(this.addr1).batchBurn([this.notBurnedTokenId4]);
Expand Down
27 changes: 23 additions & 4 deletions test/extensions/ERC721ABatchTransferable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@ const createTestSuite = ({ contract, constructorArgs }) =>
await this.erc721aBatchTransferable['safeMint(address,uint256)'](this.addr3.address, 7);
});

it('test safe batch transfer with data', async function () {
const transferFn = 'safeBatchTransferFrom(address,address,uint256[],bytes)';
const tokensToTransfer = this.addr1.expected.tokens;
await expect(this.erc721aBatchTransferable.connect(this.addr1)[transferFn](
this.addr1.address, this.receiver.address, tokensToTransfer, '0x01'
)).to.be.revertedWith('reverted in the receiver contract!');
});

it('batch transfer nothing', async function () {
const transferFn = 'safeBatchTransferFrom(address,address,uint256[])';
await this.erc721aBatchTransferable.connect(this.addr1)[transferFn](
this.addr1.address, this.receiver.address, []);
});

context('test batch transfer functionality', function () {
const testSuccessfulBatchTransfer = function (transferFn, transferToContract = true) {
describe('successful transfers', async function () {
Expand Down Expand Up @@ -93,8 +107,8 @@ const createTestSuite = ({ contract, constructorArgs }) =>
// Transfer part of uninitialized tokens
this.tokensToTransferAlt = [25, 26, 27];
this.transferTxAlt = await this.erc721aBatchTransferable.connect(this.addr3)[transferFn](
this.addr3.address, this.addr5.address, this.tokensToTransferAlt
);
this.addr3.address, this.addr5.address, this.tokensToTransferAlt
);
});

it('emits Transfers event', async function () {
Expand Down Expand Up @@ -383,14 +397,18 @@ const createTestSuite = ({ contract, constructorArgs }) =>
await this.erc721aBatchTransferable.connect(this.addr2).setApprovalForAll(this.sender.address, true);
await expect(
this.erc721aBatchTransferable
.connect(this.sender)['directBatchTransferFrom'](
.connect(this.sender)['directBatchTransferFrom(address,address,address,uint256[])'](
ZERO_ADDRESS, ZERO_ADDRESS, this.sender.address, this.tokenIds
)
).to.be.revertedWith('TransferFromIncorrectOwner');
this.erc721aBatchTransferable
.connect(this.sender)['directBatchTransferFrom'](
.connect(this.sender)['directBatchTransferFrom(address,address,address,uint256[])'](
ZERO_ADDRESS, this.addr2.address, this.sender.address, this.tokenIds
);
this.erc721aBatchTransferable
.connect(this.addr2)['directBatchTransferFrom(address,address,uint256[])'](
this.sender.address, this.addr2.address, this.tokenIds
);
});

it('rejects transfer to zero address', async function () {
Expand Down Expand Up @@ -478,6 +496,7 @@ const createTestSuite = ({ contract, constructorArgs }) =>
testApproveBatchTransfer(fn, false);
});
});

context('safeBatchTransferFrom', function (fn = 'safeBatchTransferFrom(address,address,uint256[])') {
describe('to contract', function () {
testSuccessfulBatchTransfer(fn);
Expand Down

0 comments on commit ab31bd7

Please sign in to comment.