Skip to content

feat(nano): miscellaneous changes [part 8]#1285

Merged
jansegre merged 1 commit intomasterfrom
feat/nano/misc
Jun 13, 2025
Merged

feat(nano): miscellaneous changes [part 8]#1285
jansegre merged 1 commit intomasterfrom
feat/nano/misc

Conversation

@glevco
Copy link
Contributor

@glevco glevco commented Jun 3, 2025

Motivation

Merge multiple minor changes from Nano.

Acceptance Criteria

  • Multiple minor changes.

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 Jun 3, 2025
@glevco glevco requested review from jansegre and msbrogli as code owners June 3, 2025 19:39
@glevco glevco changed the title feat(nano): miscellaneous changes feat(nano): miscellaneous changes [part 8] Jun 3, 2025
@glevco glevco moved this from Todo to In Progress (Done) in Hathor Network Jun 3, 2025
@glevco glevco force-pushed the feat/nano/indexes branch from e6c3a52 to a13b182 Compare June 3, 2025 20:00
@glevco glevco force-pushed the feat/nano/misc branch 3 times, most recently from ec39a24 to 0ea3582 Compare June 3, 2025 22:29
@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 34.81229% with 382 lines in your changes missing coverage. Please review.

Project coverage is 78.11%. Comparing base (47c3dd1) to head (b78fea7).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
hathor/dag_builder/vertex_exporter.py 28.44% 78 Missing ⚠️
hathor/dag_builder/builder.py 20.00% 60 Missing and 4 partials ⚠️
hathor/transaction/storage/transaction_storage.py 17.24% 48 Missing ⚠️
hathor/transaction/util.py 23.80% 32 Missing ⚠️
hathor/indexes/manager.py 35.71% 19 Missing and 8 partials ⚠️
hathor/builder/builder.py 50.00% 24 Missing and 2 partials ⚠️
hathor/dag_builder/tokenizer.py 23.07% 17 Missing and 3 partials ⚠️
hathor/dag_builder/types.py 33.33% 18 Missing ⚠️
hathor/transaction/transaction.py 34.61% 13 Missing and 4 partials ⚠️
hathor/dag_builder/utils.py 33.33% 14 Missing ⚠️
... and 7 more

❌ Your project status has failed because the head coverage (78.11%) 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    #1285      +/-   ##
==========================================
- Coverage   78.83%   78.11%   -0.72%     
==========================================
  Files         412      413       +1     
  Lines       29171    29699     +528     
  Branches     4500     4590      +90     
==========================================
+ Hits        22996    23200     +204     
- Misses       5402     5710     +308     
- Partials      773      789      +16     

☔ 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 glevco force-pushed the feat/nano/indexes branch 2 times, most recently from eefcf44 to 9e759b4 Compare June 11, 2025 18:46
@glevco glevco moved this from In Progress (Done) to In Review (WIP) in Hathor Network Jun 11, 2025
@glevco glevco force-pushed the feat/nano/misc branch 3 times, most recently from 9cbb5c8 to 77f963d Compare June 11, 2025 19:30
Comment on lines +233 to +236
self.check_or_raise(
not self._args.nc_history_index,
'--nc-history-index has been deprecated, use --nc-indices instead',
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

help='Create an index of transactions by address and allow searching queries')
parser.add_argument('--utxo-index', action='store_true',
help='Create an index of UTXOs by token/address/amount and allow searching queries')
parser.add_argument('--nc-history-index', action='store_true', help=SUPPRESS) # moved to --nc-indices
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

x_enable_ipv6: bool
x_disable_ipv4: bool
localnet: bool
nc_history_index: bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

Comment on lines +1153 to +1157
try:
contract_storage = block_storage.get_contract_storage(ContractId(NCVertexId(contract_id)))
except KeyError:
from hathor.nanocontracts.exception import NanoContractDoesNotExist
raise NanoContractDoesNotExist(contract_id.hex())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this exception handling because it's now done in get_contract_storage itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

@glevco
Copy link
Contributor Author

glevco commented Jun 11, 2025

LGTM ✅

@glevco glevco force-pushed the feat/nano/indexes branch from 9e759b4 to cb8cc56 Compare June 13, 2025 13:47
@jansegre jansegre deleted the branch master June 13, 2025 14:57
@jansegre jansegre closed this Jun 13, 2025
@github-project-automation github-project-automation bot moved this from In Review (WIP) to Waiting to be deployed in Hathor Network Jun 13, 2025
@glevco glevco moved this from Waiting to be deployed to In Review (Done) in Hathor Network Jun 13, 2025
@glevco glevco moved this from In Review (Done) to In Review (WIP) in Hathor Network Jun 13, 2025
@jansegre jansegre reopened this Jun 13, 2025
@jansegre jansegre changed the base branch from feat/nano/indexes to master June 13, 2025 15:07
@github-actions
Copy link

github-actions bot commented Jun 13, 2025

🐰 Bencher Report

Branchfeat/nano/misc
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark 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.63 m
(-0.15%)Baseline: 1.63 m
1.47 m
(90.14%)
1.79 m
(90.77%)
🐰 View full continuous benchmarking report in Bencher

jansegre
jansegre previously approved these changes Jun 13, 2025
Comment on lines +120 to +121
parser.add_argument('--nc-indices', action='store_true',
help='Enable indices related to nano contracts')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While "indices" is technically correct, "indexes" is also correct, I'd rather use that since it's what we've been using before, there's also this style guide which opts for indexes too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

MULTISIG_VERSION_BYTE=b'\x87',
NETWORK_NAME='nano-testnet-alpha',
BOOTSTRAP_DNS=[],
BOOTSTRAP_DNS=['alpha.nano-testnet.hathor.network'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it going to be alpha or bravo now? It probably doesn't sync with the current alpha so we might as well bump it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

raise NotImplementedError('temporarily removed during nano merge')
# used to initialize self.text with
match self.kind:
case CodeKind.PYTHON_GZIP:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be PYTHON_ZLIB now, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes aren't making anything right? If anything I'd rather move away from string annotations instead of moving to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postponed thread.

@glevco glevco moved this from In Review (WIP) to In Review (Done) in Hathor Network Jun 13, 2025
@jansegre jansegre merged commit bb1ff58 into master Jun 13, 2025
7 checks passed
@jansegre jansegre deleted the feat/nano/misc branch June 13, 2025 19:26
@github-project-automation github-project-automation bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Jun 13, 2025
Copy link
Member

@jansegre jansegre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ approved

@jansegre jansegre mentioned this pull request Jul 22, 2025
2 tasks
@jansegre jansegre moved this from Waiting to be deployed to Done in Hathor Network Jul 22, 2025
@jansegre jansegre mentioned this pull request Aug 7, 2025
2 tasks
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.

2 participants