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

Support new hash format in morph nns client #2063

Merged

Conversation

vdomnich-yadro
Copy link
Contributor

In #1748 we've introduced a small inconsistency in NNS hash parsing:

  • In function parseNNSResolveResult we support 2 hash formats (Uint160DecodeStringLE и StringToUint160).
  • In function nnsResolve we support only 1 format (Uint160DecodeStringLE).

This PR updates implementation of nnsResolve so that it is consistent with parseNNSResolveResult.

Per discussion with @fyrchik for now we just add missing logic to nnsResolve. In the future we will refactor implementation so that the logic is implemented in a single place.

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #2063 (9153ae3) into support/v0.34 (584f465) will increase coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 9153ae3 differs from pull request most recent head fe28484. Consider uploading reports for the commit fe28484 to get more accurate results

@@              Coverage Diff               @@
##           support/v0.34    #2063   +/-   ##
==============================================
  Coverage          30.66%   30.66%           
==============================================
  Files                380      380           
  Lines              28080    28077    -3     
==============================================
  Hits                8610     8610           
+ Misses             18731    18728    -3     
  Partials             739      739           
Impacted Files Coverage Δ
...neofs-adm/internal/modules/morph/initialize_nns.go 0.00% <ø> (ø)
pkg/morph/client/nns.go 0.00% <0.00%> (ø)
pkg/local_object_storage/writecache/flush.go 60.23% <0.00%> (+3.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

fyrchik
fyrchik previously approved these changes Nov 17, 2022
Copy link
Contributor

@fyrchik fyrchik left a comment

Choose a reason for hiding this comment

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

Could you add morph/client: prefix to your commit message?

@sami-nspcc
Copy link

I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing.

carpawell
carpawell previously approved these changes Nov 17, 2022
@sami-nspcc
Copy link

I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing.

@vdomnich-yadro
Copy link
Contributor Author

vdomnich-yadro commented Nov 17, 2022

Thanks for approvals!

@realloc I don't have permissions to merge PR by myself 😔

@sami-nspcc
Copy link

I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing.

@alexchetaev alexchetaev added U3 Regular and removed 2022Q4 labels Nov 18, 2022
…lient

Signed-off-by: Vladimir Domnich <v.domnich@yadro.com>
@sami-nspcc
Copy link

I have recieved a notification of a new pull request. I am starting the build of images and binaries for further testing.

@fyrchik fyrchik merged commit b9a24e9 into nspcc-dev:support/v0.34 Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working U3 Regular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants