Skip to content

Commit a6af08c

Browse files
RogerPodacterRogerPodacter
authored
Merge pull request #147 from ethscriptions-protocol/fix_collections
Refactor token ID handling in ERC20FixedDenomination and ERC404NullOw…
2 parents 6b89550 + 83ef4a3 commit a6af08c

File tree

3 files changed

+40
-54
lines changed

3 files changed

+40
-54
lines changed

‎contracts/src/ERC20FixedDenomination.sol‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ contract ERC20FixedDenomination is ERC404NullOwnerCappedUpgradeable {
9090
_transferERC20(from, to, units());
9191

9292
// Transfer the specific NFT using the proper function
93-
uint256 id = ID_ENCODING_PREFIX + nftId;
94-
_transferERC721(from, to, id);
93+
_transferERC721(from, to, nftId);
9594
}
9695

9796
// =============================================================
@@ -154,25 +153,14 @@ contract ERC20FixedDenomination is ERC404NullOwnerCappedUpgradeable {
154153

155154
/// @notice Returns metadata URI for NFT tokens
156155
/// @dev Returns a data URI with JSON metadata fetched from the main Ethscriptions contract
157-
function tokenURI(uint256 id_) public view virtual override returns (string memory) {
158-
// This will revert InvalidTokenId / NotFound on bad ids
159-
ownerOf(id_);
160-
161-
uint256 mintId = id_ & ~ID_ENCODING_PREFIX;
156+
function tokenURI(uint256 mintId) public view virtual override returns (string memory) {
157+
_validateTokenId(mintId);
158+
ownerOf(mintId); // reverts on invalid / nonexistent
162159

163160
// Get the ethscriptionId for this mintId from the manager
164161
ERC20FixedDenominationManager mgr = ERC20FixedDenominationManager(manager);
165162
bytes32 ethscriptionId = mgr.getMintEthscriptionId(deployEthscriptionId, mintId);
166163

167-
if (ethscriptionId == bytes32(0)) {
168-
// If no ethscription found, return minimal metadata
169-
return string(abi.encodePacked(
170-
"data:application/json;utf8,",
171-
'{"name":"', name(), ' Note #', mintId.toString(), '",',
172-
'"description":"Denomination note for ', mintAmount().toString(), ' tokens"}'
173-
));
174-
}
175-
176164
// Get the ethscription data from the main contract
177165
Ethscriptions ethscriptionsContract = Ethscriptions(Predeploys.ETHSCRIPTIONS);
178166
Ethscriptions.Ethscription memory ethscription = ethscriptionsContract.getEthscription(ethscriptionId, false);
@@ -183,7 +171,7 @@ contract ERC20FixedDenomination is ERC404NullOwnerCappedUpgradeable {
183171

184172
// Build the JSON metadata
185173
string memory jsonStart = string.concat(
186-
'{"name":"', name(), ' Note #', mintId.toString(), '"',
174+
'{"name":"', name(), ' Token #', mintId.toString(), '"',
187175
',"description":"Fixed denomination token for ', mintAmount().toString(), ' ', symbol(), ' tokens"'
188176
);
189177

‎contracts/src/ERC404NullOwnerCappedUpgradeable.sol‎

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,15 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
6969
/// keccak256(abi.encode(uint256(keccak256("ethscriptions.storage.ERC404NullOwnerCapped")) - 1)) & ~bytes32(uint256(0xff))
7070
bytes32 private constant STORAGE_LOCATION = 0x8a0c9d8e5f7b3a2c1d4e6f8a9b7c5d3e2f1a4b6c8d9e7f5a3b2c1d4e6f8a9b00;
7171

72-
/// @dev Constant for token id encoding
73-
uint256 public constant ID_ENCODING_PREFIX = 1 << 255;
74-
7572
// =============================================================
7673
// EVENTS
7774
// =============================================================
7875

7976
// ERC20 Events are inherited from IERC20 (Transfer, Approval)
8077

8178
// ERC721 Events (using different names to avoid conflicts with ERC20)
79+
// event Transfer(address indexed from, address indexed to, uint256 value);
80+
event ERC20Transfer(address indexed from, address indexed to, uint256 value);
8281
event ERC721Transfer(address indexed from, address indexed to, uint256 indexed id);
8382
event ERC721Approval(address indexed owner, address indexed spender, uint256 indexed id);
8483
event ApprovalForAll(address indexed owner, address indexed operator, bool approved);
@@ -125,14 +124,9 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
125124
) internal onlyInitializing {
126125
TokenStorage storage $ = _getS();
127126

128-
if (cap_ == 0 || cap_ > ID_ENCODING_PREFIX - 1) {
129-
revert ERC20InvalidCap(cap_);
130-
}
131-
127+
if (cap_ == 0) revert ERC20InvalidCap(cap_);
132128
uint256 base = 10 ** decimals();
133-
if (units_ == 0 || units_ % base != 0) {
134-
revert InvalidUnits(units_);
135-
}
129+
if (units_ == 0 || units_ % base != 0) revert InvalidUnits(units_);
136130

137131
$.name = name_;
138132
$.symbol = symbol_;
@@ -173,6 +167,17 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
173167
TokenStorage storage $ = _getS();
174168
return $.balances[account];
175169
}
170+
171+
function balanceOf(address owner_, uint256 id_)
172+
public
173+
view
174+
returns (uint256)
175+
{
176+
TokenStorage storage $ = _getS();
177+
TokenData storage t = $.tokens[id_];
178+
if (!t.exists) return 0;
179+
return t.owner == owner_ ? 1 : 0;
180+
}
176181

177182
function allowance(address owner, address spender) public view virtual override(IERC404, IERC20) returns (uint256) {
178183
TokenStorage storage $ = _getS();
@@ -202,16 +207,11 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
202207
}
203208

204209
function ownerOf(uint256 id_) public view virtual override(IERC404) returns (address) {
205-
if (!_isValidTokenId(id_)) {
206-
revert InvalidTokenId();
207-
}
208-
210+
_validateTokenId(id_);
209211
TokenStorage storage $ = _getS();
210212
TokenData storage t = $.tokens[id_];
211213

212-
if (!t.exists) {
213-
revert NotFound();
214-
}
214+
if (!t.exists) revert NotFound();
215215

216216
return t.owner;
217217
}
@@ -222,7 +222,10 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
222222
}
223223

224224
function getApproved(uint256 id_) public view virtual returns (address) {
225+
_validateTokenId(id_);
225226
TokenStorage storage $ = _getS();
227+
if (!$.tokens[id_].exists) revert NotFound();
228+
226229
return $.getApproved[id_];
227230
}
228231

@@ -347,9 +350,6 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
347350
if (newSupply > $.cap) {
348351
revert ERC20ExceededCap(newSupply, $.cap);
349352
}
350-
if (newSupply > ID_ENCODING_PREFIX) {
351-
revert MintLimitReached();
352-
}
353353
$.totalSupply = newSupply;
354354
} else {
355355
// Transfer
@@ -367,16 +367,16 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
367367
}
368368

369369
emit Transfer(from_, to_, value_);
370+
// emit ERC20Transfer(from_, to_, value_);
370371
}
371372

372373
/// @notice Transfer an ERC721 token
373374
function _transferERC721(address from_, address to_, uint256 id_) internal virtual {
374375
TokenStorage storage $ = _getS();
375376
TokenData storage t = $.tokens[id_];
376-
377-
if (from_ != ownerOf(id_)) {
378-
revert Unauthorized();
379-
}
377+
378+
if (!t.exists) revert NotFound();
379+
if (from_ != t.owner) revert Unauthorized();
380380

381381
if (from_ != address(0)) {
382382
// Clear approval
@@ -417,24 +417,19 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
417417
if (to_ == address(0)) {
418418
revert InvalidRecipient();
419419
}
420-
if (nftId_ == 0 || nftId_ >= ID_ENCODING_PREFIX - 1) {
421-
revert InvalidTokenId();
422-
}
420+
_validateTokenId(nftId_);
423421

424422
TokenStorage storage $ = _getS();
425423

426-
// Add the ID_ENCODING_PREFIX to the provided ID
427-
uint256 id = ID_ENCODING_PREFIX + nftId_;
428-
429-
TokenData storage t = $.tokens[id];
424+
TokenData storage t = $.tokens[nftId_];
430425

431426
// Check if this NFT already exists (including null-owner)
432427
if (t.exists) {
433428
revert AlreadyExists();
434429
}
435430

436431
t.exists = true;
437-
_transferERC721(address(0), to_, id);
432+
_transferERC721(address(0), to_, nftId_);
438433

439434
// Increment minted supply counter
440435
$.minted++;
@@ -444,8 +439,11 @@ abstract contract ERC404NullOwnerCappedUpgradeable is
444439
// HELPER FUNCTIONS
445440
// =============================================================
446441

447-
function _isValidTokenId(uint256 id_) internal pure returns (bool) {
448-
return id_ > ID_ENCODING_PREFIX && id_ != type(uint256).max;
442+
/// @dev Simple tokenId validation: nonzero and not max uint256.
443+
function _validateTokenId(uint256 id_) internal pure {
444+
if (id_ == 0 || id_ == type(uint256).max) {
445+
revert InvalidTokenId();
446+
}
449447
}
450448

451449
// =============================================================

‎contracts/test/ERC404FixedDenominationNullOwner.t.sol‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,13 @@ contract ERC404FixedDenominationNullOwnerTest is TestSetup {
108108
// Mint to bob
109109
mintNote(tokenAddr, "TNULL", 1, 1000, bytes32(uint256(0xAAAA)), bob);
110110
assertEq(token.balanceOf(bob), 1000 * 1e18);
111-
assertEq(token.ownerOf(token.ID_ENCODING_PREFIX() + 1), bob);
111+
assertEq(token.ownerOf(1), bob);
112112
assertEq(token.totalSupply(), 1000 * 1e18);
113113

114114
// Mint to null owner (initialOwner = 0) should end with 0x0 owning NFT and ERC20
115115
mintNote(tokenAddr, "TNULL", 2, 1000, bytes32(uint256(0xBBBB)), address(0));
116116
assertEq(token.balanceOf(address(0)), 1000 * 1e18);
117-
assertEq(token.ownerOf(token.ID_ENCODING_PREFIX() + 2), address(0));
117+
assertEq(token.ownerOf(2), address(0));
118118
assertEq(token.totalSupply(), 2000 * 1e18);
119119
}
120120

@@ -132,7 +132,7 @@ contract ERC404FixedDenominationNullOwnerTest is TestSetup {
132132

133133
assertEq(token.totalSupply(), supplyBefore);
134134
assertEq(token.balanceOf(address(0)), 1000 * 1e18);
135-
assertEq(token.ownerOf(token.ID_ENCODING_PREFIX() + 1), address(0));
135+
assertEq(token.ownerOf(1), address(0));
136136
}
137137

138138
function testCapEnforcedOnMint() public {

0 commit comments

Comments
 (0)