[Sealing] Validation of multiple receipts in payload #323
Conversation
durkmurder
commented
Jan 25, 2021
durkmurder
commented
- Add implementation for looking up receipt in payload
| // local storage. This is needed to handle a case when same block payload contains receipts that | ||
| // reference each other. | ||
| previousResult := func(executionResult *flow.ExecutionResult) (*flow.ExecutionResult, error) { | ||
| for _, receipt := range receipts { |
There was a problem hiding this comment.
can build a small cache here to perform lookups faster, but the question is if we need it, what is the expected average size of receipts payload?
| if err != nil { | ||
| return fmt.Errorf("could not validate receipt %v at index %v: %w", r.ID(), i, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func (v *receiptValidator) validate(receipt *flow.ExecutionReceipt) error { | ||
| func (v *receiptValidator) validate(receipt *flow.ExecutionReceipt, previousResult GetPreviousResult) error { |
There was a problem hiding this comment.
Did it this way to postpone storage lookup till it's needed and not too put validation in loop body.
Fixed, thanks. |
| // Build a functor that performs lookup first in receipts that were passed as payload and only then in | ||
| // local storage. This is needed to handle a case when same block payload contains receipts that | ||
| // reference each other. | ||
| previousResult := func(executionResult *flow.ExecutionResult) (*flow.ExecutionResult, error) { |
There was a problem hiding this comment.
when validating a receipt, we assume the previous result from the same payload is "valid", and so that if this assumption fails, eventually the entire validation will fail.
The solution is simple and elegant, however I'm still not feeling very comfortable about it, even though I couldn't think of case that leads to an incorrect behavior yet
Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>
