feat(mempool): replenishing queue in get_txs

[ayeletstarkware]

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Project coverage is 81.81%. Comparing base (93de0bd) to head (a70b025).

Files	Patch %	Lines
crates/mempool/src/mempool.rs	85.71%	0 Missing and 2 partials ⚠️
crates/mempool/src/transaction_pool.rs	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #496      +/-   ##
==========================================
+ Coverage   81.21%   81.81%   +0.60%     
==========================================
  Files          42       42              
  Lines        1826     1848      +22     
  Branches     1826     1848      +22     
==========================================
+ Hits         1483     1512      +29     
+ Misses        269      259      -10     
- Partials       74       77       +3     

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

[MohammadNassar1]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @elintul, @giladchase, and @MohammadNassar1)

[MohammadNassar1]

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

a discussion (no related file):
Bad rebase?

---

[elintul]

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

---

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

            let nonce = tx.nonce.try_increment().unwrap();
            if let Some(next_tx_reference) =
                self.tx_pool.get_by_address_and_nonce(tx.sender_address, nonce)

This is an API we're going to need in more locations; please add:

get_next_eligible_tx(account: Account) -> TransactionReference

Code quote:

self.tx_pool.get_by_address_and_nonce(tx.sender_address, nonce)

[elintul]

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

---

crates/mempool/src/mempool_test.rs line 195 at r3 (raw file):

    // Assert that the account's next tx was added the queue.
    assert_eq!(txs, &[tx_address_0_nonce_0, tx_address_0_nonce_1]);

How come this PR caused this change? I'd imagine fixing add_tx would make get_txs not return both transactions.

Code quote:

    assert_eq!(txs, &[tx_address_0_nonce_0, tx_address_0_nonce_1]);

[ayeletstarkware]

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

---

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

Previously, elintul (Elin) wrote…

This is an API we're going to need in more locations; please add:

get_next_eligible_tx(account: Account) -> TransactionReference

1 - Should this function be in Mempool or TransactionPool?
2 - The get_by_address_and_nonce function returns Option<&TransactionReference>, not TransactionReference
3- ThinTransaction doesn’t include Account. Should I create one when calling this function, or should I receive the sender address and nonce instead?

---

crates/mempool/src/mempool_test.rs line 195 at r3 (raw file):

Previously, elintul (Elin) wrote…

How come this PR caused this change? I'd imagine fixing add_tx would make get_txs not return both transactions.

correct. this PR makes get_txs to return only the first tx.

[ayeletstarkware]

Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @elintul, @giladchase, and @MohammadNassar1)

---

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

Previously, ayeletstarkware (Ayelet Zilber) wrote…

1 - Should this function be in Mempool or TransactionPool?
2 - The get_by_address_and_nonce function returns Option<&TransactionReference>, not TransactionReference
3- ThinTransaction doesn’t include Account. Should I create one when calling this function, or should I receive the sender address and nonce instead?

done

[elintul]

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

---

crates/mempool/src/mempool.rs line 114 at r5 (raw file):

        Ok(())
    }

See my comment here: taking into account Mohammad's suggestion, I think we should loop over transaction references until we collect all eligible.
Also, not sure we need a function, it's not too long.

---

crates/mempool/src/mempool_test.rs line 195 at r3 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

correct. this PR makes get_txs to return only the first tx.

It's temporary, right?

---

crates/mempool/src/transaction_pool.rs line 96 at r5 (raw file):

    ) -> MempoolResult<Option<&TransactionReference>> {
        let Account { sender_address, state: AccountState { nonce } } = current_account_state;
        let next_nonce = nonce.try_increment().map_err(|_| MempoolError::FeltOutOfRange)?;

TODO to change once SN API's errors impl. PartialEq (after mono-repo).

Code quote:

let next_nonce = nonce.try_increment().map_err(|_| MempoolError::FeltOutOfRange)?;

[ayeletstarkware]

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

a discussion (no related file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Bad rebase?

Done.

---

---

crates/mempool/src/mempool.rs line 114 at r5 (raw file):

Previously, elintul (Elin) wrote…

See my comment here: taking into account Mohammad's suggestion, I think we should loop over transaction references until we collect all eligible.
Also, not sure we need a function, it's not too long.

Your suggestion is done on the next PR. the changes here take into consideration your comment.

---

crates/mempool/src/mempool_test.rs line 195 at r3 (raw file):

Previously, elintul (Elin) wrote…

It's temporary, right?

Yes. It's changing in the following PR according to the support of each PR.

[elintul]

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

[MohammadNassar1]

Reviewed 1 of 3 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @giladchase)

[elintul]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @giladchase)