Skip to content

Commit

Permalink
[H-3] BatchMetadata modules may apply baseURI to incorrect token ids
Browse files Browse the repository at this point in the history
  • Loading branch information
@GWSzeto
GWSzeto committed Sep 28, 2024
1 parent a58c573 commit 5c9ceeb
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 169 deletions.
8 changes: 4 additions & 4 deletions src/module/token/metadata/BatchMetadataERC1155.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ contract BatchMetadataERC1155 is BatchMetadataERC721, UpdateMetadataCallbackERC1
FallbackFunction({selector: this.setBaseURI.selector, permissionBits: Role._MANAGER_ROLE});
config.fallbackFunctions[2] =
FallbackFunction({selector: this.getAllMetadataBatches.selector, permissionBits: 0});
config.fallbackFunctions[3] = FallbackFunction({selector: this.nextTokenIdToMint.selector, permissionBits: 0});
config.fallbackFunctions[4] = FallbackFunction({selector: this.getBatchId.selector, permissionBits: 0});
config.fallbackFunctions[5] = FallbackFunction({selector: this.getBatchRange.selector, permissionBits: 0});
config.fallbackFunctions[3] = FallbackFunction({selector: this.getMetadataBatch.selector, permissionBits: 0});
config.fallbackFunctions[4] = FallbackFunction({selector: this.nextTokenIdToMint.selector, permissionBits: 0});
config.fallbackFunctions[5] = FallbackFunction({selector: this.getBatchIndex.selector, permissionBits: 0});

config.requiredInterfaces = new bytes4[](1);
config.requiredInterfaces[0] = 0xd9b67a26; // ERC1155
Expand All @@ -44,7 +44,7 @@ contract BatchMetadataERC1155 is BatchMetadataERC721, UpdateMetadataCallbackERC1
if (_startTokenId < _batchMetadataStorage().nextTokenIdRangeStart) {
revert BatchMetadataMetadataAlreadySet();
}
_setMetadata(_quantity, _baseURI);
_setMetadata(_startTokenId, _quantity, _baseURI);
}

}
110 changes: 39 additions & 71 deletions src/module/token/metadata/BatchMetadataERC721.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ library BatchMetadataStorage {
keccak256(abi.encode(uint256(keccak256("token.metadata.batch")) - 1)) & ~bytes32(uint256(0xff));

struct Data {
// tokenId range end
uint256[] tokenIdRangeEnd;
// next tokenId as range start
uint256 nextTokenIdRangeStart;
// tokenId range end => baseURI of range
mapping(uint256 => string) baseURIOfTokenIdRange;
// tokenId range end
BatchMetadataERC721.MetadataBatch[] metadataBatches;
}

function data() internal pure returns (Data storage data_) {
Expand All @@ -42,7 +40,7 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {
/**
* @notice MetadataBatch struct to store metadata for a range of tokenIds.
* @param startTokenIdInclusive The first tokenId in the range.
* @param endTokenIdNonInclusive The last tokenId in the range.
* @param endTokenIdInclusive The last tokenId in the range.
* @param baseURI The base URI for the range.
*/
struct MetadataBatch {
Expand All @@ -69,7 +67,7 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {
//////////////////////////////////////////////////////////////*/

/// @dev ERC-4906 Metadata Update.
event BatchMetadataUpdate(uint256 _fromTokenId, uint256 _toTokenId);
event BatchMetadataUpdate(uint256 startTokenIdIncluside, uint256 endTokenIdInclusive, string baseURI);

/*//////////////////////////////////////////////////////////////
MODULE CONFIG
Expand All @@ -89,9 +87,9 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {
FallbackFunction({selector: this.setBaseURI.selector, permissionBits: Role._MANAGER_ROLE});
config.fallbackFunctions[2] =
FallbackFunction({selector: this.getAllMetadataBatches.selector, permissionBits: 0});
config.fallbackFunctions[3] = FallbackFunction({selector: this.nextTokenIdToMint.selector, permissionBits: 0});
config.fallbackFunctions[4] = FallbackFunction({selector: this.getBatchId.selector, permissionBits: 0});
config.fallbackFunctions[5] = FallbackFunction({selector: this.getBatchRange.selector, permissionBits: 0});
config.fallbackFunctions[3] = FallbackFunction({selector: this.getMetadataBatch.selector, permissionBits: 0});
config.fallbackFunctions[4] = FallbackFunction({selector: this.nextTokenIdToMint.selector, permissionBits: 0});
config.fallbackFunctions[5] = FallbackFunction({selector: this.getBatchIndex.selector, permissionBits: 0});

config.requiredInterfaces = new bytes4[](1);
config.requiredInterfaces[0] = 0x80ac58cd; // ERC721.
Expand Down Expand Up @@ -121,7 +119,7 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {
if (_startTokenId < _batchMetadataStorage().nextTokenIdRangeStart) {
revert BatchMetadataMetadataAlreadySet();
}
_setMetadata(_quantity, _baseURI);
_setMetadata(_startTokenId, _quantity, _baseURI);
}

/*//////////////////////////////////////////////////////////////
Expand All @@ -130,71 +128,42 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {

/// @notice Returns all metadata batches for a token.
function getAllMetadataBatches() external view returns (MetadataBatch[] memory) {
uint256[] memory rangeEnds = _batchMetadataStorage().tokenIdRangeEnd;
uint256 numOfBatches = rangeEnds.length;

MetadataBatch[] memory batches = new MetadataBatch[](rangeEnds.length);

uint256 rangeStart = 0;
for (uint256 i = 0; i < numOfBatches; i += 1) {
batches[i] = MetadataBatch({
startTokenIdInclusive: rangeStart,
endTokenIdInclusive: rangeEnds[i] - 1,
baseURI: _batchMetadataStorage().baseURIOfTokenIdRange[rangeEnds[i]]
});
rangeStart = rangeEnds[i];
}
return _batchMetadataStorage().metadataBatches;
}

return batches;
/// @dev returns the metadata batch for a given batchIndex
function getMetadataBatch(uint256 _batchIndex) public view returns (MetadataBatch memory) {
return _batchMetadataStorage().metadataBatches[_batchIndex];
}

/// @notice Uploads metadata for a range of tokenIds.
function uploadMetadata(uint256 _amount, string calldata _baseURI) external virtual {
_setMetadata(_amount, _baseURI);
_setMetadata(_batchMetadataStorage().nextTokenIdRangeStart, _amount, _baseURI);
}

function nextTokenIdToMint() external view returns (uint256) {
return _batchMetadataStorage().nextTokenIdRangeStart;
}

/// @dev Returns the id for the batch of tokens the given tokenId belongs to.
function getBatchId(uint256 _tokenId) public view virtual returns (uint256 batchId, uint256 index) {
uint256[] memory rangeEnds = _batchMetadataStorage().tokenIdRangeEnd;
uint256 numOfBatches = rangeEnds.length;

for (uint256 i = 0; i < numOfBatches; i += 1) {
if (_tokenId < rangeEnds[i]) {
index = i;
batchId = rangeEnds[i];

return (batchId, index);
}
}
revert BatchMetadataNoMetadataForTokenId();
}

/// @dev returns the starting tokenId of a given batchId.
function getBatchRange(uint256 _batchID) public view returns (uint256, uint256) {
uint256[] memory rangeEnds = _batchMetadataStorage().tokenIdRangeEnd;
uint256 numOfBatches = rangeEnds.length;
/// @dev Returns the index for the batch of tokens the given tokenId belongs to.
function getBatchIndex(uint256 _tokenId) public view virtual returns (uint256) {
MetadataBatch[] memory batches = _batchMetadataStorage().metadataBatches;
uint256 numOfBatches = batches.length;

for (uint256 i = 0; i < numOfBatches; i += 1) {
if (_batchID == rangeEnds[i]) {
if (i > 0) {
return (rangeEnds[i - 1], rangeEnds[i] - 1);
}
return (0, rangeEnds[i] - 1);
if (_tokenId >= batches[i].startTokenIdInclusive && _tokenId <= batches[i].endTokenIdInclusive) {
return i;
}
}

revert BatchMetadataNoMetadataForTokenId();
}

/// @dev Sets the base URI for the batch of tokens with the given batchId.
function setBaseURI(uint256 _batchId, string memory _baseURI) external virtual {
_batchMetadataStorage().baseURIOfTokenIdRange[_batchId] = _baseURI;
(uint256 startTokenId,) = getBatchRange(_batchId);
emit BatchMetadataUpdate(startTokenId, _batchId);
/// @dev Sets the base URI for the batch based on the batchIndex.
function setBaseURI(uint256 _batchIndex, string memory _baseURI) external virtual {
MetadataBatch memory batch = _batchMetadataStorage().metadataBatches[_batchIndex];
batch.baseURI = _baseURI;
_batchMetadataStorage().metadataBatches[_batchIndex] = batch;
emit BatchMetadataUpdate(batch.startTokenIdInclusive, batch.endTokenIdInclusive, batch.baseURI);
}

/*//////////////////////////////////////////////////////////////
Expand All @@ -203,35 +172,34 @@ contract BatchMetadataERC721 is Module, UpdateMetadataCallbackERC721 {

/// @dev Returns the baseURI for a token. The intended metadata URI for the token is baseURI + indexInBatch.
function _getBaseURI(uint256 _tokenId) internal view returns (string memory baseUri, uint256 indexInBatch) {
uint256[] memory rangeEnds = _batchMetadataStorage().tokenIdRangeEnd;
uint256 numOfBatches = rangeEnds.length;
BatchMetadataERC721.MetadataBatch[] memory batches = _batchMetadataStorage().metadataBatches;
uint256 numOfBatches = batches.length;

for (uint256 i = 0; i < numOfBatches; i += 1) {
if (_tokenId < rangeEnds[i]) {
uint256 rangeStart = 0;
if (i > 0) {
rangeStart = rangeEnds[i - 1];
}
return (_batchMetadataStorage().baseURIOfTokenIdRange[rangeEnds[i]], _tokenId - rangeStart);
if (_tokenId >= batches[i].startTokenIdInclusive && _tokenId <= batches[i].endTokenIdInclusive) {
return (batches[i].baseURI, _tokenId - batches[i].startTokenIdInclusive);
}
}
revert BatchMetadataNoMetadataForTokenId();
}

/// @notice sets the metadata for a range of tokenIds.
function _setMetadata(uint256 _amount, string calldata _baseURI) internal virtual {
function _setMetadata(uint256 _rangeStart, uint256 _amount, string calldata _baseURI) internal virtual {
if (_amount == 0) {
revert BatchMetadataZeroAmount();
}

uint256 rangeStart = _batchMetadataStorage().nextTokenIdRangeStart;
uint256 rangeEndNonInclusive = rangeStart + _amount;
uint256 rangeEndNonInclusive = _rangeStart + _amount;

MetadataBatch memory batch = MetadataBatch({
startTokenIdInclusive: _rangeStart,
endTokenIdInclusive: rangeEndNonInclusive - 1,
baseURI: _baseURI
});
_batchMetadataStorage().metadataBatches.push(batch);
_batchMetadataStorage().nextTokenIdRangeStart = rangeEndNonInclusive;
_batchMetadataStorage().tokenIdRangeEnd.push(rangeEndNonInclusive);
_batchMetadataStorage().baseURIOfTokenIdRange[rangeEndNonInclusive] = _baseURI;

emit BatchMetadataUpdate(rangeStart, rangeEndNonInclusive - 1);
emit BatchMetadataUpdate(_rangeStart, rangeEndNonInclusive, _baseURI);
}

function _batchMetadataStorage() internal pure returns (BatchMetadataStorage.Data storage) {
Expand Down
41 changes: 21 additions & 20 deletions test/module/metadata/BatchMetadataERC1155.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,52 +53,53 @@ contract BatchMetadataERC1155Test is Test {
}

/*///////////////////////////////////////////////////////////////
Unit tests: `getBatchId`
Unit tests: `getBatchIndex`
//////////////////////////////////////////////////////////////*/

function test_state_getBatchId() public {
function test_state_getBatchIndex() public {
vm.prank(owner);
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base/");
(uint256 batchId, uint256 index) = BatchMetadataExt(address(core)).getBatchId(0);
uint256 batchIndex = BatchMetadataExt(address(core)).getBatchIndex(50);

assertEq(batchId, 100);
assertEq(index, 0);
assertEq(batchIndex, 0);
}

function test_revert_getBatchId() public {
function test_revert_getBatchIndex_noMetadata() public {
vm.prank(owner);
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base/");

vm.expectRevert(BatchMetadataERC721.BatchMetadataNoMetadataForTokenId.selector);
(uint256 batchId, uint256 index) = BatchMetadataExt(address(core)).getBatchId(101);
BatchMetadataExt(address(core)).getBatchIndex(101);
}

/*///////////////////////////////////////////////////////////////
Unit tests: `getBatchRange`
Unit tests: `getMetadataBatch`
//////////////////////////////////////////////////////////////*/

function test_state_getBatchRange() public {
function test_state_getMetadataBatch() public {
vm.startPrank(owner);
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base/");
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base/");
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base2/");
vm.stopPrank();

(uint256 startTokenId1, uint256 endTokenId1) = BatchMetadataExt(address(core)).getBatchRange(100);
(uint256 startTokenId2, uint256 endTokenId2) = BatchMetadataExt(address(core)).getBatchRange(200);
BatchMetadataERC1155.MetadataBatch memory batch = BatchMetadataExt(address(core)).getMetadataBatch(0);
BatchMetadataERC1155.MetadataBatch memory batch2 = BatchMetadataExt(address(core)).getMetadataBatch(1);

assertEq(startTokenId1, 0);
assertEq(endTokenId1, 99);
assertEq(startTokenId2, 100);
assertEq(endTokenId2, 199);
assertEq(batch.startTokenIdInclusive, 0);
assertEq(batch.endTokenIdInclusive, 99);
assertEq(batch.baseURI, "ipfs://base/");
assertEq(batch2.startTokenIdInclusive, 100);
assertEq(batch2.endTokenIdInclusive, 199);
assertEq(batch2.baseURI, "ipfs://base2/");
}

function test_revert_getBatchRange() public {
function test_revert_getMetadataBatch_invalidIndex() public {
vm.prank(owner);
BatchMetadataExt(address(core)).uploadMetadata(100, "ipfs://base/");

vm.expectRevert(BatchMetadataERC721.BatchMetadataNoMetadataForTokenId.selector);
vm.expectRevert();
vm.prank(owner);
BatchMetadataExt(address(core)).getBatchRange(101);
BatchMetadataExt(address(core)).getMetadataBatch(1);
}

/*///////////////////////////////////////////////////////////////
Expand All @@ -116,7 +117,7 @@ contract BatchMetadataERC1155Test is Test {
assertEq(batches[0].baseURI, "ipfs://base/");

vm.prank(owner);
BatchMetadataExt(address(core)).setBaseURI(100, "ipfs://base2/");
BatchMetadataExt(address(core)).setBaseURI(0, "ipfs://base2/");

// get metadata batches
BatchMetadataExt.MetadataBatch[] memory batches2 = BatchMetadataExt(address(core)).getAllMetadataBatches();
Expand Down
Loading

0 comments on commit 5c9ceeb

Please sign in to comment.