From 3bd691300e7adbae89978ecd2d08205cfb4557a7 Mon Sep 17 00:00:00 2001 From: subtly Date: Sun, 5 Apr 2015 17:01:57 +0200 Subject: [PATCH 01/26] fix first whisper test --- libwhisper/WhisperPeer.cpp | 4 +- test/whisperTopic.cpp | 77 ++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 44 deletions(-) diff --git a/libwhisper/WhisperPeer.cpp b/libwhisper/WhisperPeer.cpp index 53ea91a9e..06957ed04 100644 --- a/libwhisper/WhisperPeer.cpp +++ b/libwhisper/WhisperPeer.cpp @@ -71,10 +71,8 @@ bool WhisperPeer::interpret(unsigned _id, RLP const& _r) } case MessagesPacket: { - unsigned n = 0; for (auto i: _r) - if (n++) - host()->inject(Envelope(i), this); + host()->inject(Envelope(i), this); break; } default: diff --git a/test/whisperTopic.cpp b/test/whisperTopic.cpp index 0ea681b67..f2aa818ee 100644 --- a/test/whisperTopic.cpp +++ b/test/whisperTopic.cpp @@ -32,33 +32,27 @@ using namespace dev::shh; BOOST_AUTO_TEST_SUITE(whisper) -#if ALEX_HASH_FIXED_NETWORKING BOOST_AUTO_TEST_CASE(topic) { cnote << "Testing Whisper..."; auto oldLogVerbosity = g_logVerbosity; - g_logVerbosity = 0; + g_logVerbosity = 11; - Host host1("Test", NetworkPreferences(30303, "127.0.0.1", false, true)); + Host host1("Test", NetworkPreferences("127.0.0.1", 30303, false)); + host1.setIdealPeerCount(1); auto whost1 = host1.registerCapability(new WhisperHost()); host1.start(); - while (!host1.isStarted()) - this_thread::sleep_for(chrono::milliseconds(2)); - - bool started = false; + bool host1Ready = false; unsigned result = 0; std::thread listener([&]() { setThreadName("other"); - started = true; - + /// Only interested in odd packets auto w = whost1->installWatch(BuildTopicMask("odd")); - - started = true; + host1Ready = true; set received; - for (int iterout = 0, last = 0; iterout < 200 && last < 81; ++iterout) { for (auto i: whost1->checkWatch(w)) @@ -76,21 +70,21 @@ BOOST_AUTO_TEST_CASE(topic) }); - Host host2("Test", NetworkPreferences(30300, "127.0.0.1", false, true)); + Host host2("Test", NetworkPreferences("127.0.0.1", 30300, false)); + host1.setIdealPeerCount(1); auto whost2 = host2.registerCapability(new WhisperHost()); host2.start(); - - while (!host2.isStarted()) - this_thread::sleep_for(chrono::milliseconds(2)); - - this_thread::sleep_for(chrono::milliseconds(100)); - host2.addNode(host1.id(), "127.0.0.1", 30303, 30303); - - this_thread::sleep_for(chrono::milliseconds(500)); - - while (!started) - this_thread::sleep_for(chrono::milliseconds(2)); - + + while (!host1.haveNetwork()) + this_thread::sleep_for(chrono::milliseconds(5)); + host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30303, 30303); + + // wait for nodes to connect + this_thread::sleep_for(chrono::milliseconds(1000)); + + while (!host1Ready) + this_thread::sleep_for(chrono::milliseconds(10)); + KeyPair us = KeyPair::create(); for (int i = 0; i < 10; ++i) { @@ -104,6 +98,7 @@ BOOST_AUTO_TEST_CASE(topic) BOOST_REQUIRE_EQUAL(result, 1 + 9 + 25 + 49 + 81); } +#if ALEX_HASH_FIXED_NETWORKING BOOST_AUTO_TEST_CASE(forwarding) { cnote << "Testing Whisper forwarding..."; @@ -111,11 +106,11 @@ BOOST_AUTO_TEST_CASE(forwarding) g_logVerbosity = 0; // Host must be configured not to share peers. - Host host1("Listner", NetworkPreferences(30303, "", false, true)); + Host host1("Listner", NetworkPreferences("127.0.0.1", 30303, false)); host1.setIdealPeerCount(0); auto whost1 = host1.registerCapability(new WhisperHost()); host1.start(); - while (!host1.isStarted()) + while (!host1.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); unsigned result = 0; @@ -146,11 +141,11 @@ BOOST_AUTO_TEST_CASE(forwarding) // Host must be configured not to share peers. - Host host2("Forwarder", NetworkPreferences(30305, "", false, true)); + Host host2("Forwarder", NetworkPreferences("127.0.0.1", 30305, false)); host2.setIdealPeerCount(1); auto whost2 = host2.registerCapability(new WhisperHost()); host2.start(); - while (!host2.isStarted()) + while (!host2.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); Public fwderid; @@ -163,7 +158,7 @@ BOOST_AUTO_TEST_CASE(forwarding) this_thread::sleep_for(chrono::milliseconds(50)); this_thread::sleep_for(chrono::milliseconds(500)); - host2.addNode(host1.id(), "127.0.0.1", 30303, 30303); + host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30303, 30303); startedForwarder = true; @@ -184,12 +179,12 @@ BOOST_AUTO_TEST_CASE(forwarding) while (!startedForwarder) this_thread::sleep_for(chrono::milliseconds(50)); - Host ph("Sender", NetworkPreferences(30300, "", false, true)); + Host ph("Sender", NetworkPreferences("127.0.0.1", 30300, false)); ph.setIdealPeerCount(1); shared_ptr wh = ph.registerCapability(new WhisperHost()); ph.start(); - ph.addNode(host2.id(), "127.0.0.1", 30305, 30305); - while (!ph.isStarted()) + ph.addNode(host2.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); + while (!ph.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(10)); KeyPair us = KeyPair::create(); @@ -214,11 +209,11 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) bool done = false; // Host must be configured not to share peers. - Host host1("Forwarder", NetworkPreferences(30305, "", false, true)); + Host host1("Forwarder", NetworkPreferences("127.0.0.1", 30305, false)); host1.setIdealPeerCount(1); auto whost1 = host1.registerCapability(new WhisperHost()); host1.start(); - while (!host1.isStarted()) + while (!host1.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); bool startedForwarder = false; @@ -249,13 +244,13 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) this_thread::sleep_for(chrono::milliseconds(2)); { - Host host2("Sender", NetworkPreferences(30300, "", false, true)); + Host host2("Sender", NetworkPreferences("127.0.0.1", 30300, false)); host2.setIdealPeerCount(1); shared_ptr whost2 = host2.registerCapability(new WhisperHost()); host2.start(); - while (!host2.isStarted()) + while (!host2.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); - host2.addNode(host1.id(), "127.0.0.1", 30305, 30305); + host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); while (!host2.peerCount()) this_thread::sleep_for(chrono::milliseconds(5)); @@ -266,13 +261,13 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) } { - Host ph("Listener", NetworkPreferences(30300, "", false, true)); + Host ph("Listener", NetworkPreferences("127.0.0.1", 30300, false)); ph.setIdealPeerCount(1); shared_ptr wh = ph.registerCapability(new WhisperHost()); ph.start(); - while (!ph.isStarted()) + while (!ph.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); - ph.addNode(host1.id(), "127.0.0.1", 30305, 30305); + ph.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); /// Only interested in odd packets auto w = wh->installWatch(BuildTopicMask("test")); From ce1f7798e3936af0d701497527e48e304eb9f9ee Mon Sep 17 00:00:00 2001 From: subtly Date: Sun, 5 Apr 2015 17:10:29 +0200 Subject: [PATCH 02/26] reenable whisper tests --- test/whisperTopic.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/whisperTopic.cpp b/test/whisperTopic.cpp index f2aa818ee..b43ba708e 100644 --- a/test/whisperTopic.cpp +++ b/test/whisperTopic.cpp @@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(topic) { cnote << "Testing Whisper..."; auto oldLogVerbosity = g_logVerbosity; - g_logVerbosity = 11; + g_logVerbosity = 0; Host host1("Test", NetworkPreferences("127.0.0.1", 30303, false)); host1.setIdealPeerCount(1); @@ -98,7 +98,6 @@ BOOST_AUTO_TEST_CASE(topic) BOOST_REQUIRE_EQUAL(result, 1 + 9 + 25 + 49 + 81); } -#if ALEX_HASH_FIXED_NETWORKING BOOST_AUTO_TEST_CASE(forwarding) { cnote << "Testing Whisper forwarding..."; @@ -291,6 +290,5 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) BOOST_REQUIRE_EQUAL(result, 1); } -#endif BOOST_AUTO_TEST_SUITE_END() From 0f50be0cc8a6d2cc06527ab890d8ef2f55dc2040 Mon Sep 17 00:00:00 2001 From: subtly Date: Fri, 8 May 2015 18:19:31 +0200 Subject: [PATCH 03/26] update whisper test to use fixture for new network code, to allow private addresses and encapsulate host/port information in NodeIPEndpoint. --- test/libwhisper/whisperTopic.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/test/libwhisper/whisperTopic.cpp b/test/libwhisper/whisperTopic.cpp index c49debf61..291b62c17 100644 --- a/test/libwhisper/whisperTopic.cpp +++ b/test/libwhisper/whisperTopic.cpp @@ -30,7 +30,13 @@ using namespace dev; using namespace dev::p2p; using namespace dev::shh; -BOOST_AUTO_TEST_SUITE(whisper) +struct P2PFixture +{ + P2PFixture() { dev::p2p::NodeIPEndpoint::test_allowLocal = true; } + ~P2PFixture() { dev::p2p::NodeIPEndpoint::test_allowLocal = false; } +}; + +BOOST_FIXTURE_TEST_SUITE(whisper, P2PFixture) BOOST_AUTO_TEST_CASE(topic) { @@ -77,7 +83,7 @@ BOOST_AUTO_TEST_CASE(topic) while (!host1.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(5)); - host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30303, 30303); + host2.addNode(host1.id(), NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 30303, 30303)); // wait for nodes to connect this_thread::sleep_for(chrono::milliseconds(1000)); @@ -157,7 +163,7 @@ BOOST_AUTO_TEST_CASE(forwarding) this_thread::sleep_for(chrono::milliseconds(50)); this_thread::sleep_for(chrono::milliseconds(500)); - host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30303, 30303); + host2.addNode(host1.id(), NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 30303, 30303)); startedForwarder = true; @@ -182,7 +188,7 @@ BOOST_AUTO_TEST_CASE(forwarding) ph.setIdealPeerCount(1); shared_ptr wh = ph.registerCapability(new WhisperHost()); ph.start(); - ph.addNode(host2.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); + ph.addNode(host2.id(), NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 30305, 30305)); while (!ph.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(10)); @@ -221,7 +227,6 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) setThreadName("forwarder"); this_thread::sleep_for(chrono::milliseconds(500)); -// ph.addNode("127.0.0.1", 30303, 30303); startedForwarder = true; @@ -249,7 +254,7 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) host2.start(); while (!host2.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); - host2.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); + host2.addNode(host1.id(), NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 30305, 30305)); while (!host2.peerCount()) this_thread::sleep_for(chrono::milliseconds(5)); @@ -266,7 +271,7 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) ph.start(); while (!ph.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(2)); - ph.addNode(host1.id(), bi::address::from_string("127.0.0.1"), 30305, 30305); + ph.addNode(host1.id(), NodeIPEndpoint(bi::address::from_string("127.0.0.1"), 30305, 30305)); /// Only interested in odd packets auto w = wh->installWatch(BuildTopicMask("test")); From 2c0d9e99689332d39b6329a4f23726db2e5b493f Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Wed, 3 Jun 2015 18:06:49 +0200 Subject: [PATCH 04/26] corrected the calculation of gas for send --- libsolidity/Compiler.cpp | 2 +- libsolidity/ExpressionCompiler.cpp | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 6425367dd..50fb8e3db 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -42,7 +42,7 @@ class StackHeightChecker public: StackHeightChecker(CompilerContext const& _context): m_context(_context), stackHeight(m_context.getStackHeight()) {} - void check() { solAssert(m_context.getStackHeight() == stackHeight, "I sense a disturbance in the stack."); } + void check() {/* solAssert(m_context.getStackHeight() == stackHeight, "I sense a disturbance in the stack."); */} private: CompilerContext const& m_context; unsigned stackHeight; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index bb3260770..51bdfbc43 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -497,6 +498,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { // stack layout: contract_address function_id [gas] [value] _functionCall.getExpression().accept(*this); + arguments.front()->accept(*this); appendTypeConversion(*arguments.front()->getType(), IntegerType(256), true); // Note that function is not the original function, but the ".gas" function. @@ -519,7 +521,6 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) break; case Location::Send: _functionCall.getExpression().accept(*this); - m_context << u256(0); // 0 gas, we do not want to execute code arguments.front()->accept(*this); appendTypeConversion(*arguments.front()->getType(), *function.getParameterTypes().front(), true); @@ -531,7 +532,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) strings(), Location::Bare, false, - true, + false, true ), {} @@ -1098,7 +1099,10 @@ void ExpressionCompiler::appendExternalFunctionCall( else // send all gas except the amount needed to execute "SUB" and "CALL" // @todo this retains too much gas for now, needs to be fine-tuned. - m_context << u256(50 + (_functionType.valueSet() ? 9000 : 0) + 25000) << eth::Instruction::GAS << eth::Instruction::SUB; + m_context << + u256(eth::c_callGas + 10 + (_functionType.valueSet() ? eth::c_callValueTransferGas : 0) + eth::c_callNewAccountGas) << + eth::Instruction::GAS << + eth::Instruction::SUB; if ( _functionType.getLocation() == FunctionType::Location::CallCode || _functionType.getLocation() == FunctionType::Location::BareCallCode From 4f925169c1144615b10037e1f23085e7f5c7314d Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 4 Jun 2015 11:46:10 +0200 Subject: [PATCH 05/26] fixed assertion --- libsolidity/Compiler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 50fb8e3db..6425367dd 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -42,7 +42,7 @@ class StackHeightChecker public: StackHeightChecker(CompilerContext const& _context): m_context(_context), stackHeight(m_context.getStackHeight()) {} - void check() {/* solAssert(m_context.getStackHeight() == stackHeight, "I sense a disturbance in the stack."); */} + void check() { solAssert(m_context.getStackHeight() == stackHeight, "I sense a disturbance in the stack."); } private: CompilerContext const& m_context; unsigned stackHeight; From a7732d201326ea569422da704570a708dbffa2d1 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Mon, 1 Jun 2015 22:00:27 +0200 Subject: [PATCH 06/26] test Conflicts: test/libsolidity/SolidityEndToEndTest.cpp --- test/libsolidity/SolidityEndToEndTest.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 9f6f27d7d..e7287b399 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4172,6 +4172,17 @@ BOOST_AUTO_TEST_CASE(evm_exceptions_in_constructor_out_of_baund) BOOST_CHECK(compileAndRunWthoutCheck(sourceCode, 0, "A").empty()); } +BOOST_AUTO_TEST_CASE(positive_integers_to_signed) +{ + char const* sourceCode = R"( + contract test { + int8 public x = 2; + } + )"; + compileAndRun(sourceCode, 0, "test"); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(2)); +} + BOOST_AUTO_TEST_SUITE_END() } From 4757651b6453bac1057a2c7c970e1475a70678be Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Wed, 3 Jun 2015 16:14:23 +0200 Subject: [PATCH 07/26] - conversion of positive literals to signed int - tests --- libsolidity/AST.cpp | 12 ++++-- libsolidity/Types.cpp | 36 +++++++++++------ test/libsolidity/SolidityEndToEndTest.cpp | 4 ++ .../SolidityNameAndTypeResolution.cpp | 40 +++++++++++++++++++ 4 files changed, 77 insertions(+), 15 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 248abfdb9..a209235dc 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -686,9 +686,15 @@ void Expression::expectType(Type const& _expectedType) checkTypeRequirements(nullptr); Type const& type = *getType(); if (!type.isImplicitlyConvertibleTo(_expectedType)) - BOOST_THROW_EXCEPTION(createTypeError("Type " + type.toString() + - " not implicitly convertible to expected type " - + _expectedType.toString() + ".")); + BOOST_THROW_EXCEPTION( + createTypeError( + "Type " + + type.toString() + + " is not implicitly convertible to expected type " + + _expectedType.toString() + + "." + ) + ); } void Expression::requireLValue() diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 0e9ea9876..c4238f773 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -361,17 +361,28 @@ IntegerConstantType::IntegerConstantType(Literal const& _literal) bool IntegerConstantType::isImplicitlyConvertibleTo(Type const& _convertTo) const { - shared_ptr integerType = getIntegerType(); - if (!integerType) - return false; - - if (_convertTo.getCategory() == Category::FixedBytes) + if (IntegerType const* integerType = dynamic_cast(&_convertTo)) { - FixedBytesType const& convertTo = dynamic_cast(_convertTo); - return convertTo.getNumBytes() * 8 >= integerType->getNumBits(); + if (m_value == 0) + return true; + int forSignBit = (integerType->isSigned() ? 1 : 0); + if (m_value > 0) + { + if (m_value <= (u256(-1) >> (256 - integerType->getNumBits() + forSignBit))) + return true; + } + else if (-m_value <= (u256(1) << (integerType->getNumBits() - forSignBit))) + return true; + return false; } - - return integerType->isImplicitlyConvertibleTo(_convertTo); + else + if (_convertTo.getCategory() == Category::FixedBytes) + { + FixedBytesType const& fixedBytes = dynamic_cast(_convertTo); + return fixedBytes.getNumBytes() * 8 >= getIntegerType()->getNumBits(); + } + else + return false; } bool IntegerConstantType::isExplicitlyConvertibleTo(Type const& _convertTo) const @@ -514,9 +525,10 @@ shared_ptr IntegerConstantType::getIntegerType() const if (value > u256(-1)) return shared_ptr(); else - return make_shared(max(bytesRequired(value), 1u) * 8, - negative ? IntegerType::Modifier::Signed - : IntegerType::Modifier::Unsigned); + return make_shared( + max(bytesRequired(value), 1u) * 8, + negative ? IntegerType::Modifier::Signed : IntegerType::Modifier::Unsigned + ); } shared_ptr FixedBytesType::smallestTypeForLiteral(string const& _literal) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index e7287b399..1d0f1fc22 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4177,10 +4177,14 @@ BOOST_AUTO_TEST_CASE(positive_integers_to_signed) char const* sourceCode = R"( contract test { int8 public x = 2; + int8 public y = 127; + int16 public q = 250; } )"; compileAndRun(sourceCode, 0, "test"); BOOST_CHECK(callContractFunction("x()") == encodeArgs(2)); + BOOST_CHECK(callContractFunction("y()") == encodeArgs(127)); + BOOST_CHECK(callContractFunction("q()") == encodeArgs(250)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 48404aaac..a35b7bbe1 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1816,6 +1816,46 @@ BOOST_AUTO_TEST_CASE(string_length) BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); } +BOOST_AUTO_TEST_CASE(negative_integers_to_signed_out_of_bound) +{ + char const* sourceCode = R"( + contract test { + int8 public i = -129; + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(negative_integers_to_signed_min) +{ + char const* sourceCode = R"( + contract test { + int8 public i = -128; + } + )"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(sourceCode)); +} + +BOOST_AUTO_TEST_CASE(positive_integers_to_signed_out_of_bound) +{ + char const* sourceCode = R"( + contract test { + int8 public j = 128; + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(positive_integers_to_signed_out_of_bound_max) +{ + char const* sourceCode = R"( + contract test { + int8 public j = 127; + } + )"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(sourceCode)); +} + BOOST_AUTO_TEST_SUITE_END() } From fdff985a300617877567cf1b9d20e4f0eee47a03 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 4 Jun 2015 19:06:01 +0900 Subject: [PATCH 08/26] --ask and --bid --- eth/main.cpp | 45 +++++++++++++++++++++++++++++------- libethereum/EthereumHost.cpp | 20 ++++++++-------- libethereum/EthereumHost.h | 4 ++-- libethereum/State.h | 13 ++++++++--- 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/eth/main.cpp b/eth/main.cpp index 095540a1c..27e5f4ca0 100644 --- a/eth/main.cpp +++ b/eth/main.cpp @@ -124,9 +124,11 @@ void help() << " --password Give a password for a private key." << endl << endl << "Client transacting:" << endl - << " -B,--block-fees Set the block fee profit in the reference unit e.g. ¢ (default: 15)." << endl + /*<< " -B,--block-fees Set the block fee profit in the reference unit e.g. ¢ (default: 15)." << endl << " -e,--ether-price Set the ether price in the reference unit e.g. ¢ (default: 30.679)." << endl - << " -P,--priority <0 - 100> Default % priority of a transaction (default: 50)." << endl + << " -P,--priority <0 - 100> Default % priority of a transaction (default: 50)." << endl*/ + << " --ask Set the minimum ask gas price under which no transactions will be mined (default 500000000000)." << endl + << " --bid Set the bid gas price for to pay for transactions (default 500000000000)." << endl << endl << "Client mining:" << endl << " -a,--address Set the coinbase (mining payout) address to addr (default: auto)." << endl @@ -299,8 +301,10 @@ int main(int argc, char** argv) /// Transaction params TransactionPriority priority = TransactionPriority::Medium; - double etherPrice = 30.679; - double blockFees = 15.0; +// double etherPrice = 30.679; +// double blockFees = 15.0; + u256 askPrice("500000000000"); + u256 bidPrice("500000000000"); // javascript console bool useConsole = false; @@ -464,7 +468,7 @@ int main(int argc, char** argv) } else if ((arg == "-d" || arg == "--path" || arg == "--db-path") && i + 1 < argc) dbPath = argv[++i]; - else if ((arg == "-B" || arg == "--block-fees") && i + 1 < argc) +/* else if ((arg == "-B" || arg == "--block-fees") && i + 1 < argc) { try { @@ -487,6 +491,30 @@ int main(int argc, char** argv) cerr << "Bad " << arg << " option: " << argv[i] << endl; return -1; } + }*/ + else if (arg == "--ask" && i + 1 < argc) + { + try + { + askPrice = u256(argv[++i]); + } + catch (...) + { + cerr << "Bad " << arg << " option: " << argv[i] << endl; + return -1; + } + } + else if (arg == "--bid" && i + 1 < argc) + { + try + { + bidPrice = u256(argv[++i]); + } + catch (...) + { + cerr << "Bad " << arg << " option: " << argv[i] << endl; + return -1; + } } else if ((arg == "-P" || arg == "--priority") && i + 1 < argc) { @@ -730,7 +758,8 @@ int main(int argc, char** argv) cout << ethCredits(); web3.setIdealPeerCount(peers); - std::shared_ptr gasPricer = make_shared(u256(double(ether / 1000) / etherPrice), u256(blockFees * 1000)); +// std::shared_ptr gasPricer = make_shared(u256(double(ether / 1000) / etherPrice), u256(blockFees * 1000)); + std::shared_ptr gasPricer = make_shared(askPrice, bidPrice); eth::Client* c = nodeMode == NodeMode::Full ? web3.ethereum() : nullptr; StructuredLogger::starting(clientImplString, dev::Version); if (c) @@ -829,7 +858,7 @@ int main(int argc, char** argv) iss >> enable; c->setForceMining(isTrue(enable)); } - else if (c && cmd == "setblockfees") +/* else if (c && cmd == "setblockfees") { iss >> blockFees; try @@ -884,7 +913,7 @@ int main(int argc, char** argv) cerr << "Unknown priority: " << m << endl; } cout << "Priority: " << (int)priority << "/8" << endl; - } + }*/ else if (cmd == "verbosity") { if (iss.peek() != -1) diff --git a/libethereum/EthereumHost.cpp b/libethereum/EthereumHost.cpp index ebbde4d07..68619d627 100644 --- a/libethereum/EthereumHost.cpp +++ b/libethereum/EthereumHost.cpp @@ -54,7 +54,7 @@ EthereumHost::EthereumHost(BlockChain const& _ch, TransactionQueue& _tq, BlockQu EthereumHost::~EthereumHost() { - forEachPeer([](EthereumPeer* _p) { _p->abortSync(); }); + foreachPeer([](EthereumPeer* _p) { _p->abortSync(); }); } bool EthereumHost::ensureInitialised() @@ -74,7 +74,7 @@ bool EthereumHost::ensureInitialised() void EthereumHost::reset() { - forEachPeer([](EthereumPeer* _p) { _p->abortSync(); }); + foreachPeer([](EthereumPeer* _p) { _p->abortSync(); }); m_man.resetToChain(h256s()); m_hashMan.reset(m_chain.number() + 1); m_needSyncBlocks = true; @@ -105,7 +105,7 @@ void EthereumHost::doWork() } } - forEachPeer([](EthereumPeer* _p) { _p->tick(); }); + foreachPeer([](EthereumPeer* _p) { _p->tick(); }); // return netChange; // TODO: Figure out what to do with netChange. @@ -125,7 +125,7 @@ void EthereumHost::maintainTransactions() } for (auto const& t: ts) m_transactionsSent.insert(t.first); - forEachPeerPtr([&](shared_ptr _p) + foreachPeerPtr([&](shared_ptr _p) { bytes b; unsigned n = 0; @@ -148,16 +148,16 @@ void EthereumHost::maintainTransactions() }); } -void EthereumHost::forEachPeer(std::function const& _f) const +void EthereumHost::foreachPeer(std::function const& _f) const { - forEachPeerPtr([&](std::shared_ptr _p) + foreachPeerPtr([&](std::shared_ptr _p) { if (_p) _f(_p.get()); }); } -void EthereumHost::forEachPeerPtr(std::function)> const& _f) const +void EthereumHost::foreachPeerPtr(std::function)> const& _f) const { for (auto s: peerSessions()) _f(s.first->cap()); @@ -551,7 +551,7 @@ void EthereumHost::onPeerTransactions(EthereumPeer* _peer, RLP const& _r) void EthereumHost::continueSync() { clog(NetAllDetail) << "Getting help with downloading hashes and blocks"; - forEachPeer([&](EthereumPeer* _p) + foreachPeer([&](EthereumPeer* _p) { if (_p->m_asking == Asking::Nothing) continueSync(_p); @@ -564,7 +564,7 @@ void EthereumHost::continueSync(EthereumPeer* _peer) bool otherPeerSync = false; if (m_needSyncHashes && peerShouldGrabChain(_peer)) { - forEachPeer([&](EthereumPeer* _p) + foreachPeer([&](EthereumPeer* _p) { if (_p != _peer && _p->m_asking == Asking::Hashes && _p->m_protocolVersion != protocolVersion()) otherPeerSync = true; // Already have a peer downloading hash chain with old protocol, do nothing @@ -630,7 +630,7 @@ bool EthereumHost::isSyncing_UNSAFE() const /// We need actual peer information here to handle the case when we are the first ever peer on the network to mine. /// I.e. on a new private network the first node mining has noone to sync with and should start block propogation immediately. bool syncing = false; - forEachPeer([&](EthereumPeer* _p) + foreachPeer([&](EthereumPeer* _p) { if (_p->m_asking != Asking::Nothing) syncing = true; diff --git a/libethereum/EthereumHost.h b/libethereum/EthereumHost.h index ff0fc9607..8ca815a17 100644 --- a/libethereum/EthereumHost.h +++ b/libethereum/EthereumHost.h @@ -92,8 +92,8 @@ public: private: std::tuple>, std::vector>, std::vector>> randomSelection(unsigned _percent = 25, std::function const& _allow = [](EthereumPeer const*){ return true; }); - void forEachPeerPtr(std::function)> const& _f) const; - void forEachPeer(std::function const& _f) const; + void foreachPeerPtr(std::function)> const& _f) const; + void foreachPeer(std::function const& _f) const; bool isSyncing_UNSAFE() const; /// Sync with the BlockChain. It might contain one of our mined blocks, we might have new candidates from the network. diff --git a/libethereum/State.h b/libethereum/State.h index afdf41735..ea54d153d 100644 --- a/libethereum/State.h +++ b/libethereum/State.h @@ -84,9 +84,16 @@ public: class TrivialGasPricer: public GasPricer { -protected: - u256 ask(State const&) const override { return 10 * szabo; } - u256 bid(TransactionPriority = TransactionPriority::Medium) const override { return 10 * szabo; } +public: + TrivialGasPricer() = default; + TrivialGasPricer(u256 const& _ask, u256 const& _bid): m_ask(_ask), m_bid(_bid) {} + + u256 ask(State const&) const override { return m_ask; } + u256 bid(TransactionPriority = TransactionPriority::Medium) const override { return m_bid; } + +private: + u256 m_ask = 10 * szabo; + u256 m_bid = 10 * szabo; }; enum class Permanence From 9da464ee05c0914017394d735c7edb90b48e318a Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 4 Jun 2015 12:42:55 +0200 Subject: [PATCH 09/26] - style fixes - added test for uint8 = -1 which doesn't fail; todo: fix that --- libsolidity/AST.cpp | 15 +++++++-------- libsolidity/Types.cpp | 11 +++++------ .../libsolidity/SolidityNameAndTypeResolution.cpp | 10 ++++++++++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index a209235dc..c6aebd4f4 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -686,14 +686,13 @@ void Expression::expectType(Type const& _expectedType) checkTypeRequirements(nullptr); Type const& type = *getType(); if (!type.isImplicitlyConvertibleTo(_expectedType)) - BOOST_THROW_EXCEPTION( - createTypeError( - "Type " + - type.toString() + - " is not implicitly convertible to expected type " + - _expectedType.toString() + - "." - ) + BOOST_THROW_EXCEPTION(createTypeError( + "Type " + + type.toString() + + " is not implicitly convertible to expected type " + + _expectedType.toString() + + "." + ) ); } diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index c4238f773..c10e29eaa 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -361,22 +361,21 @@ IntegerConstantType::IntegerConstantType(Literal const& _literal) bool IntegerConstantType::isImplicitlyConvertibleTo(Type const& _convertTo) const { - if (IntegerType const* integerType = dynamic_cast(&_convertTo)) + if (auto targetType = dynamic_cast(&_convertTo)) { if (m_value == 0) return true; - int forSignBit = (integerType->isSigned() ? 1 : 0); + int forSignBit = (targetType->isSigned() ? 1 : 0); if (m_value > 0) { - if (m_value <= (u256(-1) >> (256 - integerType->getNumBits() + forSignBit))) + if (m_value <= (u256(-1) >> (256 - targetType->getNumBits() + forSignBit))) return true; } - else if (-m_value <= (u256(1) << (integerType->getNumBits() - forSignBit))) + else if (-m_value <= (u256(1) << (targetType->getNumBits() - forSignBit))) return true; return false; } - else - if (_convertTo.getCategory() == Category::FixedBytes) + else if (_convertTo.getCategory() == Category::FixedBytes) { FixedBytesType const& fixedBytes = dynamic_cast(_convertTo); return fixedBytes.getNumBytes() * 8 >= getIntegerType()->getNumBits(); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index a35b7bbe1..4f7b82ec7 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1856,6 +1856,16 @@ BOOST_AUTO_TEST_CASE(positive_integers_to_signed_out_of_bound_max) BOOST_CHECK_NO_THROW(parseTextAndResolveNames(sourceCode)); } +BOOST_AUTO_TEST_CASE(negative_integers_to_unsigned) +{ + char const* sourceCode = R"( + contract test { + uint8 public x = -1; + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() } From ee2b5e7cb42f6eeec238ec17d8037a1341c940b0 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 4 Jun 2015 14:09:19 +0200 Subject: [PATCH 10/26] fixed assigning negative number to unsigned --- libsolidity/Types.cpp | 2 +- test/libsolidity/SolidityNameAndTypeResolution.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index c10e29eaa..bf79be310 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -371,7 +371,7 @@ bool IntegerConstantType::isImplicitlyConvertibleTo(Type const& _convertTo) cons if (m_value <= (u256(-1) >> (256 - targetType->getNumBits() + forSignBit))) return true; } - else if (-m_value <= (u256(1) << (targetType->getNumBits() - forSignBit))) + else if (targetType->isSigned() && -m_value <= (u256(1) << (targetType->getNumBits() - forSignBit))) return true; return false; } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 4f7b82ec7..3691868cc 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1866,6 +1866,16 @@ BOOST_AUTO_TEST_CASE(negative_integers_to_unsigned) BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); } +BOOST_AUTO_TEST_CASE(positive_integers_to_unsigned_out_of_bound) +{ + char const* sourceCode = R"( + contract test { + uint8 public x = 700; + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() } From 8593be1b6a114e7cd64ad0db800d88fa2f4a6c9a Mon Sep 17 00:00:00 2001 From: subtly Date: Thu, 4 Jun 2015 14:53:27 +0200 Subject: [PATCH 11/26] Log warning and disconnect when remote passes bad rlp during handshake. --- libp2p/RLPxHandshake.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libp2p/RLPxHandshake.cpp b/libp2p/RLPxHandshake.cpp index 65116cd58..d7c2e5e3b 100644 --- a/libp2p/RLPxHandshake.cpp +++ b/libp2p/RLPxHandshake.cpp @@ -265,8 +265,17 @@ void RLPXHandshake::transition(boost::system::error_code _ech) } clog(NetTriviaSummary) << (m_originated ? "p2p.connect.egress" : "p2p.connect.ingress") << "hello frame: success. starting session."; - RLP rlp(frame.cropped(1), RLP::ThrowOnFail | RLP::FailIfTooSmall); - m_host->startPeerSession(m_remote, rlp, m_io, m_socket->remoteEndpoint()); + try + { + RLP rlp(frame.cropped(1), RLP::ThrowOnFail | RLP::FailIfTooSmall); + m_host->startPeerSession(m_remote, rlp, m_io, m_socket->remoteEndpoint()); + } + catch (std::exception const& _e) + { + clog(NetWarn) << "Handshake causing an exception:" << _e.what(); + m_nextState = Error; + transition(); + } } }); } From f26bbb21e491e0d1492570d87027c1edb05307e9 Mon Sep 17 00:00:00 2001 From: subtly Date: Thu, 4 Jun 2015 16:10:25 +0200 Subject: [PATCH 12/26] Drop socket and log when peer disconnect occurs mid-read. --- libp2p/Session.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index 2a16007ca..462fea7b1 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -447,8 +447,12 @@ void Session::doRead() clog(NetWarn) << "Error reading: " << ec.message(); drop(TCPError); } - else if (ec && length == 0) + else if (ec && length < tlen) + { + clog(NetWarn) << "Error reading - Abrupt peer disconnect: " << ec.message(); + drop(TCPError); return; + } else { if (!m_io->authAndDecryptFrame(bytesRef(m_data.data(), tlen))) From ce367d3da09c673dae759720cd4f9a942eb28cd1 Mon Sep 17 00:00:00 2001 From: subtly Date: Thu, 4 Jun 2015 17:27:35 +0200 Subject: [PATCH 13/26] update whisper tests --- test/libwhisper/whisperTopic.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/libwhisper/whisperTopic.cpp b/test/libwhisper/whisperTopic.cpp index 291b62c17..ba487a92e 100644 --- a/test/libwhisper/whisperTopic.cpp +++ b/test/libwhisper/whisperTopic.cpp @@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(topic) { for (auto i: whost1->checkWatch(w)) { - Message msg = whost1->envelope(i).open(whost1->fullTopic(w)); + Message msg = whost1->envelope(i).open(whost1->fullTopics(w)); last = RLP(msg.payload()).toInt(); if (received.count(last)) continue; @@ -112,7 +112,7 @@ BOOST_AUTO_TEST_CASE(forwarding) // Host must be configured not to share peers. Host host1("Listner", NetworkPreferences("127.0.0.1", 30303, false)); - host1.setIdealPeerCount(0); + host1.setIdealPeerCount(1); auto whost1 = host1.registerCapability(new WhisperHost()); host1.start(); while (!host1.haveNetwork()) @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(forwarding) { for (auto i: whost1->checkWatch(w)) { - Message msg = whost1->envelope(i).open(whost1->fullTopic(w)); + Message msg = whost1->envelope(i).open(whost1->fullTopics(w)); unsigned last = RLP(msg.payload()).toInt(); cnote << "New message from:" << msg.from() << RLP(msg.payload()).toInt(); result = last; @@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE(forwarding) { for (auto i: whost2->checkWatch(w)) { - Message msg = whost2->envelope(i).open(whost2->fullTopic(w)); + Message msg = whost2->envelope(i).open(whost2->fullTopics(w)); cnote << "New message from:" << msg.from() << RLP(msg.payload()).toInt(); } this_thread::sleep_for(chrono::milliseconds(50)); @@ -192,6 +192,9 @@ BOOST_AUTO_TEST_CASE(forwarding) while (!ph.haveNetwork()) this_thread::sleep_for(chrono::milliseconds(10)); + while (!ph.peerCount()) + this_thread::sleep_for(chrono::milliseconds(10)); + KeyPair us = KeyPair::create(); wh->post(us.sec(), RLPStream().append(1).out(), BuildTopic("test")); this_thread::sleep_for(chrono::milliseconds(250)); @@ -237,7 +240,7 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) { for (auto i: whost1->checkWatch(w)) { - Message msg = whost1->envelope(i).open(whost1->fullTopic(w)); + Message msg = whost1->envelope(i).open(whost1->fullTopics(w)); cnote << "New message from:" << msg.from() << RLP(msg.payload()).toInt(); } this_thread::sleep_for(chrono::milliseconds(50)); @@ -280,7 +283,7 @@ BOOST_AUTO_TEST_CASE(asyncforwarding) { for (auto i: wh->checkWatch(w)) { - Message msg = wh->envelope(i).open(wh->fullTopic(w)); + Message msg = wh->envelope(i).open(wh->fullTopics(w)); unsigned last = RLP(msg.payload()).toInt(); cnote << "New message from:" << msg.from() << RLP(msg.payload()).toInt(); result = last; From 3a5e67eaabf531a8b2ea36369a25e98d6a20da6b Mon Sep 17 00:00:00 2001 From: subtly Date: Thu, 4 Jun 2015 17:59:41 +0200 Subject: [PATCH 14/26] Fix network tests: delete host pointers after use. --- test/libp2p/peer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/libp2p/peer.cpp b/test/libp2p/peer.cpp index 192dacd7b..a5a4a64dc 100644 --- a/test/libp2p/peer.cpp +++ b/test/libp2p/peer.cpp @@ -116,6 +116,9 @@ BOOST_AUTO_TEST_CASE(saveNodes) BOOST_REQUIRE(i.itemCount() == 4 || i.itemCount() == 11); BOOST_REQUIRE(i[0].size() == 4 || i[0].size() == 16); } + + for (auto host: hosts) + delete host; } BOOST_AUTO_TEST_SUITE_END() From f604ab72ee1f0d69ee486757eedcb6a8854e7ef8 Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Thu, 4 Jun 2015 18:06:06 +0200 Subject: [PATCH 15/26] Update Types.cpp --- libsolidity/Types.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index bf79be310..33eafb150 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -376,12 +376,12 @@ bool IntegerConstantType::isImplicitlyConvertibleTo(Type const& _convertTo) cons return false; } else if (_convertTo.getCategory() == Category::FixedBytes) - { - FixedBytesType const& fixedBytes = dynamic_cast(_convertTo); - return fixedBytes.getNumBytes() * 8 >= getIntegerType()->getNumBits(); - } - else - return false; + { + FixedBytesType const& fixedBytes = dynamic_cast(_convertTo); + return fixedBytes.getNumBytes() * 8 >= getIntegerType()->getNumBits(); + } + else + return false; } bool IntegerConstantType::isExplicitlyConvertibleTo(Type const& _convertTo) const From c008fe856faeb81393cd2bf8e9663c604e19c9b5 Mon Sep 17 00:00:00 2001 From: arkpar Date: Thu, 4 Jun 2015 19:59:31 +0200 Subject: [PATCH 16/26] fixed iterating over random peers --- libethereum/EthereumHost.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libethereum/EthereumHost.cpp b/libethereum/EthereumHost.cpp index 68619d627..3965a214e 100644 --- a/libethereum/EthereumHost.cpp +++ b/libethereum/EthereumHost.cpp @@ -120,7 +120,8 @@ void EthereumHost::maintainTransactions() for (auto const& i: ts) { bool unsent = !m_transactionsSent.count(i.first); - for (auto const& p: get<1>(randomSelection(0, [&](EthereumPeer* p) { return p->m_requireTransactions || (unsent && !p->m_knownTransactions.count(i.first)); }))) + auto peers = get<1>(randomSelection(0, [&](EthereumPeer* p) { return p->m_requireTransactions || (unsent && !p->m_knownTransactions.count(i.first)); })); + for (auto const& p: peers) peerTransactions[p].push_back(i.first); } for (auto const& t: ts) From 51b90324c0768924a1b3b32bf90cb3a3de983553 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 5 Jun 2015 11:17:21 +0900 Subject: [PATCH 17/26] Allow configurable gas price in AZ & eth. --- alethzero/CMakeLists.txt | 3 +- alethzero/GasPricing.ui | 181 +++++++++++++++++++++++++++++++++++++++ alethzero/Main.ui | 12 +++ alethzero/MainWin.cpp | 37 ++++++++ alethzero/MainWin.h | 3 + ethminer/farm.json | 2 + libethereum/Client.h | 1 + libethereum/State.h | 4 + 8 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 alethzero/GasPricing.ui diff --git a/alethzero/CMakeLists.txt b/alethzero/CMakeLists.txt index 595668cd1..fe1f2f82f 100644 --- a/alethzero/CMakeLists.txt +++ b/alethzero/CMakeLists.txt @@ -24,6 +24,7 @@ qt5_wrap_ui(ui_Debugger.h Debugger.ui) qt5_wrap_ui(ui_Transact.h Transact.ui) qt5_wrap_ui(ui_ExportState.h ExportState.ui) qt5_wrap_ui(ui_GetPassword.h GetPassword.ui) +qt5_wrap_ui(ui_GasPricing.h GasPricing.ui) file(GLOB HEADERS "*.h") @@ -36,7 +37,7 @@ endif () # eth_add_executable is defined in cmake/EthExecutableHelper.cmake eth_add_executable(${EXECUTABLE} ICON alethzero - UI_RESOURCES alethzero.icns Main.ui Connect.ui Debugger.ui Transact.ui ExportState.ui GetPassword.ui + UI_RESOURCES alethzero.icns Main.ui Connect.ui Debugger.ui Transact.ui ExportState.ui GetPassword.ui GasPricing.ui WIN_RESOURCES alethzero.rc ) diff --git a/alethzero/GasPricing.ui b/alethzero/GasPricing.ui new file mode 100644 index 000000000..39a3b1661 --- /dev/null +++ b/alethzero/GasPricing.ui @@ -0,0 +1,181 @@ + + + GasPricing + + + + 0 + 0 + 416 + 286 + + + + Gas Pricing + + + + + + Qt::Vertical + + + + 398 + 45 + + + + + + + + &Bid: The minimum grice of gas that is accepting into a block which we mine. + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop + + + true + + + bidValue + + + + + + + + + + 430000000 + + + 0 + + + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + Close + + + true + + + + + + + + + &Ask: The minimum grice of gas that is accepting into a block which we mine. + + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop + + + true + + + askValue + + + + + + + + + + + + + 430000000 + + + 0 + + + + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + + + close + clicked() + GasPricing + accept() + + + 387 + 264 + + + 240 + 262 + + + + + diff --git a/alethzero/Main.ui b/alethzero/Main.ui index ecdc07ab6..efb6256af 100644 --- a/alethzero/Main.ui +++ b/alethzero/Main.ui @@ -212,12 +212,19 @@ + + + &Config + + + + @@ -1763,6 +1770,11 @@ font-size: 14pt Re-Encrypt All Keys... + + + &Gas Prices... + + diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 59698bac3..327392df1 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -76,6 +76,7 @@ #include "ExportState.h" #include "ui_Main.h" #include "ui_GetPassword.h" +#include "ui_GasPricing.h" using namespace std; using namespace dev; using namespace dev::p2p; @@ -262,6 +263,42 @@ bool Main::confirm() const return ui->natSpec->isChecked(); } +void Main::on_gasPrices_triggered() +{ + QDialog d; + Ui_GasPricing gp; + gp.setupUi(&d); + d.setWindowTitle("Gas Pricing"); + + initUnits(gp.bidUnits); + u256 bid = static_cast(ethereum()->gasPricer().get())->bid(); + gp.bidUnits->setCurrentIndex(0); + while (bid > 50000 && gp.bidUnits->currentIndex() < (int)(units().size() - 2)) + { + bid /= 1000; + gp.bidUnits->setCurrentIndex(gp.bidUnits->currentIndex() + 1); + } + gp.bidValue->setValue((unsigned)bid); + + // TODO: refactor into a single function. + initUnits(gp.askUnits); + u256 ask = static_cast(ethereum()->gasPricer().get())->ask(); + gp.askUnits->setCurrentIndex(0); + while (ask > 50000 && gp.askUnits->currentIndex() < (int)(units().size() - 2)) + { + ask /= 1000; + gp.askUnits->setCurrentIndex(gp.askUnits->currentIndex() + 1); + } + gp.askValue->setValue((unsigned)ask); + + if (d.exec() == QDialog::Accepted) + { + // TODO: refactor into a single function. + static_cast(ethereum()->gasPricer().get())->setBid(gp.bidValue->value() * units()[units().size() - 1 - gp.bidUnits->currentIndex()].first); + static_cast(ethereum()->gasPricer().get())->setAsk(gp.askValue->value() * units()[units().size() - 1 - gp.askUnits->currentIndex()].first); + } +} + void Main::on_newIdentity_triggered() { KeyPair kp = KeyPair::create(); diff --git a/alethzero/MainWin.h b/alethzero/MainWin.h index 29cd0dbf3..bffab225c 100644 --- a/alethzero/MainWin.h +++ b/alethzero/MainWin.h @@ -194,6 +194,9 @@ private slots: void on_newIdentity_triggered(); void on_post_clicked(); + // Config + void on_gasPrices_triggered(); + void refreshWhisper(); void refreshBlockChain(); void addNewId(QString _ids); diff --git a/ethminer/farm.json b/ethminer/farm.json index 1f4142d00..ff0a2e9e7 100644 --- a/ethminer/farm.json +++ b/ethminer/farm.json @@ -1,4 +1,6 @@ [ { "name": "eth_getWork", "params": [], "order": [], "returns": []}, { "name": "eth_submitWork", "params": ["", "", ""], "order": [], "returns": true} + { "name": "eth_awaitNewWork", "params": [], "order": [], "returns": []}, + { "name": "eth_progress", "params": [], "order": [], "returns": true} ] diff --git a/libethereum/Client.h b/libethereum/Client.h index c77cb6034..5132b5a30 100644 --- a/libethereum/Client.h +++ b/libethereum/Client.h @@ -133,6 +133,7 @@ public: /// Resets the gas pricer to some other object. void setGasPricer(std::shared_ptr _gp) { m_gp = _gp; } + std::shared_ptr gasPricer() const { return m_gp; } /// Blocks until all pending transactions have been processed. virtual void flushTransactions() override; diff --git a/libethereum/State.h b/libethereum/State.h index ea54d153d..55d5cfb4c 100644 --- a/libethereum/State.h +++ b/libethereum/State.h @@ -88,6 +88,10 @@ public: TrivialGasPricer() = default; TrivialGasPricer(u256 const& _ask, u256 const& _bid): m_ask(_ask), m_bid(_bid) {} + void setAsk(u256 const& _ask) { m_ask = _ask; } + void setBid(u256 const& _bid) { m_bid = _bid; } + + u256 ask() const { return m_ask; } u256 ask(State const&) const override { return m_ask; } u256 bid(TransactionPriority = TransactionPriority::Medium) const override { return m_bid; } From 0bd1b56c914b6ed80201cd1c12e42c154680bd3c Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 5 Jun 2015 11:53:41 +0900 Subject: [PATCH 18/26] Save/load gas pricing. Full integration. --- alethzero/Context.cpp | 18 +++++++++++++++ alethzero/Context.h | 4 ++++ alethzero/MainWin.cpp | 52 +++++++++++++++++++----------------------- alethzero/MainWin.h | 4 ++-- alethzero/Transact.cpp | 13 +++++++---- alethzero/Transact.h | 1 + alethzero/Transact.ui | 2 +- 7 files changed, 57 insertions(+), 37 deletions(-) diff --git a/alethzero/Context.cpp b/alethzero/Context.cpp index 7a5a986d1..9c64362dc 100644 --- a/alethzero/Context.cpp +++ b/alethzero/Context.cpp @@ -21,6 +21,7 @@ #include "Context.h" #include +#include #include using namespace std; using namespace dev; @@ -34,6 +35,23 @@ Context::~Context() { } +void setValueUnits(QComboBox* _units, QSpinBox* _value, u256 _v) +{ + initUnits(_units); + _units->setCurrentIndex(0); + while (_v > 50000 && _units->currentIndex() < (int)(units().size() - 2)) + { + _v /= 1000; + _units->setCurrentIndex(_units->currentIndex() + 1); + } + _value->setValue((unsigned)_v); +} + +u256 fromValueUnits(QComboBox* _units, QSpinBox* _value) +{ + return _value->value() * units()[units().size() - 1 - _units->currentIndex()].first; +} + void initUnits(QComboBox* _b) { for (auto n = (unsigned)units().size(); n-- != 0; ) diff --git a/alethzero/Context.h b/alethzero/Context.h index 4a52366db..a49d2be12 100644 --- a/alethzero/Context.h +++ b/alethzero/Context.h @@ -28,6 +28,7 @@ #include class QComboBox; +class QSpinBox; namespace dev { namespace eth { struct StateDiff; class KeyManager; } } @@ -37,6 +38,8 @@ namespace dev { namespace eth { struct StateDiff; class KeyManager; } } #define Span(S) "" void initUnits(QComboBox* _b); +void setValueUnits(QComboBox* _units, QSpinBox* _value, dev::u256 _v); +dev::u256 fromValueUnits(QComboBox* _units, QSpinBox* _value); std::vector keysAsVector(QList const& _keys); @@ -67,5 +70,6 @@ public: virtual dev::Secret retrieveSecret(dev::Address const& _a) const = 0; virtual dev::eth::KeyManager& keyManager() = 0; + virtual dev::u256 gasPrice() const = 0; }; diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 327392df1..10038a80e 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -129,7 +129,7 @@ static QString filterOutTerminal(QString _s) Main::Main(QWidget *parent) : QMainWindow(parent), ui(new Ui::Main), - m_transact(this, this), + m_transact(nullptr), m_dappLoader(nullptr), m_webPage(nullptr) { @@ -233,6 +233,11 @@ Main::Main(QWidget *parent) : // inspector->setPage(page); setBeneficiary(*m_keyManager.accounts().begin()); readSettings(); + + m_transact = new Transact(this, this); + m_transact->setWindowFlags(Qt::Dialog); + m_transact->setWindowModality(Qt::WindowModal); + #if !ETH_FATDB removeDockWidget(ui->dockWidget_accounts); #endif @@ -269,33 +274,14 @@ void Main::on_gasPrices_triggered() Ui_GasPricing gp; gp.setupUi(&d); d.setWindowTitle("Gas Pricing"); - - initUnits(gp.bidUnits); - u256 bid = static_cast(ethereum()->gasPricer().get())->bid(); - gp.bidUnits->setCurrentIndex(0); - while (bid > 50000 && gp.bidUnits->currentIndex() < (int)(units().size() - 2)) - { - bid /= 1000; - gp.bidUnits->setCurrentIndex(gp.bidUnits->currentIndex() + 1); - } - gp.bidValue->setValue((unsigned)bid); - - // TODO: refactor into a single function. - initUnits(gp.askUnits); - u256 ask = static_cast(ethereum()->gasPricer().get())->ask(); - gp.askUnits->setCurrentIndex(0); - while (ask > 50000 && gp.askUnits->currentIndex() < (int)(units().size() - 2)) - { - ask /= 1000; - gp.askUnits->setCurrentIndex(gp.askUnits->currentIndex() + 1); - } - gp.askValue->setValue((unsigned)ask); + setValueUnits(gp.bidUnits, gp.bidValue, static_cast(ethereum()->gasPricer().get())->bid()); + setValueUnits(gp.askUnits, gp.askValue, static_cast(ethereum()->gasPricer().get())->ask()); if (d.exec() == QDialog::Accepted) { - // TODO: refactor into a single function. - static_cast(ethereum()->gasPricer().get())->setBid(gp.bidValue->value() * units()[units().size() - 1 - gp.bidUnits->currentIndex()].first); - static_cast(ethereum()->gasPricer().get())->setAsk(gp.askValue->value() * units()[units().size() - 1 - gp.askUnits->currentIndex()].first); + static_cast(ethereum()->gasPricer().get())->setBid(fromValueUnits(gp.bidUnits, gp.bidValue)); + static_cast(ethereum()->gasPricer().get())->setAsk(fromValueUnits(gp.askUnits, gp.askValue)); + m_transact->resetGasPrice(); } } @@ -504,10 +490,8 @@ void Main::load(QString _s) void Main::on_newTransaction_triggered() { - m_transact.setEnvironment(m_keyManager.accounts(), ethereum(), &m_natSpecDB); - m_transact.setWindowFlags(Qt::Dialog); - m_transact.setWindowModality(Qt::WindowModal); - m_transact.show(); + m_transact->setEnvironment(m_keyManager.accounts(), ethereum(), &m_natSpecDB); + m_transact->show(); } void Main::on_loadJS_triggered() @@ -690,6 +674,11 @@ void Main::on_paranoia_triggered() ethereum()->setParanoia(ui->paranoia->isChecked()); } +dev::u256 Main::gasPrice() const +{ + return ethereum()->gasPricer()->bid(); +} + void Main::writeSettings() { QSettings s("ethereum", "alethzero"); @@ -706,6 +695,8 @@ void Main::writeSettings() s.setValue("identities", b); } + s.setValue("askPrice", QString::fromStdString(toString(static_cast(ethereum()->gasPricer().get())->ask()))); + s.setValue("bidPrice", QString::fromStdString(toString(static_cast(ethereum()->gasPricer().get())->bid()))); s.setValue("upnp", ui->upnp->isChecked()); s.setValue("forceAddress", ui->forcePublicIP->text()); s.setValue("forceMining", ui->forceMining->isChecked()); @@ -789,6 +780,9 @@ void Main::readSettings(bool _skipGeometry) } } + static_cast(ethereum()->gasPricer().get())->setAsk(u256(s.value("askPrice", "500000000000").toString().toStdString())); + static_cast(ethereum()->gasPricer().get())->setBid(u256(s.value("bidPrice", "500000000000").toString().toStdString())); + ui->upnp->setChecked(s.value("upnp", true).toBool()); ui->forcePublicIP->setText(s.value("forceAddress", "").toString()); ui->dropPeers->setChecked(false); diff --git a/alethzero/MainWin.h b/alethzero/MainWin.h index bffab225c..193f8e364 100644 --- a/alethzero/MainWin.h +++ b/alethzero/MainWin.h @@ -91,7 +91,7 @@ public: QList owned() const { return m_myIdentities; } - dev::u256 gasPrice() const { return 10 * dev::eth::szabo; } + dev::u256 gasPrice() const override; dev::eth::KeyManager& keyManager() override { return m_keyManager; } bool doConfirm(); @@ -282,7 +282,7 @@ private: static std::string fromRaw(dev::h256 _n, unsigned* _inc = nullptr); NatspecHandler m_natSpecDB; - Transact m_transact; + Transact* m_transact; std::unique_ptr m_dappHost; DappLoader* m_dappLoader; QWebEnginePage* m_webPage; diff --git a/alethzero/Transact.cpp b/alethzero/Transact.cpp index b485091d9..e61092f33 100644 --- a/alethzero/Transact.cpp +++ b/alethzero/Transact.cpp @@ -58,11 +58,9 @@ Transact::Transact(Context* _c, QWidget* _parent): { ui->setupUi(this); - initUnits(ui->gasPriceUnits); - initUnits(ui->valueUnits); - ui->valueUnits->setCurrentIndex(6); - ui->gasPriceUnits->setCurrentIndex(4); - ui->gasPrice->setValue(10); + resetGasPrice(); + setValueUnits(ui->valueUnits, ui->value, 0); + on_destination_currentTextChanged(QString()); } @@ -92,6 +90,11 @@ void Transact::setEnvironment(AddressHash const& _accounts, dev::eth::Client* _e ui->from->setCurrentIndex(0); } +void Transact::resetGasPrice() +{ + setValueUnits(ui->gasPriceUnits, ui->gasPrice, m_context->gasPrice()); +} + bool Transact::isCreation() const { return ui->destination->currentText().isEmpty() || ui->destination->currentText() == "(Create Contract)"; diff --git a/alethzero/Transact.h b/alethzero/Transact.h index c14fcc7e1..b8b5134a4 100644 --- a/alethzero/Transact.h +++ b/alethzero/Transact.h @@ -41,6 +41,7 @@ public: explicit Transact(Context* _context, QWidget* _parent = 0); ~Transact(); + void resetGasPrice(); void setEnvironment(dev::AddressHash const& _accounts, dev::eth::Client* _eth, NatSpecFace* _natSpecDB); private slots: diff --git a/alethzero/Transact.ui b/alethzero/Transact.ui index f7a5e7c0e..87a8ebd99 100644 --- a/alethzero/Transact.ui +++ b/alethzero/Transact.ui @@ -159,7 +159,7 @@ @ - 1 + 0 430000000 From ac79514e63426e8196fcdc8ddbb3a44622d0cb35 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 11:07:50 +0200 Subject: [PATCH 19/26] Ability to specify the storage location of a reference type. --- libsolidity/AST.cpp | 19 +++- libsolidity/AST.h | 17 +++- libsolidity/ArrayUtils.cpp | 50 +++++----- libsolidity/CompilerUtils.cpp | 4 +- libsolidity/ExpressionCompiler.cpp | 16 ++-- libsolidity/NameAndTypeResolver.cpp | 47 +++++++++- libsolidity/Parser.cpp | 94 ++++++++++++++----- libsolidity/Parser.h | 9 +- libsolidity/Token.h | 3 + libsolidity/Types.cpp | 25 +++-- libsolidity/Types.h | 41 +++++--- .../SolidityNameAndTypeResolution.cpp | 34 +++++++ test/libsolidity/SolidityParser.cpp | 41 ++++++++ test/libsolidity/SolidityTypes.cpp | 14 +-- 14 files changed, 311 insertions(+), 103 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index c6aebd4f4..4c7168afa 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -528,6 +528,17 @@ void VariableDeclaration::checkTypeRequirements() BOOST_THROW_EXCEPTION(createTypeError("Internal type is not allowed for public state variables.")); } +bool VariableDeclaration::isFunctionParameter() const +{ + auto const* function = dynamic_cast(getScope()); + if (!function) + return false; + for (auto const& variable: function->getParameters() + function->getReturnParameters()) + if (variable.get() == this) + return true; + return false; +} + bool VariableDeclaration::isExternalFunctionParameter() const { auto const* function = dynamic_cast(getScope()); @@ -879,7 +890,7 @@ void MemberAccess::checkTypeRequirements(TypePointers const* _argumentTypes) { auto const& arrayType(dynamic_cast(type)); m_isLValue = (*m_memberName == "length" && - arrayType.getLocation() != ArrayType::Location::CallData && arrayType.isDynamicallySized()); + arrayType.location() != ReferenceType::Location::CallData && arrayType.isDynamicallySized()); } else m_isLValue = false; @@ -902,7 +913,7 @@ void IndexAccess::checkTypeRequirements(TypePointers const*) m_type = make_shared(1); else m_type = type.getBaseType(); - m_isLValue = type.getLocation() != ArrayType::Location::CallData; + m_isLValue = type.location() != ReferenceType::Location::CallData; break; } case Type::Category::Mapping: @@ -919,7 +930,7 @@ void IndexAccess::checkTypeRequirements(TypePointers const*) { TypeType const& type = dynamic_cast(*m_base->getType()); if (!m_index) - m_type = make_shared(make_shared(ArrayType::Location::Memory, type.getActualType())); + m_type = make_shared(make_shared(ReferenceType::Location::Memory, type.getActualType())); else { m_index->checkTypeRequirements(nullptr); @@ -927,7 +938,7 @@ void IndexAccess::checkTypeRequirements(TypePointers const*) if (!length) BOOST_THROW_EXCEPTION(m_index->createTypeError("Integer constant expected.")); m_type = make_shared(make_shared( - ArrayType::Location::Memory, type.getActualType(), length->literalValue(nullptr))); + ReferenceType::Location::Memory, type.getActualType(), length->literalValue(nullptr))); } break; } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index be5c9a6cd..d75347127 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -474,22 +474,26 @@ private: class VariableDeclaration: public Declaration { public: + enum Location { Default, Storage, Memory }; + VariableDeclaration( - SourceLocation const& _location, + SourceLocation const& _sourceLocation, ASTPointer const& _type, ASTPointer const& _name, ASTPointer _value, Visibility _visibility, bool _isStateVar = false, bool _isIndexed = false, - bool _isConstant = false + bool _isConstant = false, + Location _referenceLocation = Location::Default ): - Declaration(_location, _name, _visibility), + Declaration(_sourceLocation, _name, _visibility), m_typeName(_type), m_value(_value), m_isStateVariable(_isStateVar), m_isIndexed(_isIndexed), - m_isConstant(_isConstant){} + m_isConstant(_isConstant), + m_location(_referenceLocation) {} virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTConstVisitor& _visitor) const override; @@ -507,10 +511,14 @@ public: void checkTypeRequirements(); bool isLocalVariable() const { return !!dynamic_cast(getScope()); } + /// @returns true if this variable is a parameter or return parameter of a function. + bool isFunctionParameter() const; + /// @returns true if this variable is a parameter (not return parameter) of an external function. bool isExternalFunctionParameter() const; bool isStateVariable() const { return m_isStateVariable; } bool isIndexed() const { return m_isIndexed; } bool isConstant() const { return m_isConstant; } + Location referenceLocation() const { return m_location; } protected: Visibility getDefaultVisibility() const override { return Visibility::Internal; } @@ -521,6 +529,7 @@ private: bool m_isStateVariable; ///< Whether or not this is a contract state variable bool m_isIndexed; ///< Whether this is an indexed variable (used by events). bool m_isConstant; ///< Whether the variable is a compile-time constant. + Location m_location; ///< Location of the variable if it is of reference type. std::shared_ptr m_type; ///< derived type, initially empty }; diff --git a/libsolidity/ArrayUtils.cpp b/libsolidity/ArrayUtils.cpp index 79ea49535..531ab8af1 100644 --- a/libsolidity/ArrayUtils.cpp +++ b/libsolidity/ArrayUtils.cpp @@ -38,10 +38,10 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // need to leave "target_ref target_byte_off" on the stack at the end // stack layout: [source_ref] [source_byte_off] [source length] target_ref target_byte_off (top) - solAssert(_targetType.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_targetType.location() == ReferenceType::Location::Storage, ""); solAssert( - _sourceType.getLocation() == ArrayType::Location::CallData || - _sourceType.getLocation() == ArrayType::Location::Storage, + _sourceType.location() == ReferenceType::Location::CallData || + _sourceType.location() == ReferenceType::Location::Storage, "Given array location not implemented." ); @@ -51,7 +51,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // TODO unroll loop for small sizes - bool sourceIsStorage = _sourceType.getLocation() == ArrayType::Location::Storage; + bool sourceIsStorage = _sourceType.location() == ReferenceType::Location::Storage; bool directCopy = sourceIsStorage && sourceBaseType->isValueType() && *sourceBaseType == *targetBaseType; bool haveByteOffsetSource = !directCopy && sourceIsStorage && sourceBaseType->getStorageBytes() <= 16; bool haveByteOffsetTarget = !directCopy && targetBaseType->getStorageBytes() <= 16; @@ -69,7 +69,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons m_context << eth::Instruction::POP; // stack: target_ref source_ref [source_length] // retrieve source length - if (_sourceType.getLocation() != ArrayType::Location::CallData || !_sourceType.isDynamicallySized()) + if (_sourceType.location() != ReferenceType::Location::CallData || !_sourceType.isDynamicallySized()) retrieveLength(_sourceType); // otherwise, length is already there // stack: target_ref source_ref source_length m_context << eth::Instruction::DUP3; @@ -82,7 +82,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons if (sourceBaseType->getCategory() == Type::Category::Mapping) { solAssert(targetBaseType->getCategory() == Type::Category::Mapping, ""); - solAssert(_sourceType.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_sourceType.location() == ReferenceType::Location::Storage, ""); // nothing to copy m_context << eth::Instruction::POP << eth::Instruction::POP @@ -106,7 +106,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons eth::AssemblyItem copyLoopEndWithoutByteOffset = m_context.newTag(); m_context.appendConditionalJumpTo(copyLoopEndWithoutByteOffset); - if (_sourceType.getLocation() == ArrayType::Location::Storage && _sourceType.isDynamicallySized()) + if (_sourceType.location() == ReferenceType::Location::Storage && _sourceType.isDynamicallySized()) CompilerUtils(m_context).computeHashStatic(); // stack: target_ref target_data_end source_length target_data_pos source_data_pos m_context << eth::Instruction::SWAP2; @@ -155,7 +155,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons // checking is easier. // stack: target_ref target_data_end source_data_pos target_data_pos source_data_end [target_byte_offset] [source_byte_offset] m_context << eth::dupInstruction(3 + byteOffsetSize); - if (_sourceType.getLocation() == ArrayType::Location::Storage) + if (_sourceType.location() == ReferenceType::Location::Storage) { if (haveByteOffsetSource) m_context << eth::Instruction::DUP2; @@ -228,7 +228,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons void ArrayUtils::clearArray(ArrayType const& _type) const { unsigned stackHeightStart = m_context.getStackHeight(); - solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_type.location() == ReferenceType::Location::Storage, ""); if (_type.getBaseType()->getStorageBytes() < 32) { solAssert(_type.getBaseType()->isValueType(), "Invalid storage size for non-value type."); @@ -283,7 +283,7 @@ void ArrayUtils::clearArray(ArrayType const& _type) const void ArrayUtils::clearDynamicArray(ArrayType const& _type) const { - solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_type.location() == ReferenceType::Location::Storage, ""); solAssert(_type.isDynamicallySized(), ""); unsigned stackHeightStart = m_context.getStackHeight(); @@ -311,7 +311,7 @@ void ArrayUtils::clearDynamicArray(ArrayType const& _type) const void ArrayUtils::resizeDynamicArray(const ArrayType& _type) const { - solAssert(_type.getLocation() == ArrayType::Location::Storage, ""); + solAssert(_type.location() == ReferenceType::Location::Storage, ""); solAssert(_type.isDynamicallySized(), ""); if (!_type.isByteArray() && _type.getBaseType()->getStorageBytes() < 32) solAssert(_type.getBaseType()->isValueType(), "Invalid storage size for non-value type."); @@ -396,7 +396,7 @@ void ArrayUtils::clearStorageLoop(Type const& _type) const void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) const { - if (_arrayType.getLocation() == ArrayType::Location::Storage) + if (_arrayType.location() == ReferenceType::Location::Storage) { if (_arrayType.getBaseType()->getStorageSize() <= 1) { @@ -432,15 +432,15 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType) const else { m_context << eth::Instruction::DUP1; - switch (_arrayType.getLocation()) + switch (_arrayType.location()) { - case ArrayType::Location::CallData: + case ReferenceType::Location::CallData: // length is stored on the stack break; - case ArrayType::Location::Memory: + case ReferenceType::Location::Memory: m_context << eth::Instruction::MLOAD; break; - case ArrayType::Location::Storage: + case ReferenceType::Location::Storage: m_context << eth::Instruction::SLOAD; break; } @@ -449,16 +449,16 @@ void ArrayUtils::retrieveLength(ArrayType const& _arrayType) const void ArrayUtils::accessIndex(ArrayType const& _arrayType) const { - ArrayType::Location location = _arrayType.getLocation(); + ReferenceType::Location location = _arrayType.location(); eth::Instruction load = - location == ArrayType::Location::Storage ? eth::Instruction::SLOAD : - location == ArrayType::Location::Memory ? eth::Instruction::MLOAD : + location == ReferenceType::Location::Storage ? eth::Instruction::SLOAD : + location == ReferenceType::Location::Memory ? eth::Instruction::MLOAD : eth::Instruction::CALLDATALOAD; // retrieve length if (!_arrayType.isDynamicallySized()) m_context << _arrayType.getLength(); - else if (location == ArrayType::Location::CallData) + else if (location == ReferenceType::Location::CallData) // length is stored on the stack m_context << eth::Instruction::SWAP1; else @@ -473,15 +473,15 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const m_context << eth::Instruction::SWAP1; if (_arrayType.isDynamicallySized()) { - if (location == ArrayType::Location::Storage) + if (location == ReferenceType::Location::Storage) CompilerUtils(m_context).computeHashStatic(); - else if (location == ArrayType::Location::Memory) + else if (location == ReferenceType::Location::Memory) m_context << u256(32) << eth::Instruction::ADD; } // stack: switch (location) { - case ArrayType::Location::CallData: + case ReferenceType::Location::CallData: if (!_arrayType.isByteArray()) m_context << eth::Instruction::SWAP1 @@ -496,7 +496,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const false ); break; - case ArrayType::Location::Storage: + case ReferenceType::Location::Storage: m_context << eth::Instruction::SWAP1; if (_arrayType.getBaseType()->getStorageBytes() <= 16) { @@ -524,7 +524,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType) const m_context << eth::Instruction::ADD << u256(0); } break; - case ArrayType::Location::Memory: + case ReferenceType::Location::Memory: solAssert(false, "Memory lvalues not yet implemented."); } } diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index 07bc3cdab..e3d8f7f90 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -81,7 +81,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound auto const& type = dynamic_cast(_type); solAssert(type.isByteArray(), "Non byte arrays not yet implemented here."); - if (type.getLocation() == ArrayType::Location::CallData) + if (type.location() == ReferenceType::Location::CallData) { // stack: target source_offset source_len m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 << eth::Instruction::DUP5 @@ -92,7 +92,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound } else { - solAssert(type.getLocation() == ArrayType::Location::Storage, "Memory arrays not yet implemented."); + solAssert(type.location() == ReferenceType::Location::Storage, "Memory arrays not yet implemented."); m_context << eth::Instruction::POP; // remove offset, arrays always start new slot m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; // stack here: memory_offset storage_offset length_bytes diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 51bdfbc43..62df9205f 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -770,12 +770,12 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) m_context << type.getLength(); } else - switch (type.getLocation()) + switch (type.location()) { - case ArrayType::Location::CallData: + case ReferenceType::Location::CallData: m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; break; - case ArrayType::Location::Storage: + case ReferenceType::Location::Storage: setLValue(_memberAccess, type); break; default: @@ -816,13 +816,13 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) solAssert(_indexAccess.getIndexExpression(), "Index expression expected."); // remove storage byte offset - if (arrayType.getLocation() == ArrayType::Location::Storage) + if (arrayType.location() == ReferenceType::Location::Storage) m_context << eth::Instruction::POP; _indexAccess.getIndexExpression()->accept(*this); // stack layout: [] ArrayUtils(m_context).accessIndex(arrayType); - if (arrayType.getLocation() == ArrayType::Location::Storage) + if (arrayType.location() == ReferenceType::Location::Storage) { if (arrayType.isByteArray()) { @@ -1169,13 +1169,13 @@ void ExpressionCompiler::appendArgumentsCopyToMemory( auto const& arrayType = dynamic_cast(*_arguments[i]->getType()); // move memory reference to top of stack CompilerUtils(m_context).moveToStackTop(arrayType.getSizeOnStack()); - if (arrayType.getLocation() == ArrayType::Location::CallData) + if (arrayType.location() == ReferenceType::Location::CallData) m_context << eth::Instruction::DUP2; // length is on stack - else if (arrayType.getLocation() == ArrayType::Location::Storage) + else if (arrayType.location() == ReferenceType::Location::Storage) m_context << eth::Instruction::DUP3 << eth::Instruction::SLOAD; else { - solAssert(arrayType.getLocation() == ArrayType::Location::Memory, ""); + solAssert(arrayType.location() == ReferenceType::Location::Memory, ""); m_context << eth::Instruction::DUP2 << eth::Instruction::MLOAD; } appendTypeMoveToMemory(IntegerType(256), true); diff --git a/libsolidity/NameAndTypeResolver.cpp b/libsolidity/NameAndTypeResolver.cpp index 5ef14f60b..22232014f 100644 --- a/libsolidity/NameAndTypeResolver.cpp +++ b/libsolidity/NameAndTypeResolver.cpp @@ -424,10 +424,49 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable) if (_variable.getTypeName()) { TypePointer type = _variable.getTypeName()->toType(); - // All array parameter types should point to call data - if (_variable.isExternalFunctionParameter()) - if (auto const* arrayType = dynamic_cast(type.get())) - type = arrayType->copyForLocation(ArrayType::Location::CallData); + using Location = VariableDeclaration::Location; + Location loc = _variable.referenceLocation(); + // References are forced to calldata for external function parameters (not return) + // and memory for parameters (also return) of publicly visible functions. + // They default to memory for function parameters and storage for local variables. + if (auto ref = dynamic_cast(type.get())) + { + if (_variable.isExternalFunctionParameter()) + { + // force location of external function parameters (not return) to calldata + if (loc != Location::Default) + BOOST_THROW_EXCEPTION(_variable.createTypeError( + "Location has to be calldata for external functions " + "(remove the \"memory\" or \"storage\" keyword)." + )); + type = ref->copyForLocation(ReferenceType::Location::CallData); + } + else if (_variable.isFunctionParameter() && _variable.getScope()->isPublic()) + { + // force locations of public or external function (return) parameters to memory + if (loc == VariableDeclaration::Location::Storage) + BOOST_THROW_EXCEPTION(_variable.createTypeError( + "Location has to be memory for publicly visible functions " + "(remove the \"storage\" keyword)." + )); + type = ref->copyForLocation(ReferenceType::Location::Memory); + } + else + { + if (loc == Location::Default) + loc = _variable.isFunctionParameter() ? Location::Memory : Location::Storage; + type = ref->copyForLocation( + loc == Location::Memory ? + ReferenceType::Location::Memory : + ReferenceType::Location::Storage + ); + } + } + else if (loc != Location::Default && !ref) + BOOST_THROW_EXCEPTION(_variable.createTypeError( + "Storage location can only be given for array or struct types." + )); + _variable.setType(type); if (!_variable.getType()) diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index 37c358d3b..851f1669e 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -224,7 +224,9 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* name = make_shared(); // anonymous function else name = expectIdentifierToken(); - ASTPointer parameters(parseParameterList()); + VarDeclParserOptions options; + options.allowLocationSpecifier = true; + ASTPointer parameters(parseParameterList(options)); bool isDeclaredConst = false; Declaration::Visibility visibility(Declaration::Visibility::Default); vector> modifiers; @@ -252,7 +254,7 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* { bool const permitEmptyParameterList = false; m_scanner->next(); - returnParameters = parseParameterList(permitEmptyParameterList); + returnParameters = parseParameterList(options, permitEmptyParameterList); } else returnParameters = createEmptyParameterList(); @@ -319,7 +321,9 @@ ASTPointer Parser::parseEnumDefinition() } ASTPointer Parser::parseVariableDeclaration( - VarDeclParserOptions const& _options, ASTPointer const& _lookAheadArrayType) + VarDeclParserOptions const& _options, + ASTPointer const& _lookAheadArrayType +) { ASTNodeFactory nodeFactory = _lookAheadArrayType ? ASTNodeFactory(*this, _lookAheadArrayType) : ASTNodeFactory(*this); @@ -334,20 +338,41 @@ ASTPointer Parser::parseVariableDeclaration( } bool isIndexed = false; bool isDeclaredConst = false; - ASTPointer identifier; - Token::Value token = m_scanner->getCurrentToken(); Declaration::Visibility visibility(Declaration::Visibility::Default); - if (_options.isStateVariable && Token::isVariableVisibilitySpecifier(token)) - visibility = parseVisibilitySpecifier(token); - if (_options.allowIndexed && token == Token::Indexed) - { - isIndexed = true; - m_scanner->next(); - } - if (token == Token::Const) + VariableDeclaration::Location location = VariableDeclaration::Location::Default; + ASTPointer identifier; + + while (true) { - isDeclaredConst = true; - m_scanner->next(); + Token::Value token = m_scanner->getCurrentToken(); + if (_options.isStateVariable && Token::isVariableVisibilitySpecifier(token)) + { + if (visibility != Declaration::Visibility::Default) + BOOST_THROW_EXCEPTION(createParserError("Visibility already specified.")); + visibility = parseVisibilitySpecifier(token); + } + else + { + if (_options.allowIndexed && token == Token::Indexed) + isIndexed = true; + else if (token == Token::Const) + isDeclaredConst = true; + else if (_options.allowLocationSpecifier && Token::isLocationSpecifier(token)) + { + if (location != VariableDeclaration::Location::Default) + BOOST_THROW_EXCEPTION(createParserError("Location already specified.")); + if (!type) + BOOST_THROW_EXCEPTION(createParserError("Location specifier needs explicit type name.")); + location = ( + token == Token::Memory ? + VariableDeclaration::Location::Memory : + VariableDeclaration::Location::Storage + ); + } + else + break; + m_scanner->next(); + } } nodeFactory.markEndPosition(); @@ -371,7 +396,7 @@ ASTPointer Parser::parseVariableDeclaration( } return nodeFactory.createNode(type, identifier, value, visibility, _options.isStateVariable, - isIndexed, isDeclaredConst); + isIndexed, isDeclaredConst, location); } ASTPointer Parser::parseModifierDefinition() @@ -388,7 +413,12 @@ ASTPointer Parser::parseModifierDefinition() ASTPointer name(expectIdentifierToken()); ASTPointer parameters; if (m_scanner->getCurrentToken() == Token::LParen) - parameters = parseParameterList(); + { + VarDeclParserOptions options; + options.allowIndexed = true; + options.allowLocationSpecifier = true; + parameters = parseParameterList(options); + } else parameters = createEmptyParameterList(); ASTPointer block = parseBlock(); @@ -407,7 +437,11 @@ ASTPointer Parser::parseEventDefinition() ASTPointer name(expectIdentifierToken()); ASTPointer parameters; if (m_scanner->getCurrentToken() == Token::LParen) - parameters = parseParameterList(true, true); + { + VarDeclParserOptions options; + options.allowIndexed = true; + parameters = parseParameterList(options); + } else parameters = createEmptyParameterList(); bool anonymous = false; @@ -505,12 +539,14 @@ ASTPointer Parser::parseMapping() return nodeFactory.createNode(keyType, valueType); } -ASTPointer Parser::parseParameterList(bool _allowEmpty, bool _allowIndexed) +ASTPointer Parser::parseParameterList( + VarDeclParserOptions const& _options, + bool _allowEmpty +) { ASTNodeFactory nodeFactory(*this); vector> parameters; - VarDeclParserOptions options; - options.allowIndexed = _allowIndexed; + VarDeclParserOptions options(_options); options.allowEmptyName = true; expectToken(Token::LParen); if (!_allowEmpty || m_scanner->getCurrentToken() != Token::RParen) @@ -691,7 +727,7 @@ ASTPointer Parser::parseSimpleStatement() } while (m_scanner->getCurrentToken() == Token::LBrack); - if (m_scanner->getCurrentToken() == Token::Identifier) + if (m_scanner->getCurrentToken() == Token::Identifier || Token::isLocationSpecifier(m_scanner->getCurrentToken())) return parseVariableDeclarationStatement(typeNameIndexAccessStructure(primary, indices)); else return parseExpressionStatement(expressionFromIndexAccessStructure(primary, indices)); @@ -703,6 +739,7 @@ ASTPointer Parser::parseVariableDeclarationStateme VarDeclParserOptions options; options.allowVar = true; options.allowInitialValue = true; + options.allowLocationSpecifier = true; ASTPointer variable = parseVariableDeclaration(options, _lookAheadArrayType); ASTNodeFactory nodeFactory(*this, variable); return nodeFactory.createNode(variable); @@ -944,11 +981,16 @@ Parser::LookAheadInfo Parser::peekStatementType() const Token::Value token(m_scanner->getCurrentToken()); bool mightBeTypeName = (Token::isElementaryTypeName(token) || token == Token::Identifier); - if (token == Token::Mapping || token == Token::Var || - (mightBeTypeName && m_scanner->peekNextToken() == Token::Identifier)) + if (token == Token::Mapping || token == Token::Var) return LookAheadInfo::VariableDeclarationStatement; - if (mightBeTypeName && m_scanner->peekNextToken() == Token::LBrack) - return LookAheadInfo::IndexAccessStructure; + if (mightBeTypeName) + { + Token::Value next = m_scanner->peekNextToken(); + if (next == Token::Identifier || Token::isLocationSpecifier(next)) + return LookAheadInfo::VariableDeclarationStatement; + if (m_scanner->peekNextToken() == Token::LBrack) + return LookAheadInfo::IndexAccessStructure; + } return LookAheadInfo::ExpressionStatement; } diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h index 08c47c252..d667aa3e1 100644 --- a/libsolidity/Parser.h +++ b/libsolidity/Parser.h @@ -47,13 +47,15 @@ private: /// End position of the current token int getEndPosition() const; - struct VarDeclParserOptions { + struct VarDeclParserOptions + { VarDeclParserOptions() {} bool allowVar = false; bool isStateVariable = false; bool allowIndexed = false; bool allowEmptyName = false; bool allowInitialValue = false; + bool allowLocationSpecifier = false; }; ///@{ @@ -74,7 +76,10 @@ private: ASTPointer parseIdentifier(); ASTPointer parseTypeName(bool _allowVar); ASTPointer parseMapping(); - ASTPointer parseParameterList(bool _allowEmpty = true, bool _allowIndexed = false); + ASTPointer parseParameterList( + VarDeclParserOptions const& _options, + bool _allowEmpty = true + ); ASTPointer parseBlock(); ASTPointer parseStatement(); ASTPointer parseIfStatement(); diff --git a/libsolidity/Token.h b/libsolidity/Token.h index bce16ed17..3e8c1c1d1 100644 --- a/libsolidity/Token.h +++ b/libsolidity/Token.h @@ -161,12 +161,14 @@ namespace solidity K(Import, "import", 0) \ K(Is, "is", 0) \ K(Mapping, "mapping", 0) \ + K(Memory, "memory", 0) \ K(Modifier, "modifier", 0) \ K(New, "new", 0) \ K(Public, "public", 0) \ K(Private, "private", 0) \ K(Return, "return", 0) \ K(Returns, "returns", 0) \ + K(Storage, "storage", 0) \ K(Struct, "struct", 0) \ K(Var, "var", 0) \ K(While, "while", 0) \ @@ -370,6 +372,7 @@ public: static bool isShiftOp(Value op) { return (SHL <= op) && (op <= SHR); } static bool isVisibilitySpecifier(Value op) { return isVariableVisibilitySpecifier(op) || op == External; } static bool isVariableVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Internal; } + static bool isLocationSpecifier(Value op) { return op == Memory || op == Storage; } static bool isEtherSubdenomination(Value op) { return op == SubWei || op == SubSzabo || op == SubFinney || op == SubEther; } static bool isTimeSubdenomination(Value op) { return op == SubSecond || op == SubMinute || op == SubHour || op == SubDay || op == SubWeek || op == SubYear; } diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 33eafb150..03e8e4ee0 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -144,9 +144,9 @@ TypePointer Type::fromElementaryTypeName(Token::Value _typeToken) else if (_typeToken == Token::Bool) return make_shared(); else if (_typeToken == Token::Bytes) - return make_shared(ArrayType::Location::Storage); + return make_shared(ReferenceType::Location::Storage); else if (_typeToken == Token::String) - return make_shared(ArrayType::Location::Storage, true); + return make_shared(ReferenceType::Location::Storage, true); else BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unable to convert elementary typename " + std::string(Token::toString(_typeToken)) + " to type.")); @@ -196,10 +196,10 @@ TypePointer Type::fromArrayTypeName(TypeName& _baseTypeName, Expression* _length auto const* length = dynamic_cast(_length->getType().get()); if (!length) BOOST_THROW_EXCEPTION(_length->createTypeError("Invalid array length.")); - return make_shared(ArrayType::Location::Storage, baseType, length->literalValue(nullptr)); + return make_shared(ReferenceType::Location::Storage, baseType, length->literalValue(nullptr)); } else - return make_shared(ArrayType::Location::Storage, baseType); + return make_shared(ReferenceType::Location::Storage, baseType); } TypePointer Type::forLiteral(Literal const& _literal) @@ -674,7 +674,7 @@ bool ArrayType::isImplicitlyConvertibleTo(const Type& _convertTo) const return false; auto& convertTo = dynamic_cast(_convertTo); // let us not allow assignment to memory arrays for now - if (convertTo.getLocation() != Location::Storage) + if (convertTo.location() != Location::Storage) return false; if (convertTo.isByteArray() != isByteArray() || convertTo.isString() != isString()) return false; @@ -778,12 +778,12 @@ TypePointer ArrayType::externalType() const return std::make_shared(Location::CallData, m_baseType->externalType(), m_length); } -shared_ptr ArrayType::copyForLocation(ArrayType::Location _location) const +TypePointer ArrayType::copyForLocation(ReferenceType::Location _location) const { auto copy = make_shared(_location); copy->m_arrayKind = m_arrayKind; - if (m_baseType->getCategory() == Type::Category::Array) - copy->m_baseType = dynamic_cast(*m_baseType).copyForLocation(_location); + if (auto ref = dynamic_cast(m_baseType.get())) + copy->m_baseType = ref->copyForLocation(_location); else copy->m_baseType = m_baseType; copy->m_hasDynamicLength = m_hasDynamicLength; @@ -934,6 +934,13 @@ MemberList const& StructType::getMembers() const return *m_members; } +TypePointer StructType::copyForLocation(ReferenceType::Location _location) const +{ + auto copy = make_shared(m_struct); + copy->m_location = _location; + return copy; +} + pair const& StructType::getStorageOffsetsOfMember(string const& _name) const { auto const* offsets = getMembers().getMemberStorageOffset(_name); @@ -1466,7 +1473,7 @@ MagicType::MagicType(MagicType::Kind _kind): {"sender", make_shared(0, IntegerType::Modifier::Address)}, {"gas", make_shared(256)}, {"value", make_shared(256)}, - {"data", make_shared(ArrayType::Location::CallData)}, + {"data", make_shared(ReferenceType::Location::CallData)}, {"sig", make_shared(4)} })); break; diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 65f6e4474..3ec925395 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -353,6 +353,24 @@ public: virtual TypePointer externalType() const override { return shared_from_this(); } }; +/** + * Trait used by types which are not value types and can be stored either in storage, memory + * or calldata. This is currently used by arrays and structs. + */ +class ReferenceType +{ +public: + enum class Location { Storage, CallData, Memory }; + explicit ReferenceType(Location _location): m_location(_location) {} + Location location() const { return m_location; } + + /// @returns a copy of this type with location (recursively) changed to @a _location. + virtual TypePointer copyForLocation(Location _location) const = 0; + +protected: + Location m_location = Location::Storage; +}; + /** * The type of an array. The flavours are byte array (bytes), statically- ([]) * and dynamically-sized array ([]). @@ -360,27 +378,26 @@ public: * one slot). Dynamically sized arrays (including byte arrays) start with their size as a uint and * thus start on their own slot. */ -class ArrayType: public Type +class ArrayType: public Type, public ReferenceType { public: - enum class Location { Storage, CallData, Memory }; virtual Category getCategory() const override { return Category::Array; } /// Constructor for a byte array ("bytes") and string. explicit ArrayType(Location _location, bool _isString = false): - m_location(_location), + ReferenceType(_location), m_arrayKind(_isString ? ArrayKind::String : ArrayKind::Bytes), m_baseType(std::make_shared(1)) {} /// Constructor for a dynamically sized array type ("type[]") ArrayType(Location _location, const TypePointer &_baseType): - m_location(_location), + ReferenceType(_location), m_baseType(_baseType) {} /// Constructor for a fixed-size array type ("type[20]") ArrayType(Location _location, const TypePointer &_baseType, u256 const& _length): - m_location(_location), + ReferenceType(_location), m_baseType(_baseType), m_hasDynamicLength(false), m_length(_length) @@ -400,7 +417,6 @@ public: } virtual TypePointer externalType() const override; - Location getLocation() const { return m_location; } /// @returns true if this is a byte array or a string bool isByteArray() const { return m_arrayKind != ArrayKind::Ordinary; } /// @returns true if this is a string @@ -408,15 +424,12 @@ public: TypePointer const& getBaseType() const { solAssert(!!m_baseType, ""); return m_baseType;} u256 const& getLength() const { return m_length; } - /// @returns a copy of this type with location changed to @a _location - /// @todo this might move as far up as Type later - std::shared_ptr copyForLocation(Location _location) const; + TypePointer copyForLocation(Location _location) const override; private: /// String is interpreted as a subtype of Bytes. enum class ArrayKind { Ordinary, Bytes, String }; - Location m_location; ///< Byte arrays ("bytes") and strings have different semantics from ordinary arrays. ArrayKind m_arrayKind = ArrayKind::Ordinary; TypePointer m_baseType; @@ -484,11 +497,13 @@ private: /** * The type of a struct instance, there is one distinct type per struct definition. */ -class StructType: public Type +class StructType: public Type, public ReferenceType { public: virtual Category getCategory() const override { return Category::Struct; } - explicit StructType(StructDefinition const& _struct): m_struct(_struct) {} + explicit StructType(StructDefinition const& _struct): + //@todo only storage until we have non-storage structs + ReferenceType(Location::Storage), m_struct(_struct) {} virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual bool operator==(Type const& _other) const override; virtual u256 getStorageSize() const override; @@ -498,6 +513,8 @@ public: virtual MemberList const& getMembers() const override; + TypePointer copyForLocation(Location _location) const override; + std::pair const& getStorageOffsetsOfMember(std::string const& _name) const; private: diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3691868cc..73bbcb162 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1876,6 +1876,40 @@ BOOST_AUTO_TEST_CASE(positive_integers_to_unsigned_out_of_bound) BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); } +BOOST_AUTO_TEST_CASE(overwrite_memory_location_external) +{ + char const* sourceCode = R"( + contract C { + function f(uint[] memory a) external {} + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(overwrite_storage_location_external) +{ + char const* sourceCode = R"( + contract C { + function f(uint[] storage a) external {} + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError); +} + +BOOST_AUTO_TEST_CASE(storage_location_local_variables) +{ + char const* sourceCode = R"( + contract C { + function f() { + uint[] storage x; + uint[] memory y; + uint[] memory z; + } + } + )"; + BOOST_CHECK_NO_THROW(parseTextAndResolveNames(sourceCode)); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp index cad0e1f2c..438e650bf 100644 --- a/test/libsolidity/SolidityParser.cpp +++ b/test/libsolidity/SolidityParser.cpp @@ -873,6 +873,47 @@ BOOST_AUTO_TEST_CASE(var_array) BOOST_CHECK_THROW(parseText(text), ParserError); } +BOOST_AUTO_TEST_CASE(location_specifiers_for_params) +{ + char const* text = R"( + contract Foo { + function f(uint[] storage constant x, uint[] memory y) { } + } + )"; + BOOST_CHECK_NO_THROW(parseText(text)); +} + +BOOST_AUTO_TEST_CASE(location_specifiers_for_locals) +{ + char const* text = R"( + contract Foo { + function f() { + uint[] storage x; + uint[] memory y; + } + } + )"; + BOOST_CHECK_NO_THROW(parseText(text)); +} + +BOOST_AUTO_TEST_CASE(location_specifiers_for_state) +{ + char const* text = R"( + contract Foo { + uint[] memory x; + })"; + BOOST_CHECK_THROW(parseText(text), ParserError); +} + +BOOST_AUTO_TEST_CASE(location_specifiers_with_var) +{ + char const* text = R"( + contract Foo { + function f() { var memory x; } + })"; + BOOST_CHECK_THROW(parseText(text), ParserError); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/SolidityTypes.cpp b/test/libsolidity/SolidityTypes.cpp index 6b6306479..718798a5a 100644 --- a/test/libsolidity/SolidityTypes.cpp +++ b/test/libsolidity/SolidityTypes.cpp @@ -77,13 +77,13 @@ BOOST_AUTO_TEST_CASE(storage_layout_mapping) BOOST_AUTO_TEST_CASE(storage_layout_arrays) { - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(1), 32).getStorageSize() == 1); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(1), 33).getStorageSize() == 2); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(2), 31).getStorageSize() == 2); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(7), 8).getStorageSize() == 2); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(7), 9).getStorageSize() == 3); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(31), 9).getStorageSize() == 9); - BOOST_CHECK(ArrayType(ArrayType::Location::Storage, make_shared(32), 9).getStorageSize() == 9); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(1), 32).getStorageSize() == 1); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(1), 33).getStorageSize() == 2); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(2), 31).getStorageSize() == 2); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(7), 8).getStorageSize() == 2); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(7), 9).getStorageSize() == 3); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(31), 9).getStorageSize() == 9); + BOOST_CHECK(ArrayType(ReferenceType::Location::Storage, make_shared(32), 9).getStorageSize() == 9); } BOOST_AUTO_TEST_SUITE_END() From 2b9e46d8f6f19271c4f013b3608a3dabc98f56b2 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 14:45:47 +0200 Subject: [PATCH 20/26] Style. --- libsolidity/AST.h | 10 +++++----- libsolidity/Parser.cpp | 13 ++++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/libsolidity/AST.h b/libsolidity/AST.h index d75347127..578709c1e 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -524,11 +524,11 @@ protected: Visibility getDefaultVisibility() const override { return Visibility::Internal; } private: - ASTPointer m_typeName; ///< can be empty ("var") - ASTPointer m_value; ///< the assigned value, can be missing - bool m_isStateVariable; ///< Whether or not this is a contract state variable - bool m_isIndexed; ///< Whether this is an indexed variable (used by events). - bool m_isConstant; ///< Whether the variable is a compile-time constant. + ASTPointer m_typeName; ///< can be empty ("var") + ASTPointer m_value; ///< the assigned value, can be missing + bool m_isStateVariable; ///< Whether or not this is a contract state variable + bool m_isIndexed; ///< Whether this is an indexed variable (used by events). + bool m_isConstant; ///< Whether the variable is a compile-time constant. Location m_location; ///< Location of the variable if it is of reference type. std::shared_ptr m_type; ///< derived type, initially empty diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index 851f1669e..548b7dc21 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -394,9 +394,16 @@ ASTPointer Parser::parseVariableDeclaration( nodeFactory.setEndPositionFromNode(value); } } - return nodeFactory.createNode(type, identifier, value, - visibility, _options.isStateVariable, - isIndexed, isDeclaredConst, location); + return nodeFactory.createNode( + type, + identifier, + value, + visibility, + _options.isStateVariable, + isIndexed, + isDeclaredConst, + location + ); } ASTPointer Parser::parseModifierDefinition() From 3fc61e9087acd291f247821ef956d9355cefeecb Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 1 Jun 2015 12:32:59 +0200 Subject: [PATCH 21/26] Compute constants --- libevmasm/Assembly.cpp | 14 +- libevmasm/Assembly.h | 9 +- libevmasm/ConstantOptimiser.cpp | 225 ++++++++++++++++++ libevmasm/ConstantOptimiser.h | 147 ++++++++++++ libevmasm/GasMeter.cpp | 7 +- libevmasm/GasMeter.h | 4 +- libevmcore/Instruction.cpp | 4 +- libsolidity/Compiler.cpp | 8 +- libsolidity/Compiler.h | 17 +- libsolidity/CompilerContext.h | 5 +- libsolidity/CompilerStack.cpp | 6 +- libsolidity/CompilerStack.h | 2 +- solc/CommandLineInterface.cpp | 9 +- test/libsolidity/SolidityOptimizer.cpp | 45 ++++ test/libsolidity/solidityExecutionFramework.h | 3 +- 15 files changed, 480 insertions(+), 25 deletions(-) create mode 100644 libevmasm/ConstantOptimiser.cpp create mode 100644 libevmasm/ConstantOptimiser.h diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index a9ee96199..8642824f6 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -22,9 +22,12 @@ #include "Assembly.h" #include #include +#include #include #include #include +#include +#include #include using namespace std; using namespace dev; @@ -302,7 +305,7 @@ inline bool matches(AssemblyItemsConstRef _a, AssemblyItemsConstRef _b) struct OptimiserChannel: public LogChannel { static const char* name() { return "OPT"; } static const int verbosity = 12; }; #define copt dev::LogOutputStream() -Assembly& Assembly::optimise(bool _enable) +Assembly& Assembly::optimise(bool _enable, bool _isCreation, size_t _runs) { if (!_enable) return *this; @@ -364,10 +367,17 @@ Assembly& Assembly::optimise(bool _enable) } } + total += ConstantOptimisationMethod::optimiseConstants( + _isCreation, + _isCreation ? 1 : _runs, + *this, + m_items + ); + copt << total << " optimisations done."; for (auto& sub: m_subs) - sub.optimise(true); + sub.optimise(true, false, _runs); return *this; } diff --git a/libevmasm/Assembly.h b/libevmasm/Assembly.h index 3c82125a1..1457173bc 100644 --- a/libevmasm/Assembly.h +++ b/libevmasm/Assembly.h @@ -49,6 +49,7 @@ public: AssemblyItem newPushTag() { return AssemblyItem(PushTag, m_usedTags++); } AssemblyItem newData(bytes const& _data) { h256 h = (u256)std::hash()(asString(_data)); m_data[h] = _data; return AssemblyItem(PushData, h); } AssemblyItem newSub(Assembly const& _sub) { m_subs.push_back(_sub); return AssemblyItem(PushSub, m_subs.size() - 1); } + Assembly const& getSub(size_t _sub) const { return m_subs.at(_sub); } AssemblyItem newPushString(std::string const& _data) { h256 h = (u256)std::hash()(_data); m_strings[h] = _data; return AssemblyItem(PushString, h); } AssemblyItem newPushSubSize(u256 const& _subId) { return AssemblyItem(PushSubSize, _subId); } @@ -92,7 +93,13 @@ public: void setSourceLocation(SourceLocation const& _location) { m_currentSourceLocation = _location; } bytes assemble() const; - Assembly& optimise(bool _enable); + bytes const& data(h256 const& _i) const { return m_data[_i]; } + + /// Modify (if @a _enable is set) and return the current assembly such that creation and + /// execution gas usage is optimised. @a _isCreation should be true for the top-level assembly. + /// @a _runs specifes an estimate on how often each opcode in this assembly will be executed, + /// i.e. use a small value to optimise for size and a large value to optimise for runtime. + Assembly& optimise(bool _enable, bool _isCreation = true, size_t _runs = 200); Json::Value stream( std::ostream& _out, std::string const& _prefix = "", diff --git a/libevmasm/ConstantOptimiser.cpp b/libevmasm/ConstantOptimiser.cpp new file mode 100644 index 000000000..77d1bdfaa --- /dev/null +++ b/libevmasm/ConstantOptimiser.cpp @@ -0,0 +1,225 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file ConstantOptimiser.cpp + * @author Christian + * @date 2015 + */ + +#include "libevmasm/ConstantOptimiser.h" +#include +#include +#include +using namespace std; +using namespace dev; +using namespace dev::eth; + +unsigned ConstantOptimisationMethod::optimiseConstants( + bool _isCreation, + size_t _runs, + Assembly& _assembly, + AssemblyItems& _items +) +{ + unsigned optimisations = 0; + map pushes; + for (AssemblyItem const& item: _items) + if (item.type() == Push) + pushes[item]++; + for (auto it: pushes) + { + AssemblyItem const& item = it.first; + if (item.data() < 0x100) + continue; + Params params; + params.multiplicity = it.second; + params.isCreation = _isCreation; + params.runs = _runs; + LiteralMethod lit(params, item.data()); + bigint literalGas = lit.gasNeeded(); + CodeCopyMethod copy(params, item.data()); + bigint copyGas = copy.gasNeeded(); + ComputeMethod compute(params, item.data()); + bigint computeGas = compute.gasNeeded(); + if (copyGas < literalGas && copyGas < computeGas) + { + copy.execute(_assembly, _items); + optimisations++; + } + else if (computeGas < literalGas && computeGas < copyGas) + { + compute.execute(_assembly, _items); + optimisations++; + } + } + return optimisations; +} + +bigint ConstantOptimisationMethod::simpleRunGas(AssemblyItems const& _items) +{ + bigint gas = 0; + for (AssemblyItem const& item: _items) + if (item.type() == Push) + gas += GasMeter::runGas(eth::Instruction::PUSH1); + else if (item.type() == Operation) + gas += GasMeter::runGas(item.instruction()); + return gas; +} + +bigint ConstantOptimisationMethod::dataGas(bytes const& _data) const +{ + if (m_params.isCreation) + { + bigint gas; + for (auto b: _data) + gas += b ? c_txDataNonZeroGas : c_txDataZeroGas; + return gas; + } + else + return c_createDataGas * dataSize(); +} + +size_t ConstantOptimisationMethod::bytesRequired(AssemblyItems const& _items) +{ + size_t size = 0; + for (AssemblyItem const& item: _items) + size += item.bytesRequired(3); // assume 3 byte addresses + return size; +} + +void ConstantOptimisationMethod::replaceConstants( + AssemblyItems& _items, + AssemblyItems const& _replacement +) const +{ + assertThrow(_items.size() > 0, OptimizerException, ""); + for (size_t i = 0; i < _items.size(); ++i) + { + if (_items.at(i) != AssemblyItem(m_value)) + continue; + _items[i] = _replacement[0]; + _items.insert(_items.begin() + i + 1, _replacement.begin() + 1, _replacement.end()); + i += _replacement.size() - 1; + } +} + +bigint LiteralMethod::gasNeeded() +{ + return combineGas( + simpleRunGas({eth::Instruction::PUSH1}), + // PUSHX plus data + (m_params.isCreation ? c_txDataNonZeroGas : c_createDataGas) + dataGas(), + 0 + ); +} + +CodeCopyMethod::CodeCopyMethod(Params const& _params, u256 const& _value): + ConstantOptimisationMethod(_params, _value), + m_copyRoutine{ + u256(0), + eth::Instruction::DUP1, + eth::Instruction::MLOAD, // back up memory + u256(32), + AssemblyItem(PushData, u256(1) << 16), // has to be replaced + eth::Instruction::DUP4, + eth::Instruction::CODECOPY, + eth::Instruction::DUP2, + eth::Instruction::MLOAD, + eth::Instruction::SWAP2, + eth::Instruction::MSTORE + } +{ +} + +bigint CodeCopyMethod::gasNeeded() +{ + return combineGas( + // Run gas: we ignore memory increase costs + simpleRunGas(m_copyRoutine) + c_copyGas, + // Data gas for copy routines: Some bytes are zero, but we ignore them. + bytesRequired(m_copyRoutine) * (m_params.isCreation ? c_txDataNonZeroGas : c_createDataGas), + // Data gas for data itself + dataGas(toBigEndian(m_value)) + ); +} + +void CodeCopyMethod::execute(Assembly& _assembly, AssemblyItems& _items) +{ + bytes data = toBigEndian(m_value); + m_copyRoutine[4] = _assembly.newData(data); + replaceConstants(_items, m_copyRoutine); +} + +AssemblyItems ComputeMethod::findRepresentation(u256 const& _value) +{ + if (_value < 0x10000) + // Very small value, not worth computing + return AssemblyItems{_value}; + else if (dev::bytesRequired(~_value) < dev::bytesRequired(_value)) + // Negated is shorter to represent + return findRepresentation(~_value) + AssemblyItems{Instruction::NOT}; + else + { + // Decompose value into a * 2**k + b where abs(b) << 2**k + // Is not always better, try literal and decomposition method. + AssemblyItems routine{u256(_value)}; + bigint bestGas = gasNeeded(routine); + for (unsigned bits = 255; bits > 8; --bits) + { + unsigned gapDetector = unsigned(_value >> (bits - 8)) & 0x1ff; + if (gapDetector != 0xff && gapDetector != 0x100) + continue; + + u256 powerOfTwo = u256(1) << bits; + u256 upperPart = _value >> bits; + bigint lowerPart = _value & (powerOfTwo - 1); + if (abs(powerOfTwo - lowerPart) < lowerPart) + lowerPart = lowerPart - powerOfTwo; // make it negative + if (abs(lowerPart) >= (powerOfTwo >> 8)) + continue; + + AssemblyItems newRoutine; + if (lowerPart != 0) + newRoutine += findRepresentation(u256(abs(lowerPart))); + newRoutine += AssemblyItems{u256(bits), u256(2), Instruction::EXP}; + if (upperPart != 1 && upperPart != 0) + newRoutine += findRepresentation(upperPart) + AssemblyItems{Instruction::MUL}; + if (lowerPart > 0) + newRoutine += AssemblyItems{Instruction::ADD}; + else if (lowerPart < 0) + newRoutine.push_back(eth::Instruction::SUB); + + bigint newGas = gasNeeded(newRoutine); + if (newGas < bestGas) + { + bestGas = move(newGas); + routine = move(newRoutine); + } + } + return routine; + } +} + +bigint ComputeMethod::gasNeeded(AssemblyItems const& _routine) +{ + size_t numExps = count(_routine.begin(), _routine.end(), eth::Instruction::EXP); + return combineGas( + simpleRunGas(_routine) + numExps * (c_expGas + c_expByteGas), + // Data gas for routine: Some bytes are zero, but we ignore them. + bytesRequired(_routine) * (m_params.isCreation ? c_txDataNonZeroGas : c_createDataGas), + 0 + ); +} diff --git a/libevmasm/ConstantOptimiser.h b/libevmasm/ConstantOptimiser.h new file mode 100644 index 000000000..e75eff380 --- /dev/null +++ b/libevmasm/ConstantOptimiser.h @@ -0,0 +1,147 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file ConstantOptimiser.cpp + * @author Christian + * @date 2015 + */ + +#pragma once + +#include +#include +#include + +namespace dev +{ +namespace eth +{ + +class AssemblyItem; +using AssemblyItems = std::vector; +class Assembly; + +/** + * Abstract base class for one way to change how constants are represented in the code. + */ +class ConstantOptimisationMethod +{ +public: + /// Tries to optimised how constants are represented in the source code and modifies + /// @a _assembly and its @a _items. + /// @returns zero if no optimisations could be performed. + static unsigned optimiseConstants( + bool _isCreation, + size_t _runs, + Assembly& _assembly, + AssemblyItems& _items + ); + + struct Params + { + bool isCreation; ///< Whether this is called during contract creation or runtime. + size_t runs; ///< Estimated number of calls per opcode oven the lifetime of the contract. + size_t multiplicity; ///< Number of times the constant appears in the code. + }; + + explicit ConstantOptimisationMethod(Params const& _params, u256 const& _value): + m_params(_params), m_value(_value) {} + virtual bigint gasNeeded() = 0; + virtual void execute(Assembly& _assembly, AssemblyItems& _items) = 0; + +protected: + size_t dataSize() const { return std::max(1, dev::bytesRequired(m_value)); } + + /// @returns the run gas for the given items ignoring special gas costs + static bigint simpleRunGas(AssemblyItems const& _items); + /// @returns the gas needed to store the given data literally + bigint dataGas(bytes const& _data) const; + /// @returns the gas needed to store the value literally + bigint dataGas() const { return dataGas(toCompactBigEndian(m_value, 1)); } + static size_t bytesRequired(AssemblyItems const& _items); + /// @returns the combined estimated gas usage taking @a m_params into account. + bigint combineGas( + bigint const& _runGas, + bigint const& _repeatedDataGas, + bigint const& _uniqueDataGas + ) + { + // _runGas is not multiplied by _multiplicity because the runs are "per opcode" + return m_params.runs * _runGas + m_params.multiplicity * _repeatedDataGas + _uniqueDataGas; + } + + /// Replaces the constant by the code given in @a _replacement. + void replaceConstants(AssemblyItems& _items, AssemblyItems const& _replacement) const; + + Params m_params; + u256 const& m_value; +}; + +/** + * Optimisation method that pushes the constant to the stack literally. This is the default method, + * i.e. executing it does not alter the Assembly. + */ +class LiteralMethod: public ConstantOptimisationMethod +{ +public: + explicit LiteralMethod(Params const& _params, u256 const& _value): + ConstantOptimisationMethod(_params, _value) {} + virtual bigint gasNeeded() override; + virtual void execute(Assembly&, AssemblyItems&) override {} +}; + +/** + * Method that stores the data in the .data section of the code and copies it to the stack. + */ +class CodeCopyMethod: public ConstantOptimisationMethod +{ +public: + explicit CodeCopyMethod(Params const& _params, u256 const& _value); + virtual bigint gasNeeded() override; + virtual void execute(Assembly& _assembly, AssemblyItems& _items) override; + +protected: + AssemblyItems m_copyRoutine; +}; + +/** + * Method that tries to compute the constant. + */ +class ComputeMethod: public ConstantOptimisationMethod +{ +public: + explicit ComputeMethod(Params const& _params, u256 const& _value): + ConstantOptimisationMethod(_params, _value) + { + m_routine = findRepresentation(m_value); + } + + virtual bigint gasNeeded() override { return gasNeeded(m_routine); } + virtual void execute(Assembly&, AssemblyItems& _items) override + { + replaceConstants(_items, m_routine); + } + +protected: + /// Tries to recursively find a way to compute @a _value. + AssemblyItems findRepresentation(u256 const& _value); + bigint gasNeeded(AssemblyItems const& _routine); + + AssemblyItems m_routine; +}; + +} +} diff --git a/libevmasm/GasMeter.cpp b/libevmasm/GasMeter.cpp index 4e5289e38..42a5bed2e 100644 --- a/libevmasm/GasMeter.cpp +++ b/libevmasm/GasMeter.cpp @@ -201,13 +201,14 @@ GasMeter::GasConsumption GasMeter::memoryGas(int _stackPosOffset, int _stackPosS })); } -GasMeter::GasConsumption GasMeter::runGas(Instruction _instruction) +u256 GasMeter::runGas(Instruction _instruction) { if (_instruction == Instruction::JUMPDEST) - return GasConsumption(1); + return 1; int tier = instructionInfo(_instruction).gasPriceTier; - return tier == InvalidTier ? GasConsumption::infinite() : c_tierStepGas[tier]; + assertThrow(tier != InvalidTier, OptimizerException, "Invalid gas tier."); + return c_tierStepGas[tier]; } diff --git a/libevmasm/GasMeter.h b/libevmasm/GasMeter.h index 6949c193e..90f151fc4 100644 --- a/libevmasm/GasMeter.h +++ b/libevmasm/GasMeter.h @@ -66,6 +66,8 @@ public: u256 const& largestMemoryAccess() const { return m_largestMemoryAccess; } + static u256 runGas(Instruction _instruction); + private: /// @returns _multiplier * (_value + 31) / 32, if _value is a known constant and infinite otherwise. GasConsumption wordGas(u256 const& _multiplier, ExpressionClasses::Id _value); @@ -76,8 +78,6 @@ private: /// given as values on the stack at the given relative positions. GasConsumption memoryGas(int _stackPosOffset, int _stackPosSize); - static GasConsumption runGas(Instruction _instruction); - std::shared_ptr m_state; /// Largest point where memory was accessed since the creation of this object. u256 m_largestMemoryAccess; diff --git a/libevmcore/Instruction.cpp b/libevmcore/Instruction.cpp index 03b6ccf2f..61489d127 100644 --- a/libevmcore/Instruction.cpp +++ b/libevmcore/Instruction.cpp @@ -300,7 +300,7 @@ void dev::eth::eachInstruction( function const& _onInstruction ) { - for (auto it = _mem.begin(); it != _mem.end(); ++it) + for (auto it = _mem.begin(); it < _mem.end(); ++it) { Instruction instr = Instruction(*it); size_t additional = 0; @@ -310,7 +310,7 @@ void dev::eth::eachInstruction( for (size_t i = 0; i < additional; ++i) { data <<= 8; - if (it != _mem.end() && ++it != _mem.end()) + if (++it < _mem.end()) data |= *it; } _onInstruction(instr, data); diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 6425367dd..6dc728405 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -69,6 +69,8 @@ void Compiler::compileContract(ContractDefinition const& _contract, swap(m_context, m_runtimeContext); initializeContext(_contract, _contracts); packIntoContractCreator(_contract, m_runtimeContext); + if (m_optimize) + m_context.optimise(m_optimizeRuns); } eth::AssemblyItem Compiler::getFunctionEntryLabel(FunctionDefinition const& _function) const @@ -120,9 +122,11 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp else if (auto c = m_context.getNextConstructor(_contract)) appendBaseConstructor(*c); - eth::AssemblyItem sub = m_context.addSubroutine(_runtimeContext.getAssembly()); + eth::AssemblyItem runtimeSub = m_context.addSubroutine(_runtimeContext.getAssembly()); + solAssert(runtimeSub.data() < numeric_limits::max(), ""); + m_runtimeSub = size_t(runtimeSub.data()); // stack contains sub size - m_context << eth::Instruction::DUP1 << sub << u256(0) << eth::Instruction::CODECOPY; + m_context << eth::Instruction::DUP1 << runtimeSub << u256(0) << eth::Instruction::CODECOPY; m_context << u256(0) << eth::Instruction::RETURN; // note that we have to include the functions again because of absolute jump labels diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 13b8639dd..670c74673 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -34,13 +34,18 @@ namespace solidity { class Compiler: private ASTConstVisitor { public: - explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_context(), - m_returnTag(m_context.newTag()) {} + explicit Compiler(bool _optimize = false, unsigned _runs = 200): + m_optimize(_optimize), + m_optimizeRuns(_runs), + m_context(), + m_returnTag(m_context.newTag()) + { + } void compileContract(ContractDefinition const& _contract, std::map const& _contracts); - bytes getAssembledBytecode() { return m_context.getAssembledBytecode(m_optimize); } - bytes getRuntimeBytecode() { return m_runtimeContext.getAssembledBytecode(m_optimize);} + bytes getAssembledBytecode() { return m_context.getAssembledBytecode(); } + bytes getRuntimeBytecode() { return m_context.getAssembledRuntimeBytecode(m_runtimeSub); } /// @arg _sourceCodes is the map of input files to source code strings /// @arg _inJsonFromat shows whether the out should be in Json format Json::Value streamAssembly(std::ostream& _stream, StringMap const& _sourceCodes = StringMap(), bool _inJsonFormat = false) const @@ -50,7 +55,7 @@ public: /// @returns Assembly items of the normal compiler context eth::AssemblyItems const& getAssemblyItems() const { return m_context.getAssembly().getItems(); } /// @returns Assembly items of the runtime compiler context - eth::AssemblyItems const& getRuntimeAssemblyItems() const { return m_runtimeContext.getAssembly().getItems(); } + eth::AssemblyItems const& getRuntimeAssemblyItems() const { return m_context.getAssembly().getSub(m_runtimeSub).getItems(); } /// @returns the entry label of the given function. Might return an AssemblyItem of type /// UndefinedItem if it does not exist yet. @@ -93,7 +98,9 @@ private: void compileExpression(Expression const& _expression, TypePointer const& _targetType = TypePointer()); bool const m_optimize; + unsigned const m_optimizeRuns; CompilerContext m_context; + size_t m_runtimeSub = size_t(-1); ///< Identifier of the runtime sub-assembly CompilerContext m_runtimeContext; std::vector m_breakTags; ///< tag to jump to for a "break" statement std::vector m_continueTags; ///< tag to jump to for a "continue" statement diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index 573e0b576..998b0a2f7 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -126,6 +126,8 @@ public: CompilerContext& operator<<(u256 const& _value) { m_asm.append(_value); return *this; } CompilerContext& operator<<(bytes const& _data) { m_asm.append(_data); return *this; } + void optimise(unsigned _runs = 200) { m_asm.optimise(true, true, _runs); } + eth::Assembly const& getAssembly() const { return m_asm; } /// @arg _sourceCodes is the map of input files to source code strings /// @arg _inJsonFormat shows whether the out should be in Json format @@ -134,7 +136,8 @@ public: return m_asm.stream(_stream, "", _sourceCodes, _inJsonFormat); } - bytes getAssembledBytecode(bool _optimize = false) { return m_asm.optimise(_optimize).assemble(); } + bytes getAssembledBytecode() { return m_asm.assemble(); } + bytes getAssembledRuntimeBytecode(size_t _subIndex) { m_asm.assemble(); return m_asm.data(u256(_subIndex)); } /** * Helper class to pop the visited nodes stack when a scope closes diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index ebb988768..a3399823e 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -145,7 +145,7 @@ vector CompilerStack::getContractNames() const } -void CompilerStack::compile(bool _optimize) +void CompilerStack::compile(bool _optimize, unsigned _runs) { if (!m_parseSuccessful) parse(); @@ -157,9 +157,9 @@ void CompilerStack::compile(bool _optimize) { if (!contract->isFullyImplemented()) continue; - shared_ptr compiler = make_shared(_optimize); + shared_ptr compiler = make_shared(_optimize, _runs); compiler->compileContract(*contract, contractBytecode); - Contract& compiledContract = m_contracts[contract->getName()]; + Contract& compiledContract = m_contracts.at(contract->getName()); compiledContract.bytecode = compiler->getAssembledBytecode(); compiledContract.runtimeBytecode = compiler->getRuntimeBytecode(); compiledContract.compiler = move(compiler); diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 6be45aec1..a7c6ea3ba 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -90,7 +90,7 @@ public: std::string defaultContractName() const; /// Compiles the source units that were previously added and parsed. - void compile(bool _optimize = false); + void compile(bool _optimize = false, unsigned _runs = 200); /// Parses and compiles the given source code. /// @returns the compiled bytecode bytes const& compile(std::string const& _sourceCode, bool _optimize = false); diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 8129f60c5..860b4c3b8 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -299,7 +299,8 @@ bool CommandLineInterface::parseArguments(int argc, char** argv) desc.add_options() ("help", "Show help message and exit") ("version", "Show version and exit") - ("optimize", po::value()->default_value(false), "Optimize bytecode for size") + ("optimize", po::value()->default_value(false), "Optimize bytecode") + ("optimize-runs", po::value()->default_value(200), "Estimated number of contract runs for optimizer.") ("add-std", po::value()->default_value(false), "Add standard contracts") ("input-file", po::value>(), "input file") ( @@ -409,7 +410,11 @@ bool CommandLineInterface::processInput() for (auto const& sourceCode: m_sourceCodes) m_compiler->addSource(sourceCode.first, sourceCode.second); // TODO: Perhaps we should not compile unless requested - m_compiler->compile(m_args["optimize"].as()); + bool optimize = m_args["optimize"].as(); + unsigned runs = m_args["optimize-runs"].as(); + if (m_args.count("optimize-runs")) + optimize = true; + m_compiler->compile(optimize, runs); } catch (ParserError const& _exception) { diff --git a/test/libsolidity/SolidityOptimizer.cpp b/test/libsolidity/SolidityOptimizer.cpp index 827d8833a..4ed081fdb 100644 --- a/test/libsolidity/SolidityOptimizer.cpp +++ b/test/libsolidity/SolidityOptimizer.cpp @@ -1036,6 +1036,51 @@ BOOST_AUTO_TEST_CASE(block_deduplicator_loops) BOOST_CHECK_EQUAL(pushTags.size(), 1); } +BOOST_AUTO_TEST_CASE(computing_constants) +{ + char const* sourceCode = R"( + contract c { + uint a; + uint b; + uint c; + function set() returns (uint a, uint b, uint c) { + a = 0x77abc0000000000000000000000000000000000000000000000000000000001; + b = 0x817416927846239487123469187231298734162934871263941234127518276; + g(); + } + function g() { + b = 0x817416927846239487123469187231298734162934871263941234127518276; + c = 0x817416927846239487123469187231298734162934871263941234127518276; + } + function get() returns (uint ra, uint rb, uint rc) { + ra = a; + rb = b; + rc = c ; + } + } + )"; + compileBothVersions(sourceCode); + compareVersions("set()"); + compareVersions("get()"); + + m_optimize = true; + m_optimizeRuns = 1; + bytes optimizedBytecode = compileAndRun(sourceCode, 0, "c"); + bytes complicatedConstant = toBigEndian(u256("0x817416927846239487123469187231298734162934871263941234127518276")); + unsigned occurrences = 0; + for (auto iter = optimizedBytecode.cbegin(); iter < optimizedBytecode.cend(); ++occurrences) + iter = search(iter, optimizedBytecode.cend(), complicatedConstant.cbegin(), complicatedConstant.cend()) + 1; + BOOST_CHECK_EQUAL(2, occurrences); + + bytes constantWithZeros = toBigEndian(u256("0x77abc0000000000000000000000000000000000000000000000000000000001")); + BOOST_CHECK(search( + optimizedBytecode.cbegin(), + optimizedBytecode.cend(), + constantWithZeros.cbegin(), + constantWithZeros.cend() + ) == optimizedBytecode.cend()); +} + BOOST_AUTO_TEST_SUITE_END() } diff --git a/test/libsolidity/solidityExecutionFramework.h b/test/libsolidity/solidityExecutionFramework.h index 1e11ae90c..e81b4d7b5 100644 --- a/test/libsolidity/solidityExecutionFramework.h +++ b/test/libsolidity/solidityExecutionFramework.h @@ -46,7 +46,7 @@ public: { m_compiler.reset(false, m_addStandardSources); m_compiler.addSource("", _sourceCode); - ETH_TEST_REQUIRE_NO_THROW(m_compiler.compile(m_optimize), "Compiling contract failed"); + ETH_TEST_REQUIRE_NO_THROW(m_compiler.compile(m_optimize, m_optimizeRuns), "Compiling contract failed"); bytes code = m_compiler.getBytecode(_contractName); sendMessage(code, true, _value); return m_output; @@ -180,6 +180,7 @@ protected: m_logs = executive.logs(); } + size_t m_optimizeRuns = 200; bool m_optimize = false; bool m_addStandardSources = false; dev::solidity::CompilerStack m_compiler; From 1c8eabf6ed41d8f57d8c4b21d43401c542ec99f5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 4 Jun 2015 11:02:34 +0200 Subject: [PATCH 22/26] MSVC fix. --- libevmasm/ConstantOptimiser.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libevmasm/ConstantOptimiser.cpp b/libevmasm/ConstantOptimiser.cpp index 77d1bdfaa..80a2dc180 100644 --- a/libevmasm/ConstantOptimiser.cpp +++ b/libevmasm/ConstantOptimiser.cpp @@ -127,8 +127,9 @@ bigint LiteralMethod::gasNeeded() } CodeCopyMethod::CodeCopyMethod(Params const& _params, u256 const& _value): - ConstantOptimisationMethod(_params, _value), - m_copyRoutine{ + ConstantOptimisationMethod(_params, _value) +{ + m_copyRoutine = AssemblyItems{ u256(0), eth::Instruction::DUP1, eth::Instruction::MLOAD, // back up memory @@ -140,8 +141,7 @@ CodeCopyMethod::CodeCopyMethod(Params const& _params, u256 const& _value): eth::Instruction::MLOAD, eth::Instruction::SWAP2, eth::Instruction::MSTORE - } -{ + }; } bigint CodeCopyMethod::gasNeeded() From 660a6eb37b35e5f7e007687ace931d7ef566fd41 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 17:34:20 +0200 Subject: [PATCH 23/26] Remove namespace prefixes. --- libevmasm/ConstantOptimiser.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libevmasm/ConstantOptimiser.cpp b/libevmasm/ConstantOptimiser.cpp index 80a2dc180..88874d81c 100644 --- a/libevmasm/ConstantOptimiser.cpp +++ b/libevmasm/ConstantOptimiser.cpp @@ -73,7 +73,7 @@ bigint ConstantOptimisationMethod::simpleRunGas(AssemblyItems const& _items) bigint gas = 0; for (AssemblyItem const& item: _items) if (item.type() == Push) - gas += GasMeter::runGas(eth::Instruction::PUSH1); + gas += GasMeter::runGas(Instruction::PUSH1); else if (item.type() == Operation) gas += GasMeter::runGas(item.instruction()); return gas; @@ -119,7 +119,7 @@ void ConstantOptimisationMethod::replaceConstants( bigint LiteralMethod::gasNeeded() { return combineGas( - simpleRunGas({eth::Instruction::PUSH1}), + simpleRunGas({Instruction::PUSH1}), // PUSHX plus data (m_params.isCreation ? c_txDataNonZeroGas : c_createDataGas) + dataGas(), 0 @@ -131,16 +131,16 @@ CodeCopyMethod::CodeCopyMethod(Params const& _params, u256 const& _value): { m_copyRoutine = AssemblyItems{ u256(0), - eth::Instruction::DUP1, - eth::Instruction::MLOAD, // back up memory + Instruction::DUP1, + Instruction::MLOAD, // back up memory u256(32), AssemblyItem(PushData, u256(1) << 16), // has to be replaced - eth::Instruction::DUP4, - eth::Instruction::CODECOPY, - eth::Instruction::DUP2, - eth::Instruction::MLOAD, - eth::Instruction::SWAP2, - eth::Instruction::MSTORE + Instruction::DUP4, + Instruction::CODECOPY, + Instruction::DUP2, + Instruction::MLOAD, + Instruction::SWAP2, + Instruction::MSTORE }; } @@ -200,7 +200,7 @@ AssemblyItems ComputeMethod::findRepresentation(u256 const& _value) if (lowerPart > 0) newRoutine += AssemblyItems{Instruction::ADD}; else if (lowerPart < 0) - newRoutine.push_back(eth::Instruction::SUB); + newRoutine.push_back(Instruction::SUB); bigint newGas = gasNeeded(newRoutine); if (newGas < bestGas) @@ -215,7 +215,7 @@ AssemblyItems ComputeMethod::findRepresentation(u256 const& _value) bigint ComputeMethod::gasNeeded(AssemblyItems const& _routine) { - size_t numExps = count(_routine.begin(), _routine.end(), eth::Instruction::EXP); + size_t numExps = count(_routine.begin(), _routine.end(), Instruction::EXP); return combineGas( simpleRunGas(_routine) + numExps * (c_expGas + c_expByteGas), // Data gas for routine: Some bytes are zero, but we ignore them. From b60da1924141e86aef1d260f4d8b88207cb51378 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 12:25:10 +0200 Subject: [PATCH 24/26] Fallback takes constant amount of gas, and send to gas with send. --- libsolidity/Compiler.cpp | 16 +++++++++++++++- libsolidity/ExpressionCompiler.cpp | 3 ++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 6425367dd..5398ed711 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -174,6 +174,16 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) map, FunctionTypePointer> interfaceFunctions = _contract.getInterfaceFunctions(); map, const eth::AssemblyItem> callDataUnpackerEntryPoints; + FunctionDefinition const* fallback = _contract.getFallbackFunction(); + eth::AssemblyItem notFound = m_context.newTag(); + // shortcut messages without data if we have many functions in order to be able to receive + // ether with constant gas + if (interfaceFunctions.size() > 5 || fallback) + { + m_context << eth::Instruction::CALLDATASIZE << eth::Instruction::ISZERO; + m_context.appendConditionalJumpTo(notFound); + } + // retrieve the function signature hash from the calldata if (!interfaceFunctions.empty()) CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true); @@ -185,7 +195,10 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) m_context << eth::dupInstruction(1) << u256(FixedHash<4>::Arith(it.first)) << eth::Instruction::EQ; m_context.appendConditionalJumpTo(callDataUnpackerEntryPoints.at(it.first)); } - if (FunctionDefinition const* fallback = _contract.getFallbackFunction()) + m_context.appendJumpTo(notFound); + + m_context << notFound; + if (fallback) { eth::AssemblyItem returnTag = m_context.pushNewTag(); fallback->accept(*this); @@ -194,6 +207,7 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) } else m_context << eth::Instruction::STOP; // function not found + for (auto const& it: interfaceFunctions) { FunctionTypePointer const& functionType = it.second; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 62df9205f..f91839edf 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -521,6 +521,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) break; case Location::Send: _functionCall.getExpression().accept(*this); + m_context << u256(0); // do not send gas (there still is the stipend) arguments.front()->accept(*this); appendTypeConversion(*arguments.front()->getType(), *function.getParameterTypes().front(), true); @@ -532,7 +533,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) strings(), Location::Bare, false, - false, + true, true ), {} From 0f8c4a7fbb34b8c92252e0e3096bdf0482929bec Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 14:34:10 +0200 Subject: [PATCH 25/26] Bare functions return success condition. --- libsolidity/ExpressionCompiler.cpp | 35 ++++++++++++++++------- libsolidity/Types.cpp | 6 ++-- test/libsolidity/SolidityEndToEndTest.cpp | 22 ++++++++++++++ 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index f91839edf..11de73082 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -1058,10 +1058,15 @@ void ExpressionCompiler::appendExternalFunctionCall( unsigned gasStackPos = m_context.currentToBaseStackOffset(gasValueSize); unsigned valueStackPos = m_context.currentToBaseStackOffset(1); + bool returnSuccessCondition = + _functionType.getLocation() == FunctionType::Location::Bare || + _functionType.getLocation() == FunctionType::Location::BareCallCode; //@todo only return the first return value for now Type const* firstType = _functionType.getReturnParameterTypes().empty() ? nullptr : _functionType.getReturnParameterTypes().front().get(); unsigned retSize = firstType ? firstType->getCalldataEncodedSize() : 0; + if (returnSuccessCondition) + retSize = 0; // return value actually is success condition m_context << u256(retSize) << u256(0); if (_functionType.isBareCall()) @@ -1112,19 +1117,27 @@ void ExpressionCompiler::appendExternalFunctionCall( else m_context << eth::Instruction::CALL; - //Propagate error condition (if CALL pushes 0 on stack). - m_context << eth::Instruction::ISZERO; - m_context.appendConditionalJumpTo(m_context.errorTag()); + unsigned remainsSize = 1 + // contract address + _functionType.valueSet() + + _functionType.gasSet() + + !_functionType.isBareCall(); - if (_functionType.valueSet()) - m_context << eth::Instruction::POP; - if (_functionType.gasSet()) - m_context << eth::Instruction::POP; - if (!_functionType.isBareCall()) - m_context << eth::Instruction::POP; - m_context << eth::Instruction::POP; // pop contract address + if (returnSuccessCondition) + m_context << eth::swapInstruction(remainsSize); + else + { + //Propagate error condition (if CALL pushes 0 on stack). + m_context << eth::Instruction::ISZERO; + m_context.appendConditionalJumpTo(m_context.errorTag()); + } - if (_functionType.getLocation() == FunctionType::Location::RIPEMD160) + CompilerUtils(m_context).popStackSlots(remainsSize); + + if (returnSuccessCondition) + { + // already there + } + else if (_functionType.getLocation() == FunctionType::Location::RIPEMD160) { // fix: built-in contract returns right-aligned data CompilerUtils(m_context).loadFromMemory(0, IntegerType(160), false, true); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 03e8e4ee0..1316bbc37 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -317,9 +317,9 @@ TypePointer IntegerType::binaryOperatorResult(Token::Value _operator, TypePointe const MemberList IntegerType::AddressMemberList({ {"balance", make_shared(256)}, - {"call", make_shared(strings(), strings(), FunctionType::Location::Bare, true)}, - {"callcode", make_shared(strings(), strings(), FunctionType::Location::BareCallCode, true)}, - {"send", make_shared(strings{"uint"}, strings{}, FunctionType::Location::Send)} + {"call", make_shared(strings(), strings{"bool"}, FunctionType::Location::Bare, true)}, + {"callcode", make_shared(strings(), strings{"bool"}, FunctionType::Location::BareCallCode, true)}, + {"send", make_shared(strings{"uint"}, strings{"bool"}, FunctionType::Location::Send)} }); IntegerConstantType::IntegerConstantType(Literal const& _literal) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 1d0f1fc22..89ed81e23 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4187,6 +4187,28 @@ BOOST_AUTO_TEST_CASE(positive_integers_to_signed) BOOST_CHECK(callContractFunction("q()") == encodeArgs(250)); } +BOOST_AUTO_TEST_CASE(failing_send) +{ + char const* sourceCode = R"( + contract Helper { + uint[] data; + function () { + data[9]; // trigger exception + } + } + contract Main { + function callHelper(address _a) returns (bool r, uint bal) { + r = !_a.send(5); + bal = this.balance; + } + } + )"; + compileAndRun(sourceCode, 0, "Helper"); + u160 const c_helperAddress = m_contractAddress; + compileAndRun(sourceCode, 20, "Main"); + BOOST_REQUIRE(callContractFunction("callHelper(address)", c_helperAddress) == encodeArgs(true, 20)); +} + BOOST_AUTO_TEST_SUITE_END() } From 089c9477b45a5cdf8fdbf69b922b73262f76625f Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 5 Jun 2015 17:38:06 +0200 Subject: [PATCH 26/26] Style. --- libsolidity/ExpressionCompiler.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 11de73082..bac967d84 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -1117,10 +1117,11 @@ void ExpressionCompiler::appendExternalFunctionCall( else m_context << eth::Instruction::CALL; - unsigned remainsSize = 1 + // contract address - _functionType.valueSet() + - _functionType.gasSet() + - !_functionType.isBareCall(); + unsigned remainsSize = + 1 + // contract address + _functionType.valueSet() + + _functionType.gasSet() + + !_functionType.isBareCall(); if (returnSuccessCondition) m_context << eth::swapInstruction(remainsSize);