feat(nano): implement fields module [part 5] by glevco · Pull Request #1280 · HathorNetwork/hathor-core

glevco · 2025-05-30T17:31:00Z

Motivation

Implement Blueprint and fields module.

Acceptance Criteria

Implement Blueprint and fields module.

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 · 2025-06-03T02:45:06Z

Codecov Report

Attention: Patch coverage is 53.37079% with 249 lines in your changes missing coverage. Please review.

Project coverage is 80.91%. Comparing base (a096b87) to head (80d9f31).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
hathor/nanocontracts/fields/deque_field.py	44.37%	89 Missing ⚠️
hathor/nanocontracts/fields/set_field.py	49.01%	52 Missing ⚠️
hathor/nanocontracts/fields/dict_field.py	51.51%	48 Missing ⚠️
hathor/nanocontracts/blueprint.py	33.33%	33 Missing and 1 partial ⚠️
hathor/nanocontracts/fields/container_field.py	79.16%	10 Missing ⚠️
hathor/nanocontracts/fields/field.py	74.28%	9 Missing ⚠️
hathor/nanocontracts/fields/nc_type_field.py	76.19%	5 Missing ⚠️
hathor/nanocontracts/fields/__init__.py	86.66%	2 Missing ⚠️

❌ Your project status has failed because the head coverage (80.91%) is below the target coverage (82.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1280      +/-   ##
==========================================
- Coverage   81.57%   80.91%   -0.66%     
==========================================
  Files         373      382       +9     
  Lines       26697    27231     +534     
  Branches     4060     4155      +95     
==========================================
+ Hits        21778    22035     +257     
- Misses       4188     4453     +265     
- Partials      731      743      +12

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

glevco · 2025-06-09T12:28:36Z

hathor/nanocontracts/fields/field.py

+    class TypeMap(NamedTuple):
+        alias_map: TypeAliasMap
+        nc_types_map: TypeToNCTypeMap
+        fields_map: TypeToFieldMap


I think these attributes need documentation.

Postponed thread.

hathor/nanocontracts/fields/container_field.py

glevco · 2025-06-09T12:52:34Z

hathor/nanocontracts/fields/deque_field.py

+    def __check_name_and_type__(cls, name: str, type_: type[Sequence[T]]) -> None:
+        if not name.isidentifier():
+            raise TypeError('field name must be a valid identifier')
+        origin_type: type[Sequence[T]] = get_origin(type_) or type_
+        if not issubclass(origin_type, Sequence):
+            raise TypeError('expected Sequence type')
+        args = get_args(type_)
+        if not args or len(args) != 1:
+            raise TypeError(f'expected {type_.__name__}[<item type>]')


Check that item type is serializable?

I'm not sure we need to check this here, because this method is only called during creation of a Field instance, which is already only called when a field is serializable (that is, supported by our system). But maybe we indeed do for the type args of containers. Confirm this.

Postponed thread.

glevco · 2025-06-09T13:01:17Z

hathor/nanocontracts/fields/deque_field.py

+    def __to_db_key(self, index: SupportsIndex) -> bytes:
+        return f'{self.__name}{KEY_SEPARATOR}'.encode() + _INDEX_NC_TYPE.to_bytes(index.__index__())


Why not int instead of SupportsIndex? It's more specific, and the _INDEX_NC_TYPE expects an int.

Or maybe just assert int if we need this signature to satisfy mypy.

The generic typing of deque and also other indexable container, use SupportsIndex for describing what types can be used, I've let this spill a bit into some methods (instead of immediately using index.__index__() to get an int) to simplify some implementations, but I guess it's fine to do the opposite and mostly use int and expose SupportsIndex only at the outer surfaces.

Postponed thread.

glevco · 2025-06-09T13:06:45Z

hathor/nanocontracts/fields/deque_field.py

+        self.__update_metadata(new_metadata)
+        return item
+
+    def __to_internal_index(self, *, index: SupportsIndex) -> int:


Same here (use int?).

Postponed thread.

glevco · 2025-06-09T13:21:49Z

hathor/nanocontracts/fields/set_field.py

+    # XXX: what to do with this:
+    # __hash__: ClassVar[None]  # type: ignore[assignment]


In the Deque field, it's uncommented.

Postponed thread.

hathor/nanocontracts/fields/set_field.py

hathor/nanocontracts/fields/dict_field.py

hathor/nanocontracts/fields/set_field.py

hathor/nanocontracts/fields/container_field.py

hathor/nanocontracts/fields/deque_field.py

hathor/nanocontracts/fields/set_field.py

hathor/nanocontracts/fields/dict_field.py

github-actions · 2025-06-09T19:09:08Z

Bencher Report

Branch	feat/nano/fields
Testbed	ubuntu-22.04

Click to view all benchmark results

Benchmark	Latency	Benchmark Result minutes (m) (Result Δ%)	Lower Boundary minutes (m) (Limit %)	Upper Boundary minutes (m) (Limit %)
sync-v2 (up to 20000 blocks)	📈 view plot 🚷 view threshold	1.60 m (-1.72%) Baseline: 1.63 m	1.47 m (91.57%)	1.79 m (89.35%)

🐰 View full continuous benchmarking report in Bencher

jansegre

Currently I have changes that touch on all of the container fields, I will look at these comments to already implement at least the trivial fixes.

jansegre · 2025-06-09T19:34:09Z

hathor/nanocontracts/fields/__init__.py

+
+DEFAULT_TYPE_TO_FIELD_MAP: TypeToFieldMap = {
+    dict: DictField,
+    list: DequeField,  # XXX: we should really make a ListField, a deque is different from a list


I think we should do this. At least the "backend" implementation (how it maps the exposed interface to the storage) can be exactly the same, but Deque is not just the implementation it is the exposed interface, which IMO should be different when only a list (and not a deque), is requested.

Postponed thread.

hathor/nanocontracts/fields/container_field.py

jansegre · 2025-06-09T21:38:10Z

hathor/nanocontracts/fields/deque_field.py

+    def __to_db_key(self, index: SupportsIndex) -> bytes:
+        return f'{self.__name}{KEY_SEPARATOR}'.encode() + _INDEX_NC_TYPE.to_bytes(index.__index__())


The generic typing of deque and also other indexable container, use SupportsIndex for describing what types can be used, I've let this spill a bit into some methods (instead of immediately using index.__index__() to get an int) to simplify some implementations, but I guess it's fine to do the opposite and mostly use int and expose SupportsIndex only at the outer surfaces.

hathor/nanocontracts/fields/dict_field.py

glevco · 2025-06-10T17:05:10Z

hathor/nanocontracts/fields/container_field.py

For all container fields, review which methods we want to provide. See #1280 (comment)

Postponed thread.

glevco · 2025-06-10T18:13:04Z

LGTM ✅

Co-authored-by: Marcelo Salhab Brogliato <msbrogli@gmail.com> Co-authored-by: Jan Segre <jan@hathor.network>

glevco self-assigned this May 30, 2025

glevco requested review from jansegre and msbrogli as code owners May 30, 2025 17:31

glevco added this to Hathor Network May 30, 2025

github-project-automation bot moved this to Todo in Hathor Network May 30, 2025

glevco moved this from Todo to In Progress (Done) in Hathor Network May 30, 2025

glevco force-pushed the feat/nano/storages branch from ccbd984 to 646105d Compare June 3, 2025 02:26

glevco force-pushed the feat/nano/fields branch from 23eafc3 to fb47abd Compare June 3, 2025 02:26

glevco force-pushed the feat/nano/storages branch from 646105d to 5a6ca04 Compare June 3, 2025 18:43

glevco force-pushed the feat/nano/fields branch from fb47abd to af0c314 Compare June 3, 2025 18:44

glevco force-pushed the feat/nano/storages branch from 5a6ca04 to ded9471 Compare June 3, 2025 19:58

glevco force-pushed the feat/nano/fields branch from af0c314 to 63ba20c Compare June 3, 2025 19:58

glevco force-pushed the feat/nano/storages branch 3 times, most recently from 72d6938 to d2eaf78 Compare June 6, 2025 19:54

jansegre deleted the branch master June 6, 2025 22:15

jansegre closed this Jun 6, 2025

github-project-automation bot moved this from In Progress (Done) to Waiting to be deployed in Hathor Network Jun 6, 2025

glevco moved this from Waiting to be deployed to In Review (Done) in Hathor Network Jun 9, 2025

glevco reopened this Jun 9, 2025

glevco changed the base branch from feat/nano/storages to master June 9, 2025 12:08

glevco force-pushed the feat/nano/fields branch from 63ba20c to 8857503 Compare June 9, 2025 12:10

glevco moved this from In Review (Done) to In Progress (Done) in Hathor Network Jun 9, 2025

glevco force-pushed the feat/nano/fields branch from 8857503 to 8052bb7 Compare June 9, 2025 12:18

glevco commented Jun 9, 2025

View reviewed changes

glevco moved this from In Progress (Done) to In Review (WIP) in Hathor Network Jun 9, 2025

glevco commented Jun 9, 2025

View reviewed changes

glevco force-pushed the feat/nano/fields branch from 8052bb7 to 4065616 Compare June 9, 2025 19:01

jansegre previously approved these changes Jun 9, 2025

View reviewed changes

glevco moved this from In Review (WIP) to In Review (Done) in Hathor Network Jun 10, 2025

glevco dismissed jansegre’s stale review via bba8a09 June 10, 2025 16:54

glevco commented Jun 10, 2025

View reviewed changes

jansegre approved these changes Jun 10, 2025

View reviewed changes

feat(nano): implement fields module

80d9f31

Co-authored-by: Marcelo Salhab Brogliato <msbrogli@gmail.com> Co-authored-by: Jan Segre <jan@hathor.network>

glevco force-pushed the feat/nano/fields branch from bba8a09 to 80d9f31 Compare June 10, 2025 18:51

jansegre merged commit f5befca into master Jun 10, 2025
7 checks passed

jansegre deleted the feat/nano/fields branch June 10, 2025 18:53

github-project-automation bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Jun 10, 2025

jansegre mentioned this pull request Jul 22, 2025

Release-candidate v0.64.0-rc.1 #1331

Merged

2 tasks

jansegre moved this from Waiting to be deployed to Done in Hathor Network Jul 22, 2025

jansegre mentioned this pull request Aug 7, 2025

Release v0.64.0 #1352

Merged

2 tasks

		def __to_db_key(self, index: SupportsIndex) -> bytes:
		return f'{self.__name}{KEY_SEPARATOR}'.encode() + _INDEX_NC_TYPE.to_bytes(index.__index__())

		# XXX: what to do with this:
		# __hash__: ClassVar[None] # type: ignore[assignment]

Conversation

glevco commented May 30, 2025

Motivation

Acceptance Criteria

Checklist

Uh oh!

codecov bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jansegre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glevco commented Jun 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jun 9, 2025 •

edited

Loading