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(core-transaction-pool): increase coverage to 100% #3553

Merged
merged 48 commits into from
Mar 9, 2020

Conversation

rainydio
Copy link
Contributor

@rainydio rainydio commented Feb 29, 2020

Summary

In addition to tests there are changes that are fixing various issues that were discovered.

Transaction has expired error

Previously it reported how many blocks ago transaction expired. But when transaction is entering the pool its expiration is checked against next height. So when current height is 100 and transaction expiration is set to 101 it is considered expired and cannot enter. It's unclear how to report blocks ago in such case. I changed error message to report expiration height instead of how many blocks ago it expired.

Sender state race condition

Although hypothetical it was possible to cause races due to async nature of transaction handler. Now sener's transactions are acquiring lock while applying or manipulating stored transactions array.

Error during revert and in-flight transactions

If error was raised during revert (should not happen) transactions that were waiting for lock to release cannot be applied anymore. State is considered corrupted, there isn't much that can be done with it. Any transactions that were waiting for lock to release will raise an error.

Mempool

Sender state was split into two classes SenderState that has only apply and revert functions and also performs transaction related checks and SenderMempool that keeps list of applied transactions and performs sender related checks. Memory was renamed into Mempool.

Fixes #3482

Checklist

  • Collator
  • Dynamic fee matcher
  • Errors
  • Expiration service
  • Mempool
  • Processor
  • Query
  • Sender mempool
  • Sender state
  • Service
  • Storage
  • Utils
  • Functional tests

@ghost ghost added Complexity: High labels Feb 29, 2020
@codecov
Copy link

codecov bot commented Feb 29, 2020

Codecov Report

Merging #3553 into develop will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3553   +/-   ##
========================================
  Coverage    66.86%   66.86%           
========================================
  Files          448      448           
  Lines        10181    10181           
  Branches      1322     1322           
========================================
  Hits          6808     6808           
  Misses        3351     3351           
  Partials        22       22           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77646c1...77646c1. Read the comment docs.

@faustbrian
Copy link
Contributor

@faustbrian faustbrian changed the title test(core-transaction-pool): Increase test coverage of core-transaction-pool to 100% test(core-transaction-pool): increase coverage to 100% Mar 1, 2020
@faustbrian faustbrian marked this pull request as ready for review March 9, 2020 10:10
@faustbrian faustbrian merged commit 462314a into develop Mar 9, 2020
@ghost ghost deleted the test/core-transaction-pool branch March 9, 2020 10:10
@ghost ghost removed the Status: Needs Review label Mar 9, 2020
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.

2 participants