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

adm: never use GetContractStateByID(1) to retrieve NNS hash #2841

Merged
merged 1 commit into from
May 6, 2024

Conversation

roman-khimov
Copy link
Member

And use a bit more of NNS wrapper.

Notice that this changes "alphabet X" to "alphabetX" in dump output, but I consider this to be an improvement (since that's the contract name).

It also deoptimizes some carefully crafted code to extract a number of names in one call. Likely it's not a performance issue you care about and it's actually somewhat broken in that it assumes one result per invocation (but you can get zero or more than one).

Many more things can be improved, but let's start with this.

Refs. #2826.

@roman-khimov roman-khimov added the neofs-adm NeoFS Adm application issues label Apr 27, 2024
@roman-khimov roman-khimov added this to the v0.42.0 milestone Apr 27, 2024
Copy link

codecov bot commented Apr 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 108 lines in your changes are missing coverage. Please review.

Project coverage is 23.65%. Comparing base (dd9cee4) to head (1eace4a).
Report is 9 commits behind head on master.

Files Patch % Lines
...md/neofs-adm/internal/modules/morph/dump_hashes.go 0.00% 15 Missing ⚠️
...neofs-adm/internal/modules/morph/initialize_nns.go 0.00% 15 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/deploy.go 0.00% 13 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/container.go 0.00% 9 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/initialize.go 0.00% 9 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/config.go 0.00% 8 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/balance.go 0.00% 7 Missing ⚠️
...fs-adm/internal/modules/morph/initialize_deploy.go 0.00% 6 Missing ⚠️
cmd/neofs-adm/internal/modules/morph/estimation.go 0.00% 5 Missing ⚠️
...d/neofs-adm/internal/modules/morph/renew_domain.go 0.00% 5 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2841      +/-   ##
==========================================
- Coverage   31.34%   23.65%   -7.70%     
==========================================
  Files         435      769     +334     
  Lines       32639    44564   +11925     
==========================================
+ Hits        10231    10541     +310     
- Misses      21517    33172   +11655     
+ Partials      891      851      -40     

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

And use a bit more of NNS wrapper.

Notice that this changes "alphabet X" to "alphabetX" in dump output, but I
consider this to be an improvement (since that's the contract name).

It also deoptimizes some carefully crafted code to extract a number of names in
one call. Likely it's not a performance issue you care about and it's actually
somewhat broken in that it assumes one result per invocation (but you can get
zero or more than one).

Many more things can be improved, but let's start with this.

Refs. #2826.

Signed-off-by: Roman Khimov <[email protected]>
@roman-khimov roman-khimov merged commit ce9a936 into master May 6, 2024
14 of 19 checks passed
@roman-khimov roman-khimov deleted the adm-nns branch May 6, 2024 19:39
roman-khimov added a commit to nspcc-dev/neofs-contract that referenced this pull request May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-adm NeoFS Adm application issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants