-
Notifications
You must be signed in to change notification settings - Fork 50
Support setAttribute container contract method #3728
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
ac904c2 to
18979b1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3728 +/- ##
==========================================
- Coverage 26.88% 26.85% -0.04%
==========================================
Files 658 659 +1
Lines 41829 41873 +44
==========================================
- Hits 11246 11243 -3
- Misses 29548 29595 +47
Partials 1035 1035 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| cp.log.Debug("notification", | ||
| zap.String("type", "set attribute"), | ||
| zap.String("cID", string(e.CID)), |
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.
| zap.String("cID", string(e.CID)), | |
| zap.String("cID", base58.Encode(e.CID)), |
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.
updated
| err := cp.cnrClient.Morph().NotarySignAndInvokeTX(e.NotaryRequest.MainTransaction, false) | ||
| if err != nil { | ||
| cp.log.Error("could not approve set attribute", | ||
| zap.String("cID", string(e.CID)), |
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.
here too
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.
Updated
| } | ||
|
|
||
| err := cp.objectPool.Submit(func() { | ||
| err := cp.cnrClient.Morph().NotarySignAndInvokeTX(e.NotaryRequest.MainTransaction, false) |
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.
so 3rd party can add attribute to my container w/o my approval?
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.
I think no. I consider, we will check this with token parameter inside contact setAttribute, when it be done in contracts
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
18979b1 to
eef8b6f
Compare
|
|
||
| // set attribute | ||
| p.SetRequestType(containerEvent.SetAttributeNotaryEvent) | ||
| p.SetParser(containerEvent.ParseSetAttribute) |
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.
| p.SetParser(containerEvent.ParseSetAttribute) | |
| p.SetParser(containerEvent.ParseSetAttribute) | |
| pp = append(pp, p) |
|
|
||
| // ParseSetAttribute from NotaryEvent into container event structure. | ||
| func ParseSetAttribute(ne event.NotaryEvent) (event.Event, error) { | ||
| const expectedItemNumAnnounceLoad = 4 |
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.
| const expectedItemNumAnnounceLoad = 4 | |
| const expectedItemNumSetAttribute = 4 |
| // SetAttribute represents structure of notification about modified container attributes | ||
| // coming from NeoFS Container contract. | ||
| type SetAttribute struct { | ||
| CID []byte |
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.
we can validate it there. currently, any size of CID and any token can be signed by the alphabet and sent to the contract
|
Replaced by #3733. |
No description provided.