-
Notifications
You must be signed in to change notification settings - Fork 85
Fix null handling in manifest #3523
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
Conversation
IsValid() is used by both compiler and ContractManagement then. Signed-off-by: Roman Khimov <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3523 +/- ##
==========================================
- Coverage 86.10% 86.10% -0.01%
==========================================
Files 330 330
Lines 38646 38660 +14
==========================================
+ Hits 33276 33288 +12
- Misses 3830 3833 +3
+ Partials 1540 1539 -1 ☔ View full report in Codecov by Sentry. |
Refs. #3522. Signed-off-by: Roman Khimov <[email protected]>
Refs. #3522. The core problem is the same as for groups/features: we can't allow empty trusts when they're unmarshalled from JSON. But unlike others we can't easily differentiate missing any value with other cases because the default value for WildPermissionDescs is a valid thing. Adding an additional field makes it invalid and we can build around it. Other options are implementing custom UnmarshalJSON for Manifest (too much for this) or making Trusts a pointer (an option, but can fail in too many ways). Signed-off-by: Roman Khimov <[email protected]>
c993f2b
to
b10af1e
Compare
The fix is confirmed for testnet up to 4372513. |
Mainnet is OK up to 5759367. |
NeoFS mainnet is OK up to 6080412. |
NeoFS testnet is OK up to 3967861. |
Starting from b10af1e (*WildPermissionDescs).Add method's call is not enough to construct a proper restricted permission descriptor, because Wildcard field should be set properly at the same time. Ref. #3523. Signed-off-by: Anna Shaleva <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a tiny adjustment: #3544.
Starting from b10af1e (*WildPermissionDescs).Add method's call is not enough to construct a proper restricted permission descriptor, because Wildcard field should be set properly at the same time. Ref. #3523. Signed-off-by: Anna Shaleva <[email protected]>
Fix #3522.