fix(mempool): fix add-tx method by updating the queue according to in…

[MohammadNassar1]

…put state

---

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.87%. Comparing base (93de0bd) to head (538c0c0).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #510      +/-   ##
==========================================
+ Coverage   81.21%   81.87%   +0.65%     
==========================================
  Files          42       42              
  Lines        1826     1826              
  Branches     1826     1826              
==========================================
+ Hits         1483     1495      +12     
+ Misses        269      256      -13     
- Partials       74       75       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MohammadNassar1]

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

a discussion (no related file):
The AddTx method now takes into account the input account state.
It updates the queue accordingly, e.g. a transaction hole can be closed by the accountState.

---

[ayeletstarkware]

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

---

crates/mempool/src/mempool.rs line 109 at r1 (raw file):

        self.tx_pool.insert(tx)?;

        if self.tx_queue.get_nonce(sender_address).is_none() {

out of scope but I think get_address_nonce is better

Code quote:

get_nonce

---

crates/mempool/src/mempool.rs line 109 at r1 (raw file):

        self.tx_pool.insert(tx)?;

        if self.tx_queue.get_nonce(sender_address).is_none() {

Consider adding a comment about the meaning of this if. I think get_nonce.none is not trivial for checking whether the account is in the queue already

[MohammadNassar1]

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

---

crates/mempool/src/mempool.rs line 109 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

Consider adding a comment about the meaning of this if. I think get_nonce.none is not trivial for checking whether the account is in the queue already

Done

---

crates/mempool/src/mempool.rs line 109 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

out of scope but I think get_address_nonce is better

I think get_nonce is a good name for the function. The input argument is the address, so it is clear that it returns the nonce of a specific address.

[ayeletstarkware]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @elintul, @giladchase, and @MohammadNassar1)

---

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

        // 1. If the input transaction can be added to the queue, this can happen when the input
        //    transaction's nonce equals the next nonce.
        // 2. If the input state fills a gap in the transaction queue, insert it into the queue.

// If no account transaction is in the queue, insert if nonce matches account's.

Code quote:

        // If the queue is empty, check if a transaction can be inserted into the transaction queue:
        // 1. If the input transaction can be added to the queue, this can happen when the input
        //    transaction's nonce equals the next nonce.
        // 2. If the input state fills a gap in the transaction queue, insert it into the queue.

[elintul]

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

[elintul]

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

---

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

        if tx.nonce < nonce {
            return Err(MempoolError::DuplicateTransaction { tx_hash: tx.tx_hash });
        }

Okay, but error is inaccurate, and also should be moved to should_insert method from the other PR.

Code quote:

        // Invalid input.
        if tx.nonce < nonce {
            return Err(MempoolError::DuplicateTransaction { tx_hash: tx.tx_hash });
        }

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

Okay, but error is inaccurate, and also should be moved to should_insert method from the other PR.

Added a TODO.

---

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

Previously, ayeletstarkware (Ayelet Zilber) wrote…

// If no account transaction is in the queue, insert if nonce matches account's.

Note that another tx may be added to the queue, not only the specific one.

[elintul]

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

[elintul]

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

---

crates/mempool/src/mempool.rs line 116 at r6 (raw file):

                self.tx_queue.insert(*tx_reference);
            }
        }

Suggestion:

        // Maybe close nonce gap.
        if self.tx_queue.get_nonce(sender_address).is_none() {
            if let Some(tx_reference) = self.tx_pool.get_by_address_and_nonce(sender_address, nonce)
            {
                self.tx_queue.insert(*tx_reference);
            }
        }

[elintul]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ayeletstarkware and @giladchase)

[elintul]

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