Merge pull request #145 from ethscriptions-protocol/fix_collections

Enhance ERC721 Ethscriptions Collection Parser with Initial Owner Support

Author: RogerPodacter

app/models/erc721_ethscriptions_collection_parser.rb

@@ -12,8 +12,8 @@ class Erc721EthscriptionsCollectionParser
   # Operation schemas defining exact structure and ABI encoding
   OPERATION_SCHEMAS = {
     'create_collection' => {
-      keys: %w[name symbol max_supply description logo_image_uri banner_image_uri background_color website_link twitter_link discord_link merkle_root],
-      abi_type: '(string,string,uint256,string,string,string,string,string,string,string,bytes32)',
+      keys: %w[name symbol max_supply description logo_image_uri banner_image_uri background_color website_link twitter_link discord_link merkle_root initial_owner],
+      abi_type: '(string,string,uint256,string,string,string,string,string,string,string,bytes32,address)',
       validators: {
         'name' => :string,
         'symbol' => :string,
@@ -25,14 +25,15 @@ class Erc721EthscriptionsCollectionParser
         'website_link' => :string,
         'twitter_link' => :string,
         'discord_link' => :string,
-        'merkle_root' => :bytes32
+        'merkle_root' => :bytes32,
+        'initial_owner' => :optional_address
       }
     },
     # New combined create op name used by the contract; keep legacy alias below
     'create_collection_and_add_self' => {
       keys: %w[metadata item],
-      # ((CollectionParams),(ItemData)) - ItemData now includes contentHash as first field
-      abi_type: '((string,string,uint256,string,string,string,string,string,string,string,bytes32),(bytes32,uint256,string,string,string,(string,string)[],bytes32[]))',
+      # ((CollectionParams),(ItemData)) - CollectionParams now includes initialOwner
+      abi_type: '((string,string,uint256,string,string,string,string,string,string,string,bytes32,address),(bytes32,uint256,string,string,string,(string,string)[],bytes32[]))',
       validators: {
         'metadata' => :collection_metadata,
         'item' => :single_item
@@ -41,7 +42,7 @@ class Erc721EthscriptionsCollectionParser
     # Legacy alias retained for backwards compatibility
     'create_and_add_self' => {
       keys: %w[metadata item],
-      abi_type: '((string,string,uint256,string,string,string,string,string,string,string,bytes32),(bytes32,uint256,string,string,string,(string,string)[],bytes32[]))',
+      abi_type: '((string,string,uint256,string,string,string,string,string,string,string,bytes32,address),(bytes32,uint256,string,string,string,(string,string)[],bytes32[]))',
       validators: {
         'metadata' => :collection_metadata,
         'item' => :single_item
@@ -132,21 +133,22 @@ class ValidationError < StandardError; end
 
   # New API: validate and encode protocol params
   # Unified interface - accepts all possible parameters, uses what it needs
-  def self.validate_and_encode(decoded_content:, operation:, params:, source:, ethscription_id: nil, **_extras)
+  def self.validate_and_encode(decoded_content:, operation:, params:, source:, ethscription_id: nil, eth_transaction: nil, **_extras)
     new.validate_and_encode(
       decoded_content: decoded_content,
       operation: operation,
       params: params,
       source: source,
-      ethscription_id: ethscription_id
+      ethscription_id: ethscription_id,
+      eth_transaction: eth_transaction
     )
   end
 
-  def validate_and_encode(decoded_content:, operation:, params:, source:, ethscription_id: nil)
+  def validate_and_encode(decoded_content:, operation:, params:, source:, ethscription_id: nil, eth_transaction: nil)
     # Check import fallback first (if ethscription_id provided)
     if ethscription_id
       normalized_id = normalize_id(ethscription_id)
-      if normalized_id && (preplanned = build_import_encoded_params(normalized_id, decoded_content))
+      if normalized_id && (preplanned = build_import_encoded_params(normalized_id, decoded_content, eth_transaction))
         return preplanned
       end
     end
@@ -217,7 +219,7 @@ def normalize_id(value)
   # -------------------- Import fallback --------------------
 
   # Returns [protocol, operation, encoded_data] or nil
-  def build_import_encoded_params(id, decoded_content)
+  def build_import_encoded_params(id, decoded_content, eth_transaction = nil)
     data = self.class.load_import_data(
       items_path: DEFAULT_ITEMS_PATH,
       collections_path: DEFAULT_COLLECTIONS_PATH
@@ -247,7 +249,7 @@ def build_import_encoded_params(id, decoded_content)
       operation = 'create_collection_and_add_self'
       schema = OPERATION_SCHEMAS[operation]
       encoding_data = {
-        'metadata' => build_metadata_object(metadata),
+        'metadata' => build_metadata_object(metadata, eth_transaction: eth_transaction),
         'item' => build_item_object(item: item, item_index: item_index, content_hash: content_hash)
       }
       encoded_data = encode_operation(operation, encoding_data, schema, content_hash: content_hash)
@@ -355,7 +357,7 @@ def load_import_data(items_path:, collections_path:)
   end
 
   # Build ordered JSON objects to match strict parser expectations
-  def build_metadata_object(meta)
+  def build_metadata_object(meta, eth_transaction: nil)
     name = safe_string(meta['name'])
     symbol = safe_string(meta['symbol'] || meta['slug'] || meta['name'])
     max_supply = safe_uint_string(meta['max_supply'] || meta['total_supply'] || 0)
@@ -381,6 +383,24 @@ def build_metadata_object(meta)
     ]
     merkle_root = meta.fetch('merkle_root')
     result['merkle_root'] = to_bytes32_hex(merkle_root)
+
+    # Handle initial_owner based on should_renounce flag
+    if meta['should_renounce'] == true
+      # address(0) means renounce ownership
+      result['initial_owner'] = '0x0000000000000000000000000000000000000000'
+    elsif meta['initial_owner']
+      # Use explicitly specified initial owner
+      result['initial_owner'] = to_address_hex(meta['initial_owner'])
+    elsif eth_transaction && eth_transaction.respond_to?(:from_address)
+      # Use the transaction sender as the actual owner
+      result['initial_owner'] = eth_transaction.from_address.to_hex
+    else
+      # No transaction context - this shouldn't happen in production
+      # For import, we always have the transaction
+      # Return nil to indicate we can't determine the owner
+      raise ValidationError, "Cannot determine initial owner without transaction context"
+    end
+
     result
   end
 
@@ -408,6 +428,12 @@ def to_bytes32_hex(val)
     h
   end
 
+  def to_address_hex(val)
+    h = safe_string(val).downcase
+    raise ValidationError, "Invalid address hex: #{val}" unless h.match?(/\A0x[0-9a-f]{40}\z/)
+    h
+  end
+
   # Integer coercion helper for import computations
   def safe_uint(val)
     case val
@@ -564,6 +590,14 @@ def validate_address(value, field_name)
     value.downcase
   end
 
+  def validate_optional_address(value, field_name)
+    unless value.is_a?(String) && value.match?(/\A0x[0-9a-f]{40}\z/i)
+      raise ValidationError, "Invalid address for #{field_name}: #{value}"
+    end
+    # Allow address(0) for renouncement
+    value.downcase
+  end
+
   def validate_bytes32_array(value, field_name)
     unless value.is_a?(Array)
       raise ValidationError, "Expected array for #{field_name}"
@@ -598,8 +632,8 @@ def validate_collection_metadata(value, field_name)
     unless value.is_a?(Hash)
       raise ValidationError, "Expected object for #{field_name}"
     end
-    # Expected keys for metadata (merkle_root optional)
-    expected_keys = %w[name symbol max_supply description logo_image_uri banner_image_uri background_color website_link twitter_link discord_link merkle_root]
+    # Expected keys for metadata (now includes initial_owner)
+    expected_keys = %w[name symbol max_supply description logo_image_uri banner_image_uri background_color website_link twitter_link discord_link merkle_root initial_owner]
     unless value.keys == expected_keys
       raise ValidationError, "Invalid metadata keys or order"
     end
@@ -615,7 +649,8 @@ def validate_collection_metadata(value, field_name)
       websiteLink: validate_string(value['website_link'], 'website_link'),
       twitterLink: validate_string(value['twitter_link'], 'twitter_link'),
       discordLink: validate_string(value['discord_link'], 'discord_link'),
-      merkleRoot: validate_bytes32(value['merkle_root'], 'merkle_root')
+      merkleRoot: validate_bytes32(value['merkle_root'], 'merkle_root'),
+      initialOwner: validate_optional_address(value['initial_owner'], 'initial_owner')
     }
   end
 
@@ -681,15 +716,16 @@ def build_create_collection_values(data)
       data['website_link'],
       data['twitter_link'],
       data['discord_link'],
-      data['merkle_root']
+      data['merkle_root'],
+      data['initial_owner']
     ]
   end
 
   def build_create_and_add_self_values(data, content_hash:)
     meta = data['metadata']
     item = data['item']
 
-    # Metadata tuple with optional merkleRoot
+    # Metadata tuple with merkleRoot and initialOwner
     merkle_root = meta[:merkleRoot] || ["".ljust(64, '0')].pack('H*')
     metadata_tuple = [
       meta[:name],
@@ -702,7 +738,8 @@ def build_create_and_add_self_values(data, content_hash:)
       meta[:websiteLink],
       meta[:twitterLink],
       meta[:discordLink],
-      merkle_root
+      merkle_root,
+      meta[:initialOwner]
     ]
 
     # Item tuple - contentHash comes first (keccak256 of ethscription content)

app/models/ethscription_transaction.rb

@@ -180,10 +180,10 @@ def build_create_calldata
     esip6 = DataUri.esip6?(content_uri) || false
 
     # Extract protocol params - returns [protocol, operation, encoded_data]
-    # Pass the ethscription_id context so parsers can inject it when needed
+    # Pass the eth_transaction for context (includes from_address and transaction_hash)
     protocol, operation, encoded_data = ProtocolParser.for_calldata(
       content_uri,
-      ethscription_id: eth_transaction.transaction_hash
+      eth_transaction: eth_transaction
     )
 
     # Hash the content for protocol uniqueness

app/models/protocol_parser.rb

@@ -11,7 +11,7 @@ class ProtocolParser
     'erc-721-ethscriptions-collection' => Erc721EthscriptionsCollectionParser
   }.freeze
 
-  def self.extract(content_uri, ethscription_id: nil)
+  def self.extract(content_uri, eth_transaction: nil, ethscription_id: nil)
     # Parse data URI and extract protocol info
     parsed = parse_data_uri_and_protocol(content_uri)
 
@@ -33,7 +33,8 @@ def self.extract(content_uri, ethscription_id: nil)
           operation: nil,
           params: {},
           source: :json,
-          ethscription_id: ethscription_id
+          ethscription_id: ethscription_id,
+          eth_transaction: eth_transaction
         )
 
         if encoded != DEFAULT_PARAMS
@@ -61,7 +62,8 @@ def self.extract(content_uri, ethscription_id: nil)
       operation: parsed[:operation],
       params: parsed[:params],
       source: parsed[:source],
-      ethscription_id: ethscription_id
+      ethscription_id: ethscription_id,
+      eth_transaction: eth_transaction
     )
 
     # Check if parsing succeeded
@@ -83,8 +85,10 @@ def self.extract(content_uri, ethscription_id: nil)
 
   # Get protocol data formatted for L2 calldata
   # Returns [protocol, operation, encoded_data] for contract consumption
-  def self.for_calldata(content_uri, ethscription_id: nil)
-    result = extract(content_uri, ethscription_id: ethscription_id)
+  def self.for_calldata(content_uri, eth_transaction: nil, ethscription_id: nil)
+    # Support both for backward compatibility
+    ethscription_id ||= eth_transaction&.transaction_hash
+    result = extract(content_uri, eth_transaction: eth_transaction, ethscription_id: ethscription_id)
 
     if result.nil?
       # No protocol detected - return empty protocol params

contracts/src/ERC721EthscriptionsCollection.sol

@@ -45,7 +45,14 @@ contract ERC721EthscriptionsCollection is ERC721EthscriptionsEnumerableUpgradeab
         bytes32 collectionId_
     ) external initializer {
         __ERC721_init(name_, symbol_);
-        __Ownable_init(initialOwner_);
+        
+        if (initialOwner_ == address(0)) {
+            __Ownable_init(address(1));
+            _transferOwnership(address(0));
+        } else {
+            __Ownable_init(initialOwner_);
+        }
+        
         manager = ERC721EthscriptionsCollectionManager(msg.sender);
         collectionId = collectionId_;
     }

contracts/src/ERC721EthscriptionsCollectionManager.sol

@@ -31,6 +31,7 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
         string twitterLink;
         string discordLink;
         bytes32 merkleRoot;
+        address initialOwner;
     }
 
     struct CollectionRecord {
@@ -345,21 +346,20 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
 
     // -------------------- Helpers --------------------
 
-    function _createCollection(bytes32 collectionId, CollectionParams memory metadata) internal {
-        require(!collectionExists(collectionId), "Collection already exists");
+    function _initializeCollection(Proxy collectionProxy, bytes32 collectionId, CollectionParams memory metadata) private {
 
-        Proxy collectionProxy = new Proxy{salt: collectionId}(address(this));
-        
         collectionProxy.upgradeToAndCall(collectionsImplementation, abi.encodeWithSelector(
             ERC721EthscriptionsCollection.initialize.selector,
             metadata.name,
             metadata.symbol,
-            ethscriptions.ownerOf(collectionId),
+            metadata.initialOwner,
             collectionId
         ));
-        
+
         collectionProxy.changeAdmin(Predeploys.PROXY_ADMIN);
+    }
 
+    function _storeCollectionData(bytes32 collectionId, address collectionContract, CollectionParams memory metadata) private {
         // Store string fields using DedupedBlobStore
         (, bytes32 nameRef) = DedupedBlobStore.storeMemory(bytes(metadata.name), collectionBlobStorage);
         (, bytes32 symbolRef) = DedupedBlobStore.storeMemory(bytes(metadata.symbol), collectionBlobStorage);
@@ -372,7 +372,7 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
         (, bytes32 discordLinkRef) = DedupedBlobStore.storeMemory(bytes(metadata.discordLink), collectionBlobStorage);
 
         collectionStore[collectionId] = CollectionRecord({
-            collectionContract: address(collectionProxy),
+            collectionContract: collectionContract,
             locked: false,
             nameRef: nameRef,
             symbolRef: symbolRef,
@@ -386,7 +386,19 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
             discordLinkRef: discordLinkRef,
             merkleRoot: metadata.merkleRoot
         });
-        
+    }
+
+    function _createCollection(bytes32 collectionId, CollectionParams memory metadata) internal {
+        require(!collectionExists(collectionId), "Collection already exists");
+
+        Proxy collectionProxy = new Proxy{salt: collectionId}(address(this));
+
+        // Initialize the collection
+        _initializeCollection(collectionProxy, collectionId, metadata);
+
+        // Store collection metadata
+        _storeCollectionData(collectionId, address(collectionProxy), metadata);
+
         collectionAddressToId[address(collectionProxy)] = collectionId;
         collectionIds.push(collectionId);
 

contracts/test/AddressPrediction.t.sol

@@ -73,7 +73,8 @@ contract AddressPredictionTest is TestSetup {
                 websiteLink: "https://example.com",
                 twitterLink: "",
                 discordLink: "",
-                merkleRoot: bytes32(0)
+                merkleRoot: bytes32(0),
+                initialOwner: address(this)  // Use test contract as owner
             });
 
         // Manually compute predicted proxy address

contracts/test/CollectionURIResolution.t.sol

@@ -170,7 +170,8 @@ contract CollectionURIResolutionTest is TestSetup {
                 websiteLink: "",
                 twitterLink: "",
                 discordLink: "",
-                merkleRoot: bytes32(0)
+                merkleRoot: bytes32(0),
+                initialOwner: alice  // Use alice as owner
             });
 
         string memory collectionContent = string.concat(

contracts/test/CollectionsManager.t.sol

@@ -40,7 +40,8 @@ contract ERC721EthscriptionsCollectionManagerTest is TestSetup {
                 websiteLink: "https://example.com",
                 twitterLink: "https://twitter.com/test",
                 discordLink: "https://discord.gg/test",
-                merkleRoot: bytes32(0)
+                merkleRoot: bytes32(0),
+                initialOwner: alice  // Use alice as owner
             });
 
         Ethscriptions.CreateEthscriptionParams memory params = Ethscriptions.CreateEthscriptionParams({
@@ -94,7 +95,8 @@ contract ERC721EthscriptionsCollectionManagerTest is TestSetup {
                 websiteLink: "https://example.com",
                 twitterLink: "",
                 discordLink: "",
-                merkleRoot: bytes32(0)
+                merkleRoot: bytes32(0),
+                initialOwner: alice  // Use alice as owner
             });
 
         // Prepare item data
@@ -1317,7 +1319,8 @@ contract ERC721EthscriptionsCollectionManagerTest is TestSetup {
                 websiteLink: "",
                 twitterLink: "",
                 discordLink: "",
-                merkleRoot: merkleRoot
+                merkleRoot: merkleRoot,
+                initialOwner: alice  // Use alice as owner
             });
 
         string memory collectionContent =