remove internal locking from DBTransaction (openethereum#2003)

ethcore/src/blockchain/blockchain.rs

@@ -380,7 +380,7 @@ impl BlockChain {
 					children: vec![]
 				};
 
-				let batch = DBTransaction::new(&db);
+				let mut batch = DBTransaction::new(&db);
 				batch.put(db::COL_HEADERS, &hash, block.header_rlp().as_raw());
 				batch.put(db::COL_BODIES, &hash, &Self::block_to_body(genesis));
 
@@ -419,7 +419,7 @@ impl BlockChain {
 					}
 				}
 
-				let batch = db.transaction();
+				let mut batch = db.transaction();
 				batch.put(db::COL_EXTRA, b"first", &hash);
 				db.write(batch).expect("Low level database error.");
 
@@ -451,7 +451,7 @@ impl BlockChain {
 	#[cfg(test)]
 	fn rewind(&self) -> Option<H256> {
 		use db::Key;
-		let batch = self.db.transaction();
+		let mut batch =self.db.transaction();
 		// track back to the best block we have in the blocks database
 		if let Some(best_block_hash) = self.db.get(db::COL_EXTRA, b"best").unwrap() {
 			let best_block_hash = H256::from_slice(&best_block_hash);
@@ -604,7 +604,7 @@ impl BlockChain {
 
 		assert!(self.pending_best_block.read().is_none());
 
-		let batch = self.db.transaction();
+		let mut batch = self.db.transaction();
 
 		let block_rlp = UntrustedRlp::new(bytes);
 		let compressed_header = block_rlp.at(0).unwrap().compress(RlpType::Blocks);
@@ -625,7 +625,7 @@ impl BlockChain {
 				location: BlockLocation::CanonChain,
 			};
 
-			self.prepare_update(&batch, ExtrasUpdate {
+			self.prepare_update(&mut batch, ExtrasUpdate {
 				block_hashes: self.prepare_block_hashes_update(bytes, &info),
 				block_details: self.prepare_block_details_update(bytes, &info),
 				block_receipts: self.prepare_block_receipts_update(receipts, &info),
@@ -659,7 +659,7 @@ impl BlockChain {
 			let mut update = HashMap::new();
 			update.insert(hash, block_details);
 
-			self.prepare_update(&batch, ExtrasUpdate {
+			self.prepare_update(&mut batch, ExtrasUpdate {
 				block_hashes: self.prepare_block_hashes_update(bytes, &info),
 				block_details: update,
 				block_receipts: self.prepare_block_receipts_update(receipts, &info),
@@ -682,7 +682,7 @@ impl BlockChain {
 		let mut parent_details = self.block_details(&block_hash)
 			.unwrap_or_else(|| panic!("Invalid block hash: {:?}", block_hash));
 
-		let batch = self.db.transaction();
+		let mut batch = self.db.transaction();
 		parent_details.children.push(child_hash);
 
 		let mut update = HashMap::new();
@@ -701,7 +701,7 @@ impl BlockChain {
 	/// Inserts the block into backing cache database.
 	/// Expects the block to be valid and already verified.
 	/// If the block is already known, does nothing.
-	pub fn insert_block(&self, batch: &DBTransaction, bytes: &[u8], receipts: Vec<Receipt>) -> ImportRoute {
+	pub fn insert_block(&self, batch: &mut DBTransaction, bytes: &[u8], receipts: Vec<Receipt>) -> ImportRoute {
 		// create views onto rlp
 		let block = BlockView::new(bytes);
 		let header = block.header_view();
@@ -782,7 +782,7 @@ impl BlockChain {
 	}
 
 	/// Prepares extras update.
-	fn prepare_update(&self, batch: &DBTransaction, update: ExtrasUpdate, is_best: bool) {
+	fn prepare_update(&self, batch: &mut DBTransaction, update: ExtrasUpdate, is_best: bool) {
 		{
 			let block_hashes: Vec<_> = update.block_details.keys().cloned().collect();
 
@@ -1147,8 +1147,8 @@ mod tests {
 		assert_eq!(bc.best_block_number(), 0);
 
 		// when
-		let batch = db.transaction();
-		bc.insert_block(&batch, &first, vec![]);
+		let mut batch =db.transaction();
+		bc.insert_block(&mut batch, &first, vec![]);
 		assert_eq!(bc.best_block_number(), 0);
 		bc.commit();
 		// NOTE no db.write here (we want to check if best block is cached)
@@ -1177,8 +1177,8 @@ mod tests {
 		assert_eq!(bc.block_hash(1), None);
 		assert_eq!(bc.block_details(&genesis_hash).unwrap().children, vec![]);
 
-		let batch = db.transaction();
-		bc.insert_block(&batch, &first, vec![]);
+		let mut batch =db.transaction();
+		bc.insert_block(&mut batch, &first, vec![]);
 		db.write(batch).unwrap();
 		bc.commit();
 
@@ -1203,11 +1203,11 @@ mod tests {
 		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 
 		let mut block_hashes = vec![genesis_hash.clone()];
-		let batch = db.transaction();
+		let mut batch =db.transaction();
 		for _ in 0..10 {
 			let block = canon_chain.generate(&mut finalizer).unwrap();
 			block_hashes.push(BlockView::new(&block).header_view().sha3());
-			bc.insert_block(&batch, &block, vec![]);
+			bc.insert_block(&mut batch, &block, vec![]);
 			bc.commit();
 		}
 		db.write(batch).unwrap();
@@ -1238,20 +1238,20 @@ mod tests {
 		let db = new_db(temp.as_str());
 		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 
-		let batch = db.transaction();
+		let mut batch =db.transaction();
 		for b in &[&b1a, &b1b, &b2a, &b2b, &b3a, &b3b, &b4a, &b4b, &b5a, &b5b] {
-			bc.insert_block(&batch, b, vec![]);
+			bc.insert_block(&mut batch, b, vec![]);
 			bc.commit();
 		}
-		bc.insert_block(&batch, &b1b, vec![]);
-		bc.insert_block(&batch, &b2a, vec![]);
-		bc.insert_block(&batch, &b2b, vec![]);
-		bc.insert_block(&batch, &b3a, vec![]);
-		bc.insert_block(&batch, &b3b, vec![]);
-		bc.insert_block(&batch, &b4a, vec![]);
-		bc.insert_block(&batch, &b4b, vec![]);
-		bc.insert_block(&batch, &b5a, vec![]);
-		bc.insert_block(&batch, &b5b, vec![]);
+		bc.insert_block(&mut batch, &b1b, vec![]);
+		bc.insert_block(&mut batch, &b2a, vec![]);
+		bc.insert_block(&mut batch, &b2b, vec![]);
+		bc.insert_block(&mut batch, &b3a, vec![]);
+		bc.insert_block(&mut batch, &b3b, vec![]);
+		bc.insert_block(&mut batch, &b4a, vec![]);
+		bc.insert_block(&mut batch, &b4b, vec![]);
+		bc.insert_block(&mut batch, &b5a, vec![]);
+		bc.insert_block(&mut batch, &b5b, vec![]);
 		db.write(batch).unwrap();
 
 		assert_eq!(
@@ -1286,17 +1286,17 @@ mod tests {
 		let db = new_db(temp.as_str());
 		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 
-		let batch = db.transaction();
-		let ir1 = bc.insert_block(&batch, &b1, vec![]);
+		let mut batch =db.transaction();
+		let ir1 = bc.insert_block(&mut batch, &b1, vec![]);
 		bc.commit();
-		let ir2 = bc.insert_block(&batch, &b2, vec![]);
+		let ir2 = bc.insert_block(&mut batch, &b2, vec![]);
 		bc.commit();
-		let ir3b = bc.insert_block(&batch, &b3b, vec![]);
+		let ir3b = bc.insert_block(&mut batch, &b3b, vec![]);
 		bc.commit();
 		db.write(batch).unwrap();
 		assert_eq!(bc.block_hash(3).unwrap(), b3b_hash);
-		let batch = db.transaction();
-		let ir3a = bc.insert_block(&batch, &b3a, vec![]);
+		let mut batch =db.transaction();
+		let ir3a = bc.insert_block(&mut batch, &b3a, vec![]);
 		bc.commit();
 		db.write(batch).unwrap();
 
@@ -1402,8 +1402,8 @@ mod tests {
 			let db = new_db(temp.as_str());
 			let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 			assert_eq!(bc.best_block_hash(), genesis_hash);
-			let batch = db.transaction();
-			bc.insert_block(&batch, &first, vec![]);
+			let mut batch =db.transaction();
+			bc.insert_block(&mut batch, &first, vec![]);
 			db.write(batch).unwrap();
 			bc.commit();
 			assert_eq!(bc.best_block_hash(), first_hash);
@@ -1467,8 +1467,8 @@ mod tests {
 		let temp = RandomTempPath::new();
 		let db = new_db(temp.as_str());
 		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
-		let batch = db.transaction();
-		bc.insert_block(&batch, &b1, vec![]);
+		let mut batch =db.transaction();
+		bc.insert_block(&mut batch, &b1, vec![]);
 		db.write(batch).unwrap();
 		bc.commit();
 
@@ -1480,8 +1480,8 @@ mod tests {
 	}
 
 	fn insert_block(db: &Arc<Database>, bc: &BlockChain, bytes: &[u8], receipts: Vec<Receipt>) -> ImportRoute {
-		let batch = db.transaction();
-		let res = bc.insert_block(&batch, bytes, receipts);
+		let mut batch =db.transaction();
+		let res = bc.insert_block(&mut batch, bytes, receipts);
 		db.write(batch).unwrap();
 		bc.commit();
 		res
@@ -1569,16 +1569,16 @@ mod tests {
 			let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 			let uncle = canon_chain.fork(1).generate(&mut finalizer.fork()).unwrap();
 
-			let batch = db.transaction();
+			let mut batch =db.transaction();
 			// create a longer fork
 			for _ in 0..5 {
 				let canon_block = canon_chain.generate(&mut finalizer).unwrap();
-				bc.insert_block(&batch, &canon_block, vec![]);
+				bc.insert_block(&mut batch, &canon_block, vec![]);
 				bc.commit();
 			}
 
 			assert_eq!(bc.best_block_number(), 5);
-			bc.insert_block(&batch, &uncle, vec![]);
+			bc.insert_block(&mut batch, &uncle, vec![]);
 			db.write(batch).unwrap();
 			bc.commit();
 		}
@@ -1604,10 +1604,10 @@ mod tests {
 		let db = new_db(temp.as_str());
 		let bc = BlockChain::new(Config::default(), &genesis, db.clone());
 
-		let batch = db.transaction();
-		bc.insert_block(&batch, &first, vec![]);
+		let mut batch =db.transaction();
+		bc.insert_block(&mut batch, &first, vec![]);
 		bc.commit();
-		bc.insert_block(&batch, &second, vec![]);
+		bc.insert_block(&mut batch, &second, vec![]);
 		bc.commit();
 		db.write(batch).unwrap();
 

ethcore/src/client/client.rs

@@ -172,8 +172,8 @@ impl Client {
 
 		let mut state_db = journaldb::new(db.clone(), config.pruning, ::db::COL_STATE);
 		if state_db.is_empty() && try!(spec.ensure_db_good(state_db.as_hashdb_mut())) {
-			let batch = DBTransaction::new(&db);
-			try!(state_db.commit(&batch, 0, &spec.genesis_header().hash(), None));
+			let mut batch = DBTransaction::new(&db);
+			try!(state_db.commit(&mut batch, 0, &spec.genesis_header().hash(), None));
 			try!(db.write(batch).map_err(ClientError::Database));
 		}
 
@@ -431,14 +431,14 @@ impl Client {
 
 		//let traces = From::from(block.traces().clone().unwrap_or_else(Vec::new));
 
-		let batch = DBTransaction::new(&self.db);
+		let mut batch = DBTransaction::new(&self.db);
 		// CHECK! I *think* this is fine, even if the state_root is equal to another
 		// already-imported block of the same number.
 		// TODO: Prove it with a test.
-		block.drain().commit(&batch, number, hash, ancient).expect("DB commit failed.");
+		block.drain().commit(&mut batch, number, hash, ancient).expect("DB commit failed.");
 
-		let route = self.chain.insert_block(&batch, block_data, receipts);
-		self.tracedb.import(&batch, TraceImportRequest {
+		let route = self.chain.insert_block(&mut batch, block_data, receipts);
+		self.tracedb.import(&mut batch, TraceImportRequest {
 			traces: traces.into(),
 			block_hash: hash.clone(),
 			block_number: number,

ethcore/src/db.rs

@@ -83,10 +83,10 @@ pub trait Key<T> {
 /// Should be used to write value into database.
 pub trait Writable {
 	/// Writes the value into the database.
-	fn write<T, R>(&self, col: Option<u32>, key: &Key<T, Target = R>, value: &T) where T: Encodable, R: Deref<Target = [u8]>;
+	fn write<T, R>(&mut self, col: Option<u32>, key: &Key<T, Target = R>, value: &T) where T: Encodable, R: Deref<Target = [u8]>;
 
 	/// Writes the value into the database and updates the cache.
-	fn write_with_cache<K, T, R>(&self, col: Option<u32>, cache: &mut Cache<K, T>, key: K, value: T, policy: CacheUpdatePolicy) where
+	fn write_with_cache<K, T, R>(&mut self, col: Option<u32>, cache: &mut Cache<K, T>, key: K, value: T, policy: CacheUpdatePolicy) where
 	K: Key<T, Target = R> + Hash + Eq,
 	T: Encodable,
 	R: Deref<Target = [u8]> {
@@ -102,7 +102,7 @@ pub trait Writable {
 	}
 
 	/// Writes the values into the database and updates the cache.
-	fn extend_with_cache<K, T, R>(&self, col: Option<u32>, cache: &mut Cache<K, T>, values: HashMap<K, T>, policy: CacheUpdatePolicy) where
+	fn extend_with_cache<K, T, R>(&mut self, col: Option<u32>, cache: &mut Cache<K, T>, values: HashMap<K, T>, policy: CacheUpdatePolicy) where
 	K: Key<T, Target = R> + Hash + Eq,
 	T: Encodable,
 	R: Deref<Target = [u8]> {
@@ -169,7 +169,7 @@ pub trait Readable {
 }
 
 impl Writable for DBTransaction {
-	fn write<T, R>(&self, col: Option<u32>, key: &Key<T, Target = R>, value: &T) where T: Encodable, R: Deref<Target = [u8]> {
+	fn write<T, R>(&mut self, col: Option<u32>, key: &Key<T, Target = R>, value: &T) where T: Encodable, R: Deref<Target = [u8]> {
 		self.put(col, &key.key(), &encode(value));
 	}
 }

ethcore/src/snapshot/mod.rs

@@ -492,8 +492,8 @@ impl StateRebuilder {
 		}
 
 		let backing = self.db.backing().clone();
-		let batch = backing.transaction();
-		try!(self.db.inject(&batch));
+		let mut batch = backing.transaction();
+		try!(self.db.inject(&mut batch));
 		try!(backing.write(batch).map_err(::util::UtilError::SimpleString));
 		trace!(target: "snapshot", "current state root: {:?}", self.state_root);
 		Ok(())

ethcore/src/snapshot/tests/blocks.rs

@@ -43,14 +43,16 @@ fn chunk_and_restore(amount: u64) {
 	let bc = BlockChain::new(Default::default(), &genesis, old_db.clone());
 
 	// build the blockchain.
+	let mut batch = old_db.transaction();
 	for _ in 0..amount {
 		let block = canon_chain.generate(&mut finalizer).unwrap();
-		let batch = old_db.transaction();
-		bc.insert_block(&batch, &block, vec![]);
+		bc.insert_block(&mut batch, &block, vec![]);
 		bc.commit();
-		old_db.write(batch).unwrap();
 	}
 
+	old_db.write(batch).unwrap();
+
+
 	let best_hash = bc.best_block_hash();
 
 	// snapshot it.

ethcore/src/tests/helpers.rs

@@ -259,9 +259,9 @@ pub fn generate_dummy_blockchain(block_number: u32) -> GuardedTempResult<BlockCh
 	let db = new_db(temp.as_str());
 	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone());
 
-	let batch = db.transaction();
+	let mut batch = db.transaction();
 	for block_order in 1..block_number {
-		bc.insert_block(&batch, &create_unverifiable_block(block_order, bc.best_block_hash()), vec![]);
+		bc.insert_block(&mut batch, &create_unverifiable_block(block_order, bc.best_block_hash()), vec![]);
 		bc.commit();
 	}
 	db.write(batch).unwrap();
@@ -278,9 +278,9 @@ pub fn generate_dummy_blockchain_with_extra(block_number: u32) -> GuardedTempRes
 	let bc = BlockChain::new(BlockChainConfig::default(), &create_unverifiable_block(0, H256::zero()), db.clone());
 
 
-	let batch = db.transaction();
+	let mut batch = db.transaction();
 	for block_order in 1..block_number {
-		bc.insert_block(&batch, &create_unverifiable_block_with_extra(block_order, bc.best_block_hash(), None), vec![]);
+		bc.insert_block(&mut batch, &create_unverifiable_block_with_extra(block_order, bc.best_block_hash(), None), vec![]);
 		bc.commit();
 	}
 	db.write(batch).unwrap();