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

add events #76

Merged
merged 2 commits into from
Dec 8, 2022
Merged

add events #76

merged 2 commits into from
Dec 8, 2022

Conversation

auryn-macmillan
Copy link
Member

@auryn-macmillan auryn-macmillan commented Dec 8, 2022

This PR adds events and custom errors to the Reality contracts, along with some other housekeeping (bumps solidity version, additional set functions, updates tests, fixed some comments, adds hardhat-size-contracts).

Closes #75
Closes #74
Closes #69

Audit checklist:

  • RealityModule.sol
  • RealityModuleERC20.sol
  • RealityModuleETH.sol

@auryn-macmillan auryn-macmillan marked this pull request as ready for review December 8, 2022 15:48
@coveralls
Copy link

coveralls commented Dec 8, 2022

Pull Request Test Coverage Report for Build 3649813124

  • 19 of 19 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.6%) to 99.408%

Totals Coverage Status
Change from base Build 3316194613: -0.6%
Covered Lines: 115
Relevant Lines: 115

💛 - Coveralls

Copy link

@cristovaoth cristovaoth left a comment

Choose a reason for hiding this comment

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

Pretty solid and straightforward code changes, along with the introduction of events. 👌

@auryn-macmillan auryn-macmillan merged commit a02e707 into main Dec 8, 2022
@auryn-macmillan auryn-macmillan deleted the add-events branch December 8, 2022 17:48
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2022
Copy link
Contributor

@asgeir-s asgeir-s left a comment

Choose a reason for hiding this comment

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

Nice, it will be much easier to view the interactions off-chain now.

Added some comments.

Also, is it possible to add an event for when a proposal is marked as invalid? So that we know we do not have to watch for that proposalId anymore and can mark it as invalid (even if it has not expired).

contracts/RealityModule.sol Show resolved Hide resolved
contracts/RealityModule.sol Show resolved Hide resolved
contracts/RealityModule.sol Show resolved Hide resolved
@auryn-macmillan
Copy link
Member Author

Ok, I've added the extra events.

Given that I added extra functions, these changes are going to need to be audited. That being the case, I've also bumped the solidity version to 0.8.6 and added custom errors.

I'll hold off on merging this until it has been audited.

@auryn-macmillan
Copy link
Member Author

Are there any other changes that we should put in while we're at it?

Perhaps it's worthwhile also adding these changes?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants