Skip to content

refactor: Prepare for sync-v2#697

Merged
msbrogli merged 1 commit intomasterfrom
refactor/prepare-sync-v2
Jul 12, 2023
Merged

refactor: Prepare for sync-v2#697
msbrogli merged 1 commit intomasterfrom
refactor/prepare-sync-v2

Conversation

@msbrogli
Copy link
Member

@msbrogli msbrogli commented Jul 12, 2023

Motivation

This PR contains refactors needed by sync v2. These changes DO NOT change any external behavior. They are mostly adjustments and fixes so tests can be easily adapted to run with sync v2.

Acceptance Criteria

  1. Add asserts on FeatureService to gracefully fail and prevent infinite loop.
  2. Fix scope verification on TransactionStorage.get_all_transactions().
  3. Assert that a transaction is fully validated on ConsensusAlgorithm.update().
  4. Fix RocksDBHeightIndex._add() method. It was wrongfully identifying a reorg when a reorg wasn't happening.
  5. Prepare HathorManager to initialize with partial and invalid transactions stored in the database.
  6. Add missing @abstractmethod decorators on SyncManager.
  7. Improve simulator FakeConnection with an auto-reconnect feature.
  8. Add FakeConnection.reconnect() and FakeConnection.is_both_synced() methods.
  9. Add simulator triggers (StopWhenTrue, StopWhenSynced, and All).
  10. Add private attribute TransactionRocksDBStorage. _rocksdb_storage: RocksDBStorage.
  11. Change tests/p2p/test_sync_rate_limiter.py to run for sync v1 only since it tests a rate limiter that exists only in sync v1.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@msbrogli msbrogli requested a review from jansegre as a code owner July 12, 2023 12:26
@msbrogli msbrogli force-pushed the refactor/prepare-sync-v2 branch from 80d4fc3 to 75a81bf Compare July 12, 2023 12:44
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #697 (3695ad2) into master (67e0848) will decrease coverage by 0.12%.
The diff coverage is 66.66%.

❗ Current head 3695ad2 differs from pull request most recent head 78123f0. Consider uploading reports for the commit 78123f0 to get more accurate results

@@            Coverage Diff             @@
##           master     #697      +/-   ##
==========================================
- Coverage   84.13%   84.01%   -0.12%     
==========================================
  Files         246      246              
  Lines       20450    20519      +69     
  Branches     2760     2769       +9     
==========================================
+ Hits        17206    17240      +34     
- Misses       2672     2706      +34     
- Partials      572      573       +1     
Impacted Files Coverage Δ
hathor/transaction/base_transaction.py 91.60% <ø> (+0.97%) ⬆️
hathor/transaction/block.py 91.45% <0.00%> (-0.79%) ⬇️
hathor/util.py 81.30% <ø> (ø)
hathor/manager.py 77.46% <38.70%> (+0.21%) ⬆️
hathor/simulator/fake_connection.py 71.18% <73.91%> (-11.55%) ⬇️
hathor/simulator/trigger.py 95.83% <87.50%> (-4.17%) ⬇️
hathor/consensus/consensus.py 97.87% <100.00%> (+0.02%) ⬆️
hathor/feature_activation/feature_service.py 97.53% <100.00%> (+0.06%) ⬆️
hathor/indexes/rocksdb_height_index.py 95.55% <100.00%> (ø)
hathor/p2p/sync_manager.py 100.00% <100.00%> (ø)
... and 2 more

... and 3 files with indirect coverage changes

@msbrogli msbrogli self-assigned this Jul 12, 2023
jansegre
jansegre previously approved these changes Jul 12, 2023
@msbrogli msbrogli force-pushed the refactor/prepare-sync-v2 branch from 3695ad2 to 78123f0 Compare July 12, 2023 22:21
@msbrogli msbrogli merged commit 78123f0 into master Jul 12, 2023
@msbrogli msbrogli deleted the refactor/prepare-sync-v2 branch July 12, 2023 22:21
@jansegre jansegre mentioned this pull request Jul 24, 2023
2 tasks
This was referenced Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants