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(metadata): move metadata attributes to static metadata [part 2/12] #1014

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

glevco
Copy link
Contributor

@glevco glevco commented May 1, 2024

Motivation

This PR continues and completes the implementation for a working version of the static metadata refactor started in the previous PR, #1013. Read its description for more detailed information.

It moves the height, min_height, and feature_activation_bit_counts metadata attributes to the new static structure. The feature_states attribute will be migrated in the next PR.

As a consequence, it introduces several benefits for metadata handling, including:

  • Blocks and Transactions now have separate types for their static metadata attributes, which are different. No more handling of non-existent attributes for different types, like setting height=0 for transactions.
  • All vertices are required to have static metadata set before they're saved to the storage, meaning when a vertex is retrieved from the storage, it's guaranteed to have static metadata. No need to perform updates and such.
  • Static metadata can be interpreted more as attributes that are as constant and present as intrinsic vertex attributes, and less like dynamic metadata attributes (like voided_by and validation).

Acceptance Criteria

  • Remove height, min_height, and feature_activation_bit_counts attributes from metadata, moving them to static metadata.
    • Provide new equivalent metadata properties that are a forward from static metadata, for backwards compatibility.
    • Keep those properties in the to_json() method, for backwards compatibility with APIs and etc.
  • Move some metadata-related methods from BaseTransaction and its subclasses to the static metadata dataclasses, and remove some that are not necessary anymore.
    • Changes were made so they do not depend on a storage anymore.
  • Implement create() and create_from_storage() methods for static metadata dataclasses.
  • Implement migrate_static_metadata Migration to move existing metadata attributes to the new static metadata structure.
  • Changes in TransactionStorage and its subclasses:
    • Save static metadata in save_transaction() and retrieve/set it in get_transaction() and get_all_transactions().
    • Remove _validate_block_height_metadata().
    • Implement iter_all_raw_metadata() method used in the migration.
  • Fix and update code and tests accordingly.

Breaking Changes

  • Before, all API calls that returned metadata included both a height and a feature_activation_bit_counts field for all vertices. For transactions, they were always falsy values (0, None, or []). Now, they were removed from transactions and are only present for blocks.

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 May 1, 2024
Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 83.20312% with 43 lines in your changes missing coverage. Please review.

Project coverage is 84.54%. Comparing base (9bce2fa) to head (701b787).
Report is 2 commits behind head on master.

Files Patch % Lines
hathor/transaction/storage/rocksdb_storage.py 48.57% 18 Missing ⚠️
...tion/storage/migrations/migrate_static_metadata.py 53.84% 12 Missing ⚠️
hathor/manager.py 50.00% 1 Missing and 4 partials ⚠️
hathor/transaction/transaction_metadata.py 86.95% 0 Missing and 3 partials ⚠️
...on/storage/migrations/remove_first_nop_features.py 33.33% 2 Missing ⚠️
hathor/mining/block_template.py 0.00% 1 Missing ⚠️
hathor/transaction/storage/cache_storage.py 75.00% 1 Missing ⚠️
hathor/verification/transaction_verifier.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1014      +/-   ##
==========================================
- Coverage   84.90%   84.54%   -0.37%     
==========================================
  Files         316      317       +1     
  Lines       24079    24149      +70     
  Branches     3645     3665      +20     
==========================================
- Hits        20445    20417      -28     
- Misses       2918     3018     +100     
+ Partials      716      714       -2     

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

@glevco glevco added the refactor label May 1, 2024
@glevco glevco force-pushed the feat/metadata/create-static-metadata-1 branch from ce2cc12 to c7a3ff8 Compare May 2, 2024 15:18
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 2 times, most recently from 942358a to 3912750 Compare May 2, 2024 15:57
@glevco glevco marked this pull request as ready for review May 2, 2024 16:30
@glevco glevco requested review from jansegre and msbrogli as code owners May 2, 2024 16:30
@glevco glevco changed the title refactor(metadata): move metadata attributes to static metadata refactor(metadata): move metadata attributes to static metadata [part 2] May 2, 2024
@glevco glevco mentioned this pull request May 2, 2024
1 task
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 2 times, most recently from 1372d65 to 9e960f7 Compare May 3, 2024 01:16
@glevco glevco force-pushed the feat/metadata/create-static-metadata-1 branch from c7a3ff8 to 079afcc Compare May 3, 2024 21:39
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 2 times, most recently from a43cb95 to bd946c3 Compare May 8, 2024 17:43
@glevco glevco force-pushed the feat/metadata/create-static-metadata-1 branch from 079afcc to 17e34da Compare May 10, 2024 01:50
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 4 times, most recently from 2aa27b2 to f805f3c Compare May 10, 2024 02:47
@glevco glevco force-pushed the feat/metadata/create-static-metadata-1 branch from 17e34da to 3d51375 Compare May 13, 2024 14:24
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from f805f3c to 4b95949 Compare May 13, 2024 14:24
@glevco glevco changed the title refactor(metadata): move metadata attributes to static metadata [part 2] refactor(metadata): move metadata attributes to static metadata [part 2/12] May 13, 2024
@glevco glevco force-pushed the feat/metadata/create-static-metadata-1 branch from 3d51375 to def1db9 Compare July 16, 2024 02:02
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from 4b95949 to cc05543 Compare July 16, 2024 02:06
jansegre
jansegre previously approved these changes Aug 16, 2024
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 4 times, most recently from 7e7fdc8 to 31f9b03 Compare August 19, 2024 16:27
@glevco glevco changed the base branch from master to chore/improve-bench August 19, 2024 18:09
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from 31f9b03 to a4e34f1 Compare August 19, 2024 18:17
Base automatically changed from chore/improve-bench to master August 19, 2024 19:06
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from a4e34f1 to 8d4f344 Compare August 19, 2024 19:08
jansegre
jansegre previously approved these changes Aug 20, 2024
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from 8d4f344 to 98e21ad Compare August 20, 2024 21:09
@glevco glevco dismissed stale reviews from jansegre and msbrogli via 4a88618 August 20, 2024 21:33
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch 6 times, most recently from 534b280 to fcbcac1 Compare August 22, 2024 15:55
@glevco glevco force-pushed the refactor/metadata/migrate-static-metadata-2 branch from fcbcac1 to 701b787 Compare August 22, 2024 16:35
@glevco glevco merged commit f6ddf3c into master Aug 22, 2024
12 of 13 checks passed
@glevco glevco deleted the refactor/metadata/migrate-static-metadata-2 branch August 22, 2024 17:30
@jansegre jansegre mentioned this pull request Oct 4, 2024
2 tasks
@jansegre jansegre mentioned this pull request Dec 11, 2024
2 tasks
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