Conversation
| // AddPeer accepts information about the network map candidate in the FrostFS | ||
| // binary protocol format, identifies the caller and behaves depending on different | ||
| // conditions listed below. |
There was a problem hiding this comment.
It produces a notification in case 1a, I don't think we can remove it safely. Have you checked whether it is called in node?
There was a problem hiding this comment.
In our case this method degrades to following lines:
common.CheckWitness(publicKey)
return
In CheckWitness we are producing notification?
As for code in node, yes, we have few places where we invoke AddPeer.
For example here pkg/morph/client/netmap/add_peer.go:24
So if it is really produces some notifications in CheckWitness I'll revert this changes. In other case we need to do some cleanup in node too.
There was a problem hiding this comment.
Yes, but storage node creates a notary transaction with this method which produces a notary notification (implicit here). I think we should leave this method for now and revisit how we handle node addition/state update later.
I think we considered producing AddPeer with runtime.Notify here too, but I forgot the outcome of this discussion. cc @alexvanin
There was a problem hiding this comment.
Changes for AddPeer were reverted.
alphabet/alphabet_contract.go
Outdated
| if data.([]interface{})[0].(bool) { | ||
| panic(common.PanicMsgForNotaryDisabledEnv) | ||
| } |
There was a problem hiding this comment.
It looks complex, can we move args := data.(struct cast here (it is already done below) and check args.notaryDisabled field?
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
| ctx := storage.GetContext() | ||
|
|
||
| //TODO(@acid-ant): #9 remove notaryDisabled from args in future version | ||
| if data.([]interface{})[0].(bool) { |
There was a problem hiding this comment.
Same code in many files, it's better to move to method.
4 participants
Close #7
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com