container: add object meta notification support #448
Conversation
carpawell
requested review from
cthulhu-rider and
roman-khimov
as code owners
| estimatePostfixSize = 10 | ||
| singleEstimatePrefix = "est" | ||
| estimateKeyPrefix = "cnr" | ||
| containersWithMetaPrefix = 'm' |
There was a problem hiding this comment.
just a remark
i very like (no) how gofmt makes a 10-line change when one is actually added/changed. Takin bout why linebreaks are good sometimes, and const/var blocks aint
| // PutObject registers successful object PUT operation and notifies about | ||
| // it. metaInformation must be signed by container nodes according to | ||
| // container's placement, see [VerifyPlacementSignatures]. metaInformation | ||
| // must contain information about an object placed to a container that was |
There was a problem hiding this comment.
must contain information in what form? Lets describe, []byte is hard to guess
There was a problem hiding this comment.
what form
i think this will somehow appear somewhere. at least it should appear before node's 0.44.0 release and contract's 0.21. however, currently node also uses it as is and the format is not fixed or described
tests/container_test.go
Outdated
| require.NoError(t, err) | ||
|
|
||
| c.Invoke(t, stackitem.Null{}, "put", cnt.value, cnt.sig, cnt.pub, cnt.token, true) | ||
| c.Invoke(t, stackitem.Null{}, "putObject", metaInfo, nil) |
There was a problem hiding this comment.
why is it OK w/o signatures?
There was a problem hiding this comment.
for test purposes, container was added without any nodes (again, IMO signature verification should not be tested in this PR), so no nodes can add its signature but also no nodes are quired to do it, every iterator over node's put keys does nothing
tests/container_test.go
Outdated
| require.NoError(t, err) | ||
|
|
||
| c.Invoke(t, stackitem.Null{}, "put", cnt.value, cnt.sig, cnt.pub, cnt.token, true) | ||
| c.Invoke(t, stackitem.Null{}, "putObject", metaInfo, nil) |
There was a problem hiding this comment.
if we have tools to test whether notification event is expectedly thrown, lets do it. If no, suggest to create an issue for this. Suppose other methods/contracts need this too
There was a problem hiding this comment.
we use neo-go's test framework. i tried my best, @roman-khimov, check if there is more optimal way to test events
| // PutMeta is the same as [Put] (and exposed as put from the contract via | ||
| // overload), but allows container's meta-information be handled and notified | ||
| // using the chain. | ||
| func PutMeta(container []byte, signature interop.Signature, publicKey interop.PublicKey, token []byte, metaOnChain bool) { |
There was a problem hiding this comment.
we put container and just configure meta support, with this PutMeta name can confuse. I can suggest PutWithTunedMeta, but not very against ur proposal
There was a problem hiding this comment.
i have adopted @roman-khimov's approach with method overloading. no one will call it putMeta, it is the same put but with one more arg. i really liked it
There was a problem hiding this comment.
put but with one more arg - putWithMeta to me
contracts/container/contract.go
Outdated
| @@ -238,6 +239,42 @@ func Update(script []byte, manifest []byte, data any) { | |||
| runtime.Log("container contract updated") | |||
| } | |||
|
|
|||
| // PutObject registers successful object PUT operation and notifies about | |||
There was a problem hiding this comment.
what meaning do u put into registers here? In proposed implementation it verifies and notifies only
There was a problem hiding this comment.
what meaning do u put
dont remember, maybe i meant how it really will be used: some meta service will wait for this notification. this is like the proof that an object exists. until this method is called it is not guaranteed
Containers now can be created with new `put` method with one more bool parameter for meta-on-chain feature (TX must still be signed by the Alphabet, and it also must check that container was created with this feature, see nspcc-dev/neofs-api#309). If enabled, new `PutObject` method allows checking object meta information's signature and producing corresponding notification. Closes #414. Signed-off-by: Pavel Karpy <[email protected]>
carpawell
force-pushed
the
feat/container-events
branch
from
8b78eb8 to
3865e8a
Compare
Containers now can be created with new
putmethod with one more bool parameter for meta-on-chain feature (TX must still be signed by the Alphabet, and it also must check that container was created with this feature, see nspcc-dev/neofs-api#309). If enabled, newPutObjectmethod allows checking object meta information's signature and producing corresponding notification. Closes #414.