pass slices around and not refs to vecs (#3404)

* pass slices around and not refs to vecs

* use slice.swap()

* use inputs() not body.inputs

core/src/core/block.rs

@@ -538,7 +538,7 @@ impl Block {
 	#[warn(clippy::new_ret_no_self)]
 	pub fn new(
 		prev: &BlockHeader,
-		txs: Vec<Transaction>,
+		txs: &[Transaction],
 		difficulty: Difficulty,
 		reward_output: (Output, TxKernel),
 	) -> Result<Block, Error> {
@@ -557,7 +557,7 @@ impl Block {
 	/// Hydrate a block from a compact block.
 	/// Note: caller must validate the block themselves, we do not validate it
 	/// here.
-	pub fn hydrate_from(cb: CompactBlock, txs: Vec<Transaction>) -> Result<Block, Error> {
+	pub fn hydrate_from(cb: CompactBlock, txs: &[Transaction]) -> Result<Block, Error> {
 		trace!("block: hydrate_from: {}, {} txs", cb.hash(), txs.len(),);
 
 		let header = cb.header.clone();
@@ -568,10 +568,9 @@ impl Block {
 
 		// collect all the inputs, outputs and kernels from the txs
 		for tx in txs {
-			let tb: TransactionBody = tx.into();
-			all_inputs.extend(tb.inputs);
-			all_outputs.extend(tb.outputs);
-			all_kernels.extend(tb.kernels);
+			all_inputs.extend(tx.inputs());
+			all_outputs.extend(tx.outputs());
+			all_kernels.extend(tx.kernels());
 		}
 
 		// include the coinbase output(s) and kernel(s) from the compact_block
@@ -587,7 +586,7 @@ impl Block {
 		let all_kernels = Vec::from_iter(all_kernels);
 
 		// Initialize a tx body and sort everything.
-		let body = TransactionBody::init(all_inputs, all_outputs, all_kernels, false)?;
+		let body = TransactionBody::init(&all_inputs, &all_outputs, &all_kernels, false)?;
 
 		// Finally return the full block.
 		// Note: we have not actually validated the block here,
@@ -608,7 +607,7 @@ impl Block {
 	/// that all transactions are valid and calculates the Merkle tree.
 	pub fn from_reward(
 		prev: &BlockHeader,
-		txs: Vec<Transaction>,
+		txs: &[Transaction],
 		reward_out: Output,
 		reward_kern: TxKernel,
 		difficulty: Difficulty,
@@ -663,32 +662,32 @@ impl Block {
 	}
 
 	/// Get inputs
-	pub fn inputs(&self) -> &Vec<Input> {
+	pub fn inputs(&self) -> &[Input] {
 		&self.body.inputs
 	}
 
 	/// Get inputs mutable
-	pub fn inputs_mut(&mut self) -> &mut Vec<Input> {
+	pub fn inputs_mut(&mut self) -> &mut [Input] {
 		&mut self.body.inputs
 	}
 
 	/// Get outputs
-	pub fn outputs(&self) -> &Vec<Output> {
+	pub fn outputs(&self) -> &[Output] {
 		&self.body.outputs
 	}
 
 	/// Get outputs mutable
-	pub fn outputs_mut(&mut self) -> &mut Vec<Output> {
+	pub fn outputs_mut(&mut self) -> &mut [Output] {
 		&mut self.body.outputs
 	}
 
 	/// Get kernels
-	pub fn kernels(&self) -> &Vec<TxKernel> {
+	pub fn kernels(&self) -> &[TxKernel] {
 		&self.body.kernels
 	}
 
 	/// Get kernels mut
-	pub fn kernels_mut(&mut self) -> &mut Vec<TxKernel> {
+	pub fn kernels_mut(&mut self) -> &mut [TxKernel] {
 		&mut self.body.kernels
 	}
 
@@ -702,14 +701,12 @@ impl Block {
 	/// elimination is stable with respect to the order of inputs and outputs.
 	/// Method consumes the block.
 	pub fn cut_through(self) -> Result<Block, Error> {
-		let mut inputs = self.inputs().clone();
-		let mut outputs = self.outputs().clone();
-		transaction::cut_through(&mut inputs, &mut outputs)?;
-
-		let kernels = self.kernels().clone();
+		let mut inputs = self.inputs().to_vec();
+		let mut outputs = self.outputs().to_vec();
+		let (inputs, outputs) = transaction::cut_through(&mut inputs, &mut outputs)?;
 
 		// Initialize tx body and sort everything.
-		let body = TransactionBody::init(inputs, outputs, kernels, false)?;
+		let body = TransactionBody::init(inputs, outputs, self.kernels(), false)?;
 
 		Ok(Block {
 			header: self.header,
@@ -809,7 +806,7 @@ impl Block {
 
 	// Verify any absolute kernel lock heights.
 	fn verify_kernel_lock_heights(&self) -> Result<(), Error> {
-		for k in &self.body.kernels {
+		for k in self.kernels() {
 			// check we have no kernels with lock_heights greater than current height
 			// no tx can be included in a block earlier than its lock_height
 			if let KernelFeatures::HeightLocked { lock_height, .. } = k.features {
@@ -825,7 +822,7 @@ impl Block {
 	// NRD kernels were introduced in HF3 and are not valid for block version < 4.
 	// Blocks prior to HF3 containing any NRD kernel(s) are invalid.
 	fn verify_nrd_kernels_for_header_version(&self) -> Result<(), Error> {
-		if self.body.kernels.iter().any(|k| k.is_nrd()) {
+		if self.kernels().iter().any(|k| k.is_nrd()) {
 			if !global::is_nrd_enabled() {
 				return Err(Error::NRDKernelNotEnabled);
 			}

core/src/core/compact_block.rs

@@ -146,17 +146,17 @@ impl CompactBlock {
 	}
 
 	/// Get kern_ids
-	pub fn kern_ids(&self) -> &Vec<ShortId> {
+	pub fn kern_ids(&self) -> &[ShortId] {
 		&self.body.kern_ids
 	}
 
 	/// Get full (coinbase) kernels
-	pub fn kern_full(&self) -> &Vec<TxKernel> {
+	pub fn kern_full(&self) -> &[TxKernel] {
 		&self.body.kern_full
 	}
 
 	/// Get full (coinbase) outputs
-	pub fn out_full(&self) -> &Vec<Output> {
+	pub fn out_full(&self) -> &[Output] {
 		&self.body.out_full
 	}
 }

core/src/core/transaction.rs

@@ -440,7 +440,7 @@ impl From<committed::Error> for Error {
 /// amount to zero.
 /// The signature signs the fee and the lock_height, which are retained for
 /// signature validation.
-#[derive(Serialize, Deserialize, Debug, Clone)]
+#[derive(Serialize, Deserialize, Debug, Clone, Copy)]
 pub struct TxKernel {
 	/// Options for a kernel's structure or use
 	pub features: KernelFeatures,
@@ -710,7 +710,7 @@ impl Readable for TransactionBody {
 		let kernels = read_multi(reader, kernel_len)?;
 
 		// Initialize tx body and verify everything is sorted.
-		let body = TransactionBody::init(inputs, outputs, kernels, true)
+		let body = TransactionBody::init(&inputs, &outputs, &kernels, true)
 			.map_err(|_| ser::Error::CorruptedData)?;
 
 		Ok(body)
@@ -737,6 +737,12 @@ impl Default for TransactionBody {
 	}
 }
 
+impl From<Transaction> for TransactionBody {
+	fn from(tx: Transaction) -> Self {
+		tx.body
+	}
+}
+
 impl TransactionBody {
 	/// Creates a new empty transaction (no inputs or outputs, zero fee).
 	pub fn empty() -> TransactionBody {
@@ -758,15 +764,15 @@ impl TransactionBody {
 	/// the provided inputs, outputs and kernels.
 	/// Guarantees inputs, outputs, kernels are sorted lexicographically.
 	pub fn init(
-		inputs: Vec<Input>,
-		outputs: Vec<Output>,
-		kernels: Vec<TxKernel>,
+		inputs: &[Input],
+		outputs: &[Output],
+		kernels: &[TxKernel],
 		verify_sorted: bool,
 	) -> Result<TransactionBody, Error> {
 		let mut body = TransactionBody {
-			inputs,
-			outputs,
-			kernels,
+			inputs: inputs.to_vec(),
+			outputs: outputs.to_vec(),
+			kernels: kernels.to_vec(),
 		};
 
 		if verify_sorted {
@@ -1075,12 +1081,6 @@ impl PartialEq for Transaction {
 	}
 }
 
-impl Into<TransactionBody> for Transaction {
-	fn into(self) -> TransactionBody {
-		self.body
-	}
-}
-
 /// Implementation of Writeable for a fully blinded transaction, defines how to
 /// write the transaction as binary.
 impl Writeable for Transaction {
@@ -1140,7 +1140,7 @@ impl Transaction {
 
 	/// Creates a new transaction initialized with
 	/// the provided inputs, outputs, kernels
-	pub fn new(inputs: Vec<Input>, outputs: Vec<Output>, kernels: Vec<TxKernel>) -> Transaction {
+	pub fn new(inputs: &[Input], outputs: &[Output], kernels: &[TxKernel]) -> Transaction {
 		let offset = BlindingFactor::zero();
 
 		// Initialize a new tx body and sort everything.
@@ -1195,32 +1195,32 @@ impl Transaction {
 	}
 
 	/// Get inputs
-	pub fn inputs(&self) -> &Vec<Input> {
+	pub fn inputs(&self) -> &[Input] {
 		&self.body.inputs
 	}
 
 	/// Get inputs mutable
-	pub fn inputs_mut(&mut self) -> &mut Vec<Input> {
+	pub fn inputs_mut(&mut self) -> &mut [Input] {
 		&mut self.body.inputs
 	}
 
 	/// Get outputs
-	pub fn outputs(&self) -> &Vec<Output> {
+	pub fn outputs(&self) -> &[Output] {
 		&self.body.outputs
 	}
 
 	/// Get outputs mutable
-	pub fn outputs_mut(&mut self) -> &mut Vec<Output> {
+	pub fn outputs_mut(&mut self) -> &mut [Output] {
 		&mut self.body.outputs
 	}
 
 	/// Get kernels
-	pub fn kernels(&self) -> &Vec<TxKernel> {
+	pub fn kernels(&self) -> &[TxKernel] {
 		&self.body.kernels
 	}
 
 	/// Get kernels mut
-	pub fn kernels_mut(&mut self) -> &mut Vec<TxKernel> {
+	pub fn kernels_mut(&mut self) -> &mut [TxKernel] {
 		&mut self.body.kernels
 	}
 
@@ -1290,7 +1290,10 @@ impl Transaction {
 /// from the Vec. Provides a simple way to cut-through a block or aggregated
 /// transaction. The elimination is stable with respect to the order of inputs
 /// and outputs.
-pub fn cut_through(inputs: &mut Vec<Input>, outputs: &mut Vec<Output>) -> Result<(), Error> {
+pub fn cut_through<'a>(
+	inputs: &'a mut [Input],
+	outputs: &'a mut [Output],
+) -> Result<(&'a [Input], &'a [Output]), Error> {
 	// assemble output commitments set, checking they're all unique
 	outputs.sort_unstable();
 	if outputs.windows(2).any(|pair| pair[0] == pair[1]) {
@@ -1303,11 +1306,11 @@ pub fn cut_through(inputs: &mut Vec<Input>, outputs: &mut Vec<Output>) -> Result
 	while inputs_idx < inputs.len() && outputs_idx < outputs.len() {
 		match inputs[inputs_idx].hash().cmp(&outputs[outputs_idx].hash()) {
 			Ordering::Less => {
-				inputs[inputs_idx - ncut] = inputs[inputs_idx];
+				inputs.swap(inputs_idx - ncut, inputs_idx);
 				inputs_idx += 1;
 			}
 			Ordering::Greater => {
-				outputs[outputs_idx - ncut] = outputs[outputs_idx];
+				outputs.swap(outputs_idx - ncut, outputs_idx);
 				outputs_idx += 1;
 			}
 			Ordering::Equal => {
@@ -1317,27 +1320,40 @@ pub fn cut_through(inputs: &mut Vec<Input>, outputs: &mut Vec<Output>) -> Result
 			}
 		}
 	}
-	// Cut elements that have already been copied
-	outputs.drain(outputs_idx - ncut..outputs_idx);
-	inputs.drain(inputs_idx - ncut..inputs_idx);
-	Ok(())
+
+	// Make sure we move any the remaining inputs into the slice to be returned.
+	while inputs_idx < inputs.len() {
+		inputs.swap(inputs_idx - ncut, inputs_idx);
+		inputs_idx += 1;
+	}
+
+	// Make sure we move any the remaining outputs into the slice to be returned.
+	while outputs_idx < outputs.len() {
+		outputs.swap(outputs_idx - ncut, outputs_idx);
+		outputs_idx += 1;
+	}
+
+	Ok((
+		&inputs[..inputs.len() - ncut],
+		&outputs[..outputs.len() - ncut],
+	))
 }
 
 /// Aggregate a vec of txs into a multi-kernel tx with cut_through.
-pub fn aggregate(mut txs: Vec<Transaction>) -> Result<Transaction, Error> {
+pub fn aggregate(txs: &[Transaction]) -> Result<Transaction, Error> {
 	// convenience short-circuiting
 	if txs.is_empty() {
 		return Ok(Transaction::empty());
 	} else if txs.len() == 1 {
-		return Ok(txs.pop().unwrap());
+		return Ok(txs[0].clone());
 	}
 	let mut n_inputs = 0;
 	let mut n_outputs = 0;
 	let mut n_kernels = 0;
 	for tx in txs.iter() {
-		n_inputs += tx.body.inputs.len();
-		n_outputs += tx.body.outputs.len();
-		n_kernels += tx.body.kernels.len();
+		n_inputs += tx.inputs().len();
+		n_outputs += tx.outputs().len();
+		n_kernels += tx.kernels().len();
 	}
 
 	let mut inputs: Vec<Input> = Vec::with_capacity(n_inputs);
@@ -1347,17 +1363,17 @@ pub fn aggregate(mut txs: Vec<Transaction>) -> Result<Transaction, Error> {
 	// we will sum these together at the end to give us the overall offset for the
 	// transaction
 	let mut kernel_offsets: Vec<BlindingFactor> = Vec::with_capacity(txs.len());
-	for mut tx in txs {
+	for tx in txs {
 		// we will sum these later to give a single aggregate offset
-		kernel_offsets.push(tx.offset);
+		kernel_offsets.push(tx.offset.clone());
 
-		inputs.append(&mut tx.body.inputs);
-		outputs.append(&mut tx.body.outputs);
-		kernels.append(&mut tx.body.kernels);
+		inputs.extend_from_slice(tx.inputs());
+		outputs.extend_from_slice(tx.outputs());
+		kernels.extend_from_slice(tx.kernels());
 	}
 
 	// Sort inputs and outputs during cut_through.
-	cut_through(&mut inputs, &mut outputs)?;
+	let (inputs, outputs) = cut_through(&mut inputs, &mut outputs)?;
 
 	// Now sort kernels.
 	kernels.sort_unstable();
@@ -1371,14 +1387,14 @@ pub fn aggregate(mut txs: Vec<Transaction>) -> Result<Transaction, Error> {
 	//   * cut-through outputs
 	//   * full set of tx kernels
 	//   * sum of all kernel offsets
-	let tx = Transaction::new(inputs, outputs, kernels).with_offset(total_kernel_offset);
+	let tx = Transaction::new(inputs, outputs, &kernels).with_offset(total_kernel_offset);
 
 	Ok(tx)
 }
 
 /// Attempt to deaggregate a multi-kernel transaction based on multiple
 /// transactions
-pub fn deaggregate(mk_tx: Transaction, txs: Vec<Transaction>) -> Result<Transaction, Error> {
+pub fn deaggregate(mk_tx: Transaction, txs: &[Transaction]) -> Result<Transaction, Error> {
 	let mut inputs: Vec<Input> = vec![];
 	let mut outputs: Vec<Output> = vec![];
 	let mut kernels: Vec<TxKernel> = vec![];
@@ -1389,19 +1405,19 @@ pub fn deaggregate(mk_tx: Transaction, txs: Vec<Transaction>) -> Result<Transact
 
 	let tx = aggregate(txs)?;
 
-	for mk_input in mk_tx.body.inputs {
-		if !tx.body.inputs.contains(&mk_input) && !inputs.contains(&mk_input) {
-			inputs.push(mk_input);
+	for mk_input in mk_tx.inputs() {
+		if !tx.inputs().contains(&mk_input) && !inputs.contains(mk_input) {
+			inputs.push(*mk_input);
 		}
 	}
-	for mk_output in mk_tx.body.outputs {
-		if !tx.body.outputs.contains(&mk_output) && !outputs.contains(&mk_output) {
-			outputs.push(mk_output);
+	for mk_output in mk_tx.outputs() {
+		if !tx.outputs().contains(&mk_output) && !outputs.contains(mk_output) {
+			outputs.push(*mk_output);
 		}
 	}
-	for mk_kernel in mk_tx.body.kernels {
-		if !tx.body.kernels.contains(&mk_kernel) && !kernels.contains(&mk_kernel) {
-			kernels.push(mk_kernel);
+	for mk_kernel in mk_tx.kernels() {
+		if !tx.kernels().contains(&mk_kernel) && !kernels.contains(mk_kernel) {
+			kernels.push(*mk_kernel);
 		}
 	}
 
@@ -1436,7 +1452,7 @@ pub fn deaggregate(mk_tx: Transaction, txs: Vec<Transaction>) -> Result<Transact
 	kernels.sort_unstable();
 
 	// Build a new tx from the above data.
-	let tx = Transaction::new(inputs, outputs, kernels).with_offset(total_kernel_offset);
+	let tx = Transaction::new(&inputs, &outputs, &kernels).with_offset(total_kernel_offset);
 	Ok(tx)
 }
 

core/src/libtx/build.rs

@@ -182,7 +182,7 @@ where
 ///
 pub fn partial_transaction<K, B>(
 	tx: Transaction,
-	elems: Vec<Box<Append<K, B>>>,
+	elems: &[Box<Append<K, B>>],
 	keychain: &K,
 	builder: &B,
 ) -> Result<(Transaction, BlindingFactor), Error>
@@ -203,7 +203,7 @@ where
 /// In the real world we use signature aggregation across multiple participants.
 pub fn transaction<K, B>(
 	features: KernelFeatures,
-	elems: Vec<Box<Append<K, B>>>,
+	elems: &[Box<Append<K, B>>],
 	keychain: &K,
 	builder: &B,
 ) -> Result<Transaction, Error>
@@ -230,7 +230,7 @@ where
 /// NOTE: Only used in tests (for convenience).
 /// Cannot recommend passing private excess around like this in the real world.
 pub fn transaction_with_kernel<K, B>(
-	elems: Vec<Box<Append<K, B>>>,
+	elems: &[Box<Append<K, B>>],
 	kernel: TxKernel,
 	excess: BlindingFactor,
 	keychain: &K,
@@ -284,7 +284,7 @@ mod test {
 
 		let tx = transaction(
 			KernelFeatures::Plain { fee: 2 },
-			vec![input(10, key_id1), input(12, key_id2), output(20, key_id3)],
+			&[input(10, key_id1), input(12, key_id2), output(20, key_id3)],
 			&keychain,
 			&builder,
 		)
@@ -306,7 +306,7 @@ mod test {
 
 		let tx = transaction(
 			KernelFeatures::Plain { fee: 2 },
-			vec![input(10, key_id1), input(12, key_id2), output(20, key_id3)],
+			&[input(10, key_id1), input(12, key_id2), output(20, key_id3)],
 			&keychain,
 			&builder,
 		)
@@ -327,7 +327,7 @@ mod test {
 
 		let tx = transaction(
 			KernelFeatures::Plain { fee: 4 },
-			vec![input(6, key_id1), output(2, key_id2)],
+			&[input(6, key_id1), output(2, key_id2)],
 			&keychain,
 			&builder,
 		)