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

Modules cleanup #123

Merged
merged 27 commits into from
Nov 1, 2024
Merged

Modules cleanup #123

merged 27 commits into from
Nov 1, 2024

Conversation

adamgall
Copy link
Member

@adamgall adamgall commented Oct 31, 2024

Did a lot here, in an attempt to clean up our new "Modules"

  • Totally just removed the DecentHats_0_1_0 contract. Oh well. I guess I'm coming around to the fact that there's really no real need to keep a hold on this thing. It's not "installed" on any Safe permanently. So anyway, yeah, it has been superseded by the DecentHatsCreationModule.
    • This also allowed me to get rid of that half-baked IHats interface.
    • This also allowed me to remove the full subdirectory in those hats and sablier interface directories.
      • This is going to force a redeployment of the DecentSablierStreamManagement contract, but I'm ok with that.
        • So while we're at it, I've also renamed this contract to DecentSablierStreamManagementModule
  • Moved all of the new "Modules" into a "modules" directory.
  • Tightened and cleaned up the structs used in our three-contract HatsModule contracts (Create, Modify, and Utils)
    • Modified the input params to the Create contract to support multiple Hats/Roles at once
    • Created a standalone struct for these input params
      • Now the implementation of DecentHatsModificationModule is super clean
  • Renamed DecentHatsUtils to DecentHatsModuleUtils for naming consistency.
  • Reordered some deployment scripts
  • Removed deployment artifacts for DecentHats_0_1_0 and DecentSablierStreamManagement contracts on all networks
  • Beefed up the MockHatsElectionEligibility contract to more accurately mimic the real thing.
  • Fix a bug in DecentAutonomousAdmin
    • Via more accurate tests.
  • Fixed all the mocks, tests, etc

@adamgall adamgall self-assigned this Oct 31, 2024
Copy link
Contributor

@DarksightKellar DarksightKellar left a comment

Choose a reason for hiding this comment

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

A couple or so questions, but all non-blocking 👍

contracts/modules/DecentHatsCreationModule.sol Outdated Show resolved Hide resolved
contracts/modules/DecentHatsCreationModule.sol Outdated Show resolved Hide resolved
contracts/modules/DecentHatsModuleUtils.sol Show resolved Hide resolved
test/modules/DecentSablierStreamManagement.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

Kyrylo see lots of red
Kyrylo likes red
Kyrylo feels good about less files -> same functionality
Kyrylo approves
Kyrylo has bunch of comments, questions and dumb jokes - @adamgall likes dumb jokes too
@adamgall responds
@adamgall merges

@adamgall
Copy link
Member Author

adamgall commented Nov 1, 2024

Hmm, I'm also thinking that I should think more critically about the pragma solidity version numbers in the new modules files...

@adamgall adamgall merged commit 9d3d456 into develop Nov 1, 2024
3 checks passed
@adamgall adamgall deleted the modules-cleanup branch November 1, 2024 16:32
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.

4 participants