From 68a4efb2e738118088065187adb7ca315941240a Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Thu, 11 Feb 2021 09:45:28 +0100 Subject: [PATCH 1/3] Refactor overwriteRelease flag. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- libsolidity/interface/CompilerStack.cpp | 11 ++++++++++- libsolidity/interface/CompilerStack.h | 14 +++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index d518c50d3..1995c7750 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -1580,6 +1580,9 @@ private: bytes CompilerStack::createCBORMetadata(Contract const& _contract) const { + if (m_metadataFormat == MetadataFormat::NoMetadata) + return bytes{}; + bool const experimentalMode = !onlySafeExperimentalFeaturesActivated( _contract.contract->sourceUnit().annotation().experimentalFeatures ); @@ -1597,10 +1600,16 @@ bytes CompilerStack::createCBORMetadata(Contract const& _contract) const if (experimentalMode || m_viaIR) encoder.pushBool("experimental", true); - if (m_release) + if (m_metadataFormat == MetadataFormat::WithReleaseVersionTag) encoder.pushBytes("solc", VersionCompactBytes); else + { + solAssert( + m_metadataFormat == MetadataFormat::WithPrereleaseVersionTag, + "Invalid metadata format." + ); encoder.pushString("solc", VersionStringStrict); + } return encoder.serialise(); } diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index 5049561c4..db087a663 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -98,6 +98,12 @@ public: CompilationSuccessful }; + enum class MetadataFormat { + WithReleaseVersionTag, + WithPrereleaseVersionTag, + NoMetadata + }; + enum class MetadataHash { IPFS, Bzzr1, @@ -336,8 +342,10 @@ public: /// @returns a JSON representing the estimated gas usage for contract creation, internal and external functions Json::Value gasEstimates(std::string const& _contractName) const; - /// Overwrites the release/prerelease flag. Should only be used for testing. - void overwriteReleaseFlag(bool release) { m_release = release; } + /// Changes the format of the metadata appended at the end of the bytecode. + /// This is mostly a workaround to avoid bytecode and gas differences between compiler builds + /// caused by differences in metadata. Should only be used for testing. + void setMetadataFormat(MetadataFormat _metadataFormat) { m_metadataFormat = _metadataFormat; } private: /// The state per source unit. Filled gradually during parsing. struct Source @@ -496,7 +504,7 @@ private: /// Whether or not there has been an error during processing. /// If this is true, the stack will refuse to generate code. bool m_hasError = false; - bool m_release = VersionIsRelease; + MetadataFormat m_metadataFormat = VersionIsRelease ? MetadataFormat::WithReleaseVersionTag : MetadataFormat::WithPrereleaseVersionTag; }; } From b598948211a417ec8a3aed9d87f55c0cc557b627 Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Thu, 11 Feb 2021 09:46:25 +0100 Subject: [PATCH 2/3] Run gas tests on NoMetadata format only. --- test/libsolidity/GasCosts.cpp | 14 +++++++------- test/libsolidity/GasTest.cpp | 2 +- test/libsolidity/gasTests/abiv2.sol | 6 +++--- test/libsolidity/gasTests/abiv2_optimised.sol | 6 +++--- test/libsolidity/gasTests/data_storage.sol | 6 +++--- test/libsolidity/gasTests/dispatch_large.sol | 6 +++--- .../gasTests/dispatch_large_optimised.sol | 6 +++--- test/libsolidity/gasTests/dispatch_medium.sol | 6 +++--- .../gasTests/dispatch_medium_optimised.sol | 6 +++--- test/libsolidity/gasTests/dispatch_small.sol | 6 +++--- .../gasTests/dispatch_small_optimised.sol | 6 +++--- test/libsolidity/gasTests/exp.sol | 6 +++--- test/libsolidity/gasTests/exp_optimized.sol | 6 +++--- 13 files changed, 41 insertions(+), 41 deletions(-) diff --git a/test/libsolidity/GasCosts.cpp b/test/libsolidity/GasCosts.cpp index 946c435b0..a03ccbb1e 100644 --- a/test/libsolidity/GasCosts.cpp +++ b/test/libsolidity/GasCosts.cpp @@ -91,7 +91,7 @@ BOOST_AUTO_TEST_CASE(string_storage) } } )"; - m_compiler.overwriteReleaseFlag(true); + m_compiler.setMetadataFormat(CompilerStack::MetadataFormat::NoMetadata); compileAndRun(sourceCode); auto evmVersion = solidity::test::CommonOptions::get().evmVersion(); @@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(string_storage) if (CommonOptions::get().useABIEncoderV1) CHECK_DEPLOY_GAS(133045, 129731, evmVersion); else - CHECK_DEPLOY_GAS(155553, 132103, evmVersion); + CHECK_DEPLOY_GAS(144679, 121229, evmVersion); } // This is only correct on >=Constantinople. else if (!CommonOptions::get().useABIEncoderV1) @@ -110,22 +110,22 @@ BOOST_AUTO_TEST_CASE(string_storage) { // Costs with 0 are cases which cannot be triggered in tests. if (evmVersion < EVMVersion::istanbul()) - CHECK_DEPLOY_GAS(0, 120189, evmVersion); + CHECK_DEPLOY_GAS(0, 109241, evmVersion); else - CHECK_DEPLOY_GAS(0, 108541, evmVersion); + CHECK_DEPLOY_GAS(0, 97697, evmVersion); } else { if (evmVersion < EVMVersion::istanbul()) - CHECK_DEPLOY_GAS(149567, 123969, evmVersion); + CHECK_DEPLOY_GAS(138693, 123969, evmVersion); else - CHECK_DEPLOY_GAS(134123, 110969, evmVersion); + CHECK_DEPLOY_GAS(123301, 110969, evmVersion); } } else if (evmVersion < EVMVersion::istanbul()) CHECK_DEPLOY_GAS(125829, 118559, evmVersion); else - CHECK_DEPLOY_GAS(114077, 107067, evmVersion); + CHECK_DEPLOY_GAS(114077, 96461, evmVersion); if (evmVersion >= EVMVersion::byzantium()) { diff --git a/test/libsolidity/GasTest.cpp b/test/libsolidity/GasTest.cpp index 4427c7dd6..540fb0a75 100644 --- a/test/libsolidity/GasTest.cpp +++ b/test/libsolidity/GasTest.cpp @@ -106,7 +106,7 @@ TestCase::TestResult GasTest::run(ostream& _stream, string const& _linePrefix, b // Prerelease CBOR metadata varies in size due to changing version numbers and build dates. // This leads to volatile creation cost estimates. Therefore we force the compiler to // release mode for testing gas estimates. - compiler().overwriteReleaseFlag(true); + compiler().setMetadataFormat(CompilerStack::MetadataFormat::NoMetadata); OptimiserSettings settings = m_optimise ? OptimiserSettings::standard() : OptimiserSettings::minimal(); if (m_optimiseYul) { diff --git a/test/libsolidity/gasTests/abiv2.sol b/test/libsolidity/gasTests/abiv2.sol index 54164e1b0..17e42cf1b 100644 --- a/test/libsolidity/gasTests/abiv2.sol +++ b/test/libsolidity/gasTests/abiv2.sol @@ -14,9 +14,9 @@ contract C { } // ---- // creation: -// codeDepositCost: 1181400 -// executionCost: 1227 -// totalCost: 1182627 +// codeDepositCost: 1170600 +// executionCost: 1214 +// totalCost: 1171814 // external: // a(): 1130 // b(uint256): infinite diff --git a/test/libsolidity/gasTests/abiv2_optimised.sol b/test/libsolidity/gasTests/abiv2_optimised.sol index 965e4576e..33e2bfb09 100644 --- a/test/libsolidity/gasTests/abiv2_optimised.sol +++ b/test/libsolidity/gasTests/abiv2_optimised.sol @@ -17,9 +17,9 @@ contract C { // optimize-yul: true // ---- // creation: -// codeDepositCost: 583400 -// executionCost: 619 -// totalCost: 584019 +// codeDepositCost: 572600 +// executionCost: 606 +// totalCost: 573206 // external: // a(): 985 // b(uint256): 2052 diff --git a/test/libsolidity/gasTests/data_storage.sol b/test/libsolidity/gasTests/data_storage.sol index 5cdadddd8..fa0db135d 100644 --- a/test/libsolidity/gasTests/data_storage.sol +++ b/test/libsolidity/gasTests/data_storage.sol @@ -13,8 +13,8 @@ contract C { } // ---- // creation: -// codeDepositCost: 398400 -// executionCost: 436 -// totalCost: 398836 +// codeDepositCost: 387600 +// executionCost: 424 +// totalCost: 388024 // external: // f(): 428 diff --git a/test/libsolidity/gasTests/dispatch_large.sol b/test/libsolidity/gasTests/dispatch_large.sol index 8413957cd..f4332d80d 100644 --- a/test/libsolidity/gasTests/dispatch_large.sol +++ b/test/libsolidity/gasTests/dispatch_large.sol @@ -24,9 +24,9 @@ contract Large { } // ---- // creation: -// codeDepositCost: 913400 -// executionCost: 948 -// totalCost: 914348 +// codeDepositCost: 902600 +// executionCost: 942 +// totalCost: 903542 // external: // a(): 1175 // b(uint256): infinite diff --git a/test/libsolidity/gasTests/dispatch_large_optimised.sol b/test/libsolidity/gasTests/dispatch_large_optimised.sol index 6603ada5d..a3f93e99c 100644 --- a/test/libsolidity/gasTests/dispatch_large_optimised.sol +++ b/test/libsolidity/gasTests/dispatch_large_optimised.sol @@ -27,9 +27,9 @@ contract Large { // optimize-runs: 2 // ---- // creation: -// codeDepositCost: 267000 -// executionCost: 306 -// totalCost: 267306 +// codeDepositCost: 256200 +// executionCost: 300 +// totalCost: 256500 // external: // a(): 983 // b(uint256): 2337 diff --git a/test/libsolidity/gasTests/dispatch_medium.sol b/test/libsolidity/gasTests/dispatch_medium.sol index b9076c9cd..8dcf8d344 100644 --- a/test/libsolidity/gasTests/dispatch_medium.sol +++ b/test/libsolidity/gasTests/dispatch_medium.sol @@ -11,9 +11,9 @@ contract Medium { } // ---- // creation: -// codeDepositCost: 360400 -// executionCost: 399 -// totalCost: 360799 +// codeDepositCost: 349600 +// executionCost: 386 +// totalCost: 349986 // external: // a(): 1152 // b(uint256): infinite diff --git a/test/libsolidity/gasTests/dispatch_medium_optimised.sol b/test/libsolidity/gasTests/dispatch_medium_optimised.sol index d59a36d58..3370694b9 100644 --- a/test/libsolidity/gasTests/dispatch_medium_optimised.sol +++ b/test/libsolidity/gasTests/dispatch_medium_optimised.sol @@ -14,9 +14,9 @@ contract Medium { // optimize-runs: 2 // ---- // creation: -// codeDepositCost: 157400 -// executionCost: 202 -// totalCost: 157602 +// codeDepositCost: 146600 +// executionCost: 190 +// totalCost: 146790 // external: // a(): 983 // b(uint256): 2095 diff --git a/test/libsolidity/gasTests/dispatch_small.sol b/test/libsolidity/gasTests/dispatch_small.sol index 6a062e530..56af8bf2b 100644 --- a/test/libsolidity/gasTests/dispatch_small.sol +++ b/test/libsolidity/gasTests/dispatch_small.sol @@ -6,9 +6,9 @@ contract Small { } // ---- // creation: -// codeDepositCost: 123600 -// executionCost: 171 -// totalCost: 123771 +// codeDepositCost: 112800 +// executionCost: 159 +// totalCost: 112959 // external: // fallback: 129 // a(): 1107 diff --git a/test/libsolidity/gasTests/dispatch_small_optimised.sol b/test/libsolidity/gasTests/dispatch_small_optimised.sol index f9da0e5bd..c8b45e9ab 100644 --- a/test/libsolidity/gasTests/dispatch_small_optimised.sol +++ b/test/libsolidity/gasTests/dispatch_small_optimised.sol @@ -9,9 +9,9 @@ contract Small { // optimize-runs: 2 // ---- // creation: -// codeDepositCost: 72600 -// executionCost: 123 -// totalCost: 72723 +// codeDepositCost: 61800 +// executionCost: 111 +// totalCost: 61911 // external: // fallback: 118 // a(): 961 diff --git a/test/libsolidity/gasTests/exp.sol b/test/libsolidity/gasTests/exp.sol index 4d2e79634..0f69a0725 100644 --- a/test/libsolidity/gasTests/exp.sol +++ b/test/libsolidity/gasTests/exp.sol @@ -19,9 +19,9 @@ contract C { // optimize-yul: false // ---- // creation: -// codeDepositCost: 119800 -// executionCost: 165 -// totalCost: 119965 +// codeDepositCost: 109000 +// executionCost: 159 +// totalCost: 109159 // external: // exp_neg_one(uint256): 2259 // exp_one(uint256): infinite diff --git a/test/libsolidity/gasTests/exp_optimized.sol b/test/libsolidity/gasTests/exp_optimized.sol index 1e0ebc2ba..0fc3e11c4 100644 --- a/test/libsolidity/gasTests/exp_optimized.sol +++ b/test/libsolidity/gasTests/exp_optimized.sol @@ -19,9 +19,9 @@ contract C { // optimize-yul: true // ---- // creation: -// codeDepositCost: 47800 -// executionCost: 99 -// totalCost: 47899 +// codeDepositCost: 37000 +// executionCost: 87 +// totalCost: 37087 // external: // exp_neg_one(uint256): 1917 // exp_one(uint256): 1870 From 6e62cbf156dab2b6c9d5fdf8b77f20f5460504de Mon Sep 17 00:00:00 2001 From: Djordje Mijovic Date: Thu, 11 Feb 2021 09:47:03 +0100 Subject: [PATCH 3/3] Run metadata tests for every metadata format. --- test/libsolidity/Metadata.cpp | 87 +++++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 29 deletions(-) diff --git a/test/libsolidity/Metadata.cpp b/test/libsolidity/Metadata.cpp index a82199b59..ae2dd85db 100644 --- a/test/libsolidity/Metadata.cpp +++ b/test/libsolidity/Metadata.cpp @@ -37,13 +37,18 @@ namespace solidity::frontend::test namespace { -map requireParsedCBORMetadata(bytes const& _bytecode) +map requireParsedCBORMetadata(bytes const& _bytecode, CompilerStack::MetadataFormat _metadataFormat) { bytes cborMetadata = solidity::test::onlyMetadata(_bytecode); - BOOST_REQUIRE(!cborMetadata.empty()); - std::optional> tmp = solidity::test::parseCBORMetadata(cborMetadata); - BOOST_REQUIRE(tmp); - return *tmp; + if (_metadataFormat != CompilerStack::MetadataFormat::NoMetadata) + { + BOOST_REQUIRE(!cborMetadata.empty()); + std::optional> tmp = solidity::test::parseCBORMetadata(cborMetadata); + BOOST_REQUIRE(tmp); + return *tmp; + } + BOOST_REQUIRE(cborMetadata.empty()); + return {}; } optional compileAndCheckLicenseMetadata(string const& _contractName, char const* _sourceCode) @@ -83,7 +88,11 @@ BOOST_AUTO_TEST_CASE(metadata_stamp) function g(function(uint) external returns (uint) x) public {} } )"; - for (auto release: std::set{true, VersionIsRelease}) + for (auto metadataFormat: std::set{ + CompilerStack::MetadataFormat::NoMetadata, + CompilerStack::MetadataFormat::WithReleaseVersionTag, + CompilerStack::MetadataFormat::WithPrereleaseVersionTag + }) for (auto metadataHash: set{ CompilerStack::MetadataHash::IPFS, CompilerStack::MetadataHash::Bzzr1, @@ -91,7 +100,7 @@ BOOST_AUTO_TEST_CASE(metadata_stamp) }) { CompilerStack compilerStack; - compilerStack.overwriteReleaseFlag(release); + compilerStack.setMetadataFormat(metadataFormat); compilerStack.setSources({{"", std::string(sourceCode)}}); compilerStack.setEVMVersion(solidity::test::CommonOptions::get().evmVersion()); compilerStack.setOptimiserSettings(solidity::test::CommonOptions::get().optimize); @@ -101,9 +110,9 @@ BOOST_AUTO_TEST_CASE(metadata_stamp) std::string const& metadata = compilerStack.metadata("test"); BOOST_CHECK(solidity::test::isValidMetadata(metadata)); - auto const cborMetadata = requireParsedCBORMetadata(bytecode); + auto const cborMetadata = requireParsedCBORMetadata(bytecode, metadataFormat); if (metadataHash == CompilerStack::MetadataHash::None) - BOOST_CHECK(cborMetadata.size() == 1); + BOOST_CHECK(cborMetadata.size() == (metadataFormat == CompilerStack::MetadataFormat::NoMetadata ? 0 : 1)); else { bytes hash; @@ -121,16 +130,24 @@ BOOST_AUTO_TEST_CASE(metadata_stamp) hashMethod = "bzzr1"; } - BOOST_CHECK(cborMetadata.size() == 2); - BOOST_CHECK(cborMetadata.count(hashMethod) == 1); - BOOST_CHECK(cborMetadata.at(hashMethod) == util::toHex(hash)); + if (metadataFormat != CompilerStack::MetadataFormat::NoMetadata) + { + BOOST_CHECK(cborMetadata.size() == 2); + BOOST_CHECK(cborMetadata.count(hashMethod) == 1); + BOOST_CHECK(cborMetadata.at(hashMethod) == util::toHex(hash)); + } } - BOOST_CHECK(cborMetadata.count("solc") == 1); - if (release) - BOOST_CHECK(cborMetadata.at("solc") == util::toHex(VersionCompactBytes)); + if (metadataFormat == CompilerStack::MetadataFormat::NoMetadata) + BOOST_CHECK(cborMetadata.count("solc") == 0); else - BOOST_CHECK(cborMetadata.at("solc") == VersionStringStrict); + { + BOOST_CHECK(cborMetadata.count("solc") == 1); + if (metadataFormat == CompilerStack::MetadataFormat::WithReleaseVersionTag) + BOOST_CHECK(cborMetadata.at("solc") == util::toHex(VersionCompactBytes)); + else + BOOST_CHECK(cborMetadata.at("solc") == VersionStringStrict); + } } } @@ -144,7 +161,11 @@ BOOST_AUTO_TEST_CASE(metadata_stamp_experimental) function g(function(uint) external returns (uint) x) public {} } )"; - for (auto release: set{true, VersionIsRelease}) + for (auto metadataFormat: std::set{ + CompilerStack::MetadataFormat::NoMetadata, + CompilerStack::MetadataFormat::WithReleaseVersionTag, + CompilerStack::MetadataFormat::WithPrereleaseVersionTag + }) for (auto metadataHash: set{ CompilerStack::MetadataHash::IPFS, CompilerStack::MetadataHash::Bzzr1, @@ -152,7 +173,7 @@ BOOST_AUTO_TEST_CASE(metadata_stamp_experimental) }) { CompilerStack compilerStack; - compilerStack.overwriteReleaseFlag(release); + compilerStack.setMetadataFormat(metadataFormat); compilerStack.setSources({{"", std::string(sourceCode)}}); compilerStack.setEVMVersion(solidity::test::CommonOptions::get().evmVersion()); compilerStack.setOptimiserSettings(solidity::test::CommonOptions::get().optimize); @@ -162,9 +183,9 @@ BOOST_AUTO_TEST_CASE(metadata_stamp_experimental) std::string const& metadata = compilerStack.metadata("test"); BOOST_CHECK(solidity::test::isValidMetadata(metadata)); - auto const cborMetadata = requireParsedCBORMetadata(bytecode); + auto const cborMetadata = requireParsedCBORMetadata(bytecode, metadataFormat); if (metadataHash == CompilerStack::MetadataHash::None) - BOOST_CHECK(cborMetadata.size() == 2); + BOOST_CHECK(cborMetadata.size() == (metadataFormat == CompilerStack::MetadataFormat::NoMetadata ? 0 : 2)); else { bytes hash; @@ -182,18 +203,26 @@ BOOST_AUTO_TEST_CASE(metadata_stamp_experimental) hashMethod = "bzzr1"; } - BOOST_CHECK(cborMetadata.size() == 3); - BOOST_CHECK(cborMetadata.count(hashMethod) == 1); - BOOST_CHECK(cborMetadata.at(hashMethod) == util::toHex(hash)); + if (metadataFormat != CompilerStack::MetadataFormat::NoMetadata) + { + BOOST_CHECK(cborMetadata.size() == 3); + BOOST_CHECK(cborMetadata.count(hashMethod) == 1); + BOOST_CHECK(cborMetadata.at(hashMethod) == util::toHex(hash)); + } } - BOOST_CHECK(cborMetadata.count("solc") == 1); - if (release) - BOOST_CHECK(cborMetadata.at("solc") == util::toHex(VersionCompactBytes)); + if (metadataFormat == CompilerStack::MetadataFormat::NoMetadata) + BOOST_CHECK(cborMetadata.count("solc") == 0); else - BOOST_CHECK(cborMetadata.at("solc") == VersionStringStrict); - BOOST_CHECK(cborMetadata.count("experimental") == 1); - BOOST_CHECK(cborMetadata.at("experimental") == "true"); + { + BOOST_CHECK(cborMetadata.count("solc") == 1); + if (metadataFormat == CompilerStack::MetadataFormat::WithReleaseVersionTag) + BOOST_CHECK(cborMetadata.at("solc") == util::toHex(VersionCompactBytes)); + else + BOOST_CHECK(cborMetadata.at("solc") == VersionStringStrict); + BOOST_CHECK(cborMetadata.count("experimental") == 1); + BOOST_CHECK(cborMetadata.at("experimental") == "true"); + } } }