From fd5ea37e5569b531ef6678b035e0a40b3c0e6c63 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Thu, 30 Apr 2015 14:07:29 +0100 Subject: [PATCH] Transaction nonce now "sorted". Fixes #1615 --- libdevcore/Common.h | 2 + libethcore/EthashAux.cpp | 6 +-- libethereum/ClientBase.cpp | 7 +++- libethereum/Transaction.h | 11 ++--- libethereum/TransactionQueue.cpp | 49 ++++++++++++++++++++--- libethereum/TransactionQueue.h | 7 +++- libweb3jsonrpc/WebThreeStubServerBase.cpp | 4 +- 7 files changed, 64 insertions(+), 22 deletions(-) diff --git a/libdevcore/Common.h b/libdevcore/Common.h index 49491d4cc..b2d48da98 100644 --- a/libdevcore/Common.h +++ b/libdevcore/Common.h @@ -52,6 +52,8 @@ using byte = uint8_t; #define DEV_QUOTED_HELPER(s) #s #define DEV_QUOTED(s) DEV_QUOTED_HELPER(s) +#define DEV_IGNORE_EXCEPTIONS(X) try { X; } catch (...) {} + namespace dev { diff --git a/libethcore/EthashAux.cpp b/libethcore/EthashAux.cpp index 19a96f550..9cb4d9fad 100644 --- a/libethcore/EthashAux.cpp +++ b/libethcore/EthashAux.cpp @@ -40,8 +40,6 @@ using namespace chrono; using namespace dev; using namespace eth; -#define ETH_IGNORE_EXCEPTIONS(X) try { X; } catch (...) {} - EthashAux* dev::eth::EthashAux::s_this = nullptr; EthashAux::~EthashAux() @@ -171,8 +169,8 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bytesRef _dest, bool boost::filesystem::rename(oldMemoFile, memoFile); } - ETH_IGNORE_EXCEPTIONS(boost::filesystem::remove(oldMemoFile)); - ETH_IGNORE_EXCEPTIONS(boost::filesystem::remove(oldMemoFile + ".info")); + DEV_IGNORE_EXCEPTIONS(boost::filesystem::remove(oldMemoFile)); + DEV_IGNORE_EXCEPTIONS(boost::filesystem::remove(oldMemoFile + ".info")); ethash_params p = params(_seedHash); assert(!_dest || _dest.size() >= p.full_size); // must be big enough. diff --git a/libethereum/ClientBase.cpp b/libethereum/ClientBase.cpp index 7c56bce4e..eba8dbc67 100644 --- a/libethereum/ClientBase.cpp +++ b/libethereum/ClientBase.cpp @@ -48,8 +48,11 @@ State ClientBase::asOf(BlockNumber _h) const void ClientBase::submitTransaction(Secret _secret, u256 _value, Address _dest, bytes const& _data, u256 _gas, u256 _gasPrice) { prepareForTransaction(); - - u256 n = postMine().transactionsFrom(toAddress(_secret)); + + auto a = toAddress(_secret); + u256 n = postMine().transactionsFrom(a); + cdebug << "submitTx: " << a << "postMine=" << n << "; tq=" << m_tq.maxNonce(a); + n = max(n, m_tq.maxNonce(a)); Transaction t(_value, _gasPrice, _gas, _dest, _data, n, _secret); m_tq.import(t.rlp()); diff --git a/libethereum/Transaction.h b/libethereum/Transaction.h index 7276493c2..09102e0ba 100644 --- a/libethereum/Transaction.h +++ b/libethereum/Transaction.h @@ -221,19 +221,14 @@ using Transactions = std::vector; /// Simple human-readable stream-shift operator. inline std::ostream& operator<<(std::ostream& _out, Transaction const& _t) { - _out << "{"; + _out << _t.sha3().abridged() << "{"; if (_t.receiveAddress()) _out << _t.receiveAddress().abridged(); else _out << "[CREATE]"; - _out << "/" << _t.nonce() << "$" << _t.value() << "+" << _t.gas() << "@" << _t.gasPrice(); - try - { - _out << "<-" << _t.sender().abridged(); - } - catch (...) {} - _out << " #" << _t.data().size() << "}"; + _out << "/" << _t.data().size() << "$" << _t.value() << "+" << _t.gas() << "@" << _t.gasPrice(); + _out << "<-" << _t.safeSender().abridged() << " #" << _t.nonce() << "}"; return _out; } diff --git a/libethereum/TransactionQueue.cpp b/libethereum/TransactionQueue.cpp index 40eec1ac5..57429d32c 100644 --- a/libethereum/TransactionQueue.cpp +++ b/libethereum/TransactionQueue.cpp @@ -53,7 +53,7 @@ ImportResult TransactionQueue::import(bytesConstRef _transactionRLP, ImportCallb UpgradeGuard ul(l); // If valid, append to blocks. - m_current[h] = t; + insertCurrent_WITH_LOCK(make_pair(h, t)); m_known.insert(h); if (_cb) m_callbacks[h] = _cb; @@ -74,13 +74,54 @@ ImportResult TransactionQueue::import(bytesConstRef _transactionRLP, ImportCallb return ImportResult::Success; } +u256 TransactionQueue::maxNonce(Address const& _a) const +{ + cdebug << "txQ::maxNonce" << _a; + ReadGuard l(m_lock); + u256 ret = 0; + auto r = m_senders.equal_range(_a); + for (auto it = r.first; it != r.second; ++it) + { + cdebug << it->first << "1+" << m_current.at(it->second).nonce(); + DEV_IGNORE_EXCEPTIONS(ret = max(ret, m_current.at(it->second).nonce() + 1)); + } + return ret; +} + +void TransactionQueue::insertCurrent_WITH_LOCK(std::pair const& _p) +{ + cdebug << "txQ::insertCurrent" << _p.first << _p.second.sender() << _p.second.nonce(); + m_senders.insert(make_pair(_p.second.sender(), _p.first)); + m_current.insert(_p); +} + +bool TransactionQueue::removeCurrent_WITH_LOCK(h256 const& _txHash) +{ + cdebug << "txQ::removeCurrent" << _txHash; + if (m_current.count(_txHash)) + { + auto r = m_senders.equal_range(m_current[_txHash].sender()); + for (auto it = r.first; it != r.second; ++it) + if (it->second == _txHash) + { + cdebug << "=> sender" << it->first; + m_senders.erase(it); + break; + } + cdebug << "=> nonce" << m_current[_txHash].nonce(); + m_current.erase(_txHash); + return true; + } + return false; +} + void TransactionQueue::setFuture(std::pair const& _t) { WriteGuard l(m_lock); if (m_current.count(_t.first)) { - m_current.erase(_t.first); m_unknown.insert(make_pair(_t.second.sender(), _t)); + m_current.erase(_t.first); } } @@ -104,9 +145,7 @@ void TransactionQueue::drop(h256 const& _txHash) m_dropped.insert(_txHash); m_known.erase(_txHash); - if (m_current.count(_txHash)) - m_current.erase(_txHash); - else + if (!removeCurrent_WITH_LOCK(_txHash)) { for (auto i = m_unknown.begin(); i != m_unknown.end(); ++i) if (i->second.first == _txHash) diff --git a/libethereum/TransactionQueue.h b/libethereum/TransactionQueue.h index 3858949cc..16bc34641 100644 --- a/libethereum/TransactionQueue.h +++ b/libethereum/TransactionQueue.h @@ -56,6 +56,7 @@ public: std::map transactions() const { ReadGuard l(m_lock); return m_current; } std::pair items() const { ReadGuard l(m_lock); return std::make_pair(m_current.size(), m_unknown.size()); } + u256 maxNonce(Address const& _a) const; void setFuture(std::pair const& _t); void noteGood(std::pair const& _t); @@ -64,12 +65,16 @@ public: template Handler onReady(T const& _t) { return m_onReady.add(_t); } private: - mutable SharedMutex m_lock; ///< General lock. + void insertCurrent_WITH_LOCK(std::pair const& _p); + bool removeCurrent_WITH_LOCK(h256 const& _txHash); + + mutable SharedMutex m_lock; ///< General lock. std::set m_known; ///< Hashes of transactions in both sets. std::map m_current; ///< Map of SHA3(tx) to tx. std::multimap> m_unknown; ///< For transactions that have a future nonce; we map their sender address to the tx stuff, and insert once the sender has a valid TX. std::map> m_callbacks; ///< Called once. std::set m_dropped; ///< Transactions that have previously been dropped. + std::multimap m_senders; ///< Mapping from the sender address to the transaction hash; useful for determining the nonce of a given sender. Signal m_onReady; ///< Called when a subsequent call to import transactions will return a non-empty container. Be nice and exit fast. }; diff --git a/libweb3jsonrpc/WebThreeStubServerBase.cpp b/libweb3jsonrpc/WebThreeStubServerBase.cpp index 208351fc6..2a1427b16 100644 --- a/libweb3jsonrpc/WebThreeStubServerBase.cpp +++ b/libweb3jsonrpc/WebThreeStubServerBase.cpp @@ -505,7 +505,7 @@ string WebThreeStubServerBase::eth_sendTransaction(Json::Value const& _json) if (!t.gasPrice) t.gasPrice = 10 * dev::eth::szabo; // TODO: should be determined by user somehow. if (!t.gas) - t.gas = min(client()->gasLimitRemaining(), client()->balanceAt(t.from) / t.gasPrice); + t.gas = min(client()->gasLimitRemaining() / 5, client()->balanceAt(t.from) / t.gasPrice); if (m_accounts->isRealAccount(t.from)) authenticate(t, false); @@ -534,7 +534,7 @@ string WebThreeStubServerBase::eth_signTransaction(Json::Value const& _json) if (!t.gasPrice) t.gasPrice = 10 * dev::eth::szabo; // TODO: should be determined by user somehow. if (!t.gas) - t.gas = min(client()->gasLimitRemaining(), client()->balanceAt(t.from) / t.gasPrice); + t.gas = min(client()->gasLimitRemaining() / 5, client()->balanceAt(t.from) / t.gasPrice); if (m_accounts->isRealAccount(t.from)) authenticate(t, false);