Drop contract group scope #2625
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #2625 +/- ##
==========================================
+ Coverage 29.04% 29.22% +0.17%
==========================================
Files 417 416 -1
Lines 31559 31318 -241
==========================================
- Hits 9166 9152 -14
+ Misses 21590 21366 -224
+ Partials 803 800 -3
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
roman-khimov
force-pushed
the
contract-group-scope
branch
4 times, most recently
from
67b551e
to
be5bfea
Compare
roman-khimov
requested review from
carpawell and
cthulhu-rider
as code owners
carpawell
reviewed
Oct 23, 2023
| signer.Scopes = transaction.CustomGroups | ||
| signer.AllowedGroups = keys.PublicKeys{groupKey} | ||
| balanceHash, err := nnsReader.ResolveFSContract(nns.NameBalance) | ||
| if err != nil && c.Contracts[balanceContract] != nil { |
There was a problem hiding this comment.
but what if err != nil and no hash has been cached?
There was a problem hiding this comment.
It means that we don't know the hash and it stays zero. In this case it's irrelevant, we don't have data from contract, we don't have data from the network, let's hope it works, but if it doesn't we can't help much.
There was a problem hiding this comment.
passing zero hash and continuing working scares me. but ok
|
Conflicts. |
It's a part of a Signer as well and it can be used. Signed-off-by: Roman Khimov <roman@nspcc.ru>
And fail if SetGroupSignerScope() fails, it MUST work on a valid network. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
It's not the most elegant way to handle the problem since normally each contract handler should decide on these things by its own (even per-call), but given the structure we have this is the easiest "universal" set of rules. It has a downside in that it MUST be synchronized with contracts changes, but those are not very frequent, so I think it's good enough. Managing group key is far worse problem than that. Signed-off-by: Roman Khimov <roman@nspcc.ru>
No longer needed. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
Signed-off-by: Roman Khimov <roman@nspcc.ru>
roman-khimov
force-pushed
the
contract-group-scope
branch
from
be5bfea
to
e501986
Compare
carpawell
approved these changes
Oct 24, 2023
Ok, not this time. |
cthulhu-rider
added a commit
that referenced
this pull request
Nov 15, 2023
Starting from c08e2b5 (#2625), there is no need to maintain committee group and its private key because CustomGroups witness scope is no longer used. Completely purge committee group from NeoFS Sidechain auto-deployment procedure. As a result, contract manifests are no longer modified. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
cthulhu-rider
added a commit
that referenced
this pull request
Nov 15, 2023
Starting from c08e2b5 (#2625), there is no need to maintain committee group and its private key because CustomGroups witness scope is no longer used. Completely purge committee group from NeoFS Sidechain auto-deployment procedure. As a result, contract manifests are no longer modified. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
cthulhu-rider
added a commit
that referenced
this pull request
Nov 15, 2023
Starting from c08e2b5 (#2625), there is no need to maintain committee group and its private key because CustomGroups witness scope is no longer used. Completely purge committee group from NeoFS Sidechain auto-deployment procedure. As a result, contract manifests are no longer modified. Closes #2642. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
roman-khimov
added a commit
to nspcc-dev/neofs-contract
that referenced
this pull request
Nov 29, 2023
I'm not yet sure we need it, but old adm (before nspcc-dev/neofs-node#2625) creates a broken deploy script with actual data that has no fields inside. Not a problem for updates, not a problem for newer adm, but still. Signed-off-by: Roman Khimov <roman@nspcc.ru>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.