Skip to content

Commit

Permalink
GMS-991: Add safeBurn and safeBurnBatch. (#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dlevy47
dlevy47 authored Aug 24, 2023
1 parent 574cdfb commit 620be0f
Show file tree
Hide file tree
Showing 7 changed files with 229 additions and 25 deletions.
6 changes: 6 additions & 0 deletions contracts/errors/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ interface IImmutableERC721Errors {

/// @dev Caller is not approved or owner
error IImmutableERC721NotOwnerOrOperator(uint256 tokenId);

/// @dev Current token owner is not what was expected
error IImmutableERC721MismatchedTokenOwner(
uint256 tokenId,
address currentOwner
);
}

interface RoyaltyEnforcementErrors {
Expand Down
28 changes: 27 additions & 1 deletion contracts/token/erc721/abstract/ERC721Hybrid.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,32 @@ abstract contract ERC721Hybrid is ERC721PsiBurnable, ERC721, IImmutableERC721Err
}
}

/// @dev Burn a token, checking the owner of the token against the parameter first.
function safeBurn(address owner, uint256 tokenId) public virtual {
address currentOwner = ownerOf(tokenId);
if (currentOwner != owner) {
revert IImmutableERC721MismatchedTokenOwner(tokenId, currentOwner);
}

burn(tokenId);
}

/// @dev A singular safe burn request.
struct IDBurn {
address owner;
uint256[] tokenIds;
}

/// @dev Burn a batch of tokens, checking the owner of each token first.
function _safeBurnBatch(IDBurn[] memory burns) internal {
for (uint i = 0; i < burns.length; i++) {
IDBurn memory b = burns[i];
for (uint j = 0; j < b.tokenIds.length; j++) {
safeBurn(b.owner, b.tokenIds[j]);
}
}
}

/// @dev overriding erc721 and erc721psi _safemint, super calls the `_safeMint` method of
/// the erc721 implementation due to inheritance linearisation
function _safeMint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Psi) {
Expand Down Expand Up @@ -291,4 +317,4 @@ abstract contract ERC721Hybrid is ERC721PsiBurnable, ERC721, IImmutableERC721Err
}
return ERC721Psi.transferFrom(from, to, tokenId);
}
}
}
26 changes: 26 additions & 0 deletions contracts/token/erc721/abstract/ImmutableERC721Base.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ abstract contract ImmutableERC721Base is
uint256[] tokenIds;
}

/// @dev A singular safe burn request.
struct IDBurn {
address owner;
uint256[] tokenIds;
}

/// @dev A singular batch transfer request
struct TransferRequest {
address from;
Expand Down Expand Up @@ -216,6 +222,26 @@ abstract contract ImmutableERC721Base is
_totalSupply--;
}

/// @dev Burn a token, checking the owner of the token against the parameter first.
function safeBurn(address owner, uint256 tokenId) public virtual {
address currentOwner = ownerOf(tokenId);
if (currentOwner != owner) {
revert IImmutableERC721MismatchedTokenOwner(tokenId, currentOwner);
}

burn(tokenId);
}

/// @dev Burn a batch of tokens, checking the owner of each token first.
function _safeBurnBatch(IDBurn[] memory burns) public virtual {
for (uint i = 0; i < burns.length; i++) {
IDBurn memory b = burns[i];
for (uint j = 0; j < b.tokenIds.length; j++) {
safeBurn(b.owner, b.tokenIds[j]);
}
}
}

/// ===== Internal functions =====

/// @dev mints specified token ids to specified address
Expand Down
6 changes: 5 additions & 1 deletion contracts/token/erc721/preset/ImmutableERC721.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ contract ImmutableERC721 is ImmutableERC721HybridBase {
_safeMintBatchByIDToMultiple(mints);
}

function safeBurnBatch(IDBurn[] memory burns) external {
_safeBurnBatch(burns);
}

function safeTransferFromBatch(TransferRequest calldata tr) external {
if (tr.tokenIds.length != tr.tos.length) {
revert IImmutableERC721MismatchedTransferLengths();
Expand All @@ -63,4 +67,4 @@ contract ImmutableERC721 is ImmutableERC721HybridBase {
}
}

}
}
7 changes: 6 additions & 1 deletion contracts/token/erc721/preset/ImmutableERC721MintByID.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ contract ImmutableERC721MintByID is ImmutableERC721Base {
}
}

/// @dev Burn a batch of tokens, checking the owner of each token first.
function safeBurnBatch(IDBurn[] memory burns) external {
_safeBurnBatch(burns);
}

/// @dev Allows owner or operator to transfer a batch of tokens
function safeTransferFromBatch(TransferRequest calldata tr) external {
if (tr.tokenIds.length != tr.tos.length) {
Expand All @@ -99,4 +104,4 @@ contract ImmutableERC721MintByID is ImmutableERC721Base {
safeTransferFrom(tr.from, tr.tos[i], tr.tokenIds[i]);
}
}
}
}
86 changes: 76 additions & 10 deletions test/token/erc721/ImmutableERC721HybridPermissionedMintable.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,19 @@ describe("ImmutableERC721", function () {
});
it("Should allow minting of batch tokens", async function () {
const mintRequests = [
{ to: user.address, tokenIds: [9, 10, 11] },
{ to: user.address, tokenIds: [9, 10, 11, 20] },
{ to: owner.address, tokenIds: [12, 13, 14] },
];
await erc721.connect(minter).safeMintBatch(mintRequests);
expect(await erc721.balanceOf(user.address)).to.equal(8);
expect(await erc721.balanceOf(user.address)).to.equal(9);
expect(await erc721.balanceOf(owner.address)).to.equal(6);
expect(await erc721.totalSupply()).to.equal(14);
expect(await erc721.ownerOf(3)).to.equal(user.address);
expect(await erc721.ownerOf(4)).to.equal(user.address);
expect(await erc721.ownerOf(5)).to.equal(user.address);
expect(await erc721.ownerOf(6)).to.equal(owner.address);
expect(await erc721.ownerOf(7)).to.equal(owner.address);
expect(await erc721.ownerOf(8)).to.equal(owner.address);
expect(await erc721.totalSupply()).to.equal(15);
expect(await erc721.ownerOf(9)).to.equal(user.address);
expect(await erc721.ownerOf(10)).to.equal(user.address);
expect(await erc721.ownerOf(11)).to.equal(user.address);
expect(await erc721.ownerOf(12)).to.equal(owner.address);
expect(await erc721.ownerOf(13)).to.equal(owner.address);
expect(await erc721.ownerOf(14)).to.equal(owner.address);
});

it("Should allow batch minting of tokens by quantity", async function () {
Expand Down Expand Up @@ -163,6 +163,72 @@ describe("ImmutableERC721", function () {
);
});

it("Should not allow owner or approved to burn a token when specifying the incorrect owner", async function() {
await expect(
erc721.connect(user).safeBurn(owner.address, 5)
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(5, user.address);
});

it("Should allow owner or approved to safely burn a token when specifying the correct owner", async function() {
const originalBalance = await erc721.balanceOf(user.address);
const originalSupply = await erc721.totalSupply();
await erc721.connect(user).safeBurn(user.address, 5);
expect(await erc721.balanceOf(user.address)).to.equal(
originalBalance.sub(1)
);
expect(await erc721.totalSupply()).to.equal(
originalSupply.sub(1)
);
});

it("Should not allow owner or approved to burn a batch of tokens when specifying the incorrect owners", async function() {
const burns = [
{
owner: user.address,
tokenIds: [12, 13, 14],
},
{
owner: owner.address,
tokenIds: [9, 10, 11],
},
];
await expect(
erc721.connect(user).safeBurnBatch(burns)
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(12, owner.address);
});

it("Should allow owner or approved to safely burn a batch of tokens when specifying the correct owners", async function() {
const originalUserBalance = await erc721.balanceOf(user.address);
const originalOwnerBalance = await erc721.balanceOf(owner.address);
const originalSupply = await erc721.totalSupply();

// Set approval for owner to burn these tokens from user.
await erc721.connect(user).approve(owner.address, 9);
await erc721.connect(user).approve(owner.address, 10);
await erc721.connect(user).approve(owner.address, 11);

const burns = [
{
owner: owner.address,
tokenIds: [12, 13, 14],
},
{
owner: user.address,
tokenIds: [9, 10, 11],
},
];
await erc721.connect(owner).safeBurnBatch(burns);
expect(await erc721.balanceOf(user.address)).to.equal(
originalUserBalance.sub(3)
);
expect(await erc721.balanceOf(owner.address)).to.equal(
originalOwnerBalance.sub(3)
);
expect(await erc721.totalSupply()).to.equal(
originalSupply.sub(6)
);
});

it("Should prevent not approved to burn a batch of tokens", async function () {
const first = await erc721.bulkMintThreshold();
await expect(
Expand Down Expand Up @@ -197,7 +263,7 @@ describe("ImmutableERC721", function () {
});

it("Should revert with a burnt tokenId", async function () {
const tokenId = 10;
const tokenId = 20;
await erc721.connect(user).burn(tokenId);
await expect(erc721.tokenURI(tokenId)).to.be.revertedWith(
"ERC721: invalid token ID"
Expand Down
95 changes: 83 additions & 12 deletions test/token/erc721/ImmutableERC721PermissionedMintable.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,26 +99,30 @@ describe("Immutable ERC721 Permissioned Mintable Test Cases", function () {

it("Should allow safe minting of batch tokens", async function () {
const mintRequests = [
{ to: user.address, tokenIds: [2, 3, 4] },
{ to: owner.address, tokenIds: [6, 7, 8] },
{ to: user.address, tokenIds: [2, 3, 4, 5, 6] },
{ to: owner.address, tokenIds: [7, 8, 9, 10, 11] },
];
await erc721.connect(minter).safeMintBatch(mintRequests);
expect(await erc721.balanceOf(user.address)).to.equal(4);
expect(await erc721.balanceOf(owner.address)).to.equal(3);
expect(await erc721.totalSupply()).to.equal(7);
expect(await erc721.balanceOf(user.address)).to.equal(6);
expect(await erc721.balanceOf(owner.address)).to.equal(5);
expect(await erc721.totalSupply()).to.equal(11);
expect(await erc721.ownerOf(2)).to.equal(user.address);
expect(await erc721.ownerOf(3)).to.equal(user.address);
expect(await erc721.ownerOf(4)).to.equal(user.address);
expect(await erc721.ownerOf(6)).to.equal(owner.address);
expect(await erc721.ownerOf(5)).to.equal(user.address);
expect(await erc721.ownerOf(6)).to.equal(user.address);
expect(await erc721.ownerOf(7)).to.equal(owner.address);
expect(await erc721.ownerOf(8)).to.equal(owner.address);
expect(await erc721.ownerOf(9)).to.equal(owner.address);
expect(await erc721.ownerOf(10)).to.equal(owner.address);
expect(await erc721.ownerOf(11)).to.equal(owner.address);
});

it("Should allow owner or approved to burn a batch of tokens", async function () {
expect(await erc721.balanceOf(user.address)).to.equal(4);
expect(await erc721.balanceOf(user.address)).to.equal(6);
await erc721.connect(user).burnBatch([1, 2]);
expect(await erc721.balanceOf(user.address)).to.equal(2);
expect(await erc721.totalSupply()).to.equal(5);
expect(await erc721.balanceOf(user.address)).to.equal(4);
expect(await erc721.totalSupply()).to.equal(9);
});

it("Should prevent not approved to burn a batch of tokens", async function () {
Expand All @@ -133,17 +137,84 @@ describe("Immutable ERC721 Permissioned Mintable Test Cases", function () {
.to.be.revertedWith("IImmutableERC721TokenAlreadyBurned")
.withArgs(1);
});

it("Should not allow owner or approved to safely burn a token when specifying the incorrect owner", async function() {
await expect(
erc721.connect(user).safeBurn(owner.address, 3)
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(3, user.address);
});

it("Should allow owner or approved to safely burn a token when specifying the correct owner", async function() {
const originalBalance = await erc721.balanceOf(user.address);
const originalSupply = await erc721.totalSupply();
await erc721.connect(user).safeBurn(user.address, 3);
expect(await erc721.balanceOf(user.address)).to.equal(
originalBalance.sub(1)
);
expect(await erc721.totalSupply()).to.equal(
originalSupply.sub(1)
);
});

it("Should not allow owner or approved to burn a batch of tokens when specifying the incorrect owners", async function() {
const burns = [
{
owner: user.address,
tokenIds: [7, 8, 9],
},
{
owner: owner.address,
tokenIds: [4, 5, 6],
},
];

await expect(
erc721.connect(user).safeBurnBatch(burns)
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(7, owner.address);
});

it("Should allow owner or approved to safely burn a batch of tokens when specifying the correct owners", async function() {
const originalUserBalance = await erc721.balanceOf(user.address);
const originalOwnerBalance = await erc721.balanceOf(owner.address);
const originalSupply = await erc721.totalSupply();

// Set approval for owner to burn these tokens from user.
await erc721.connect(user).approve(owner.address, 4);
await erc721.connect(user).approve(owner.address, 5);
await erc721.connect(user).approve(owner.address, 6);

const burns = [
{
owner: owner.address,
tokenIds: [7, 8, 9],
},
{
owner: user.address,
tokenIds: [4, 5, 6],
},
];
await erc721.connect(owner).safeBurnBatch(burns);
expect(await erc721.balanceOf(user.address)).to.equal(
originalUserBalance.sub(3)
);
expect(await erc721.balanceOf(owner.address)).to.equal(
originalOwnerBalance.sub(3)
);
expect(await erc721.totalSupply()).to.equal(
originalSupply.sub(6)
);
});
});

describe("Base URI and Token URI", function () {
it("Should return a non-empty tokenURI when the base URI is set", async function () {
const tokenId = 10;
const tokenId = 100;
await erc721.connect(minter).mint(user.address, tokenId);
expect(await erc721.tokenURI(tokenId)).to.equal(`${baseURI}${tokenId}`);
});

it("Should revert with a burnt tokenId", async function () {
const tokenId = 10;
const tokenId = 100;
await erc721.connect(user).burn(tokenId);
await expect(erc721.tokenURI(tokenId)).to.be.revertedWith(
"ERC721: invalid token ID"
Expand Down Expand Up @@ -172,7 +243,7 @@ describe("Immutable ERC721 Permissioned Mintable Test Cases", function () {

it("Should return an empty token URI when the base URI is not set", async function () {
await erc721.setBaseURI("");
const tokenId = 12;
const tokenId = 101;
await erc721.connect(minter).mint(user.address, tokenId);
expect(await erc721.tokenURI(tokenId)).to.equal("");
});
Expand Down

0 comments on commit 620be0f

Please sign in to comment.