Pauser and renounceAdmin Drips tests #18

jferas · 2023-11-28T18:37:57Z

This PR contains test for testing manipulation (via governance) of the "pauser" and "renounceAdmin" related functions of the Drips protocol.
Additional PR(s) will test the ability (via governance) to change the "admin" of the Drips protocol.

jferas · 2023-11-28T18:38:19Z

Current dependencies on/for this PR:

main
- PR forge install: openzeppelin-contracts #1
  - PR Initial code for Radworks upgraded Governor #2
    - PR Initial tests and architecture #5
      - PR Added post-proposal Alpha tests and Bravo tests #8
        
        PR Updated README.md file #12
        
        PR Implement efficient settable quorum without library #15
        
        PR Pulled alpha governor tests out into a separate suite #14
        
        PR Added transfer of RAD tokens to ScopeLift upon successful upgrade to governor Bravo #19
        
        PR Pauser and renounceAdmin Drips tests #18 👈
        
        PR Added tests for changing admin/upgrading protocol via governance #21
        
        PR Cleanup test names / scopelint fix #22
        
        PR Added GovernorVotesCompQuorumFraction for settable quorum #13

This stack of pull requests is managed by Graphite.

test/RadworksDripsGovernance.t.sol

alexkeating · 2023-11-30T22:05:17Z

test/RadworksDripsGovernance.t.sol

+    _upgradeToBravoGovernor();
+  }
+
+  function _proposePassAndExecuteDripsProposal(string memory _description, bytes memory _callData)


Is this method necessary? It seems like a little less generic version of _queueAndVoteAndExecuteProposalWithBravoGovernor

Also the ProposalBuilder could help with the array boilerplate neeed to setup proposals.

Done: 0d211b9

test/RadworksDripsGovernance.t.sol

src/interfaces/IDrips.sol

… Drips tests

wildmolasses

looks good, mostly nits.

just double checking, is the granting/revoking of pauser (and then the admin renouncement) the only functionality we need to test? if so, looks great.

foundry.toml

wildmolasses · 2023-12-15T16:03:13Z

test/helpers/ProposalTest.sol

@@ -57,25 +59,29 @@ abstract contract ProposalTest is RadworksGovernorTest {
  // a cheat for fuzzing addresses that are payable only
  // Why is this no longer in standard cheats? JJF
  // see https://github.com/foundry-rs/foundry/issues/3631
-  function assumePayable(address addr) internal virtual {
-    (bool success,) = payable(addr).call{value: 0}("");
+  function assumePayable(address _addr) internal virtual {


nit: consistency may suggest an underscore prefix

wildmolasses · 2023-12-15T16:03:18Z

test/helpers/ProposalTest.sol

    vm.assume(success);
  }

-  function _assumeReceiver(address _receiver) internal {
-    assumePayable(_receiver);
+  function assummeNotTimelock(address _addr) internal virtual {


nit: consistency may suggest an underscore prefix. Also, assume not assumme

test/RadworksDripsGovernance.t.sol

jferas · 2023-12-20T16:17:44Z

looks good, mostly nits.

just double checking, is the granting/revoking of pauser (and then the admin renouncement) the only functionality we need to test? if so, looks great.

There is a separate PR for testing the changing of admin as well as upgrade of the Drips protocol

jferas · 2023-12-20T19:09:16Z

Added a commit that adds .DS_Store to .gitignore file for smoother MacOS git operations bf585a1

… list

github-actions · 2023-12-20T20:16:33Z

Coverage after merging drips-tests into main will be

76.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src
RadworksGovernor.sol	73.91%	75%	69.23%	76%	101, 126–127, 135, 138, 152, 203, 84

wildmolasses

i'm scraping the bottom of the barrel on my nits now. think this one's ready for merge

wildmolasses · 2023-12-20T20:19:01Z

test/RadworksDripsGovernance.t.sol

+
+  function testFuzz_grantedPauserCanPauseAndUnPause(address _newPauser) public {
+    _assumeNotTimelock(_newPauser);
+


nit: inconsistent newline

Suggested change

Fixed in a PR farther up the stack, to avoid rebases related to test filename changes: 01e2252

jferas requested review from wildmolasses and alexkeating November 28, 2023 19:22

jferas force-pushed the separate-alpha-tests branch from 672ce0f to b2af40e Compare November 28, 2023 20:03

jferas force-pushed the drips-tests branch from 9f41539 to 3d870db Compare November 28, 2023 20:03

alexkeating reviewed Nov 30, 2023

View reviewed changes

jferas mentioned this pull request Dec 4, 2023

Added transfer of RAD tokens to ScopeLift upon successful upgrade to governor Bravo #19

Merged

github-advanced-security bot found potential problems Dec 4, 2023

View reviewed changes

src/interfaces/IDrips.sol Fixed Show fixed Hide fixed

jferas force-pushed the separate-alpha-tests branch from b2af40e to 354cfbc Compare December 7, 2023 15:32

jferas force-pushed the drips-tests branch from c0ebe7a to a38f7cf Compare December 7, 2023 15:32

jferas force-pushed the separate-alpha-tests branch from 354cfbc to bf76141 Compare December 7, 2023 16:18

jferas force-pushed the drips-tests branch from a38f7cf to 8ec011a Compare December 7, 2023 16:18

jferas force-pushed the separate-alpha-tests branch from bf76141 to fe8a3aa Compare December 7, 2023 16:39

jferas force-pushed the drips-tests branch 2 times, most recently from 4b6aa36 to 3e82389 Compare December 7, 2023 17:59

jferas changed the base branch from separate-alpha-tests to main December 7, 2023 18:23

jferas force-pushed the drips-tests branch from 3e82389 to 8dd6d17 Compare December 7, 2023 18:23

alexkeating approved these changes Dec 7, 2023

View reviewed changes

This was referenced Dec 8, 2023

Added tests for changing admin/upgrading protocol via governance #21

Merged

Cleanup test names / scopelint fix #22

Merged

jferas added 2 commits December 15, 2023 13:39

Pauser and renouncAdmin Drips tests

72cea2c

Removed unneeded imports from Drips tests

9355885

jferas added 5 commits December 15, 2023 13:39

Removed mostly-redundant function and made use of proposal builder in…

44ece7b

… Drips tests

Now using IDrips interface create by 'cast interface'

2ced77b

Changed Drips setStreams function parameter names to avoice CI warning

d5e3291

Broke out pauser tests with more meaningful names

7a2ff84

Added comment about how IDrips.sol was created

2651f69

jferas force-pushed the drips-tests branch from 8dd6d17 to 2651f69 Compare December 15, 2023 19:56

wildmolasses changed the title ~~Pauser and renouncAdmin Drips tests~~ Pauser and renounceAdmin Drips tests Dec 20, 2023

wildmolasses reviewed Dec 20, 2023

View reviewed changes

Added .DS_Store to .gitignore

bf585a1

jferas added 2 commits December 20, 2023 14:34

Update test function names

b70ca2b

Added assume for pauser test that fuzzed pauser not already in pauser…

8b89ae6

… list

wildmolasses approved these changes Dec 20, 2023

View reviewed changes

jferas merged commit 1240370 into main Dec 28, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pauser and renounceAdmin Drips tests #18

Pauser and renounceAdmin Drips tests #18

jferas commented Nov 28, 2023 •

edited

Loading

jferas commented Nov 28, 2023 •

edited

Loading

alexkeating Nov 30, 2023

alexkeating Nov 30, 2023

jferas Dec 4, 2023

wildmolasses left a comment

wildmolasses Dec 15, 2023

wildmolasses Dec 15, 2023

jferas commented Dec 20, 2023

jferas commented Dec 20, 2023

github-actions bot commented Dec 20, 2023

wildmolasses left a comment

wildmolasses Dec 20, 2023

jferas Dec 28, 2023


		function testFuzz_grantedPauserCanPauseAndUnPause(address _newPauser) public {
		_assumeNotTimelock(_newPauser);

Pauser and renounceAdmin Drips tests #18

Pauser and renounceAdmin Drips tests #18

Conversation

jferas commented Nov 28, 2023 • edited Loading

jferas commented Nov 28, 2023 • edited Loading

alexkeating Nov 30, 2023

Choose a reason for hiding this comment

alexkeating Nov 30, 2023

Choose a reason for hiding this comment

jferas Dec 4, 2023

Choose a reason for hiding this comment

wildmolasses left a comment

Choose a reason for hiding this comment

wildmolasses Dec 15, 2023

Choose a reason for hiding this comment

wildmolasses Dec 15, 2023

Choose a reason for hiding this comment

jferas commented Dec 20, 2023

jferas commented Dec 20, 2023

github-actions bot commented Dec 20, 2023

wildmolasses left a comment

Choose a reason for hiding this comment

wildmolasses Dec 20, 2023

Choose a reason for hiding this comment

jferas Dec 28, 2023

Choose a reason for hiding this comment

jferas commented Nov 28, 2023 •

edited

Loading

jferas commented Nov 28, 2023 •

edited

Loading