feat(mempool): implement priority queue data structure

[MohammadNassar1]

This change is 

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 50.00000% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 61.24%. Comparing base (72a2b06) to head (eca5cf1).

Files	Patch %	Lines
crates/mempool/src/priority_queue.rs	50.00%	14 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #34      +/-   ##
==========================================
- Coverage   64.64%   61.24%   -3.41%     
==========================================
  Files           5        6       +1     
  Lines          99      129      +30     
  Branches       99      129      +30     
==========================================
+ Hits           64       79      +15     
- Misses         35       49      +14     
- Partials        0        1       +1     

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

[giladchase]

⚡

Reviewable status: 0 of 6 files reviewed, all discussions resolved (waiting on @elintul)

[elintul]

Reviewed 1 of 6 files at r3, all commit messages.
Reviewable status: 1 of 6 files reviewed, all discussions resolved

[ayeletstarkware]

Reviewed 3 of 6 files at r1, 1 of 1 files at r2, 5 of 6 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MohammadNassar1)

---

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

#[derive(Clone, Debug, derive_more::Deref)]
pub struct Transaction(pub InternalTransaction);

Consider changing the name of Transaction to make its job in the priority queue clearer or moving it to a better spot?

[giladchase]

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

---

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

Previously, ayeletstarkware (Ayelet Zilber) wrote…

Consider changing the name of Transaction to make its job in the priority queue clearer or moving it to a better spot?

What do you have in mind?
I initially thought about calling it PriorityQueueTransaction, but then it felt redundant given the location of its definition.
I was thinking, if users of the queue have a clash, they can import it as PriorityQueueTransaction.
Not sure though.

[ayeletstarkware]

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

---

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

Previously, giladchase wrote…

What do you have in mind?
I initially thought about calling it PriorityQueueTransaction, but then it felt redundant given the location of its definition.
I was thinking, if users of the queue have a clash, they can import it as PriorityQueueTransaction.
Not sure though.

That's exactly what I was thinking too. I was concerned about having too many "Transaction"s in our structs, which could make things confusing, but maybe it's unnecessary.

[giladchase]

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

---

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

Previously, ayeletstarkware (Ayelet Zilber) wrote…

That's exactly what I was thinking too. I was concerned about having too many "Transaction"s in our structs, which could make things confusing, but maybe it's unnecessary.

Mohammad is taking over so his call now :P

---

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

            InternalTransaction::Declare(declare_tx) => match &declare_tx.tx {
                DeclareTransaction::V3(tx_v3) => tx_v3.tip,
                _ => unimplemented!(),

return error

Code quote:

                _ => unimplemented!(),

---

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

    pq.push(tx3.clone());

    assert_eq!(pq.pop().unwrap(), tx1);

change back to numbers (no hash)

[MohammadNassar1]

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

---

-- commits line 4 at r3:
@giladchase, how do you add this to the commit msg?

Code quote:

New commits in r3 on 11/04/2024 at 09:21 by Gilad Chase:

---

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

Previously, giladchase wrote…

Mohammad is taking over so his call now :P

Changed to PQTransaction.

---

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

Previously, giladchase wrote…

return error

I think we can keep unimplemented, and in the mempool, we need to check that we get only V3 transactions.
WDYT?

---

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

Previously, giladchase wrote…

change back to numbers (no hash)

Done.

[giladchase]

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

---

-- commits line 4 at r3:

Previously, MohammadNassar1 (mohammad-starkware) wrote…

@giladchase, how do you add this to the commit msg?

not sure i follow what you mean.
I push-forced into this PR while you were gone, do you mean that?

---

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

Previously, MohammadNassar1 (mohammad-starkware) wrote…

I think we can keep unimplemented, and in the mempool, we need to check that we get only V3 transactions.
WDYT?

Sounds good

[giladchase]

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

[giladchase]

Reviewed 1 of 6 files at r3, 4 of 4 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MohammadNassar1)