Skip to content

Commit 620be0f

Browse files
dlevy47dlevy47
authored
GMS-991: Add safeBurn and safeBurnBatch. (#69)
1 parent 574cdfb commit 620be0f

File tree

7 files changed

+229
-25
lines changed

7 files changed

+229
-25
lines changed

‎contracts/errors/Errors.sol

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ interface IImmutableERC721Errors {
1515

1616
/// @dev Caller is not approved or owner
1717
error IImmutableERC721NotOwnerOrOperator(uint256 tokenId);
18+
19+
/// @dev Current token owner is not what was expected
20+
error IImmutableERC721MismatchedTokenOwner(
21+
uint256 tokenId,
22+
address currentOwner
23+
);
1824
}
1925

2026
interface RoyaltyEnforcementErrors {

‎contracts/token/erc721/abstract/ERC721Hybrid.sol

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,32 @@ abstract contract ERC721Hybrid is ERC721PsiBurnable, ERC721, IImmutableERC721Err
181181
}
182182
}
183183

184+
/// @dev Burn a token, checking the owner of the token against the parameter first.
185+
function safeBurn(address owner, uint256 tokenId) public virtual {
186+
address currentOwner = ownerOf(tokenId);
187+
if (currentOwner != owner) {
188+
revert IImmutableERC721MismatchedTokenOwner(tokenId, currentOwner);
189+
}
190+
191+
burn(tokenId);
192+
}
193+
194+
/// @dev A singular safe burn request.
195+
struct IDBurn {
196+
address owner;
197+
uint256[] tokenIds;
198+
}
199+
200+
/// @dev Burn a batch of tokens, checking the owner of each token first.
201+
function _safeBurnBatch(IDBurn[] memory burns) internal {
202+
for (uint i = 0; i < burns.length; i++) {
203+
IDBurn memory b = burns[i];
204+
for (uint j = 0; j < b.tokenIds.length; j++) {
205+
safeBurn(b.owner, b.tokenIds[j]);
206+
}
207+
}
208+
}
209+
184210
/// @dev overriding erc721 and erc721psi _safemint, super calls the `_safeMint` method of
185211
/// the erc721 implementation due to inheritance linearisation
186212
function _safeMint(address to, uint256 tokenId) internal virtual override(ERC721, ERC721Psi) {
@@ -291,4 +317,4 @@ abstract contract ERC721Hybrid is ERC721PsiBurnable, ERC721, IImmutableERC721Err
291317
}
292318
return ERC721Psi.transferFrom(from, to, tokenId);
293319
}
294-
}
320+
}

‎contracts/token/erc721/abstract/ImmutableERC721Base.sol

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ abstract contract ImmutableERC721Base is
4848
uint256[] tokenIds;
4949
}
5050

51+
/// @dev A singular safe burn request.
52+
struct IDBurn {
53+
address owner;
54+
uint256[] tokenIds;
55+
}
56+
5157
/// @dev A singular batch transfer request
5258
struct TransferRequest {
5359
address from;
@@ -216,6 +222,26 @@ abstract contract ImmutableERC721Base is
216222
_totalSupply--;
217223
}
218224

225+
/// @dev Burn a token, checking the owner of the token against the parameter first.
226+
function safeBurn(address owner, uint256 tokenId) public virtual {
227+
address currentOwner = ownerOf(tokenId);
228+
if (currentOwner != owner) {
229+
revert IImmutableERC721MismatchedTokenOwner(tokenId, currentOwner);
230+
}
231+
232+
burn(tokenId);
233+
}
234+
235+
/// @dev Burn a batch of tokens, checking the owner of each token first.
236+
function _safeBurnBatch(IDBurn[] memory burns) public virtual {
237+
for (uint i = 0; i < burns.length; i++) {
238+
IDBurn memory b = burns[i];
239+
for (uint j = 0; j < b.tokenIds.length; j++) {
240+
safeBurn(b.owner, b.tokenIds[j]);
241+
}
242+
}
243+
}
244+
219245
/// ===== Internal functions =====
220246

221247
/// @dev mints specified token ids to specified address

‎contracts/token/erc721/preset/ImmutableERC721.sol

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ contract ImmutableERC721 is ImmutableERC721HybridBase {
5353
_safeMintBatchByIDToMultiple(mints);
5454
}
5555

56+
function safeBurnBatch(IDBurn[] memory burns) external {
57+
_safeBurnBatch(burns);
58+
}
59+
5660
function safeTransferFromBatch(TransferRequest calldata tr) external {
5761
if (tr.tokenIds.length != tr.tos.length) {
5862
revert IImmutableERC721MismatchedTransferLengths();
@@ -63,4 +67,4 @@ contract ImmutableERC721 is ImmutableERC721HybridBase {
6367
}
6468
}
6569

66-
}
70+
}

‎contracts/token/erc721/preset/ImmutableERC721MintByID.sol

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ contract ImmutableERC721MintByID is ImmutableERC721Base {
8989
}
9090
}
9191

92+
/// @dev Burn a batch of tokens, checking the owner of each token first.
93+
function safeBurnBatch(IDBurn[] memory burns) external {
94+
_safeBurnBatch(burns);
95+
}
96+
9297
/// @dev Allows owner or operator to transfer a batch of tokens
9398
function safeTransferFromBatch(TransferRequest calldata tr) external {
9499
if (tr.tokenIds.length != tr.tos.length) {
@@ -99,4 +104,4 @@ contract ImmutableERC721MintByID is ImmutableERC721Base {
99104
safeTransferFrom(tr.from, tr.tos[i], tr.tokenIds[i]);
100105
}
101106
}
102-
}
107+
}

‎test/token/erc721/ImmutableERC721HybridPermissionedMintable.test.ts

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,19 @@ describe("ImmutableERC721", function () {
8989
});
9090
it("Should allow minting of batch tokens", async function () {
9191
const mintRequests = [
92-
{ to: user.address, tokenIds: [9, 10, 11] },
92+
{ to: user.address, tokenIds: [9, 10, 11, 20] },
9393
{ to: owner.address, tokenIds: [12, 13, 14] },
9494
];
9595
await erc721.connect(minter).safeMintBatch(mintRequests);
96-
expect(await erc721.balanceOf(user.address)).to.equal(8);
96+
expect(await erc721.balanceOf(user.address)).to.equal(9);
9797
expect(await erc721.balanceOf(owner.address)).to.equal(6);
98-
expect(await erc721.totalSupply()).to.equal(14);
99-
expect(await erc721.ownerOf(3)).to.equal(user.address);
100-
expect(await erc721.ownerOf(4)).to.equal(user.address);
101-
expect(await erc721.ownerOf(5)).to.equal(user.address);
102-
expect(await erc721.ownerOf(6)).to.equal(owner.address);
103-
expect(await erc721.ownerOf(7)).to.equal(owner.address);
104-
expect(await erc721.ownerOf(8)).to.equal(owner.address);
98+
expect(await erc721.totalSupply()).to.equal(15);
99+
expect(await erc721.ownerOf(9)).to.equal(user.address);
100+
expect(await erc721.ownerOf(10)).to.equal(user.address);
101+
expect(await erc721.ownerOf(11)).to.equal(user.address);
102+
expect(await erc721.ownerOf(12)).to.equal(owner.address);
103+
expect(await erc721.ownerOf(13)).to.equal(owner.address);
104+
expect(await erc721.ownerOf(14)).to.equal(owner.address);
105105
});
106106

107107
it("Should allow batch minting of tokens by quantity", async function () {
@@ -163,6 +163,72 @@ describe("ImmutableERC721", function () {
163163
);
164164
});
165165

166+
it("Should not allow owner or approved to burn a token when specifying the incorrect owner", async function() {
167+
await expect(
168+
erc721.connect(user).safeBurn(owner.address, 5)
169+
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(5, user.address);
170+
});
171+
172+
it("Should allow owner or approved to safely burn a token when specifying the correct owner", async function() {
173+
const originalBalance = await erc721.balanceOf(user.address);
174+
const originalSupply = await erc721.totalSupply();
175+
await erc721.connect(user).safeBurn(user.address, 5);
176+
expect(await erc721.balanceOf(user.address)).to.equal(
177+
originalBalance.sub(1)
178+
);
179+
expect(await erc721.totalSupply()).to.equal(
180+
originalSupply.sub(1)
181+
);
182+
});
183+
184+
it("Should not allow owner or approved to burn a batch of tokens when specifying the incorrect owners", async function() {
185+
const burns = [
186+
{
187+
owner: user.address,
188+
tokenIds: [12, 13, 14],
189+
},
190+
{
191+
owner: owner.address,
192+
tokenIds: [9, 10, 11],
193+
},
194+
];
195+
await expect(
196+
erc721.connect(user).safeBurnBatch(burns)
197+
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(12, owner.address);
198+
});
199+
200+
it("Should allow owner or approved to safely burn a batch of tokens when specifying the correct owners", async function() {
201+
const originalUserBalance = await erc721.balanceOf(user.address);
202+
const originalOwnerBalance = await erc721.balanceOf(owner.address);
203+
const originalSupply = await erc721.totalSupply();
204+
205+
// Set approval for owner to burn these tokens from user.
206+
await erc721.connect(user).approve(owner.address, 9);
207+
await erc721.connect(user).approve(owner.address, 10);
208+
await erc721.connect(user).approve(owner.address, 11);
209+
210+
const burns = [
211+
{
212+
owner: owner.address,
213+
tokenIds: [12, 13, 14],
214+
},
215+
{
216+
owner: user.address,
217+
tokenIds: [9, 10, 11],
218+
},
219+
];
220+
await erc721.connect(owner).safeBurnBatch(burns);
221+
expect(await erc721.balanceOf(user.address)).to.equal(
222+
originalUserBalance.sub(3)
223+
);
224+
expect(await erc721.balanceOf(owner.address)).to.equal(
225+
originalOwnerBalance.sub(3)
226+
);
227+
expect(await erc721.totalSupply()).to.equal(
228+
originalSupply.sub(6)
229+
);
230+
});
231+
166232
it("Should prevent not approved to burn a batch of tokens", async function () {
167233
const first = await erc721.bulkMintThreshold();
168234
await expect(
@@ -197,7 +263,7 @@ describe("ImmutableERC721", function () {
197263
});
198264

199265
it("Should revert with a burnt tokenId", async function () {
200-
const tokenId = 10;
266+
const tokenId = 20;
201267
await erc721.connect(user).burn(tokenId);
202268
await expect(erc721.tokenURI(tokenId)).to.be.revertedWith(
203269
"ERC721: invalid token ID"

‎test/token/erc721/ImmutableERC721PermissionedMintable.test.ts

Lines changed: 83 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -99,26 +99,30 @@ describe("Immutable ERC721 Permissioned Mintable Test Cases", function () {
9999

100100
it("Should allow safe minting of batch tokens", async function () {
101101
const mintRequests = [
102-
{ to: user.address, tokenIds: [2, 3, 4] },
103-
{ to: owner.address, tokenIds: [6, 7, 8] },
102+
{ to: user.address, tokenIds: [2, 3, 4, 5, 6] },
103+
{ to: owner.address, tokenIds: [7, 8, 9, 10, 11] },
104104
];
105105
await erc721.connect(minter).safeMintBatch(mintRequests);
106-
expect(await erc721.balanceOf(user.address)).to.equal(4);
107-
expect(await erc721.balanceOf(owner.address)).to.equal(3);
108-
expect(await erc721.totalSupply()).to.equal(7);
106+
expect(await erc721.balanceOf(user.address)).to.equal(6);
107+
expect(await erc721.balanceOf(owner.address)).to.equal(5);
108+
expect(await erc721.totalSupply()).to.equal(11);
109109
expect(await erc721.ownerOf(2)).to.equal(user.address);
110110
expect(await erc721.ownerOf(3)).to.equal(user.address);
111111
expect(await erc721.ownerOf(4)).to.equal(user.address);
112-
expect(await erc721.ownerOf(6)).to.equal(owner.address);
112+
expect(await erc721.ownerOf(5)).to.equal(user.address);
113+
expect(await erc721.ownerOf(6)).to.equal(user.address);
113114
expect(await erc721.ownerOf(7)).to.equal(owner.address);
114115
expect(await erc721.ownerOf(8)).to.equal(owner.address);
116+
expect(await erc721.ownerOf(9)).to.equal(owner.address);
117+
expect(await erc721.ownerOf(10)).to.equal(owner.address);
118+
expect(await erc721.ownerOf(11)).to.equal(owner.address);
115119
});
116120

117121
it("Should allow owner or approved to burn a batch of tokens", async function () {
118-
expect(await erc721.balanceOf(user.address)).to.equal(4);
122+
expect(await erc721.balanceOf(user.address)).to.equal(6);
119123
await erc721.connect(user).burnBatch([1, 2]);
120-
expect(await erc721.balanceOf(user.address)).to.equal(2);
121-
expect(await erc721.totalSupply()).to.equal(5);
124+
expect(await erc721.balanceOf(user.address)).to.equal(4);
125+
expect(await erc721.totalSupply()).to.equal(9);
122126
});
123127

124128
it("Should prevent not approved to burn a batch of tokens", async function () {
@@ -133,17 +137,84 @@ describe("Immutable ERC721 Permissioned Mintable Test Cases", function () {
133137
.to.be.revertedWith("IImmutableERC721TokenAlreadyBurned")
134138
.withArgs(1);
135139
});
140+
141+
it("Should not allow owner or approved to safely burn a token when specifying the incorrect owner", async function() {
142+
await expect(
143+
erc721.connect(user).safeBurn(owner.address, 3)
144+
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(3, user.address);
145+
});
146+
147+
it("Should allow owner or approved to safely burn a token when specifying the correct owner", async function() {
148+
const originalBalance = await erc721.balanceOf(user.address);
149+
const originalSupply = await erc721.totalSupply();
150+
await erc721.connect(user).safeBurn(user.address, 3);
151+
expect(await erc721.balanceOf(user.address)).to.equal(
152+
originalBalance.sub(1)
153+
);
154+
expect(await erc721.totalSupply()).to.equal(
155+
originalSupply.sub(1)
156+
);
157+
});
158+
159+
it("Should not allow owner or approved to burn a batch of tokens when specifying the incorrect owners", async function() {
160+
const burns = [
161+
{
162+
owner: user.address,
163+
tokenIds: [7, 8, 9],
164+
},
165+
{
166+
owner: owner.address,
167+
tokenIds: [4, 5, 6],
168+
},
169+
];
170+
171+
await expect(
172+
erc721.connect(user).safeBurnBatch(burns)
173+
).to.be.revertedWith('IImmutableERC721MismatchedTokenOwner').withArgs(7, owner.address);
174+
});
175+
176+
it("Should allow owner or approved to safely burn a batch of tokens when specifying the correct owners", async function() {
177+
const originalUserBalance = await erc721.balanceOf(user.address);
178+
const originalOwnerBalance = await erc721.balanceOf(owner.address);
179+
const originalSupply = await erc721.totalSupply();
180+
181+
// Set approval for owner to burn these tokens from user.
182+
await erc721.connect(user).approve(owner.address, 4);
183+
await erc721.connect(user).approve(owner.address, 5);
184+
await erc721.connect(user).approve(owner.address, 6);
185+
186+
const burns = [
187+
{
188+
owner: owner.address,
189+
tokenIds: [7, 8, 9],
190+
},
191+
{
192+
owner: user.address,
193+
tokenIds: [4, 5, 6],
194+
},
195+
];
196+
await erc721.connect(owner).safeBurnBatch(burns);
197+
expect(await erc721.balanceOf(user.address)).to.equal(
198+
originalUserBalance.sub(3)
199+
);
200+
expect(await erc721.balanceOf(owner.address)).to.equal(
201+
originalOwnerBalance.sub(3)
202+
);
203+
expect(await erc721.totalSupply()).to.equal(
204+
originalSupply.sub(6)
205+
);
206+
});
136207
});
137208

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

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

173244
it("Should return an empty token URI when the base URI is not set", async function () {
174245
await erc721.setBaseURI("");
175-
const tokenId = 12;
246+
const tokenId = 101;
176247
await erc721.connect(minter).mint(user.address, tokenId);
177248
expect(await erc721.tokenURI(tokenId)).to.equal("");
178249
});

0 commit comments

Comments
 (0)