Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add transaction getter to mempool #402

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Conversation

Yael-Starkware
Copy link
Contributor

@Yael-Starkware Yael-Starkware commented Jul 8, 2024

This change is Reviewable

@Yael-Starkware Yael-Starkware requested a review from elintul July 8, 2024 13:28
@codecov-commenter
Copy link

codecov-commenter commented Jul 8, 2024

Codecov Report

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

Project coverage is 82.65%. Comparing base (4269f5b) to head (aee5b13).
Report is 5 commits behind head on main.

Files Patch % Lines
crates/mempool/src/transaction_pool.rs 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #402      +/-   ##
==========================================
- Coverage   83.24%   82.65%   -0.59%     
==========================================
  Files          36       38       +2     
  Lines        1695     1753      +58     
  Branches     1695     1753      +58     
==========================================
+ Hits         1411     1449      +38     
- Misses        209      229      +20     
  Partials       75       75              

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

Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Yael-Starkware)


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

    }

    pub fn get(&self, tx_hash: TransactionHash) -> MempoolResult<&ThinTransaction> {

Suggestion:

get_by_tx_hash

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

    }

    pub fn get_by_account(

Suggestion:

get_by_address_and_nonce

Copy link
Contributor Author

@Yael-Starkware Yael-Starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

    }

    pub fn get(&self, tx_hash: TransactionHash) -> MempoolResult<&ThinTransaction> {

done


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

    }

    pub fn get_by_account(

done

Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware)

a discussion (no related file):
Please change the commit message: "add getter by address and nonce to TransactionPool".


Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)

Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Yael-Starkware)

@Yael-Starkware Yael-Starkware merged commit ea3e69d into main Jul 9, 2024
8 checks passed
@Yael-Starkware Yael-Starkware deleted the yael/getter branch July 9, 2024 11:17
@Yael-Starkware Yael-Starkware restored the yael/getter branch July 9, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants