Check message on-chain consistency before initializing context #1251
Conversation
awrichar
requested review from
peterbroadhurst,
nguyer and
shorsher
as code owners
| @@ -469,6 +469,12 @@ func (ag *aggregator) processMessage(ctx context.Context, manifest *core.BatchMa | |||
| case !dataAvailable: | |||
| l.Errorf("Message '%s' in batch '%s' is missing data", msgEntry.ID, manifest.ID) | |||
| default: | |||
| // Check the pin signer is valid for the message | |||
| action, err = ag.checkOnchainConsistency(ctx, msg, pin) | |||
There was a problem hiding this comment.
This was the main goal of the PR - to move this check earlier instead of having it as part of attemptMessageDispatch.
However, since this call returns a core.MessageAction, it also made sense to refactor common handling of retry/wait actions up into this method as well, so consequently I split up attemptMessageDispatch a little bit to make that happen.
There was a problem hiding this comment.
Side note: core.MessageAction is definitely imperfect and can be slightly confusing. ActionRetry means "I also should have returned a non-nil err and you should treat that as retryable" (an err is ignored for any other action). ActionConfirm kind of means "looks good to me, go ahead to confirm or continue with the next step if there is one". There could be some further thought on how to handle success vs. the 3 distinct problem scenarios (wait/retry/reject).
This prevents inserting a group from a malformed "group init" message. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
| for _, unmaskedContext := range unmaskedContexts { | ||
| state.SetContextBlockedBy(ctx, *unmaskedContext, pin.Sequence) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Note - there a few earlier branches of this method which return nil without marking the context blocked. Still trying to decide if that was intentional for some reason, or if they should all fall down to here instead.
Fixes regression from hyperledger#1251 Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Fixes regression from hyperledger#1251 Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
This prevents inserting a group from a malformed "group init" message.
Possible/partial fix for #1247