WIP: A0-1820: Add justifications and validator implementations #840
Conversation
timorl
changed the title
timorl
requested review from
DamianStraszak,
lesniak43 and
maciejnems
and removed request for
lesniak43
maciejnems
left a comment
maciejnems
left a comment
There was a problem hiding this comment.
From what I understand outside of session map, the only write lock we have is for subscribing for authority list, so there is no possibility of new deadlocks to occur after changing it to sync code.
| impl<H: SubstrateHeader<Number = BlockNumber>> Verifier<Justification<H>> for FullVerifier { | ||
| type Error = VerificationError; | ||
|
|
||
| fn verify(&self, justification: Justification<H>) -> Result<Justification<H>, Self::Error> { |
There was a problem hiding this comment.
This verify seems to be pretty heavy. On each call it locks on SessionMap, clones authorities and creates a new verifier. Still I'm not sure if there is a better way, to create a full verifier.
I guess we would have to somehow cache already existing verifiers and remove them from cache once they are not needed, which is already done in SessionMap :// maybe @DamianStraszak has an idea for that.
There was a problem hiding this comment.
I agree this is annoying... I'll look into how hard it would be to rewrite ROSM some more to avoid this.
There was a problem hiding this comment.
Ok, it seems I'll rewrite much more here after all, I'll open a separate PR for some smaller part of this and then write one for the verifier (although it will take longer). This one will likely get closed.
| fn from(authority_data: SessionAuthorityData) -> Self { | ||
| SessionVerifier { | ||
| authority_verifier: AuthorityVerifier::new(authority_data.authorities().to_vec()), | ||
| emergency_signer: authority_data.emergency_finalizer().clone(), |
There was a problem hiding this comment.
Also this clone should probably not happen, as we move SessionAuthorityData. That said I'm not sure how to get authorities and signer out of SessionAuthorityData ://
timorl
changed the title
2 participants
Description
Implements the validator and justifications. Makes the code a bit of a mess, mostly because some of this stuff was already there and still needs to be used externally while we still have the legacy justification sync.
Type of change
Checklist: