Remove reprocess RPC method and associated type

The reprocess admin RPC method is the only function which depends on the old Block::populateFromChain functionality where it replays each tx in the block. There's no need for the function so we can remove it. Additionally, check for StateDB corruption in Block::populateFromChain before setting the state root so that we can log a user-friendly error message.
ethereum · Oct 25, 2019 · c3d9fbf · c3d9fbf
1 parent 89606dd
commit c3d9fbf
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 72 deletions.
diff --git a/libethereum/Block.cpp b/libethereum/Block.cpp
@@ -136,7 +136,7 @@ void Block::noteChain(BlockChain const& _bc)
     }
 }
 
-PopulationStatistics Block::populateFromChain(BlockChain const& _bc, h256 const& _h, ImportRequirements::value _ir)
+void Block::populateFromChain(BlockChain const& _bc, h256 const& _h)
 {
     if (!_bc.isKnown(_h))
     {
@@ -147,21 +147,36 @@ PopulationStatistics Block::populateFromChain(BlockChain const& _bc, h256 const&
     auto const& blockBytes = _bc.block(_h);
     m_currentBytes = blockBytes;
 
+    // Set block headers
     auto const& blockHeader = BlockHeader{blockBytes};
     m_currentBlock = blockHeader;
 
+    if (blockHeader.number())
+        m_previousBlock = _bc.info(blockHeader.parentHash());
+    else
+        m_previousBlock = m_currentBlock;
+
     // Set state root and precommit state
+    //
+    // First check for database corruption by looking up the state root in the state database. Note
+    // that we don't technically need to do this since if the state DB is corrupt setting a new
+    // state root will throw anyway, but checking here enables us to log a user-friendly error
+    // message.
+    if (m_state.db().lookup(blockHeader.stateRoot()).empty())
+    {
+        cerr << "Unable to populate block " << blockHeader.hash() << " - state root"
+             << blockHeader.stateRoot() << " not found in database.";
+        cerr << "Database corrupt: contains block without stateRoot:" << blockHeader;
+        cerr << "Try rescuing the database by running: eth --rescue";
+        BOOST_THROW_EXCEPTION(InvalidStateRoot() << errinfo_target(blockHeader.stateRoot()));
+    }
+
     if (blockHeader.number())
         m_state.setRoot(blockHeader.stateRoot());
     else
         m_state = State{m_state.accountStartNonce(), m_state.db(), BaseState::Empty};
     m_precommit = m_state;
 
-    if (blockHeader.number())
-        m_previousBlock = _bc.info(blockHeader.hash());
-    else
-        m_previousBlock = m_currentBlock;
-
     RLP blockRLP{blockBytes};
     auto const& txListRLP = blockRLP[1];
     for (auto const& txRLP : txListRLP)
@@ -177,8 +192,6 @@ PopulationStatistics Block::populateFromChain(BlockChain const& _bc, h256 const&
 
     m_committedToSeal = false;
     m_sealEngine = _bc.sealEngine();
-
-    return PopulationStatistics{};
 }
 
 bool Block::sync(BlockChain const& _bc)

diff --git a/libethereum/Block.h b/libethereum/Block.h
@@ -35,12 +35,6 @@ class TransactionQueue;
 struct VerifiedBlockRef;
 class LastBlockHashesFace;
 
-struct PopulationStatistics
-{
-    double verify;
-    double enact;
-};
-
 DEV_SIMPLE_EXCEPTION(ChainOperationWithUnknownBlockChain);
 DEV_SIMPLE_EXCEPTION(InvalidOperationOnSealedBlock);
 
@@ -190,7 +184,7 @@ class Block
     // State-change operations
 
     /// Construct state object from arbitrary point in blockchain.
-    PopulationStatistics populateFromChain(BlockChain const& _bc, h256 const& _hash, ImportRequirements::value _ir = ImportRequirements::None);
+    void populateFromChain(BlockChain const& _bc, h256 const& _hash);
 
     /// Execute a given transaction.
     /// This will append @a _t to the transaction list and change the state accordingly.

diff --git a/libethereum/Client.cpp b/libethereum/Client.cpp
@@ -759,37 +759,19 @@ void Client::prepareForTransaction()
     startWorking();
 }
 
-Block Client::block(h256 const& _block) const
+Block Client::block(h256 const& _blockHash) const
 {
     try
     {
-        Block ret(bc(), m_stateDB);
-        ret.populateFromChain(bc(), _block);
-        return ret;
-    }
-    catch (Exception& ex)
-    {
-        ex << errinfo_block(bc().block(_block));
-        onBadBlock(ex);
-        return Block(bc());
-    }
-}
-
-Block Client::block(h256 const& _blockHash, PopulationStatistics* o_stats) const
-{
-    try
-    {
-        Block ret(bc(), m_stateDB);
-        PopulationStatistics s = ret.populateFromChain(bc(), _blockHash);
-        if (o_stats)
-            swap(s, *o_stats);
+        Block ret{bc(), m_stateDB};
+        ret.populateFromChain(bc(), _blockHash);
         return ret;
     }
     catch (Exception& ex)
     {
         ex << errinfo_block(bc().block(_blockHash));
         onBadBlock(ex);
-        return Block(bc());
+        return Block{bc()};
     }
 }
 

diff --git a/libethereum/Client.h b/libethereum/Client.h
@@ -100,10 +100,6 @@ class Client: public ClientBase, protected Worker
     /// Get the gas bid price
     u256 gasBidPrice() const override { return m_gp->bid(); }
 
-    // [PRIVATE API - only relevant for base clients, not available in general]
-    /// Get the block.
-    dev::eth::Block block(h256 const& _blockHash, PopulationStatistics* o_stats) const;
-
     /// Get the object representing the current state of Ethereum.
     dev::eth::Block postState() const { ReadGuard l(x_postSeal); return m_postSeal; }
     /// Get the object representing the current canonical blockchain.

diff --git a/libweb3jsonrpc/AdminEth.cpp b/libweb3jsonrpc/AdminEth.cpp
@@ -179,18 +179,6 @@ h256 AdminEth::blockHash(string const& _blockNumberOrHash) const
     }
 }
 
-Json::Value AdminEth::admin_eth_reprocess(string const& _blockNumberOrHash, string const& _session)
-{
-    RPC_ADMIN;
-    Json::Value ret;
-    PopulationStatistics ps;
-    m_eth.block(blockHash(_blockNumberOrHash), &ps);
-    ret["enact"] = ps.enact;
-    ret["verify"] = ps.verify;
-    ret["total"] = ps.verify + ps.enact;
-    return ret;
-}
-
 Json::Value AdminEth::admin_eth_vmTrace(string const& _blockNumberOrHash, int _txIndex, string const& _session)
 {
     RPC_ADMIN;

diff --git a/libweb3jsonrpc/AdminEth.h b/libweb3jsonrpc/AdminEth.h
@@ -38,7 +38,6 @@ class AdminEth: public AdminEthFace
 	virtual Json::Value admin_eth_newAccount(const Json::Value& _info, std::string const& _session) override;
 	virtual bool admin_eth_setMiningBenefactor(std::string const& _uuidOrAddress, std::string const& _session) override;
 	virtual Json::Value admin_eth_inspect(std::string const& _address, std::string const& _session) override;
-	virtual Json::Value admin_eth_reprocess(std::string const& _blockNumberOrHash, std::string const& _session) override;
 	virtual Json::Value admin_eth_vmTrace(std::string const& _blockNumberOrHash, int _txIndex, std::string const& _session) override;
 	virtual Json::Value admin_eth_getReceiptByHashAndIndex(std::string const& _blockNumberOrHash, int _txIndex, std::string const& _session) override;
 	virtual bool miner_start(int _threads) override;

diff --git a/libweb3jsonrpc/AdminEthFace.h b/libweb3jsonrpc/AdminEthFace.h
@@ -25,7 +25,6 @@ namespace dev {
                     this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_newAccount", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_OBJECT, "param1",jsonrpc::JSON_OBJECT,"param2",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_newAccountI);
                     this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_setMiningBenefactor", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_STRING,"param2",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_setMiningBenefactorI);
                     this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_inspect", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_OBJECT, "param1",jsonrpc::JSON_STRING,"param2",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_inspectI);
-                    this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_reprocess", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_OBJECT, "param1",jsonrpc::JSON_STRING,"param2",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_reprocessI);
                     this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_vmTrace", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_OBJECT, "param1",jsonrpc::JSON_STRING,"param2",jsonrpc::JSON_INTEGER,"param3",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_vmTraceI);
                     this->bindAndAddMethod(jsonrpc::Procedure("admin_eth_getReceiptByHashAndIndex", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_OBJECT, "param1",jsonrpc::JSON_STRING,"param2",jsonrpc::JSON_INTEGER,"param3",jsonrpc::JSON_STRING, NULL), &dev::rpc::AdminEthFace::admin_eth_getReceiptByHashAndIndexI);
                     this->bindAndAddMethod(jsonrpc::Procedure("miner_start", jsonrpc::PARAMS_BY_POSITION, jsonrpc::JSON_BOOLEAN, "param1",jsonrpc::JSON_INTEGER, NULL), &dev::rpc::AdminEthFace::miner_startI);
@@ -80,10 +79,6 @@ namespace dev {
                 {
                     response = this->admin_eth_inspect(request[0u].asString(), request[1u].asString());
                 }
-                inline virtual void admin_eth_reprocessI(const Json::Value &request, Json::Value &response)
-                {
-                    response = this->admin_eth_reprocess(request[0u].asString(), request[1u].asString());
-                }
                 inline virtual void admin_eth_vmTraceI(const Json::Value &request, Json::Value &response)
                 {
                     response = this->admin_eth_vmTrace(request[0u].asString(), request[1u].asInt(), request[2u].asString());
@@ -129,7 +124,6 @@ namespace dev {
                 virtual Json::Value admin_eth_newAccount(const Json::Value& param1, const std::string& param2) = 0;
                 virtual bool admin_eth_setMiningBenefactor(const std::string& param1, const std::string& param2) = 0;
                 virtual Json::Value admin_eth_inspect(const std::string& param1, const std::string& param2) = 0;
-                virtual Json::Value admin_eth_reprocess(const std::string& param1, const std::string& param2) = 0;
                 virtual Json::Value admin_eth_vmTrace(const std::string& param1, int param2, const std::string& param3) = 0;
                 virtual Json::Value admin_eth_getReceiptByHashAndIndex(const std::string& param1, int param2, const std::string& param3) = 0;
                 virtual bool miner_start(int param1) = 0;

diff --git a/libweb3jsonrpc/admin_eth.json b/libweb3jsonrpc/admin_eth.json
@@ -10,7 +10,6 @@
 { "name": "admin_eth_newAccount", "params": [{}, ""], "returns": {} },
 { "name": "admin_eth_setMiningBenefactor", "params": ["", ""], "returns": true },
 { "name": "admin_eth_inspect", "params": ["", ""], "returns": {} },
-{ "name": "admin_eth_reprocess", "params": ["", ""], "returns": {} },
 { "name": "admin_eth_vmTrace", "params": ["", 0, ""], "returns": {} },
 { "name": "admin_eth_getReceiptByHashAndIndex", "params": ["", 0, ""], "returns": {} },
 { "name": "miner_start", "params": [0], "returns": true },

diff --git a/test/unittests/libweb3jsonrpc/WebThreeStubClient.h b/test/unittests/libweb3jsonrpc/WebThreeStubClient.h
@@ -924,17 +924,6 @@ class WebThreeStubClient : public jsonrpc::Client
             else
                 throw jsonrpc::JsonRpcException(jsonrpc::Errors::ERROR_CLIENT_INVALID_RESPONSE, result.toStyledString());
         }
-        Json::Value admin_eth_reprocess(const std::string& param1, const std::string& param2) throw (jsonrpc::JsonRpcException)
-        {
-            Json::Value p;
-            p.append(param1);
-            p.append(param2);
-            Json::Value result = this->CallMethod("admin_eth_reprocess",p);
-            if (result.isObject())
-                return result;
-            else
-                throw jsonrpc::JsonRpcException(jsonrpc::Errors::ERROR_CLIENT_INVALID_RESPONSE, result.toStyledString());
-        }
         Json::Value admin_eth_vmTrace(const std::string& param1, int param2, const std::string& param3) throw (jsonrpc::JsonRpcException)
         {
             Json::Value p;