Merge pull request #133 from ethscriptions-protocol/improve_reading

Refactor Ethscription structure and retrieval methods

Author: RogerPodacter

contracts/script/L2Genesis.s.sol

@@ -43,7 +43,7 @@ contract GenesisEthscriptions is Ethscriptions {
         firstEthscriptionByContentUri[params.contentUriHash] = params.ethscriptionId;
 
         // Set all values including genesis-specific ones
-        ethscriptions[params.ethscriptionId] = Ethscription({
+        ethscriptions[params.ethscriptionId] = EthscriptionStorage({
             // Fixed-size fields
             contentUriHash: params.contentUriHash,
             contentSha: contentSha,

contracts/src/Ethscriptions.sol

@@ -136,9 +136,10 @@ contract EthscriptionsProver {
     /// @param ethscriptionId The Ethscription ID (L1 tx hash)
     /// @param proofInfo The queued proof info containing block data
     function _createAndSendProof(bytes32 ethscriptionId, QueuedProof memory proofInfo) internal {
-        // Get ethscription data including previous owner
-        Ethscriptions.Ethscription memory ethscription = ethscriptions.getEthscription(ethscriptionId);
-        address currentOwner = ethscriptions.ownerOf(ethscriptionId);
+        // Get ethscription data including previous owner (without content for gas efficiency)
+        Ethscriptions.Ethscription memory ethscription = ethscriptions.getEthscription(ethscriptionId, false);
+        // currentOwner is already in the struct now
+        address currentOwner = ethscription.currentOwner;
 
         // Create proof struct with all ethscription data
         EthscriptionDataProof memory proof = EthscriptionDataProof({
@@ -150,7 +151,7 @@ contract EthscriptionsProver {
             currentOwner: currentOwner,
             previousOwner: ethscription.previousOwner,
             esip6: ethscription.esip6,
-            ethscriptionNumber: ethscription.ethscriptionNumber,
+            ethscriptionNumber: uint48(ethscription.ethscriptionNumber),
             l1BlockNumber: proofInfo.l1BlockNumber,
             l2BlockNumber: proofInfo.l2BlockNumber,
             l2Timestamp: proofInfo.l2BlockTimestamp

contracts/src/EthscriptionsProver.sol

@@ -15,7 +15,7 @@ library EthscriptionsRendererLib {
     /// @param etsc Storage pointer to the ethscription
     /// @param ethscriptionId The ethscription ID (L1 tx hash)
     /// @return JSON string of attributes array
-    function buildAttributes(Ethscriptions.Ethscription storage etsc, bytes32 ethscriptionId)
+    function buildAttributes(Ethscriptions.EthscriptionStorage storage etsc, bytes32 ethscriptionId)
         internal
         view
         returns (string memory)
@@ -56,7 +56,7 @@ library EthscriptionsRendererLib {
     /// @param content The content bytes
     /// @return mediaType Either "image" or "animation_url"
     /// @return mediaUri The data URI for the media
-    function getMediaUri(Ethscriptions.Ethscription storage etsc, bytes memory content)
+    function getMediaUri(Ethscriptions.EthscriptionStorage storage etsc, bytes memory content)
         internal
         view
         returns (string memory mediaType, string memory mediaUri)
@@ -88,7 +88,7 @@ library EthscriptionsRendererLib {
     /// @param content The content bytes
     /// @return The complete base64-encoded data URI
     function buildTokenURI(
-        Ethscriptions.Ethscription storage etsc,
+        Ethscriptions.EthscriptionStorage storage etsc,
         bytes32 ethscriptionId,
         bytes memory content
     ) internal view returns (string memory) {

contracts/src/libraries/EthscriptionsRendererLib.sol

@@ -5,7 +5,7 @@ import "./TestSetup.sol";
 
 contract EthscriptionsWithContentTest is TestSetup {
 
-    function testGetEthscriptionWithContent() public {
+    function testGetEthscription() public {
         // Create a test ethscription first
         bytes32 txHash = bytes32(uint256(12345));
         address creator = address(0x1);
@@ -30,59 +30,93 @@ contract EthscriptionsWithContentTest is TestSetup {
 
         uint256 tokenId = ethscriptions.createEthscription(params);
 
-        // Test the new combined method
-        (Ethscriptions.Ethscription memory ethscription, bytes memory content) = ethscriptions.getEthscriptionWithContent(txHash);
+        // Test the new getEthscription method that returns Ethscription
+        Ethscriptions.Ethscription memory complete = ethscriptions.getEthscription(txHash);
 
         // Verify ethscription data
-        assertEq(ethscription.creator, creator);
-        assertEq(ethscription.initialOwner, initialOwner);
-        assertEq(ethscription.previousOwner, creator);
-        assertEq(ethscription.ethscriptionNumber, tokenId);
-        assertEq(ethscription.mimetype, "text/plain");
-        assertEq(ethscription.esip6, false);
+        assertEq(complete.ethscriptionId, txHash);
+        assertEq(complete.ethscriptionNumber, tokenId);
+        assertEq(complete.creator, creator);
+        assertEq(complete.initialOwner, initialOwner);
+        assertEq(complete.previousOwner, creator);
+        assertEq(complete.currentOwner, initialOwner);
+        assertEq(complete.mimetype, "text/plain");
+        assertEq(complete.esip6, false);
 
         // Verify content
-        assertEq(content, bytes(testContent));
-
-        // Compare with individual method calls to ensure they return the same data
-        Ethscriptions.Ethscription memory ethscriptionSeparate = ethscriptions.getEthscription(txHash);
-        bytes memory contentSeparate = ethscriptions.getEthscriptionContent(txHash);
-
-        // Compare structs (we'll compare individual fields since struct comparison isn't directly supported)
-        assertEq(ethscription.creator, ethscriptionSeparate.creator);
-        assertEq(ethscription.initialOwner, ethscriptionSeparate.initialOwner);
-        assertEq(ethscription.previousOwner, ethscriptionSeparate.previousOwner);
-        assertEq(ethscription.ethscriptionNumber, ethscriptionSeparate.ethscriptionNumber);
-        assertEq(ethscription.createdAt, ethscriptionSeparate.createdAt);
-        assertEq(ethscription.l1BlockNumber, ethscriptionSeparate.l1BlockNumber);
-        assertEq(ethscription.l2BlockNumber, ethscriptionSeparate.l2BlockNumber);
-        assertEq(ethscription.l1BlockHash, ethscriptionSeparate.l1BlockHash);
-
-        // Compare content fields
-        assertEq(ethscription.contentUriHash, ethscriptionSeparate.contentUriHash);
-        assertEq(ethscription.contentSha, ethscriptionSeparate.contentSha);
-        assertEq(ethscription.mimetype, ethscriptionSeparate.mimetype);
-        assertEq(ethscription.esip6, ethscriptionSeparate.esip6);
-
-        // Compare content
-        assertEq(content, contentSeparate);
+        assertEq(complete.content, bytes(testContent));
+
+        // Test the version without content using the overloaded function
+        Ethscriptions.Ethscription memory withoutContent = ethscriptions.getEthscription(txHash, false);
+
+        // Verify same metadata but empty content
+        assertEq(withoutContent.ethscriptionId, txHash);
+        assertEq(withoutContent.ethscriptionNumber, tokenId);
+        assertEq(withoutContent.creator, creator);
+        assertEq(withoutContent.currentOwner, initialOwner);
+        assertEq(withoutContent.content.length, 0, "Content should be empty");
+    }
+
+    function testGetEthscriptionByTokenId() public {
+        // Create a test ethscription first
+        bytes32 txHash = bytes32(uint256(67890));
+        address creator = address(0x5);
+        address initialOwner = address(0x6);
+        string memory testContent = "Test by token ID";
+
+        // Create the ethscription
+        vm.prank(creator);
+        Ethscriptions.CreateEthscriptionParams memory params = Ethscriptions.CreateEthscriptionParams({
+            ethscriptionId: txHash,
+            contentUriHash: keccak256(bytes("data:text/plain,Test by token ID")),
+            initialOwner: initialOwner,
+            content: bytes(testContent),
+            mimetype: "text/plain",
+            esip6: true,
+            protocolParams: Ethscriptions.ProtocolParams({
+                protocolName: "",
+                operation: "",
+                data: ""
+            })
+        });
+
+        uint256 tokenId = ethscriptions.createEthscription(params);
+
+        // Test getting by token ID
+        Ethscriptions.Ethscription memory complete = ethscriptions.getEthscription(tokenId);
+
+        // Verify ethscription data
+        assertEq(complete.ethscriptionId, txHash, "Ethscription ID should match");
+        assertEq(complete.ethscriptionNumber, tokenId, "Token ID should match");
+        assertEq(complete.creator, creator);
+        assertEq(complete.currentOwner, initialOwner);
+        assertEq(complete.content, bytes(testContent));
+
+        // Test without content version by token ID using the overloaded function
+        Ethscriptions.Ethscription memory withoutContent = ethscriptions.getEthscription(tokenId, false);
+        assertEq(withoutContent.ethscriptionId, txHash);
+        assertEq(withoutContent.content.length, 0, "Content should be empty");
     }
 
-    function testGetEthscriptionWithContentNonExistent() public {
+    function testGetEthscriptionNonExistent() public {
         bytes32 nonExistentTxHash = bytes32(uint256(99999));
 
         // Should revert with EthscriptionDoesNotExist
         vm.expectRevert(Ethscriptions.EthscriptionDoesNotExist.selector);
-        ethscriptions.getEthscriptionWithContent(nonExistentTxHash);
+        ethscriptions.getEthscription(nonExistentTxHash);
+
+        // Same for without content version using the overloaded function
+        vm.expectRevert(Ethscriptions.EthscriptionDoesNotExist.selector);
+        ethscriptions.getEthscription(nonExistentTxHash, false);
     }
 
-    function testGetEthscriptionWithContentLargeContent() public {
-        // Test with content that requires multiple SSTORE2 chunks
+    function testGetEthscriptionWithLargeContent() public {
+        // Test with content that's large (testing SSTORE2Unlimited)
         bytes32 txHash = bytes32(uint256(54321));
         address creator = address(0x3);
         address initialOwner = address(0x4);
 
-        // Create content larger than CHUNK_SIZE (24575 bytes)
+        // Create content larger than inline storage (>31 bytes)
         bytes memory largeContent = new bytes(30000);
         for (uint256 i = 0; i < 30000; i++) {
             largeContent[i] = bytes1(uint8(i % 256));
@@ -106,15 +140,68 @@ contract EthscriptionsWithContentTest is TestSetup {
 
         ethscriptions.createEthscription(params);
 
-        // Test the combined method with large content
-        (Ethscriptions.Ethscription memory ethscription, bytes memory content) = ethscriptions.getEthscriptionWithContent(txHash);
+        // Test the getEthscription method with large content
+        Ethscriptions.Ethscription memory complete = ethscriptions.getEthscription(txHash);
 
         // Verify content is correct
-        assertEq(content.length, 30000);
-        assertEq(content, largeContent);
+        assertEq(complete.content.length, 30000);
+        assertEq(complete.content, largeContent);
 
         // Verify ethscription data
-        assertEq(ethscription.creator, creator);
-        assertEq(ethscription.initialOwner, initialOwner);
+        assertEq(complete.creator, creator);
+        assertEq(complete.initialOwner, initialOwner);
+        assertEq(complete.currentOwner, initialOwner);
+
+        // Test without content - should have zero-length content using the overloaded function
+        Ethscriptions.Ethscription memory withoutContent = ethscriptions.getEthscription(txHash, false);
+        assertEq(withoutContent.content.length, 0, "Content should be empty");
+        assertEq(withoutContent.creator, creator);
+        assertEq(withoutContent.currentOwner, initialOwner);
+    }
+
+    function testGetEthscriptionWithSmallContent() public {
+        // Test with content that fits inline (≤31 bytes)
+        bytes32 txHash = bytes32(uint256(11111));
+        address creator = address(0x7);
+        address initialOwner = address(0x8);
+
+        // Create small content (10 bytes)
+        bytes memory smallContent = hex"48656c6c6f576f726c64"; // "HelloWorld"
+
+        // Create the ethscription
+        vm.prank(creator);
+        Ethscriptions.CreateEthscriptionParams memory params = Ethscriptions.CreateEthscriptionParams({
+            ethscriptionId: txHash,
+            contentUriHash: keccak256(bytes("data:text/plain,HelloWorld")),
+            initialOwner: initialOwner,
+            content: smallContent,
+            mimetype: "text/plain",
+            esip6: false,
+            protocolParams: Ethscriptions.ProtocolParams({
+                protocolName: "",
+                operation: "",
+                data: ""
+            })
+        });
+
+        uint256 tokenId = ethscriptions.createEthscription(params);
+
+        // Test the getEthscription method with small inline content
+        Ethscriptions.Ethscription memory complete = ethscriptions.getEthscription(txHash);
+
+        // Verify content is correct
+        assertEq(complete.content, smallContent);
+        assertEq(complete.content.length, 10);
+
+        // Verify ownership chain
+        assertEq(complete.creator, creator);
+        assertEq(complete.initialOwner, initialOwner);
+        assertEq(complete.currentOwner, initialOwner);
+        assertEq(complete.previousOwner, creator);
+
+        // Test getting by token ID too
+        Ethscriptions.Ethscription memory byTokenId = ethscriptions.getEthscription(tokenId);
+        assertEq(byTokenId.ethscriptionId, txHash);
+        assertEq(byTokenId.content, smallContent);
     }
-}
+}

contracts/test/EthscriptionsWithContent.t.sol

@@ -14,21 +14,21 @@ contract EthscriptionsWithTestFunctions is Ethscriptions {
     /// @notice Check if content is stored for an ethscription
     /// @dev Test-only function to check if content exists
     function hasContent(bytes32 ethscriptionId) external view returns (bool) {
-        Ethscription storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
+        EthscriptionStorage storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
         return contentStorageBySha[ethscription.contentSha] != bytes32(0);
     }
 
     /// @notice Get the content storage value for an ethscription
     /// @dev Test-only function to inspect storage (either packed bytes or SSTORE2 address)
     function getContentStorage(bytes32 ethscriptionId) external view returns (bytes32) {
-        Ethscription storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
+        EthscriptionStorage storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
         return contentStorageBySha[ethscription.contentSha];
     }
 
     /// @notice Get the content pointer for an ethscription (only for SSTORE2 stored content)
     /// @dev Test-only function to inspect SSTORE2 address
     function getContentPointer(bytes32 ethscriptionId) external view returns (address) {
-        Ethscription storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
+        EthscriptionStorage storage ethscription = _getEthscriptionOrRevert(ethscriptionId);
         bytes32 stored = contentStorageBySha[ethscription.contentSha];
 
         // Check if it's inline content using BytePackLib
@@ -46,6 +46,6 @@ contract EthscriptionsWithTestFunctions is Ethscriptions {
     /// @param ethscriptionId The ethscription ID (L1 tx hash)
     /// @return The content data
     function readContent(bytes32 ethscriptionId) external view returns (bytes memory) {
-        return getEthscriptionContent(ethscriptionId);
+        return _getEthscriptionContent(ethscriptionId);
     }
 }

contracts/test/EthscriptionsWithTestFunctions.sol

@@ -130,7 +130,7 @@ contract SSTORE2ContentSizesTest is TestSetup {
         // Test content retrieval via getEthscriptionContent
         g0 = gasleft();
         vm.resumeGasMetering();
-        bytes memory retrievedContent = eth.getEthscriptionContent(txHash);
+        bytes memory retrievedContent = eth.readContent(txHash);
         vm.pauseGasMetering();
         uint256 retrievalGas = g0 - gasleft();
 
@@ -284,8 +284,8 @@ contract SSTORE2ContentSizesTest is TestSetup {
         assertEq(pointer1, pointer2, "Pointers should be identical");
 
         // Verify content retrieval works for both
-        bytes memory content1 = eth.getEthscriptionContent(txHash1);
-        bytes memory content2 = eth.getEthscriptionContent(txHash2);
+        bytes memory content1 = eth.readContent(txHash1);
+        bytes memory content2 = eth.readContent(txHash2);
         assertEq(keccak256(content1), keccak256(content2), "Content should be identical");
         assertEq(keccak256(content1), keccak256(content), "Retrieved content should match original");