Skip to content

feat(feature-activation): implement must signal #785

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 1, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 25, 2023

Depends on #800

Motivation

Implement the mandatory signaling mechanism required by the Feature Activation process.

Acceptance Criteria

  • Improvements in Builder:
    • Add FeatureService to BuildArtifacts
    • Change implementation of some _get_or_create_* methods so they can all be used without arguments.
    • Change private _get_or_create_settings() and _get_or_create_feature_service() methods to public so they can be used externally (specifically, so the Simulator can properly customize the builder).
  • Implement FeatureService.check_must_signal().
  • Fix problem in caching mechanism in FeatureService.get_state() caused by must signal verification.
  • Remove the original Block.get_feature_activation_bit_counts(), and change calculate_feature_activation_bit_counts() to get_feature_activation_bit_counts().
  • Change Block.update_feature_state() to set_feature_state().
  • Remove BaseTransaction._update_feature_activation_bit_counts_metadata() as it's not necessary anymore.
  • Implement BlockVerifier.verify_mandatory_signaling().

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 25, 2023
@glevco glevco changed the title wip feat(feature-activation): implement must signal Sep 25, 2023
@glevco glevco force-pushed the feat/feature-activation/must-signal branch 2 times, most recently from d34ad69 to f2e6a65 Compare September 25, 2023 16:50
@glevco glevco force-pushed the refactor/verification-service branch 2 times, most recently from 1121a3c to fe3c2e9 Compare September 25, 2023 22:04
Base automatically changed from refactor/verification-service to master September 26, 2023 11:53
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from f2e6a65 to a954e43 Compare September 27, 2023 16:24
@glevco glevco changed the base branch from master to refactor/move-verification-methods September 27, 2023 16:24
@glevco glevco force-pushed the feat/feature-activation/must-signal branch 7 times, most recently from 43c3ede to dd6cfa1 Compare September 28, 2023 15:01
@glevco glevco force-pushed the refactor/move-verification-methods branch from 0bf3cfd to a5b0b94 Compare September 29, 2023 03:38
@glevco glevco force-pushed the feat/feature-activation/must-signal branch 2 times, most recently from 706bb68 to 8cb11f7 Compare September 29, 2023 03:43
@glevco glevco force-pushed the refactor/move-verification-methods branch from a5b0b94 to da50f53 Compare September 29, 2023 13:41
@glevco glevco force-pushed the feat/feature-activation/must-signal branch 3 times, most recently from 8f1dd2f to 607a83d Compare September 29, 2023 14:19
@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

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

Comparison is base (bf0d0bc) 84.61% compared to head (b6294a8) 85.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
+ Coverage   84.61%   85.00%   +0.38%     
==========================================
  Files         271      271              
  Lines       22311    22362      +51     
  Branches     3406     3416      +10     
==========================================
+ Hits        18878    19008     +130     
+ Misses       2764     2677      -87     
- Partials      669      677       +8     
Files Coverage Δ
hathor/builder/builder.py 91.10% <100.00%> (-0.21%) ⬇️
hathor/builder/cli_builder.py 74.48% <100.00%> (ø)
hathor/feature_activation/feature_service.py 98.09% <100.00%> (+0.56%) ⬆️
hathor/simulator/simulator.py 94.61% <100.00%> (+0.09%) ⬆️
hathor/transaction/base_transaction.py 94.02% <ø> (-0.13%) ⬇️
hathor/transaction/block.py 94.52% <100.00%> (+0.13%) ⬆️
hathor/transaction/exceptions.py 100.00% <100.00%> (ø)
hathor/transaction/transaction_metadata.py 92.26% <ø> (ø)
hathor/verification/verification_service.py 90.32% <100.00%> (+0.10%) ⬆️
hathor/verification/block_verifier.py 97.14% <88.88%> (-2.86%) ⬇️

... and 7 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 feat/feature-activation/must-signal branch 2 times, most recently from 808620c to c0e2be9 Compare September 29, 2023 15:26
@glevco glevco marked this pull request as ready for review September 29, 2023 15:39
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from c0e2be9 to e61761d Compare September 29, 2023 20:40
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from d6caa5c to e62cc24 Compare October 17, 2023 21:14
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from 709bf10 to ebd7ea9 Compare October 17, 2023 21:21
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from e62cc24 to 07ff57b Compare October 19, 2023 20:31
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from ebd7ea9 to afd4ade Compare October 19, 2023 20:31
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch from 07ff57b to 363be05 Compare October 24, 2023 20:55
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from afd4ade to d90d744 Compare October 24, 2023 21:09
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 4 times, most recently from f5f7fe7 to bc6f21d Compare October 25, 2023 16:00
@glevco glevco force-pushed the feat/feature-activation/must-signal branch 2 times, most recently from 6f3992c to 01c52c0 Compare October 25, 2023 16:11
@glevco glevco force-pushed the refactor/move-verification/vertex-methods branch 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 feat/feature-activation/must-signal branch from 01c52c0 to d4ee187 Compare October 25, 2023 17:15
msbrogli
msbrogli previously approved these changes Oct 31, 2023
@glevco glevco force-pushed the feat/feature-activation/must-signal branch from c070320 to b6294a8 Compare November 1, 2023 15:58
@glevco glevco merged commit b3d41ed into master Nov 1, 2023
@glevco glevco deleted the feat/feature-activation/must-signal branch November 1, 2023 19:16
@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
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants