feat(mempool): remove addresses of reverted transactions from queue in commit block method

[MohammadNassar1]

…n commit block method

---

This change is 

[codecov-commenter]

Codecov Report

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

Project coverage is 80.53%. Comparing base (44e4e35) to head (511bc56).

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

Additional details and impacted files

@@                          Coverage Diff                           @@
##           mohammad/mempool/add-mempool-state     #508      +/-   ##
======================================================================
- Coverage                               80.80%   80.53%   -0.27%     
======================================================================
  Files                                      42       42              
  Lines                                    1849     1855       +6     
  Branches                                 1849     1855       +6     
======================================================================
  Hits                                     1494     1494              
- Misses                                    279      285       +6     
  Partials                                   76       76              

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

[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 87 at r2 (raw file):

        state_changes: HashMap<ContractAddress, AccountState>,
    ) -> MempoolResult<()> {
        for (&address, AccountState { nonce }) in state_changes.iter() {

Why is this change needed? Isn't for x in iterable sugar for that?

Code quote:

for (&address, AccountState { nonce }) in state_changes.iter()

---

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

        }
        // TODO: update the tx_queue with the new state changes.

This is not necessarily true, right? The bouncer might have rejected them, so they're still valid.

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

Why is this change needed? Isn't for x in iterable sugar for that?

Not sure I get it. What so you suggest?
My code extracts address, and nonce.

---

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

Previously, elintul (Elin) wrote…

This is not necessarily true, right? The bouncer might have rejected them, so they're still valid.

Correct, I wrote " as they were not included in the block." So it's a generic comment. A TX can be rejected by the bouncer or by the executor.

[elintul]

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

---

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

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Not sure I get it. What so you suggest?
My code extracts address, and nonce.

What happens if you remove .iter()?

[MohammadNassar1]

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

---

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

Previously, elintul (Elin) wrote…

What happens if you remove .iter()?

.iter() returns ref.
if I remove it, so this line:
if !state_changes.contains_key(address)
would fail on borrowing issue

[elintul]

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

---

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

Previously, MohammadNassar1 (mohammad-starkware) wrote…

.iter() returns ref.
if I remove it, so this line:
if !state_changes.contains_key(address)
would fail on borrowing issue

Will looping over &state_changes work?

[elintul]

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

[elintul]

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

---

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

        self.mempool_state.clear();

        Ok(())

Suggestion:

        // Rewind nonces of addresses that were not included in block.
        let addresses_not_included_in_block =
            self.mempool_state.keys().filter(|&key| !state_changes.contains_key(key));
        for address in addresses_not_included_in_block {
            self.tx_queue.remove(*address);
        }
        
        self.mempool_state.clear();

        Ok(())

[elintul]

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