Remove the usage of SELFDESTRUCT in BytecodeStorage (#668)

* Remove all call-sites for purgeBytecode, before cleaning up library internals * Remove purge logic from library entirely * fix comment * Update to reflect TWO VERSIONS (https://www.youtube.com/watch\?v\=diIFhc_Kzng) * Update to reflect TWO VERSIONS (https://www.youtube.com/watch\?v\=diIFhc_Kzng) * Update contracts/libs/0.8.x/BytecodeStorageV1.sol Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com> * Update contracts/libs/0.8.x/BytecodeStorageV1.sol Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com> * Update contracts/libs/0.8.x/BytecodeStorageV1.sol Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com> * Add basic form of `BytecodeStorage` library versioning (#670) * Support basic semantic versioning – and ensure interop with backwards compatible reads (tests added) * update the version string * minor adjustment and a song: https://www.youtube.com/watch\?v\=vOreqez4v9Y * Update to reflect better v1/v0/unknown versioning semantics * Update documentation and code structuring * Refactor size == 0 check * clean up offset checks * `STORE2`-compatible reads in `BytecodeStorage` (#681) * Update BytecodeStorage library to provide backwards-compatible reads that are compatible with SSTORE2 as the fallback read option, in advance of plans to split off reads into a shared external public utility library, in-companion to the embedded internal library for writes. * Whoops, forgot to add all the files yo * add support for manual-offset reads * SSTORE2 / explicit bytes reads restructure * Comment fix * Filename fix * Filename fix * Better typing for tests * spelling fixes * Split `BytecodeStorage` into public/internal libraries (#684) * basic MVP of split library (with tests) * Adjusted tests for split library setup * minor modifier adjustment * Adjust optimizer runs * optimizer order * nit * get interface from deployed contract * get interface from deployed contract * ensure libraries are linked * library linkkinnnn * More linking fixes in tests * update Engine Flex as PoC (#688) * fix the rest of the test bindingsgit diff * OPTIMIZOOOOOR * remove unnecessary using for * Fixed comment * public constants * Update library naming convention * DEPLOYOOOOOR * DEPLOYOOOOOR * format * Address nits --------- Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com> --------- Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com> --------- Co-authored-by: ryley-o <30364988+ryley-o@users.noreply.github.com>
ArtBlocks · May 5, 2023 · 5adfb32 · 5adfb32
1 parent 3362275
commit 5adfb32
Show file tree

Hide file tree

Showing 38 changed files with 2,256 additions and 315 deletions.
diff --git a/README.md b/README.md
@@ -70,6 +70,7 @@ erDiagram
     ADMIN_ACL ||--o{ CORE_V3_V3_FLEX : manages
     ADMIN_ACL ||--o{ MINTERFILTER : manages
     CORE_V3_V3_FLEX ||--|| MINTERFILTER : uses
+    CORE_V3_V3_FLEX }o--|| BYTECODESTORAGEREADER : uses
     MINTERFILTER ||--o{ MINTER_1 : uses
     MINTERFILTER ||--o{ MINTER_2 : uses
     MINTERFILTER ||--o{ MINTER_N : uses
@@ -141,6 +142,15 @@ The following represents the current set of flagship core contracts deployed on
 
 For deployed core contracts, see the deployment details in the `/deployments/engine/[V2|V3]/<engine-partner>/` directories. For V2 core contracts, archived source code is available in the `/posterity/engine/` directory.
 
+## BytecodeStorageReader
+
+BytecodeStorageReader (currently on V1 version) is public library for reading from storage contracts. This library is intended to be deployed as a standalone contract, and provides all _read_ functionality by being used as an externally linked library within the Art Blocks ecosystem contracts that use contract storage for writes.
+
+Given that it is an externally linked library with a shared public deployment, the deployment addresses for these shared deployments are referenced in our shared deployments `constants.ts` util (in the `BYTECODE_STORAGE_READER_LIBRARY_ADDRESSES` constant) so that they may be linked at time of deployment and are also linked here below for shared reference:
+
+- V1 `BytecodeStorageReader` (goerli): https://goerli.etherscan.io/address/0xB8B806A10d16cc80dB788552B54B3ECb4A2A3C3D#code
+- V1 `BytecodeStorageReader` (mainnet): https://etherscan.io/address/0xf0585dF582A0ad119F1616FB82f3b449a98EeCd5#code
+
 ## Minter Suite
 
 For details on the Flagship and Engine Minter Suite, see the [minter suite documenation](./MINTER_SUITE.md).

diff --git a/contracts/DependencyRegistryV0.sol b/contracts/DependencyRegistryV0.sol
@@ -13,7 +13,7 @@ import "@openzeppelin-4.8/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import "@openzeppelin-4.8/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin-4.5/contracts/utils/math/SafeCast.sol";
 
-import "./libs/0.8.x/BytecodeStorage.sol";
+import "./libs/0.8.x/BytecodeStorageV1.sol";
 import "./libs/0.8.x/Bytes32Strings.sol";
 
 /**
@@ -33,8 +33,7 @@ contract DependencyRegistryV0 is
     OwnableUpgradeable,
     IDependencyRegistryV0
 {
-    using BytecodeStorage for string;
-    using BytecodeStorage for address;
+    using BytecodeStorageWriter for string;
     using Bytes32Strings for bytes32;
     using Strings for uint256;
     using EnumerableSet for EnumerableSet.Bytes32Set;
@@ -211,16 +210,6 @@ contract DependencyRegistryV0 is
             _scriptId < dependencyType.scriptCount,
             "scriptId out of range"
         );
-        // purge old contract bytecode contract from the blockchain state
-        // note: Although this does reduce usage of Ethereum state, it does not
-        // reduce the gas costs of removal transactions. We believe this is the
-        // best behavior at the time of writing, and do not expect this to
-        // result in any breaking changes in the future. All current proposals
-        // to change the self-destruct opcode are backwards compatible, but may
-        // result in not removing the bytecode from the blockchain state. This
-        // implementation is compatible with that architecture, as it does not
-        // rely on the bytecode being removed from the blockchain state.
-        dependencyType.scriptBytecodeAddresses[_scriptId].purgeBytecode();
         // store script in contract bytecode, replacing reference address from
         // the contract that no longer exists with the newly created one
         dependencyType.scriptBytecodeAddresses[_scriptId] = _script
@@ -238,19 +227,7 @@ contract DependencyRegistryV0 is
         _onlyExistingDependencyType(_dependencyType);
         Dependency storage dependency = dependencyDetails[_dependencyType];
         require(dependency.scriptCount > 0, "there are no scripts to remove");
-        // purge old contract bytecode contract from the blockchain state
-        // note: Although this does reduce usage of Ethereum state, it does not
-        // reduce the gas costs of removal transactions. We believe this is the
-        // best behavior at the time of writing, and do not expect this to
-        // result in any breaking changes in the future. All current proposals
-        // to change the self-destruct opcode are backwards compatible, but may
-        // result in not removing the bytecode from the blockchain state. This
-        // implementation is compatible with that architecture, as it does not
-        // rely on the bytecode being removed from the blockchain state.
-        dependency
-            .scriptBytecodeAddresses[dependency.scriptCount - 1]
-            .purgeBytecode();
-        // delete reference to contract address that no longer exists
+        // delete reference to old storage contract address
         delete dependency.scriptBytecodeAddresses[dependency.scriptCount - 1];
         unchecked {
             dependency.scriptCount = dependency.scriptCount - 1;
@@ -757,7 +734,7 @@ contract DependencyRegistryV0 is
             return "";
         }
 
-        return dependency.scriptBytecodeAddresses[_index].readFromBytecode();
+        return _readFromBytecode(dependency.scriptBytecodeAddresses[_index]);
     }
 
     /**
@@ -851,4 +828,14 @@ contract DependencyRegistryV0 is
         OwnableUpgradeable._transferOwnership(newOwner);
         adminACLContract = IAdminACLV0(newOwner);
     }
+
+    /**
+     * Helper for calling `BytecodeStorageReader` external library reader method,
+     * added for bytecode size reduction purposes.
+     */
+    function _readFromBytecode(
+        address _address
+    ) internal view returns (string memory) {
+        return BytecodeStorageReader.readFromBytecode(_address);
+    }
 }
diff --git a/contracts/GenArt721CoreV3.sol b/contracts/GenArt721CoreV3.sol
@@ -12,7 +12,7 @@ import "./interfaces/0.8.x/IManifold.sol";
 import "@openzeppelin-4.7/contracts/utils/Strings.sol";
 import "@openzeppelin-4.7/contracts/access/Ownable.sol";
 import "./libs/0.8.x/ERC721_PackedHashSeed.sol";
-import "./libs/0.8.x/BytecodeStorage.sol";
+import "./libs/0.8.x/BytecodeStorageV1.sol";
 import "./libs/0.8.x/Bytes32Strings.sol";
 
 /**
@@ -93,8 +93,7 @@ contract GenArt721CoreV3 is
     IGenArt721CoreContractV3,
     IGenArt721CoreContractExposesHashSeed
 {
-    using BytecodeStorage for string;
-    using BytecodeStorage for address;
+    using BytecodeStorageWriter for string;
     using Bytes32Strings for bytes32;
     using Strings for uint256;
     uint256 constant ONE_HUNDRED = 100;
@@ -242,7 +241,7 @@ contract GenArt721CoreV3 is
     bool public newProjectsForbidden;
 
     /// version & type of this core contract
-    string public constant coreVersion = "v3.1.0";
+    string public constant coreVersion = "v3.2.0";
     string public constant coreType = "GenArt721CoreV3";
 
     /// default base URI to initialize all new project projectBaseURI values to
@@ -1072,18 +1071,8 @@ contract GenArt721CoreV3 is
         _onlyNonEmptyString(_script);
         Project storage project = projects[_projectId];
         require(_scriptId < project.scriptCount, "scriptId out of range");
-        // purge old contract bytecode contract from the blockchain state
-        // note: Although this does reduce usage of Ethereum state, it does not
-        // reduce the gas costs of removal transactions. We believe this is the
-        // best behavior at the time of writing, and do not expect this to
-        // result in any breaking changes in the future. All current proposals
-        // to change the self-destruct opcode are backwards compatible, but may
-        // result in not removing the bytecode from the blockchain state. This
-        // implementation is compatible with that architecture, as it does not
-        // rely on the bytecode being removed from the blockchain state.
-        project.scriptBytecodeAddresses[_scriptId].purgeBytecode();
         // store script in contract bytecode, replacing reference address from
-        // the contract that no longer exists with the newly created one
+        // the old storage contract with the newly created one
         project.scriptBytecodeAddresses[_scriptId] = _script.writeToBytecode();
         emit ProjectUpdated(_projectId, FIELD_PROJECT_SCRIPT);
     }
@@ -1100,19 +1089,7 @@ contract GenArt721CoreV3 is
         );
         Project storage project = projects[_projectId];
         require(project.scriptCount > 0, "there are no scripts to remove");
-        // purge old contract bytecode contract from the blockchain state
-        // note: Although this does reduce usage of Ethereum state, it does not
-        // reduce the gas costs of removal transactions. We believe this is the
-        // best behavior at the time of writing, and do not expect this to
-        // result in any breaking changes in the future. All current proposals
-        // to change the self-destruct opcode are backwards compatible, but may
-        // result in not removing the bytecode from the blockchain state. This
-        // implementation is compatible with that architecture, as it does not
-        // rely on the bytecode being removed from the blockchain state.
-        project
-            .scriptBytecodeAddresses[project.scriptCount - 1]
-            .purgeBytecode();
-        // delete reference to contract address that no longer exists
+        // delete reference to old storage contract address
         delete project.scriptBytecodeAddresses[project.scriptCount - 1];
         unchecked {
             project.scriptCount = project.scriptCount - 1;
@@ -1515,7 +1492,7 @@ contract GenArt721CoreV3 is
         if (_index >= project.scriptCount) {
             return "";
         }
-        return project.scriptBytecodeAddresses[_index].readFromBytecode();
+        return _readFromBytecode(project.scriptBytecodeAddresses[_index]);
     }
 
     /**
@@ -1967,4 +1944,14 @@ contract GenArt721CoreV3 is
             (block.timestamp - projectCompletedTimestamp <
                 FOUR_WEEKS_IN_SECONDS);
     }
+
+    /**
+     * Helper for calling `BytecodeStorageReader` external library reader method,
+     * added for bytecode size reduction purposes.
+     */
+    function _readFromBytecode(
+        address _address
+    ) internal view returns (string memory) {
+        return BytecodeStorageReader.readFromBytecode(_address);
+    }
 }
diff --git a/contracts/engine/V3/GenArt721CoreV3_Engine.sol b/contracts/engine/V3/GenArt721CoreV3_Engine.sol
@@ -14,7 +14,7 @@ import "../../interfaces/0.8.x/IManifold.sol";
 import "@openzeppelin-4.7/contracts/utils/Strings.sol";
 import "@openzeppelin-4.7/contracts/access/Ownable.sol";
 import "../../libs/0.8.x/ERC721_PackedHashSeed.sol";
-import "../../libs/0.8.x/BytecodeStorage.sol";
+import "../../libs/0.8.x/BytecodeStorageV1.sol";
 import "../../libs/0.8.x/Bytes32Strings.sol";
 
 /**
@@ -100,8 +100,7 @@ contract GenArt721CoreV3_Engine is
     IGenArt721CoreContractV3_Engine,
     IGenArt721CoreContractExposesHashSeed
 {
-    using BytecodeStorage for string;
-    using BytecodeStorage for address;
+    using BytecodeStorageWriter for string;
     using Bytes32Strings for bytes32;
     using Strings for uint256;
     using Strings for address;
@@ -1134,9 +1133,8 @@ contract GenArt721CoreV3_Engine is
         _onlyNonEmptyString(_script);
         Project storage project = projects[_projectId];
         require(_scriptId < project.scriptCount, "scriptId out of range");
-
         // store script in contract bytecode, replacing reference address from
-        // the contract that no longer exists with the newly created one
+        // the old storage contract with the newly created one
         project.scriptBytecodeAddresses[_scriptId] = _script.writeToBytecode();
         emit ProjectUpdated(_projectId, FIELD_PROJECT_SCRIPT);
     }
@@ -1153,8 +1151,7 @@ contract GenArt721CoreV3_Engine is
         );
         Project storage project = projects[_projectId];
         require(project.scriptCount > 0, "No scripts to remove");
-
-        // delete reference to contract address that no longer exists
+        // delete reference to old storage contract address
         delete project.scriptBytecodeAddresses[project.scriptCount - 1];
         unchecked {
             project.scriptCount = project.scriptCount - 1;
@@ -1579,7 +1576,7 @@ contract GenArt721CoreV3_Engine is
         if (_index >= project.scriptCount) {
             return "";
         }
-        return project.scriptBytecodeAddresses[_index].readFromBytecode();
+        return _readFromBytecode(project.scriptBytecodeAddresses[_index]);
     }
 
     /**
@@ -1998,4 +1995,14 @@ contract GenArt721CoreV3_Engine is
             (block.timestamp - projectCompletedTimestamp <
                 FOUR_WEEKS_IN_SECONDS);
     }
+
+    /**
+     * Helper for calling `BytecodeStorageReader` external library reader method,
+     * added for bytecode size reduction purposes.
+     */
+    function _readFromBytecode(
+        address _address
+    ) internal view returns (string memory) {
+        return BytecodeStorageReader.readFromBytecode(_address);
+    }
 }
diff --git a/contracts/engine/V3/GenArt721CoreV3_Engine_Flex.sol b/contracts/engine/V3/GenArt721CoreV3_Engine_Flex.sol
@@ -13,7 +13,7 @@ import "../../interfaces/0.8.x/IManifold.sol";
 
 import "@openzeppelin-4.7/contracts/access/Ownable.sol";
 import "../../libs/0.8.x/ERC721_PackedHashSeed.sol";
-import "../../libs/0.8.x/BytecodeStorage.sol";
+import "../../libs/0.8.x/BytecodeStorageV1.sol";
 import "../../libs/0.8.x/Bytes32Strings.sol";
 
 /**
@@ -108,8 +108,7 @@ contract GenArt721CoreV3_Engine_Flex is
     IGenArt721CoreContractV3_Engine_Flex,
     IGenArt721CoreContractExposesHashSeed
 {
-    using BytecodeStorage for string;
-    using BytecodeStorage for address;
+    using BytecodeStorageWriter for string;
     using Bytes32Strings for bytes32;
     uint256 constant ONE_HUNDRED = 100;
     uint256 constant ONE_MILLION = 1_000_000;
@@ -1320,9 +1319,8 @@ contract GenArt721CoreV3_Engine_Flex is
         _onlyNonEmptyString(_script);
         Project storage project = projects[_projectId];
         require(_scriptId < project.scriptCount, "scriptId out of range");
-
         // store script in contract bytecode, replacing reference address from
-        // the contract that no longer exists with the newly created one
+        // the old storage contract with the newly created one
         project.scriptBytecodeAddresses[_scriptId] = _script.writeToBytecode();
         emit ProjectUpdated(_projectId, FIELD_PROJECT_SCRIPT);
     }
@@ -1339,8 +1337,7 @@ contract GenArt721CoreV3_Engine_Flex is
         );
         Project storage project = projects[_projectId];
         require(project.scriptCount > 0, "No scripts to remove");
-
-        // delete reference to contract address that no longer exists
+        // delete reference to old storage contract address
         delete project.scriptBytecodeAddresses[project.scriptCount - 1];
         unchecked {
             project.scriptCount = project.scriptCount - 1;
@@ -1765,7 +1762,7 @@ contract GenArt721CoreV3_Engine_Flex is
         if (_index >= project.scriptCount) {
             return "";
         }
-        return project.scriptBytecodeAddresses[_index].readFromBytecode();
+        return _readFromBytecode(project.scriptBytecodeAddresses[_index]);
     }
 
     /**
@@ -1996,7 +1993,7 @@ contract GenArt721CoreV3_Engine_Flex is
                 bytecodeAddress: _bytecodeAddress,
                 data: (_dependency.dependencyType ==
                     ExternalAssetDependencyType.ONCHAIN)
-                    ? _bytecodeAddress.readFromBytecode()
+                    ? _readFromBytecode(_bytecodeAddress)
                     : ""
             });
     }
@@ -2219,6 +2216,16 @@ contract GenArt721CoreV3_Engine_Flex is
                 FOUR_WEEKS_IN_SECONDS);
     }
 
+    /**
+     * Helper for calling `BytecodeStorageReader` external library reader method,
+     * added for bytecode size reduction purposes.
+     */
+    function _readFromBytecode(
+        address _address
+    ) internal view returns (string memory) {
+        return BytecodeStorageReader.readFromBytecode(_address);
+    }
+
     // strings library from OpenZeppelin, modified for no constants
 
     bytes16 private _HEX_SYMBOLS = "0123456789abcdef";

diff --git a/contracts/engine/V3/forks/GenArt721CoreV3_Engine_Flex_PROOF.sol b/contracts/engine/V3/forks/GenArt721CoreV3_Engine_Flex_PROOF.sol
@@ -12,7 +12,7 @@ import "../../../interfaces/0.8.x/IManifold.sol";
 
 import "@openzeppelin-4.7/contracts/access/Ownable.sol";
 import "../../../libs/0.8.x/ERC721_PackedHashSeed.sol";
-import "../../../libs/0.8.x/BytecodeStorage.sol";
+import "../../../libs/0.8.x/BytecodeStorageV1.sol";
 import "../../../libs/0.8.x/Bytes32Strings.sol";
 
 /**
@@ -107,8 +107,7 @@ contract GenArt721CoreV3_Engine_Flex_PROOF is
     IManifold,
     IGenArt721CoreContractV3_Engine_Flex
 {
-    using BytecodeStorage for string;
-    using BytecodeStorage for address;
+    using BytecodeStorageWriter for string;
     using Bytes32Strings for bytes32;
 
     uint256 constant ONE_HUNDRED = 100;
@@ -1773,7 +1772,7 @@ contract GenArt721CoreV3_Engine_Flex_PROOF is
         if (_index >= project.scriptCount) {
             return "";
         }
-        return project.scriptBytecodeAddresses[_index].readFromBytecode();
+        return _readFromBytecode(project.scriptBytecodeAddresses[_index]);
     }
 
     /**
@@ -2004,7 +2003,7 @@ contract GenArt721CoreV3_Engine_Flex_PROOF is
                 bytecodeAddress: _bytecodeAddress,
                 data: (_dependency.dependencyType ==
                     ExternalAssetDependencyType.ONCHAIN)
-                    ? _bytecodeAddress.readFromBytecode()
+                    ? _readFromBytecode(_bytecodeAddress)
                     : ""
             });
     }
@@ -2227,6 +2226,16 @@ contract GenArt721CoreV3_Engine_Flex_PROOF is
                 FOUR_WEEKS_IN_SECONDS);
     }
 
+    /**
+     * Helper for calling `BytecodeStorageReader` external library reader method,
+     * added for bytecode size reduction purposes.
+     */
+    function _readFromBytecode(
+        address _address
+    ) internal view returns (string memory) {
+        return BytecodeStorageReader.readFromBytecode(_address);
+    }
+
     // strings library from OpenZeppelin, modified for no constants
 
     bytes16 private _HEX_SYMBOLS = "0123456789abcdef";