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

test: custom mempool unit tests #2199

Merged
merged 24 commits into from
May 23, 2024
Merged

test: custom mempool unit tests #2199

merged 24 commits into from
May 23, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented May 16, 2024

Description

Contains unit tests for priority mempool and custom proposal handler. These tests are forked from cosmos-sdk 0.47 codebase, but also extended on someplaces where we are using custom sender_with_nonce.go to handle ethermint txs, mostly on Insert method on priority mempool and selecting txs in handler. I marked those changes with comments, tried to keep same tests style.

Closes: #2183

Type of change

Unit tests

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

Copy link

codecov bot commented May 16, 2024

Codecov Report

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

Project coverage is 68.80%. Comparing base (c0026b9) to head (c3a2402).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2199      +/-   ##
===========================================
+ Coverage    67.99%   68.80%   +0.80%     
===========================================
  Files          259      261       +2     
  Lines        16030    16250     +220     
===========================================
+ Hits         10900    11180     +280     
+ Misses        4669     4589      -80     
- Partials       461      481      +20     
Files Coverage Δ
pkg/mempool/custom_proposal_handler.go 71.92% <100.00%> (ø)
pkg/mempool/priority_nonce_mempool.go 90.42% <ø> (ø)
pkg/mempool/senders_with_nonce.go 87.50% <100.00%> (ø)
app/app.go 2.18% <0.00%> (ø)

@skosito skosito marked this pull request as ready for review May 17, 2024 22:28
Copy link

!!!WARNING!!!
nosec detected in the following files: pkg/mempool/custom_proposal_handler.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions bot added the nosec label May 20, 2024
@skosito
Copy link
Contributor Author

skosito commented May 20, 2024

I don't think we have this file in codecov scope. In general I don't think it is relevant to have the app package in codecov because it is generic logic but for this one this would make sense since we have a customized implementation.

We could also consider moving the mem pool priority logic in pkg as an independant package

moved everything mempool related to pkg file, added more tests not present in forked tests, coverage of that folder is now 83%

@skosito skosito requested a review from lumtis May 20, 2024 15:39
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

gitguardian bot commented May 23, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
11033145 Triggered Generic High Entropy Secret cbb972e cmd/zetae2e/local/accounts.go View secret
11033146 Triggered Generic High Entropy Secret cbb972e cmd/zetae2e/local/accounts.go View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@skosito skosito merged commit 1a2c964 into develop May 23, 2024
19 of 21 checks passed
@skosito skosito deleted the custom-mempool-unit-tests branch May 23, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fork unit tests for priority mempool
3 participants