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

refactor(cpu-mining): create CpuMiningService #803

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 29, 2023

Depends on #802

Motivation

This PR refactors the BaseTransaction.resolve() method, allowing its simulator patch to be removed.

Acceptance Criteria

  • Remove resolve() and start_mining() methods from BaseTransaction and from TokenCreationTransaction, moving them to the new CpuMiningService class. Code logic should not be changed, except for the addition of the TokenCreationTransaction case.
  • Add CpuMiningService to HathorManager, and update Builder and CliBuilder accordingly.
  • Rename simulator/verification.py to simulator/patches.py, adding the new SimulatorCpuMiningService class that overrides resolve().
  • Remove all previous patching-related code from Simulator, that now only uses injected customizations.
  • Update all usages of resolve() and start_mining().

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

@glevco glevco self-assigned this Sep 29, 2023
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from cf8df1d to 75f9964 Compare September 29, 2023 22:42
@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch 3 times, most recently from 4f8296a to 0ae446e Compare September 29, 2023 22:58
@glevco glevco marked this pull request as ready for review September 29, 2023 23:03
@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

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

Comparison is base (9150878) 85.09% compared to head (9cbb87e) 85.05%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #803      +/-   ##
==========================================
- Coverage   85.09%   85.05%   -0.05%     
==========================================
  Files         275      276       +1     
  Lines       22516    22514       -2     
  Branches     3434     3430       -4     
==========================================
- Hits        19161    19149      -12     
- Misses       2672     2678       +6     
- Partials      683      687       +4     
Files Coverage Δ
hathor/builder/builder.py 91.59% <100.00%> (+0.20%) ⬆️
hathor/builder/cli_builder.py 75.12% <100.00%> (+0.25%) ⬆️
hathor/manager.py 82.19% <100.00%> (+0.05%) ⬆️
hathor/simulator/simulator.py 93.38% <100.00%> (-1.10%) ⬇️
hathor/transaction/base_transaction.py 96.13% <100.00%> (+2.10%) ⬆️
hathor/transaction/token_creation_tx.py 99.11% <ø> (-0.04%) ⬇️
hathor/wallet/resources/nano_contracts/execute.py 85.00% <100.00%> (ø)
hathor/wallet/resources/send_tokens.py 98.01% <100.00%> (ø)
hathor/wallet/resources/sign_tx.py 100.00% <100.00%> (ø)
hathor/wallet/resources/thin_wallet/send_tokens.py 68.51% <100.00%> (ø)
... and 2 more

... and 5 files with indirect coverage changes

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

@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from 75f9964 to fd22533 Compare October 10, 2023 03:09
@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch 2 times, most recently from 0348cc0 to dc7d9de Compare October 10, 2023 03:35
jansegre
jansegre previously approved these changes Oct 10, 2023
@glevco glevco marked this pull request as draft October 10, 2023 21:09
msbrogli
msbrogli previously approved these changes Oct 18, 2023
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from fd22533 to 88c0162 Compare November 3, 2023 18:52
@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch from dc7d9de to 639fd12 Compare November 3, 2023 20:15
@glevco glevco marked this pull request as ready for review November 3, 2023 20:16
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from 88c0162 to 3926fa5 Compare November 3, 2023 21:09
@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch from 639fd12 to 0a780e9 Compare November 3, 2023 21:10
Base automatically changed from refactor/simulator-patches/daa-test-mode to master November 3, 2023 22:34
@glevco glevco dismissed stale reviews from msbrogli and jansegre November 3, 2023 22:34

The base branch was changed.

@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch from 0a780e9 to 0ad3797 Compare November 3, 2023 22:35
jansegre
jansegre previously approved these changes Nov 3, 2023
@glevco glevco force-pushed the refactor/simulator-patches/cpu-mining-service branch from 0ad3797 to 9cbb87e Compare November 4, 2023 18:49
@glevco glevco merged commit 3c8698e into master Nov 6, 2023
9 checks passed
@glevco glevco deleted the refactor/simulator-patches/cpu-mining-service branch November 6, 2023 19:47
@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