Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(og): L-02 - Emit event on syncing upgraded OOv3 #4487

Merged
merged 4 commits into from Mar 22, 2023

Conversation

Reinis-FRP
Copy link
Contributor

Motivation

OZ identified the following issue:

The internal _sync function changes the OptimisticOracleV3 contract used by the
OptimisticGovernor , but unlike other administrative setter functions, it does not emit an
event when a change occurs.

Summary

This PR addresses this issue by adding a new OptimisticOracleChanged event that is emitted by _sync whenever the finder contract returns a different address than that of the current optimisticOracleV3 variable.

Testing

Check a box to describe how you tested these changes and list the steps for reviewers to test.

  • Ran end-to-end test, running the code as in production
  • New unit tests created
  • Existing tests adequate, no new tests required
  • All existing tests pass
  • Untested

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
address currentOptimisticOracleV3 = finder.getImplementationAddress(OracleInterfaces.OptimisticOracleV3);
if (currentOptimisticOracleV3 != address(optimisticOracleV3)) {
optimisticOracleV3 = OptimisticOracleV3Interface(currentOptimisticOracleV3);
emit OptimisticOracleChanged(currentOptimisticOracleV3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems a bit counter intuitive to emit the currentOptimisticOracleV3 over the optimisticOracleV3. would you not want to emit the value of the new oov3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, fixed in 3ee4250 by renaming currentOptimisticOracleV3 to newOptimisticOracleV3.

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Copy link
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Reinis-FRP Reinis-FRP merged commit f3ea7a6 into master Mar 22, 2023
1 check passed
@Reinis-FRP Reinis-FRP deleted the reinis-frp/l02-og-upgrade branch March 22, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants