Implement collections and more generic protocol framework by RogerPodacter · Pull Request #113 · ethscriptions-protocol/ethscriptions-indexer

RogerPodacter · 2025-09-24T22:27:31Z

Note

Introduce a generic protocol framework with Collections support, protocol handler registry, and ERC-721 enumerable upgrades across contracts, extractors, libs, and tests.

Protocols/Extraction:
- Add GenericProtocolExtractor, CollectionsParamsExtractor, and unified ProtocolExtractor with calldata encoding; introduce DataUri parser.
- New Ruby readers/helpers: ProtocolEventReader, CollectionsReader, TokenReader.
Contracts:
- Implement CollectionsManager, EthscriptionERC721 (non-transferable ERC-721 with enumerable), and IProtocolHandler.
- Update Ethscriptions: protocol-aware creation/transfer hooks, media URI generation, batched proving interactions; add getEthscriptionWithContent.
- Extend TokenManager to protocol handler pattern; add deterministic clone/predict helpers.
- Update genesis (L2Genesis.s.sol): deploy/register protocol handlers; add predeploys for ERC721 template and CollectionsManager; adjust genesis flow.
- Upgrade ERC721EthscriptionsUpgradeable to store name/symbol and support IERC721Enumerable.
Infrastructure:
- Add/modify predeploy addresses (Predeploys.sol) incl. depositor account and new templates.
- Improve L1RpcPrefetcher (throttling, caching latest, stats); enhance GenesisGenerator (quiet logging, env flags).
Tests:
- Comprehensive Solidity and Ruby integration tests for Collections, Tokens, Prover, ERC721 enumerable, protocol registration, and E2E flows.
Rails:
- Remove facet_rails_common usage from ApplicationController.
Deps:
- Dependency updates and cleanup in Gemfile/lockfile.

^{Written by Cursor Bugbot for commit 28d6bf9. This will update automatically on new commits. Configure here.}

Copilot

Pull Request Overview

This PR implements a comprehensive protocol framework for handling collections and other generic protocols in the Ethscriptions system, building upon the existing token protocol infrastructure.

Introduces ProtocolExtractor as a unified entry point that intelligently routes between strict token protocols and flexible generic protocols
Adds GenericProtocolExtractor with automatic type inference for ABI encoding of arbitrary JSON protocol data
Implements the collections protocol with CollectionsManager and EthscriptionERC721 contracts for managing NFT collections

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
spec/models/protocol_extractor_spec.rb	Tests unified protocol extraction routing between token and generic extractors
spec/models/token_params_extractor_via_protocol_spec.rb	Verifies token protocol continues working through unified interface
spec/models/generic_protocol_*_spec.rb	Comprehensive testing of generic protocol extraction and type inference
contracts/src/protocols/	New protocol handler interfaces and ERC721 collection contract implementation
contracts/src/Ethscriptions.sol	Updated to use generic protocol framework with handler registration
contracts/src/TokenManager.sol	Refactored to implement IProtocolHandler interface
app/models/protocol_extractor.rb	Ruby implementation of unified protocol extraction
app/models/generic_protocol_extractor.rb	Generic protocol extractor with automatic type inference

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-24T22:30:14Z

+    context 'marketplace protocol' do
+      # it 'extracts create_listing with string numbers' do
+      #   content_uri = 'data:,{"p":"marketplace","op":"create_listing","tokenId":"0x4567","price":"1000000000000000000","duration":"604800"}'
+
+      #   protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)
+
+      #   expect(protocol).to eq('marketplace'.b)
+      #   expect(operation).to eq('create_listing'.b)
+
+      #   types = ['uint256', 'uint256', 'string']
+      #   decoded = Eth::Abi.decode(types, encoded_data)
+
+      #   expect(decoded[0]).to eq(604800)
+      #   expect(decoded[1]).to eq(1000000000000000000) # 1 ETH in wei
+      #   expect(decoded[2]).to eq('0x4567')
+      # end
+    end
+


This entire marketplace protocol test context contains only commented-out code. Either implement the test or remove this dead code to keep the test suite clean.

Suggested change

context 'marketplace protocol' do

# it 'extracts create_listing with string numbers' do

# content_uri = 'data:,{"p":"marketplace","op":"create_listing","tokenId":"0x4567","price":"1000000000000000000","duration":"604800"}'

# protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)

# expect(protocol).to eq('marketplace'.b)

# expect(operation).to eq('create_listing'.b)

# types = ['uint256', 'uint256', 'string']

# decoded = Eth::Abi.decode(types, encoded_data)

# expect(decoded[0]).to eq(604800)

# expect(decoded[1]).to eq(1000000000000000000) # 1 ETH in wei

# expect(decoded[2]).to eq('0x4567')

# end

end

Copilot · 2025-09-24T22:30:15Z

+      # it 'recognizes various fixed-length bytes types' do
+      #   content_uri = 'data:,{"p":"test","op":"bytes_test","byte1":"0x12","byte4":"0x12345678","byte16":"0x123456789abcdef0123456789abcdef0"}'
+
+      #   protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)
+
+      #   types = ['bytes1', 'bytes16', 'bytes4']
+      #   decoded = Eth::Abi.decode(types, encoded_data)
+
+      #   expect(decoded[0]).to eq('0x12')
+      #   expect(decoded[1]).to eq('0x123456789abcdef0123456789abcdef0')
+      #   expect(decoded[2]).to eq('0x12345678')
+      # end
+


This commented-out test for various fixed-length bytes types should either be implemented or removed. The current implementation in the extractor raises 'Not supported' for non-address/bytes32 types, so this test would fail if uncommented.

Suggested change

# it 'recognizes various fixed-length bytes types' do

# content_uri = 'data:,{"p":"test","op":"bytes_test","byte1":"0x12","byte4":"0x12345678","byte16":"0x123456789abcdef0123456789abcdef0"}'

# protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)

# types = ['bytes1', 'bytes16', 'bytes4']

# decoded = Eth::Abi.decode(types, encoded_data)

# expect(decoded[0]).to eq('0x12')

# expect(decoded[1]).to eq('0x123456789abcdef0123456789abcdef0')

# expect(decoded[2]).to eq('0x12345678')

# end

Copilot · 2025-09-24T22:30:15Z

+        // CollectionItem memory item = collectionItems[txHash];
+
+        // // If this ethscription is part of a collection, sync ownership
+        // if (item.collectionId != bytes32(0)) {
+        //     CollectionInfo memory collection = collections[item.collectionId];
+        //     if (collection.collectionContract != address(0)) {
+        //         // Sync the ownership in the ERC721 contract
+        //         EthscriptionERC721(collection.collectionContract).syncOwnership(item.tokenId);
+        //     }
+        // }


The onTransfer function contains only commented-out code. Either implement the transfer synchronization logic or remove this dead code. The function currently does nothing when ethscriptions are transferred.

Suggested change

// CollectionItem memory item = collectionItems[txHash];

// // If this ethscription is part of a collection, sync ownership

// if (item.collectionId != bytes32(0)) {

// CollectionInfo memory collection = collections[item.collectionId];

// if (collection.collectionContract != address(0)) {

// // Sync the ownership in the ERC721 contract

// EthscriptionERC721(collection.collectionContract).syncOwnership(item.tokenId);

// }

// }

CollectionItem memory item = collectionItems[txHash];

// If this ethscription is part of a collection, sync ownership

if (item.collectionId != bytes32(0)) {

CollectionInfo memory collection = collections[item.collectionId];

if (collection.collectionContract != address(0)) {

// Sync the ownership in the ERC721 contract

EthscriptionERC721(collection.collectionContract).syncOwnership(item.tokenId);

}

}

tunnckoCore · 2025-09-24T23:01:49Z

+        // Create an ethscription to add to the collection
+        vm.prank(alice);
+
+        string memory itemContent = 'data:,{"p":"collections","op":"add","collection":"0x1234","item":"artwork1"}';


what's item here? is it the item's data uri?

The tests are completely messed up rn!

Copilot

Pull Request Overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

spec/models/token_params_extractor_spec.rb:1

Remove the commented-out debugging statement.

require 'rails_helper'

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-29T22:23:58Z

+        # if receipt['status'] != '0x1'
+          # ap EthRpcClient.l2.trace_transaction(l2_tx['hash'])
+        # end


Remove the commented-out debug code or move it to a proper debug logging mechanism if it's still needed.

Copilot · 2025-09-29T22:23:58Z

+        // If they differ, update our state
+        if (actualOwner != recordedOwner) {
+            // Use internal _transfer to update state and emit event
+            _transfer(recordedOwner, actualOwner, tokenId);


The _transfer function should validate that the token exists and that the recorded owner actually owns it before performing the transfer. Consider adding these checks to prevent potential state corruption.

Copilot · 2025-09-29T22:23:59Z

+
+  def parse_json_safely(json_str)
+    # Size check
+    raise ExtractionError, "JSON too large" if json_str.bytesize > 10_000


[nitpick] Consider making the JSON size limit configurable or define it as a constant rather than hardcoding the value.

Copilot

Pull Request Overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-10T21:34:59Z

+        # if receipt['status'] != '0x1'
+          # ap EthRpcClient.l2.trace_transaction(l2_tx['hash'])
+        # end


Remove commented debugging code before merging to production. This appears to be leftover debugging logic that should be cleaned up.

Suggested change

# if receipt['status'] != '0x1'

# ap EthRpcClient.l2.trace_transaction(l2_tx['hash'])

# end

Copilot · 2025-10-10T21:35:00Z

+        "max" => "1000000",
+        "lim" => "100"
+      }
+# binding.irb


Remove debugging breakpoint before merging to production.

Suggested change

# binding.irb

Copilot · 2025-10-10T21:35:00Z

@@ -2,6 +2,7 @@
 require 'retriable'


The Memery include is added but there's no corresponding require statement for the gem. Ensure Memery is properly required or imported.

Suggested change

require 'retriable'

require 'retriable'

require 'memery'

Copilot · 2025-10-10T21:35:01Z

+// import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
+import "../ERC721EthscriptionsUpgradeable.sol";
+// import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
+// import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";


Remove commented import statements. These appear to be replaced by the actual import but should be cleaned up for code clarity.

Suggested change

// import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

Copilot · 2025-10-10T21:35:01Z

+    //     //     protocolParams: Ethscriptions.ProtocolParams({
    //     //         protocol: "",
-    //     //         tick: "",
-    //     //         max: 0,
-    //     //         lim: 0,
-    //     //         amt: 0
+    //     //         operation: "",
+    //     //         data: ""
    //     //     })


Remove large blocks of commented test code or convert to proper documentation if the code examples are meant to be preserved for reference.

Copilot

Pull Request Overview

Copilot reviewed 46 out of 47 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-13T16:33:18Z

+      # it 'extracts create_listing with string numbers' do
+      #   content_uri = 'data:,{"p":"marketplace","op":"create_listing","tokenId":"0x4567","price":"1000000000000000000","duration":"604800"}'
+
+      #   protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)
+
+      #   expect(protocol).to eq('marketplace'.b)
+      #   expect(operation).to eq('create_listing'.b)
+
+      #   types = ['uint256', 'uint256', 'string']
+      #   decoded = Eth::Abi.decode(types, encoded_data)
+
+      #   expect(decoded[0]).to eq(604800)
+      #   expect(decoded[1]).to eq(1000000000000000000) # 1 ETH in wei
+      #   expect(decoded[2]).to eq('0x4567')
+      # end


Remove large blocks of commented-out test code. If these tests are planned for future implementation, track them in issues or TODO comments instead.

Suggested change

# it 'extracts create_listing with string numbers' do

# content_uri = 'data:,{"p":"marketplace","op":"create_listing","tokenId":"0x4567","price":"1000000000000000000","duration":"604800"}'

# protocol, operation, encoded_data = GenericProtocolExtractor.extract(content_uri)

# expect(protocol).to eq('marketplace'.b)

# expect(operation).to eq('create_listing'.b)

# types = ['uint256', 'uint256', 'string']

# decoded = Eth::Abi.decode(types, encoded_data)

# expect(decoded[0]).to eq(604800)

# expect(decoded[1]).to eq(1000000000000000000) # 1 ETH in wei

# expect(decoded[2]).to eq('0x4567')

# end

# TODO: Implement test for 'create_listing' operation (see removed commented-out test)

Copilot · 2025-10-13T16:33:18Z

    //         mimeSubtype: "png",
    //         esip6: false,
    //         isCompressed: false,
-    //         tokenParams: Ethscriptions.TokenParams({
-    //             op: "", protocol: "", tick: "", max: 0, lim: 0, amt: 0
+    //         protocolParams: Ethscriptions.ProtocolParams({
+    //             protocol: "", operation: "", data: ""
    //         })


Remove large blocks of commented-out test code. If compression testing is planned for future implementation, create proper test stubs or track in issues.

Implement collections and more generic protocol framework

29d3ec9

RogerPodacter requested a review from Copilot September 24, 2025 22:28

Copilot AI reviewed Sep 24, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

tunnckoCore reviewed Sep 24, 2025

View reviewed changes

Comment thread contracts/test/CollectionsManager.t.sol Outdated

tunnckoCore reviewed Sep 24, 2025

View reviewed changes

Merge branch 'evm-backend-demo' into collection_protocol

93b33a2

This comment was marked as outdated.

Sign in to view

RogerPodacter added 2 commits September 25, 2025 18:11

Cleanup

88d6def

Generic protocol framework

823f06e

This comment was marked as outdated.

Sign in to view

RogerPodacter requested review from Copilot and tunnckoCore September 29, 2025 22:22

Copilot AI reviewed Sep 29, 2025

View reviewed changes

Simplify

24b4aa6

RogerPodacter requested a review from Copilot October 10, 2025 21:33

Copilot AI reviewed Oct 10, 2025

View reviewed changes

Disable collections by default

9d5ecff

This comment was marked as outdated.

Sign in to view

RogerPodacter added 2 commits October 10, 2025 18:26

ESIP6 fix

4af6943

Fixes

cac6d49

RogerPodacter requested a review from Copilot October 13, 2025 16:32

Copilot AI reviewed Oct 13, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

Fixes

28d6bf9

RogerPodacter merged commit ae9d933 into evm-backend-demo Oct 13, 2025
2 checks passed

RogerPodacter deleted the collection_protocol branch October 13, 2025 16:55

	# if receipt['status'] != '0x1'
	# ap EthRpcClient.l2.trace_transaction(l2_tx['hash'])
	# end

Uh oh!

Conversation

RogerPodacter commented Sep 24, 2025 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

tunnckoCore Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

RogerPodacter Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

RogerPodacter commented Sep 24, 2025 •

edited by cursor Bot

Loading