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

Make adm compatible with new contracts #2662

Merged
merged 4 commits into from
Dec 1, 2023

Conversation

roman-khimov
Copy link
Member

No description provided.

@roman-khimov roman-khimov added the neofs-adm NeoFS Adm application issues label Nov 30, 2023
@roman-khimov roman-khimov added this to the v0.39.0 milestone Nov 30, 2023
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Attention: 43 lines in your changes are missing coverage. Please review.

Comparison is base (f33f242) 30.43% compared to head (4e22e02) 30.40%.

Files Patch % Lines
cmd/neofs-adm/internal/modules/morph/initialize.go 0.00% 19 Missing ⚠️
...fs-adm/internal/modules/morph/initialize_deploy.go 0.00% 14 Missing ⚠️
...neofs-adm/internal/modules/morph/initialize_nns.go 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2662      +/-   ##
==========================================
- Coverage   30.43%   30.40%   -0.03%     
==========================================
  Files         406      406              
  Lines       30068    30095      +27     
==========================================
  Hits         9150     9150              
- Misses      20134    20161      +27     
  Partials      784      784              

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

"register" doesn't work for TLDs since 0.18.0.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
With current adm it's the easiest way to ensure domains are present when
we're deploying contracts. Additional NNS parameters are ignored by old
contracts, but used in 0.18.0+ versions.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Old (pre-0.18.0) versions will just ignore this, but newer ones will deploy
the TLD we need.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
 * netmap must be deployed first, balance and container depend on it
 * they also call it during deployment and need an appropriate witness
 * we can't send independent deployment transactions in this scenario
 * but our contract set fits into a single transaction, luckily

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@@ -290,6 +290,20 @@ func (c *initializeContext) getSigner(fancyScope bool, acc *wallet.Account) tran
}

signer = morphClient.GetUniversalSignerScope(nnsHash, balanceHash, cntHash, netmapHash, neofsIDHash)
// Deploy-only rules.
Copy link
Member

Choose a reason for hiding this comment

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

dangerous, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

But why? They're never needed during regular operation.

@roman-khimov roman-khimov merged commit 78823a3 into master Dec 1, 2023
7 of 11 checks passed
@roman-khimov roman-khimov deleted the make-adm-compatible-with-new-contracts branch December 1, 2023 19:02
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