Merge pull request #142 from ethscriptions-protocol/fix_collections

Enhance ERC721 Contracts with Base64 Metadata Encoding

Author: RogerPodacter

app/models/erc721_ethscriptions_collection_parser.rb

@@ -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[])',
@@ -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'
@@ -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
+
   def validate_bytes32_array(value, field_name)
     unless value.is_a?(Array)
       raise ValidationError, "Expected array for #{field_name}"
@@ -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

contracts/src/ERC721EthscriptionsCollection.sol

@@ -124,7 +124,7 @@ contract ERC721EthscriptionsCollection is ERC721EthscriptionsEnumerableUpgradeab
             ',"',
             mediaType,
             '":"',
-            mediaUri.escapeJSON(),
+            mediaUri,
             '"'
         );
 
@@ -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)

contracts/src/ERC721EthscriptionsCollectionManager.sol

@@ -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");
@@ -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];
@@ -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)) {
@@ -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");
 
@@ -488,4 +511,3 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
         return getCollection(collectionId);
     }
 }
-

contracts/src/NameRegistry.sol

@@ -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";
 
     struct DomainRecord {
         bytes32 packedName;
@@ -62,10 +62,6 @@ contract NameRegistry is ERC721EthscriptionsEnumerableUpgradeable, IProtocolHand
         return "NAME";
     }
 
-    function protocolName() external pure returns (string memory) {
-        return PROTOCOL_NAME;
-    }
-
     // ============================
     // Protocol handler functions
     // ============================
@@ -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,
+            '","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();

contracts/test/CollectionURIResolution.t.sol

@@ -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 *;
@@ -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 {