@@ -338,39 +338,24 @@ func (v *cachingValidator) validateJustification(ctx context.Context, valueKey *
338338 partial := valueKey != nil
339339 cacheNamespace := validationNamespaces .justification (partial )
340340
341- // It doesn't matter whether the justification is partial or not. Because, namespace
342- // separates the two.
343- cacheKey , err := v .getCacheKey (msg .Justification )
344- var alreadyValidated bool
345- if err != nil {
346- log .Warnw ("failed to get cache key for justification" , "partial" , partial , "err" , err )
347- // If we can't compute the cache key, we can't cache the justification. But we
348- // can still validate it.
349- cacheKey = nil
350- } else {
351- // Only cache the justification if:
352- // * marshalling it was successful, and
353- // * it is not yet present in the cache.
354- if alreadyValidated , err = v .isAlreadyValidated (msg .Vote .Instance , cacheNamespace , cacheKey ); err != nil {
355- log .Warnw ("failed to check if justification is already cached" , "partial" , partial , "err" , err )
356- } else if alreadyValidated {
357- metrics .validationCache .Add (ctx , 1 , metric .WithAttributes (attrCacheHit , attrCacheKindJustification , attrPartial (partial )))
358- return nil
359- } else {
360- metrics .validationCache .Add (ctx , 1 , metric .WithAttributes (attrCacheMiss , attrCacheKindJustification , attrPartial (partial )))
361- }
362- }
363-
364341 // Check that the justification is for the same instance.
365342 if msg .Vote .Instance != msg .Justification .Vote .Instance {
366343 return fmt .Errorf ("message with instanceID %v has evidence from instanceID: %v" , msg .Vote .Instance , msg .Justification .Vote .Instance )
367344 }
368345 if ! msg .Vote .SupplementalData .Eq (& msg .Justification .Vote .SupplementalData ) {
369346 return fmt .Errorf ("message and justification have inconsistent supplemental data: %v != %v" , msg .Vote .SupplementalData , msg .Justification .Vote .SupplementalData )
370347 }
348+
371349 // Check that justification vote value is a valid chain.
372350 if err := msg .Justification .Vote .Value .Validate (); err != nil {
373- return fmt .Errorf ("invalid justification vote value chain: %w" , err )
351+ return fmt .Errorf ("has invalid justification vote value chain: %w" , err )
352+ }
353+
354+ zeroKey := (& ECChain {}).Key ()
355+ msgKey := valueKey
356+ if ! partial {
357+ key := msg .Vote .Value .Key ()
358+ msgKey = & key
374359 }
375360
376361 // Check every remaining field of the justification, according to the phase requirements.
@@ -379,27 +364,27 @@ func (v *cachingValidator) validateJustification(ctx context.Context, valueKey *
379364 // Anything else is disallowed.
380365 expectations := map [Phase ]map [Phase ]struct {
381366 Round uint64
382- Value * ECChain
367+ Key * ECChainKey
383368 }{
384369 // CONVERGE is justified by a strong quorum of COMMIT for bottom,
385370 // or a strong quorum of PREPARE for the same value, from the previous round.
386371 CONVERGE_PHASE : {
387- COMMIT_PHASE : {msg .Vote .Round - 1 , & ECChain {} },
388- PREPARE_PHASE : {msg .Vote .Round - 1 , msg . Vote . Value },
372+ COMMIT_PHASE : {msg .Vote .Round - 1 , & zeroKey },
373+ PREPARE_PHASE : {msg .Vote .Round - 1 , msgKey },
389374 },
390375 // PREPARE is justified by the same rules as CONVERGE (in rounds > 0).
391376 PREPARE_PHASE : {
392- COMMIT_PHASE : {msg .Vote .Round - 1 , & ECChain {} },
393- PREPARE_PHASE : {msg .Vote .Round - 1 , msg . Vote . Value },
377+ COMMIT_PHASE : {msg .Vote .Round - 1 , & zeroKey },
378+ PREPARE_PHASE : {msg .Vote .Round - 1 , msgKey },
394379 },
395380 // COMMIT is justified by a strong quorum of PREPARE from the same round with the same value.
396381 COMMIT_PHASE : {
397- PREPARE_PHASE : {msg .Vote .Round , msg . Vote . Value },
382+ PREPARE_PHASE : {msg .Vote .Round , msgKey },
398383 },
399384 // DECIDE is justified by a strong quorum of COMMIT with the same value.
400385 // The DECIDE message doesn't specify a round.
401386 DECIDE_PHASE : {
402- COMMIT_PHASE : {math .MaxUint64 , msg . Vote . Value },
387+ COMMIT_PHASE : {math .MaxUint64 , msgKey },
403388 },
404389 }
405390
@@ -410,25 +395,15 @@ func (v *cachingValidator) validateJustification(ctx context.Context, valueKey *
410395 return fmt .Errorf ("message %v has justification from wrong round %d" , msg , msg .Justification .Vote .Round )
411396 }
412397
413- // There are 4 possible cases:
414- // 1. The justification is from a complete message with a non-zero value
415- // 2. The justification is from a complete message with a zero value
416- // 3. The justification is from a partial message with non-zero value key
417- // 4. The justification is from a partial message with zero value key
418- //
419- // In cases 1 and 2, the justification vote value must match the expected value
420- // exactly.
421- //
422- // Whereas in cases 3 and 4, the justification vote can't directly be checked and
423- // instead we rely on asserting the value via signature verification. Because the
424- // signing payload uses the value key only.
425- if partial {
426- expectedVoteValueKey = * valueKey
427- } else {
428- if ! msg .Justification .Vote .Value .Eq (expected .Value ) {
398+ // The key can be either the value key or the zero key.
399+ // Depending on which type of justification we are dealing with,
400+ // if message is not partial, then check the justification Value is same as the expected Value
401+ expectedVoteValueKey = * expected .Key
402+ if ! partial {
403+ justificationVoteValueKey := msg .Justification .Vote .Value .Key ()
404+ if ! bytes .Equal (justificationVoteValueKey [:], expected .Key [:]) {
429405 return fmt .Errorf ("message %v has justification for a different value: %v" , msg , msg .Justification .Vote .Value )
430406 }
431- expectedVoteValueKey = expected .Value .Key ()
432407 }
433408 } else {
434409 return fmt .Errorf ("message %v has justification with unexpected phase: %v" , msg , msg .Justification .Vote .Phase )
@@ -437,18 +412,30 @@ func (v *cachingValidator) validateJustification(ctx context.Context, valueKey *
437412 return fmt .Errorf ("message %v has unexpected phase for justification" , msg )
438413 }
439414
440- // Check justification power and signature.
441- justificationPower , signers , err := msg . Justification . GetSigners ( comt . PowerTable )
415+ cacheKey , err := v . getCacheKey ( msg . Justification , expectedVoteValueKey [:])
416+ var alreadyValidated bool
442417 if err != nil {
443- return fmt .Errorf ("failed to get justification signers: %w" , err )
444- }
445- if ! IsStrongQuorum (justificationPower , comt .PowerTable .ScaledTotal ) {
446- return fmt .Errorf ("message %v has justification with insufficient power: %v :%w" , msg , justificationPower , ErrValidationInvalid )
418+ log .Warnw ("failed to get cache key for justification" , "partial" , partial , "err" , err )
419+ // If we can't compute the cache key, we can't cache the justification. But we
420+ // can still validate it.
421+ cacheKey = nil
422+ } else {
423+ // Only cache the justification if:
424+ // * marshalling it was successful, and
425+ // * it is not yet present in the cache.
426+ if alreadyValidated , err = v .isAlreadyValidated (msg .Vote .Instance , cacheNamespace , cacheKey ); err != nil {
427+ log .Warnw ("failed to check if justification is already cached" , "partial" , partial , "err" , err )
428+ } else if alreadyValidated {
429+ metrics .validationCache .Add (ctx , 1 , metric .WithAttributes (attrCacheHit , attrCacheKindJustification , attrPartial (partial )))
430+ return nil
431+ } else {
432+ metrics .validationCache .Add (ctx , 1 , metric .WithAttributes (attrCacheMiss , attrCacheKindJustification , attrPartial (partial )))
433+ }
447434 }
448435
449- payload := msg . Justification . Vote . MarshalForSigningWithValueKey ( v . networkName , expectedVoteValueKey )
450- if err := comt . AggregateVerifier . VerifyAggregate ( signers , payload , msg . Justification . Signature ); err != nil {
451- return fmt .Errorf ("verification of the aggregate failed: %+v: %w" , msg . Justification , err )
436+ err = v . validateJustificationSignature ( comt , msg . Justification , expectedVoteValueKey )
437+ if err != nil {
438+ return fmt .Errorf ("internal justification validation failed: %w" , err )
452439 }
453440
454441 if len (cacheKey ) > 0 {
@@ -461,6 +448,27 @@ func (v *cachingValidator) validateJustification(ctx context.Context, valueKey *
461448 return nil
462449}
463450
451+ func (v * cachingValidator ) validateJustificationSignature (comt * Committee , justif * Justification , expectedVoteValueKey ECChainKey ) error {
452+ // It doesn't matter whether the justification is partial or not. Because, namespace
453+ // separates the two.
454+
455+ // Check justification power and signature.
456+ justificationPower , signers , err := justif .GetSigners (comt .PowerTable )
457+ if err != nil {
458+ return fmt .Errorf ("failed to get justification signers: %w" , err )
459+ }
460+ if ! IsStrongQuorum (justificationPower , comt .PowerTable .ScaledTotal ) {
461+ return fmt .Errorf ("has justification with insufficient power: %v :%w" , justificationPower , ErrValidationInvalid )
462+ }
463+
464+ payload := justif .Vote .MarshalForSigningWithValueKey (v .networkName , expectedVoteValueKey )
465+ if err := comt .AggregateVerifier .VerifyAggregate (signers , payload , justif .Signature ); err != nil {
466+ return fmt .Errorf ("verification of the aggregate failed: %+v: %w" , justif , err )
467+ }
468+
469+ return nil
470+ }
471+
464472func (v * cachingValidator ) isAlreadyValidated (group uint64 , namespace validatorNamespace , cacheKey []byte ) (bool , error ) {
465473 alreadyValidated , err := v .cache .Contains (group , namespace , cacheKey )
466474 if err != nil {
@@ -469,10 +477,13 @@ func (v *cachingValidator) isAlreadyValidated(group uint64, namespace validatorN
469477 return alreadyValidated , nil
470478}
471479
472- func (v * cachingValidator ) getCacheKey (msg cbor.Marshaler ) ([]byte , error ) {
480+ func (v * cachingValidator ) getCacheKey (msg cbor.Marshaler , additionalFields ... [] byte ) ([]byte , error ) {
473481 var buf bytes.Buffer
474482 if err := msg .MarshalCBOR (& buf ); err != nil {
475483 return nil , fmt .Errorf ("failed to get cache key: %w" , err )
476484 }
485+ for _ , field := range additionalFields {
486+ _ , _ = buf .Write (field )
487+ }
477488 return buf .Bytes (), nil
478489}
0 commit comments