Skip to content

Enhance ERC721 Contracts with Base64 Metadata Encoding #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RogerPodacter merged 2 commits into from
Nov 12, 2025
Merged

Enhance ERC721 Contracts with Base64 Metadata Encoding #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f25b1a
Enhance ERC721 Contracts with Base64 Metadata Encoding
RogerPodacter Nov 10, 2025
64230ae
Implement Ownership Transfer and Renouncement in ERC721 Ethscriptions
RogerPodacter Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions app/models/erc721_ethscriptions_collection_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,21 @@ class Erc721EthscriptionsCollectionParser
'item' => :single_item
}
},
'transfer_ownership' => {
keys: %w[collection_id new_owner],
abi_type: '(bytes32,address)',
validators: {
'collection_id' => :bytes32,
'new_owner' => :address
}
},
'renounce_ownership' => {
keys: %w[collection_id],
abi_type: 'bytes32',
validators: {
'collection_id' => :bytes32
}
},
'remove_items' => {
keys: %w[collection_id ethscription_ids],
abi_type: '(bytes32,bytes32[])',
Expand Down Expand Up @@ -399,6 +414,10 @@ def encode_operation(operation, data, schema, content_hash: nil)
build_create_and_add_self_values(validated_data, content_hash: content_hash)
when 'add_self_to_collection'
build_add_self_to_collection_values(validated_data, content_hash: content_hash)
when 'transfer_ownership'
build_transfer_ownership_values(validated_data)
when 'renounce_ownership'
build_renounce_ownership_values(validated_data)
when 'remove_items'
build_remove_items_values(validated_data)
when 'edit_collection'
Expand Down Expand Up @@ -488,6 +507,18 @@ def validate_bytes32(value, field_name)
[value[2..]].pack('H*')
end

def validate_address(value, field_name)
unless value.is_a?(String) && value.match?(/\A0x[0-9a-f]{40}\z/)
raise ValidationError, "Invalid address for #{field_name}: #{value}"
end

if value == '0x0000000000000000000000000000000000000000'
raise ValidationError, "Address cannot be zero for #{field_name}"
end

value.downcase
end

@cursor cursor Bot Nov 12, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Case Sensitivity Breaks Address Validation

The validate_address method rejects valid EIP-55 checksummed Ethereum addresses containing uppercase hex characters. The regex pattern /\A0x[0-9a-f]{40}\z/ only accepts lowercase hex, but Ethereum addresses are case-insensitive and often use mixed-case checksums. Users providing checksummed addresses (e.g., 0xAbCd...) will encounter validation errors even though these addresses are valid. The method already downcases the result, so accepting mixed case input wouldn't affect the output.

Fix in Cursor Fix in Web


def validate_bytes32_array(value, field_name)
unless value.is_a?(Array)
raise ValidationError, "Expected array for #{field_name}"
Expand Down Expand Up @@ -675,6 +706,14 @@ def build_add_self_to_collection_values(data, content_hash:)
[data['collection_id'], item_tuple]
end

def build_transfer_ownership_values(data)
[data['collection_id'], data['new_owner']]
end

def build_renounce_ownership_values(data)
data['collection_id']
end

def build_remove_items_values(data)
[data['collection_id'], data['ethscription_ids']]
end
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/ERC721EthscriptionsCollection.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ contract ERC721EthscriptionsCollection is ERC721EthscriptionsEnumerableUpgradeab
',"',
mediaType,
'":"',
mediaUri.escapeJSON(),
mediaUri,

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mediaUri is not being escaped with .escapeJSON() despite being inserted into a JSON string. This is inconsistent with line 127 (removed) which had proper escaping. This could lead to JSON injection if the media URI contains special characters like quotes or backslashes.

Suggested change
mediaUri,
mediaUri.escapeJSON(),

Copilot uses AI. Check for mistakes.
'"'
);

Expand Down Expand Up @@ -182,7 +182,7 @@ contract ERC721EthscriptionsCollection is ERC721EthscriptionsEnumerableUpgradeab
'"}'
);

return json;
return string.concat("data:application/json;base64,", Base64.encode(bytes(json)));
}

function safeTransferFrom(address, address, uint256, bytes memory)
Expand Down
36 changes: 29 additions & 7 deletions contracts/src/ERC721EthscriptionsCollectionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
event ItemsRemoved(bytes32 indexed collectionId, uint256 count, bytes32 updateTxHash);
event CollectionEdited(bytes32 indexed collectionId);
event CollectionLocked(bytes32 indexed collectionId);
event OwnershipTransferred(bytes32 indexed collectionId, address indexed previousOwner, address indexed newOwner);

modifier onlyEthscriptions() {
require(msg.sender == address(ethscriptions), "Only Ethscriptions contract");
Expand Down Expand Up @@ -184,6 +185,17 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
_addSingleItem(op.collectionId, ethscriptionId, op.item);
}

function op_transfer_ownership(bytes32 ethscriptionId, bytes calldata data) external onlyEthscriptions {
(bytes32 collectionId, address newOwner) = abi.decode(data, (bytes32, address));
require(newOwner != address(0), "New owner cannot be zero address");
_transferCollectionOwnership(ethscriptionId, collectionId, newOwner);
}

function op_renounce_ownership(bytes32 ethscriptionId, bytes calldata data) external onlyEthscriptions {
bytes32 collectionId = abi.decode(data, (bytes32));
_transferCollectionOwnership(ethscriptionId, collectionId, address(0));
}

function op_remove_items(bytes32 ethscriptionId, bytes calldata data) external onlyEthscriptions {
RemoveItemsOperation memory removeOp = abi.decode(data, (RemoveItemsOperation));
CollectionRecord storage collection = collectionStore[removeOp.collectionId];
Expand Down Expand Up @@ -266,12 +278,6 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
}

function onTransfer(bytes32 ethscriptionId, address from, address to) external override onlyEthscriptions {
if (collectionExists(ethscriptionId)) {
ERC721EthscriptionsCollection collection = ERC721EthscriptionsCollection(collectionStore[ethscriptionId].collectionContract);
collection.factoryTransferOwnership(to);
return;
}

Membership storage membership = membershipOfEthscription[ethscriptionId];

if (!collectionExists(membership.collectionId)) {
Expand Down Expand Up @@ -454,6 +460,23 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
require(currentOwner == sender, errorMessage);
}

function _transferCollectionOwnership(bytes32 ethscriptionId, bytes32 collectionId, address newOwner) private {
CollectionRecord storage collection = collectionStore[collectionId];
require(collection.collectionContract != address(0), "Collection does not exist");

address sender = _getEthscriptionCreator(ethscriptionId);
ERC721EthscriptionsCollection collectionContract = ERC721EthscriptionsCollection(collection.collectionContract);
address currentOwner = collectionContract.owner();
require(currentOwner == sender, "Only collection owner can transfer");

if (newOwner == currentOwner) {
revert("New owner must differ");
}

collectionContract.factoryTransferOwnership(newOwner);
emit OwnershipTransferred(collectionId, currentOwner, newOwner);
}

function _verifyItemMerkleProof(ItemData memory item, bytes32 merkleRoot) private pure {
require(merkleRoot != bytes32(0), "Merkle proof required");

Expand Down Expand Up @@ -488,4 +511,3 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
return getCollection(collectionId);
}
}

79 changes: 69 additions & 10 deletions contracts/src/NameRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import "@openzeppelin/contracts/utils/Strings.sol";
import {Base64} from "solady/utils/Base64.sol";
import {LibString} from "solady/utils/LibString.sol";
import "./ERC721EthscriptionsEnumerableUpgradeable.sol";
import "./interfaces/IProtocolHandler.sol";
import "./Ethscriptions.sol";
import "./libraries/Predeploys.sol";
import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";

/// @title NameRegistry
/// @notice Handles legacy word-domain registrations and mirrors ownership as an ERC-721 collection.
contract NameRegistry is ERC721EthscriptionsEnumerableUpgradeable, IProtocolHandler {
using Strings for uint256;
using LibString for *;

Ethscriptions public constant ethscriptions = Ethscriptions(Predeploys.ETHSCRIPTIONS);

string public constant PROTOCOL_NAME = "word-domains";
string public constant protocolName = "word-domains";

Copilot AI Nov 10, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing protocolName from a function to a public constant breaks the IProtocolHandler interface requirement. The interface defines protocolName() as a function that must be implemented. This will cause the contract to fail compilation or break interface compatibility.

Recommendation: Keep protocolName() as a function returning the constant string, or modify the interface if changing it is intentional.

Suggested change
string public constant protocolName = "word-domains";
function protocolName() public pure override returns (string memory) {
return "word-domains";
}

Copilot uses AI. Check for mistakes.

struct DomainRecord {
bytes32 packedName;
Expand Down Expand Up @@ -62,10 +62,6 @@ contract NameRegistry is ERC721EthscriptionsEnumerableUpgradeable, IProtocolHand
return "NAME";
}

function protocolName() external pure returns (string memory) {
return PROTOCOL_NAME;
}

// ============================
// Protocol handler functions
// ============================
Expand Down Expand Up @@ -192,27 +188,90 @@ contract NameRegistry is ERC721EthscriptionsEnumerableUpgradeable, IProtocolHand
// Get the ethscription data to extract the ethscription number
Ethscriptions.Ethscription memory ethscription = ethscriptions.getEthscription(record.ethscriptionId, false);

// Get the media URI from the ethscription
(string memory mediaType, string memory mediaUri) = ethscriptions.getMediaUri(record.ethscriptionId);

// Convert ethscriptionId to hex string (0x prefixed)
string memory ethscriptionIdHex = uint256(record.ethscriptionId).toHexString(32);

bytes memory json = abi.encodePacked(
'{"name":"',
name,
name.escapeJSON(),
'","description":"Dotless word domain"',
',"ethscription_id":"',
ethscriptionIdHex,
'","ethscription_number":',
ethscription.ethscriptionNumber.toString(),
',"attributes":[',
',"',
mediaType,
'":"',
mediaUri,

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mediaUri should be escaped with .escapeJSON() to prevent JSON injection attacks. Unlike other URIs that are resolved/processed, this is directly embedded in the JSON output without escaping. If the media URI contains characters like quotes or backslashes, it could break the JSON structure.

Suggested change
mediaUri,
mediaUri.escapeJSON(),

Copilot uses AI. Check for mistakes.
'","attributes":[',
'{"trait_type":"Name","value":"',
name,
name.escapeJSON(),
'"}',
']}'
);

return string(abi.encodePacked("data:application/json;base64,", Base64.encode(json)));
}

/// @notice OpenSea collection-level metadata
/// @return JSON string with collection metadata
function contractURI() external pure returns (string memory) {
return string(abi.encodePacked(
'data:application/json;base64,',
Base64.encode(bytes(
'{"name":"Word Domains Registry",'
'"description":"On-chain word domain name system for Ethscriptions. Register unique, dotless domain names as NFTs.",'
'"image":"data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNTAwIiBoZWlnaHQ9IjUwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iNTAwIiBoZWlnaHQ9IjUwMCIgZmlsbD0iIzEwMTAxMCIvPjx0ZXh0IHg9IjI1MCIgeT0iMjUwIiBmb250LXNpemU9IjgwIiBmb250LWZhbWlseT0ibW9ub3NwYWNlIiB0ZXh0LWFuY2hvcj0ibWlkZGxlIiBmaWxsPSIjMDBmZjAwIj5bTkFNRVNdPC90ZXh0Pjwvc3ZnPg==",'
'"external_link":"https://ethscriptions.com"}'
))
));
}

// --- Transfer/approvals blocked externally ---------------------------------

function transferFrom(address, address, uint256)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransfersDisabled();
}

function safeTransferFrom(address, address, uint256)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransfersDisabled();
}

function safeTransferFrom(address, address, uint256, bytes memory)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransfersDisabled();
}

function approve(address, uint256)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransfersDisabled();
}

function setApprovalForAll(address, bool)
public
pure
override(ERC721EthscriptionsUpgradeable, IERC721)
{
revert TransfersDisabled();
}

function _update(address to, uint256 tokenId, address auth) internal override returns (address) {
if (auth != address(0) && auth != address(this)) {
revert TransfersDisabled();
Expand Down
14 changes: 13 additions & 1 deletion contracts/test/CollectionURIResolution.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity ^0.8.24;

import "./TestSetup.sol";
import {LibString} from "solady/utils/LibString.sol";
import {Base64} from "solady/utils/Base64.sol";

contract CollectionURIResolutionTest is TestSetup {
using LibString for *;
Expand Down Expand Up @@ -33,7 +34,18 @@ contract CollectionURIResolutionTest is TestSetup {
string memory contractUri = collection.contractURI();

assertTrue(bytes(contractUri).length > 0, "Should have contractURI");
assertTrue(contractUri.contains(regularUri), "Should contain original URI");

// contractURI returns base64-encoded JSON, decode it
// Check it starts with data URI prefix
string memory prefix = "data:application/json;base64,";
assertTrue(contractUri.startsWith(prefix), "Should be a data URI");

// Extract and decode the base64 part
string memory base64Part = contractUri.slice(bytes(prefix).length);
bytes memory decodedBytes = Base64.decode(base64Part);
string memory decodedJson = string(decodedBytes);

assertTrue(decodedJson.contains(regularUri), "Should contain original URI");
}

function test_DataURIPassesThrough() public {
Expand Down
Loading
Loading