feat(mempool): insert eligible tx to tx queue in commit-block accordi…

[MohammadNassar1]

…ng to state changes

---

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.16%. Comparing base (231b340) to head (a403fef).

Files	Patch %	Lines
crates/mempool/src/mempool.rs	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #464      +/-   ##
==========================================
- Coverage   81.38%   81.16%   -0.23%     
==========================================
  Files          42       42              
  Lines        1816     1821       +5     
  Branches     1816     1821       +5     
==========================================
  Hits         1478     1478              
- Misses        264      269       +5     
  Partials       74       74              

☔ 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):
This PR adds an eligible transaction to the tx_queue based on the state changes.
It inserts new transactions in commit_block from another leader, provided the transaction is already present in the tx_pool.

---

[elintul]

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

---

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

                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, nonce) {
                    self.tx_queue.insert(*tx);
                }

Can this be moved into the scope of removing from the Q?

Code quote:

                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, nonce) {
                    self.tx_queue.insert(*tx);
                }

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

Can this be moved into the scope of removing from the Q?

Not sure I get it :)

[elintul]

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

---

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

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Not sure I get it :)

        for (address, AccountState { nonce }) in state_changes {
            if self.tx_queue.get_nonce(address).is_some_and(|queued_nonce| queued_nonce != nonce) {
                self.tx_queue.remove(address);

                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, nonce) {
                    self.tx_queue.insert(*tx);
                }
            }
        }

What am I missing?
Maybe we should add a get_next_eligible_tx(account) API?

[elintul]

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

---

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

            //    block, applicable when the block is from the same leader.
            // 2. Remove a transaction from queue with nonce greater than those committed to the
            //    block, applicable when the block is from a different leader.

I think this doc. is too extensive, not sure I want to reference consensus (it's our of scope). Perhaps saying // Align the queue with the committed nonces..
I also think it can appear in one paragraph:

// Align the queue with the committed nonces.
if self.tx_queue.get_nonce(address).is_some_and(|queued_nonce| queued_nonce != next_nonce) {
    self.tx_queue.remove(address);
}

if self.tx_queue.get_nonce(address).is_none() {
    if let Some(next_eligible_tx) = self.tx_pool.get_by_address_and_nonce(address, next_nonce) {
        self.tx_queue.insert(*next_eligible_tx);
    }
}

Where do we have get_next_eligible_tx API? It can be used here, right?

Code quote:

            // Dequeue transactions from the queue in the following cases:
            // 1. Remove a transaction from queue with nonce lower than those committed to the
            //    block, applicable when the block is from the same leader.
            // 2. Remove a transaction from queue with nonce greater than those committed to the
            //    block, applicable when the block is from a different leader.

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

I think this doc. is too extensive, not sure I want to reference consensus (it's our of scope). Perhaps saying // Align the queue with the committed nonces..
I also think it can appear in one paragraph:

// Align the queue with the committed nonces.
if self.tx_queue.get_nonce(address).is_some_and(|queued_nonce| queued_nonce != next_nonce) {
    self.tx_queue.remove(address);
}

if self.tx_queue.get_nonce(address).is_none() {
    if let Some(next_eligible_tx) = self.tx_pool.get_by_address_and_nonce(address, next_nonce) {
        self.tx_queue.insert(*next_eligible_tx);
    }
}

Where do we have get_next_eligible_tx API? It can be used here, right?

Thanks, updated.

This my my next_elegible method:
self.tx_pool.get_by_address_and_nonce(address, nonce).

---

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

Previously, elintul (Elin) wrote…

        for (address, AccountState { nonce }) in state_changes {
            if self.tx_queue.get_nonce(address).is_some_and(|queued_nonce| queued_nonce != nonce) {
                self.tx_queue.remove(address);

                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, nonce) {
                    self.tx_queue.insert(*tx);
                }
            }
        }

What am I missing?
Maybe we should add a get_next_eligible_tx(account) API?

The code has been updated. See the above comment.

[elintul]

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

---

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

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Thanks, updated.

This my my next_elegible method:
self.tx_pool.get_by_address_and_nonce(address, nonce).

I think it can be a method (can be done in a separated PR), and also shared with replenishing logic.

[elintul]

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

[elintul]

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

---

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

            }

            // TODO: remove the transactions from the tx_pool.

Can be in another PR.

Suggestion:

            }
            
            if self.tx_queue.get_nonce(address).is_none() {
                if let Some(tx) = self.tx_pool.get_by_address_and_nonce(address, nonce) {
                    self.tx_queue.insert(*tx);
                }
            }

            // TODO: remove the transactions from the tx_pool.

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

I think it can be a method (can be done in a separated PR), and also shared with replenishing logic.

Okay, we'll do in another PR.

[elintul]

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

[elintul]

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