-
Notifications
You must be signed in to change notification settings - Fork 40
Conversation
contracts/MedianOracle.sol
Outdated
@@ -35,6 +35,7 @@ contract MedianOracle is Ownable, IOracle { | |||
event ProviderAdded(address provider); | |||
event ProviderRemoved(address provider); | |||
event ReportTimestampOutOfRange(address provider); | |||
event ProviderReportPushed(address provider, uint256 payload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the timestamp as well, so that we can really rely on the log instead of getting the timestamp from the block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. and added a test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
contracts/MedianOracle.sol
Outdated
@@ -35,6 +35,7 @@ contract MedianOracle is Ownable, IOracle { | |||
event ProviderAdded(address provider); | |||
event ProviderRemoved(address provider); | |||
event ReportTimestampOutOfRange(address provider); | |||
event ProviderReportPushed(address provider, uint256 payload, uint256 timestamp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@truncs Do any of these arguments need to be indexed?
What do you expect the query behavior to be like?
e.g.
would be need to query by provider?
query for payload outliers?
query for anything after a timestamp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query for payload outliers?
query for anything after a timestamp?
Indexing doesn't help with these two cases.
Querying everything and processing after the fact is more practical than indexing and doing operations using the log query itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added indexing to provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.