Skip to content
New issue

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

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

Already on GitHub? # to your account

test: basic flow content #14

Merged
merged 7 commits into from
Aug 12, 2023
Merged

test: basic flow content #14

merged 7 commits into from
Aug 12, 2023

Conversation

petrovska-petro
Copy link
Collaborator

@petrovska-petro petrovska-petro commented Aug 10, 2023

forge test -vvv

@gosuto-inzasheru
Copy link
Contributor

getting these warnings when compiling, maybe these could be tackled once we are ready to implement our own SmartGardenRegistry?

Warning (2018): Function state mutability can be restricted to pure
  --> contracts/lib/safe-protocol/contracts/SafeProtocolRegistry.sol:95:5:
   |
95 |     function supportsInterface(bytes4 interfaceId) external view override returns (bool) {
   |     ^ (Relevant source part starts here and spans across multiple lines).

Warning (2018): Function state mutability can be restricted to pure
  --> contracts/src/modules/BaseModule.sol:98:3:
   |
98 |   function supportsInterface(
   |   ^ (Relevant source part starts here and spans across multiple lines).

@petrovska-petro
Copy link
Collaborator Author

getting these warnings when compiling?

i think is simply the compiler highligthing that in that method there is not manipulation of the blockchain state, so we could feel safe labelling them as 'pure', no?

@gosuto-inzasheru
Copy link
Contributor

console.log(safe.isModuleEnabled(address(manager))); after safe.enableModule(address(manager)); returns false for me!

Copy link
Contributor

@gosuto-inzasheru gosuto-inzasheru left a comment

Choose a reason for hiding this comment

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

test passes!!!!!

some future steps i can think of:

  • move relayer to the manager/registry (possibly an array of relayers)
  • split executeFromPlugin into a queue and execute function?
  • rename DummyModule to HarvesterPlugin, since it now has a clear role it fulfills

contracts/src/modules/DummyModule.sol Show resolved Hide resolved
contracts/src/modules/DummyModule.sol Show resolved Hide resolved
@petrovska-petro petrovska-petro merged commit d98b1de into main Aug 12, 2023
@petrovska-petro petrovska-petro deleted the feat/basic-test branch August 12, 2023 20:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants