Skip to content

Enhance ERC721 Ethscriptions Collection Parser with Initial Owner Support #145

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 Ethscriptions Collection Parser with Initial Owner Support #145

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e55b6c
Enhance ERC721 Ethscriptions Collection Parser with Initial Owner Sup…
RogerPodacter Nov 12, 2025
76710a3
Refactor ERC721 Ethscriptions Collection Parser for Address Handling
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
75 changes: 56 additions & 19 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 @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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'] = to_address_hex(eth_transaction.from_address)
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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +593 to +598
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

[nitpick] The regex pattern /\A0x[0-9a-f]{40}\z/i uses the case-insensitive flag i, but then forces lowercase with .downcase. This means uppercase addresses (including checksummed addresses) are accepted and normalized to lowercase.

However, the regex should either:

  1. Remove the i flag to strictly require lowercase hex (matching the pattern used for bytes32), or
  2. Keep the i flag if checksummed addresses need to be supported

Consider aligning with the validate_bytes32 approach which uses /\A0x[0-9a-f]{64}\z/ (no i flag) for consistency, unless there's a specific reason to accept checksummed addresses.

Copilot uses AI. Check for mistakes.
end

def validate_bytes32_array(value, field_name)
unless value.is_a?(Array)
raise ValidationError, "Expected array for #{field_name}"
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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],
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions app/models/ethscription_transaction.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions app/models/protocol_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion contracts/src/ERC721EthscriptionsCollection.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

[nitpick] This workaround pattern for renouncing ownership at initialization may be fragile and could fail if OpenZeppelin's OwnableUpgradeable implementation changes. The contract first initializes with address(1) then immediately transfers to address(0).

Consider documenting this pattern with a comment explaining why this workaround is necessary, or consider using OpenZeppelin's renounceOwnership() function after initialization with a non-zero address if the intended behavior is to have no owner.

Suggested change
_transferOwnership(address(0));
renounceOwnership();

Copilot uses AI. Check for mistakes.
} else {
__Ownable_init(initialOwner_);
}

manager = ERC721EthscriptionsCollectionManager(msg.sender);
collectionId = collectionId_;
}
Expand Down
28 changes: 20 additions & 8 deletions contracts/src/ERC721EthscriptionsCollectionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ contract ERC721EthscriptionsCollectionManager is IProtocolHandler {
string twitterLink;
string discordLink;
bytes32 merkleRoot;
address initialOwner;
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The new initialOwner field in the CollectionParams struct lacks documentation. Consider adding a comment to explain:

  • Its purpose (setting the initial owner of the collection)
  • Special behavior when set to address(0) (ownership renouncement)
  • The expected value (typically the transaction sender)

For example:

address initialOwner;  // Initial owner of the collection; use address(0) to renounce ownership immediately
Suggested change
address initialOwner;
address initialOwner; // Initial owner of the collection; use address(0) to renounce ownership immediately. Typically set to the transaction sender.

Copilot uses AI. Check for mistakes.
}

struct CollectionRecord {
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion contracts/test/AddressPrediction.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 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 @@ -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(
Expand Down
9 changes: 6 additions & 3 deletions contracts/test/CollectionsManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1317,7 +1319,8 @@ contract ERC721EthscriptionsCollectionManagerTest is TestSetup {
websiteLink: "",
twitterLink: "",
discordLink: "",
merkleRoot: merkleRoot
merkleRoot: merkleRoot,
initialOwner: alice // Use alice as owner
});

string memory collectionContent =
Expand Down
Loading
Loading