feat(mempool): add resource bounds to transaction reference type

[MohammadNassar1]

This change is 

[MohammadNassar1]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion

a discussion (no related file):
This PR adds resource_bounds to the transaction reference; this field will be used to check the gas price.
As resource_bounds doesn't implement Copy, I removed Copy from TransactionReference.

---

[giladchase]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @elintul, and @MohammadNassar1)

a discussion (no related file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

This PR adds resource_bounds to the transaction reference; this field will be used to check the gas price.
As resource_bounds doesn't implement Copy, I removed Copy from TransactionReference.

Can you ping Dori about the Copy thing? i think Meshi started implementing Copy on ResourceBoundsMapping, and she told me the task was passed to Dori's team

---

[MohammadNassar1]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @elintul, and @giladchase)

a discussion (no related file):

Previously, giladchase wrote…

Can you ping Dori about the Copy thing? i think Meshi started implementing Copy on ResourceBoundsMapping, and she told me the task was passed to Dori's team

Asked him, till then I think I can merge this PR.
Do you prefer deref on clone?

---

[elintul]

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ayeletstarkware, @giladchase, and @MohammadNassar1)

---

crates/mempool/src/mempool.rs line 113 at r2 (raw file):

            if self.tx_queue.get_nonce(address).is_none() {
                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, next_nonce) {

Throughout the code (also in transaction_queue.rs below); tx.clone() should not appear.

Suggestion:

tx_reference

---

crates/mempool/src/mempool.rs line 218 at r2 (raw file):

/// TODO(Mohammad): rename this struct to `ThinTransaction` once that name
/// becomes available, to better reflect its purpose and usage.
#[derive(Clone, Debug, Default, Eq, PartialEq)]

Add a TODO to restore the Copy once ResourceBoundsMapping implements it.

---

crates/mempool/src/mempool_test.rs line 352 at r2 (raw file):

    let pool_txs = [input_nonce_1.tx];
    let queue_txs = [];

Why are these changes suddenly needed?

[MohammadNassar1]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ayeletstarkware, @elintul, and @giladchase)

---

crates/mempool/src/mempool.rs line 113 at r2 (raw file):

Previously, elintul (Elin) wrote…

Throughout the code (also in transaction_queue.rs below); tx.clone() should not appear.

Why clone should not appear?
We cannot use deref as it doesn't imply copy.

---

crates/mempool/src/mempool.rs line 218 at r2 (raw file):

Previously, elintul (Elin) wrote…

Add a TODO to restore the Copy once ResourceBoundsMapping implements it.

Done.

---

crates/mempool/src/mempool_test.rs line 352 at r2 (raw file):

Previously, elintul (Elin) wrote…

Why are these changes suddenly needed?

queue_txs is from the TransactionReference type, and since it doesn't implement Copy, so I had to add a clone.

[elintul]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ayeletstarkware, @giladchase, and @MohammadNassar1)

---

crates/mempool/src/mempool.rs line 113 at r2 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Why clone should not appear?
We cannot use deref as it doesn't imply copy.

I meant .clone() should not be invoked on a variable named tx if it's a transaction reference. It makes the reader think there's heavy-lifting being done.

---

crates/mempool/src/mempool_test.rs line 352 at r2 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

queue_txs is from the TransactionReference type, and since it doesn't implement Copy, so I had to add a clone.

What happened before? It was Copyied implicitly?

---

crates/mempool/src/transaction_queue.rs line 25 at r3 (raw file):

    pub fn insert(&mut self, tx: TransactionReference) {
        assert_eq!(
            self.address_to_tx.insert(tx.sender_address, tx.clone()),

Suggestion:

tx_reference

[MohammadNassar1]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @ayeletstarkware, @elintul, and @giladchase)

---

crates/mempool/src/mempool.rs line 113 at r2 (raw file):

Previously, elintul (Elin) wrote…

I meant .clone() should not be invoked on a variable named tx if it's a transaction reference. It makes the reader think there's heavy-lifting being done.

I see.
I think we don't have tx.clone() anymore.

---

crates/mempool/src/mempool_test.rs line 352 at r2 (raw file):

Previously, elintul (Elin) wrote…

What happened before? It was Copyied implicitly?

Exactly.

---

crates/mempool/src/transaction_queue.rs line 25 at r3 (raw file):

    pub fn insert(&mut self, tx: TransactionReference) {
        assert_eq!(
            self.address_to_tx.insert(tx.sender_address, tx.clone()),

Done.

[elintul]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @giladchase)