added skipchain and identity-changes #607
Conversation
| @@ -110,7 +110,7 @@ func (s *Service) CreateIdentity(si *network.ServerIdentity, ai *CreateIdentity) | |||
|
|
|||
| roster := ids.Root.Roster | |||
| replies, err := manage.PropagateStartAndWait(s.Context, roster, | |||
| &PropagateIdentity{ids}, 1000, s.Propagate) | |||
| &PropagateIdentity{ids}, 10000, s.Propagate) | |||
There was a problem hiding this comment.
Instead of 10000 or 1000, it should be easier to change at a single place using an unexported (or exported?) global variable (or with getter/setter). Even better, we could specify a value to the service (service.SetTimeout()) and the service would use the default unexported global variable by default otherwise.
There was a problem hiding this comment.
It's very specific for that service and shouldn't change during runtime, so I'll put in a constant.
| @@ -150,7 +150,7 @@ func (s *Service) ProposeSend(si *network.ServerIdentity, p *ProposeSend) (netwo | |||
| } | |||
| roster := sid.Root.Roster | |||
| replies, err := manage.PropagateStartAndWait(s.Context, roster, | |||
| p, 1000, s.Propagate) | |||
| p, 10000, s.Propagate) | |||
|
|
||
| // RegisterVerification stores the verification in a map and will | ||
| // call it whenever a verification needs to be done. | ||
| func VerificationRegistration(v VerifierID, f bftcosi.VerificationFunction) error { |
There was a problem hiding this comment.
Inconsistency between names here RegisterVerification vs VerificationRegistration. (I have a preference for the former)
| @@ -397,7 +397,7 @@ func (s *Service) startPropagation(blocks []*SkipBlock) error { | |||
| roster = sb.Roster | |||
| } | |||
| replies, err := manage.PropagateStartAndWait(s.Context, roster, | |||
| block, 1000, s.PropagateSkipBlock) | |||
| block, 10000, s.PropagateSkipBlock) | |||
There was a problem hiding this comment.
same comment as for the identity service.
There was a problem hiding this comment.
Same reasoning as above. I'll put in a constant.
| @@ -443,30 +443,36 @@ func (s *Service) bftVerify(msg []byte, data []byte) bool { | |||
| return true | |||
| } | |||
| } | |||
| default: | |||
| f, ok := verifiers[sb.VerifierID] | |||
There was a problem hiding this comment.
Discussion: Maybe it's not possible directly or with too much work I don't know but since we introduce the registration of verification functions, it seems best to
- delete the two above
case - put their logic into separate verification functions
- register these verification functions in a
init()or so - get rid of this switch case
That way all is "unified" according to this new way of using verification functions (and in a few months we won't be like whut ? Why is it not used in a registration function ... ?? ).
What do you think ? From just what I see now it does not seem to be too much work but @ineiti you know better this code so what's your thoughts on this ?
There was a problem hiding this comment.
I left it here because it uses fields of the structure, but it makes sense what you say. It will be a bit clumsy, but probably nicer...
|
All changes implemented - waiting for travis. |
| @@ -411,6 +447,7 @@ func (s *Service) startPropagation(blocks []*SkipBlock) error { | |||
| // bftVerify takes a message and verifies it's valid | |||
| func (s *Service) bftVerify(msg []byte, data []byte) bool { | |||
| log.Lvlf4("%s verifying block %x", s.ServerIdentity(), msg) | |||
| // XXX What's this variable for ? | |||
| s.testVerify = true | |||
There was a problem hiding this comment.
Sorry I wrote it in the code; What's testVerify for ?
There was a problem hiding this comment.
// testVerify is set to true if a verification happened - only for testing
|
|
||
| // RegisterVerification stores the verification in a map and will | ||
| // call it whenever a verification needs to be done. | ||
| func RegisterVerification(c *sda.Context, v VerifierID, f SkipBlockVerifier) error { |
There was a problem hiding this comment.
Should be deleted I guess.
There was a problem hiding this comment.
I thought keeping it, so that Identity or any other skipchain can call skipchain.RegisterVerification and give its sda.Context, so that it gets stored correctly.
This reverts commit ed74537.
SecureTCPHostopens a connection so that it can be closed againGetUpdateChaininskipchain