Skip to content

refactor(settings): change daa to use injected settings #801

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

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 29, 2023

Depends on #800

Motivation

This PR changes the DAA module so it stops depending on global settings, allowing its simulator patch to be removed.

Acceptance Criteria

  • Move DAA functions to a new class with injected settings. No code logic should be changed.
  • Remove daa.MIN_BLOCK_WEIGHT and daa.AVG_TIME_BETWEEN_BLOCKS global variables, moving them to the class instance.
  • Add the new DAA class to be injected in HathorManager.
  • Add the new DAA class to be injected in VertexVerifiers.
  • Update Builder and CliBuilder accordingly.
  • Update Simulator to use injected DAA class, removing the DAA simulator patch.
  • Update all code using the daa module to use the DAA class.

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

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (5b59091) 85.06% compared to head (22c4e78) 85.02%.
Report is 1 commits behind head on master.

❗ Current head 22c4e78 differs from pull request most recent head 2bcbc81. Consider uploading reports for the commit 2bcbc81 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #801      +/-   ##
==========================================
- Coverage   85.06%   85.02%   -0.05%     
==========================================
  Files         275      271       -4     
  Lines       22502    22380     -122     
  Branches     3434     3419      -15     
==========================================
- Hits        19142    19029     -113     
- Misses       2674     2676       +2     
+ Partials      686      675      -11     
Files Coverage Δ
hathor/builder/builder.py 91.38% <100.00%> (+0.28%) ⬆️
hathor/builder/cli_builder.py 74.74% <100.00%> (+0.25%) ⬆️
hathor/manager.py 82.13% <100.00%> (+0.02%) ⬆️
hathor/p2p/resources/mining_info.py 92.59% <100.00%> (-0.27%) ⬇️
hathor/simulator/simulator.py 94.51% <100.00%> (-0.10%) ⬇️
hathor/simulator/tx_generator.py 96.05% <100.00%> (-0.06%) ⬇️
hathor/stratum/stratum.py 69.32% <100.00%> (ø)
hathor/transaction/resources/create_tx.py 92.45% <100.00%> (-0.14%) ⬇️
hathor/verification/block_verifier.py 97.14% <100.00%> (ø)
hathor/verification/transaction_verifier.py 92.05% <100.00%> (-0.06%) ⬇️
... and 6 more

... and 12 files with indirect coverage changes

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

@glevco glevco marked this pull request as ready for review September 29, 2023 23:03
jansegre
jansegre previously approved these changes Oct 6, 2023
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 2 times, most recently from 9145c97 to 7093fa5 Compare October 10, 2023 02:54
@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch from 09ee92f to c76fb12 Compare October 10, 2023 03:03
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 2 times, most recently from ca40831 to db9591a Compare October 10, 2023 20:57
@glevco glevco marked this pull request as draft October 10, 2023 21:09
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 2 times, most recently from d6caa5c to e62cc24 Compare October 17, 2023 21:14
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 7 times, most recently from bc6f21d to 0a78bb9 Compare October 25, 2023 17:04
Base automatically changed from refactor/move-verification/vertex-methods to master October 25, 2023 17:10
@msbrogli msbrogli dismissed jansegre’s stale review October 25, 2023 17:10

The base branch was changed.

@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch 2 times, most recently from d2f2993 to 22c4e78 Compare November 3, 2023 18:45
@glevco glevco marked this pull request as ready for review November 3, 2023 18:47
@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch from 22c4e78 to 2bcbc81 Compare November 3, 2023 21:08
@msbrogli msbrogli merged commit 2bcbc81 into master Nov 3, 2023
@msbrogli msbrogli deleted the refactor/simulator-patches/daa-settings branch November 3, 2023 21:11
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants