feat(block-producer): borrow instead of cloning txs

[Mirko-von-Leipzig]

Blocked by PR #543.

Removes the unnecessary deep cloning of transaction data by utilising Borrow. The TransactionBatch::new implementation can probably be improved to reduce the amount of iteration occurring, but I'm delaying that until the dust settles on the mempool implementation.

Closes #531

[polydez]

It seems that this PR introduces even more cloning than before, doesn't it? I found 3 cloning of transaction vector here.

[Mirko-von-Leipzig]

It seems that this PR introduces even more cloning than before, doesn't it? I found 3 cloning of transaction vector here.

Sort of. We have the following signature now:

txs: impl IntoIterator<Item = Borrow<ProvenTransaction>> + Clone

and txs gets cloned a bunch. However this gets passed in as an iterator:

txs.iter().map(AuthenticatedTransaction::raw_proven_transaction),

so the clones are clones of the iterator. This is a change from before where we cloned the inner transaction data:

txs.iter().map(AuthenticatedTransaction::raw_proven_transaction).cloned().collect()

---

I'm 95% confident this works how I expect it to. If there is a better/simpler way to express this borrow + reusable iterator bound then that would be perfect.

[polydez]

@Mirko-von-Leipzig this makes sense, thank you! The confusing thing here is that we clone IntoIterator which can be not only iterator itself, but vector as well. I've made my proposal how we can make this simpler and clearer, please take a look: #550

[Mirko-von-Leipzig]

@Mirko-von-Leipzig this makes sense, thank you! The confusing thing here is that we clone IntoIterator which can be not only iterator itself, but vector as well. I've made my proposal how we can make this simpler and clearer, please take a look: #550

Agreed; and that is indeed much clearer. I was searching for a clone + iterator combo but missed the fact that you can specify the underlying iterator within IntoIterator.

[bobbinth]

Looks good! Thank you!