Fix MinObservation to make observations per observer unique [CCIP-5050] #495
Conversation
asoliman92
changed the title
asoliman92
changed the title
asoliman92
requested review from
winder,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang,
0xnogo and
b-gopalswami
| if idFunc == nil { | ||
| idFunc = func(data T) [32]byte { | ||
| return sha3.Sum256([]byte(fmt.Sprintf("%v", data))) | ||
| } | ||
| } |
There was a problem hiding this comment.
Thinking out loud, but could this version be implemented by overloading the ID func?
if idFunc == nil {
idFunc = func(oracle int, data T) [32]byte {
return sha3.Sum256([]byte(fmt.Sprintf("%d_%v", oracle, data)))
}
} else {
idFunc = func(oracle int, data T) [32]byte {
return sha3.Sum256([]byte(fmt.Sprintf("%d_%v", oracle, idFunc(data))))
}
}
There was a problem hiding this comment.
thought about this and tried it, was't straightforward to use from the callers' though. don't remember the specific reason for it now.
| // OracleMinObservation provides a way to ensure a minimum number of observations for | ||
| // some piece of data have occurred. It maintains an internal cache and provides a list | ||
| // of valid or invalid data points. | ||
| type OracleMinObservation[T any] interface { |
There was a problem hiding this comment.
This API doc comment should outline why this would be used instead of MinObservation. I think you can say that OracleMinObservation ensures that duplicate observations from the same observer are only counted once? TBH this should be the default behavior, I'm not even sure we should have a separate MinObservation and OracleMinObservation.
cc @winder
There was a problem hiding this comment.
As mentioned in DM I agree we should have one. It's a big change that will take some time and other plugins already use maps for most observations which by design prevents duplicates.
Co-authored-by: Will Winder <wwinder.unh@gmail.com>
|
Make sure that minObservations are unique per observer.