cleanup rebate code, use zeppelin for math utils and improve rebate t…

…ests

.github/workflows/testnet.yml

@@ -11,10 +11,6 @@ on:
   release:
     types: [created]
 
-concurrency:
-  group: "testnet"
-  cancel-in-progress: true
-
 env:
   FOUNDRY_PROFILE: ci
   DEVNET_RPC_URL: ${{ secrets.ALCHEMY_DEVNET_RPC }}

abi/Governance.json

@@ -265,7 +265,7 @@
       {
         "indexed": false,
         "internalType": "uint256",
-        "name": "refund",
+        "name": "rebate",
         "type": "uint256"
       },
       {

abi/GovernanceBuilder.json

@@ -405,7 +405,7 @@
         "type": "uint256"
       }
     ],
-    "name": "withGasRefund",
+    "name": "withGasRebate",
     "outputs": [
       {
         "internalType": "contract GovernanceCreator",

contracts/CollectiveGovernance.sol

@@ -44,6 +44,7 @@
 pragma solidity ^0.8.15;
 
 import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+import "@openzeppelin/contracts/utils/math/Math.sol";
 
 import "../contracts/Storage.sol";
 import "../contracts/Governance.sol";
@@ -78,9 +79,9 @@ contract CollectiveGovernance is Governance, VoteStrategy, ERC165 {
 
     address[] private _communitySupervisorList;
 
-    uint256 public immutable _maximumGasUsedRefund;
+    uint256 public immutable _maximumGasUsedRebate;
 
-    uint256 public immutable _maximumBaseFeeRefund;
+    uint256 public immutable _maximumBaseFeeRebate;
 
     bytes32 public immutable _communityName;
 
@@ -92,13 +93,13 @@ contract CollectiveGovernance is Governance, VoteStrategy, ERC165 {
     mapping(uint256 => bool) private isVoteOpenByProposalId;
 
     /// @notice create a new collective governance contract
-    /// @dev This should be invoked through the GovernanceBuilder.  Gas refund
+    /// @dev This should be invoked through the GovernanceBuilder.  Gas Rebate
     /// is contingent on contract being funded through a transfer.
     /// @param _supervisorList the list of supervisors for this project
     /// @param _class the VoterClass for this project
     /// @param _governanceStorage The storage contract for this governance
-    /// @param _gasUsedRefund The maximum refund for gas used
-    /// @param _baseFeeRefund The maximum base fee refund
+    /// @param _gasUsedRebate The maximum rebate for gas used
+    /// @param _baseFeeRebate The maximum base fee rebate
     /// @param _name The community name
     /// @param _url The Url for this project
     /// @param _description The community description
@@ -108,24 +109,24 @@ contract CollectiveGovernance is Governance, VoteStrategy, ERC165 {
         VoterClass _class,
         Storage _governanceStorage,
         TimeLocker _timeLocker,
-        uint256 _gasUsedRefund,
-        uint256 _baseFeeRefund,
+        uint256 _gasUsedRebate,
+        uint256 _baseFeeRebate,
         bytes32 _name,
         string memory _url,
         string memory _description
     ) {
         require(_supervisorList.length > 0, "Supervisor required");
         require(Constant.len(_url) <= Constant.STRING_DATA_LIMIT, "Url too large");
         require(Constant.len(_description) <= Constant.STRING_DATA_LIMIT, "Description too large");
-        require(_gasUsedRefund >= Constant.MAXIMUM_REFUND_GAS_USED, "Insufficient gas used refund");
-        require(_baseFeeRefund >= Constant.MAXIMUM_REFUND_BASE_FEE, "Insufficient base fee refund");
+        require(_gasUsedRebate >= Constant.MAXIMUM_REBATE_GAS_USED, "Insufficient gas used rebate");
+        require(_baseFeeRebate >= Constant.MAXIMUM_REBATE_BASE_FEE, "Insufficient base fee rebate");
 
         _voterClass = _class;
         _storage = _governanceStorage;
         _timeLock = _timeLocker;
         _communitySupervisorList = _supervisorList;
-        _maximumGasUsedRefund = _gasUsedRefund;
-        _maximumBaseFeeRefund = _baseFeeRefund;
+        _maximumGasUsedRebate = _gasUsedRebate;
+        _maximumBaseFeeRebate = _baseFeeRebate;
         _communityName = _name;
         _communityUrl = _url;
         _communityDescription = _description;
@@ -677,14 +678,21 @@ contract CollectiveGovernance is Governance, VoteStrategy, ERC165 {
         if (balance == 0) {
             return;
         }
+        // determine rebate and transfer
+        (uint256 rebate, uint256 gasUsed) = calculateGasRebate(startGas, balance);
+        payable(recipient).transfer(rebate);
+        emit RebatePaid(recipient, rebate, gasUsed);
+    }
+
+    function calculateGasRebate(uint256 startGas, uint256 balance) private view returns (uint256 rebate, uint256 gasUsed) {
+        uint256 permittedBaseFee = Math.min(block.basefee, _maximumBaseFeeRebate);
+        uint256 permittedGasPrice = Math.min(tx.gasprice, permittedBaseFee + Constant.MAXIMUM_REBATE_PRIORITY_FEE);
 
         uint256 totalGasUsed = startGas - gasleft();
-        uint256 basefee = min(block.basefee, _maximumBaseFeeRefund);
-        uint256 gasPrice = min(tx.gasprice, basefee + Constant.MAXIMUM_REFUND_PRIORITY_FEE);
-        uint256 gasUsed = min(totalGasUsed + Constant.REFUND_BASE_GAS, _maximumGasUsedRefund);
-        uint256 rebateQuantity = min(gasPrice * gasUsed, balance);
-        payable(recipient).transfer(rebateQuantity);
-        emit RebatePaid(recipient, rebateQuantity, totalGasUsed);
+
+        uint256 gasUsedForRebate = Math.min(totalGasUsed + Constant.REBATE_BASE_GAS, _maximumGasUsedRebate);
+        uint256 rebateQuantity = Math.min(permittedGasPrice * gasUsedForRebate, balance);
+        return (rebateQuantity, totalGasUsed);
     }
 
     function getBlockTimestamp() internal view returns (uint256) {
@@ -695,11 +703,4 @@ contract CollectiveGovernance is Governance, VoteStrategy, ERC165 {
     function getRebateBalance() internal view returns (uint256) {
         return address(this).balance;
     }
-
-    function min(uint256 a, uint256 b) internal pure returns (uint256) {
-        if (a < b) {
-            return a;
-        }
-        return b;
-    }
 }

contracts/CollectiveGovernanceFactory.sol

@@ -60,8 +60,8 @@ library CollectiveGovernanceFactory {
     /// @param _class the VoterClass for this project
     /// @param _storage The storage contract for this governance
     /// @param _timeLock The timelock for the contract
-    /// @param _gasUsedRefund The maximum refund for gas used
-    /// @param _baseFeeRefund The maximum base fee refund
+    /// @param _gasUsedRebate The maximum rebate for gas used
+    /// @param _baseFeeRebate The maximum base fee rebate
     /// @param _name The community name
     /// @param _url The Url for this project
     /// @param _description The community description
@@ -70,8 +70,8 @@ library CollectiveGovernanceFactory {
         VoterClass _class,
         Storage _storage,
         TimeLocker _timeLock,
-        uint256 _gasUsedRefund,
-        uint256 _baseFeeRefund,
+        uint256 _gasUsedRebate,
+        uint256 _baseFeeRebate,
         bytes32 _name,
         string memory _url,
         string memory _description
@@ -81,8 +81,8 @@ library CollectiveGovernanceFactory {
             _class,
             _storage,
             _timeLock,
-            _gasUsedRefund,
-            _baseFeeRefund,
+            _gasUsedRebate,
+            _baseFeeRebate,
             _name,
             _url,
             _description

contracts/Constant.sol

@@ -74,13 +74,13 @@ library Constant {
     uint256 public constant STRING_DATA_LIMIT = 1024;
 
     /// @notice The maximum priority fee
-    uint256 public constant MAXIMUM_REFUND_PRIORITY_FEE = 2 gwei;
-    /// @notice The vote refund gas overhead, including 7K for ETH transfer and 29K for general transaction overhead
-    uint256 public constant REFUND_BASE_GAS = 36000;
-    /// @notice The maximum refundable gas fee
-    uint256 public constant MAXIMUM_REFUND_GAS_USED = 200_000;
-    /// @notice the maximum allowed gas rebate parameterization
-    uint256 public constant MAXIMUM_REFUND_BASE_FEE = 200 gwei;
+    uint256 public constant MAXIMUM_REBATE_PRIORITY_FEE = 2 gwei;
+    /// @notice The vote rebate gas overhead, including 7K for ETH transfer and 29K for general transaction overhead
+    uint256 public constant REBATE_BASE_GAS = 36000;
+    /// @notice The maximum refundable gas used
+    uint256 public constant MAXIMUM_REBATE_GAS_USED = 200_000;
+    /// @notice the maximum allowed gas fee for rebate
+    uint256 public constant MAXIMUM_REBATE_BASE_FEE = 200 gwei;
 
     uint32 public constant VERSION_1 = 1;
 

contracts/Governance.sol

@@ -92,8 +92,8 @@ interface Governance is IERC165 {
     event ProposalVeto(uint256 proposalId, address sender);
     /// @notice The contract has been funded to provide gas rebates
     event RebateFund(address sender, uint256 transfer, uint256 totalFund);
-    /// @notice Gas Rebate payment
-    event RebatePaid(address recipient, uint256 refund, uint256 gasPaid);
+    /// @notice Gas rebate payment
+    event RebatePaid(address recipient, uint256 rebate, uint256 gasPaid);
 
     /// @notice propose a vote for the community
     /// @return uint256 The id of the new proposal

contracts/GovernanceBuilder.sol

@@ -44,6 +44,7 @@
 pragma solidity ^0.8.15;
 
 import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+import "@openzeppelin/contracts/utils/math/Math.sol";
 import "@openzeppelin/contracts/interfaces/IERC165.sol";
 
 import "../contracts/Constant.sol";
@@ -169,11 +170,11 @@ contract GovernanceBuilder is GovernanceCreator, ERC165 {
         return this;
     }
 
-    /// @notice setup gas refund parameters
-    /// @param _gasUsed the maximum gas used for refund
-    /// @param _baseFee the maximum base fee for refund
+    /// @notice setup gas rebate parameters
+    /// @param _gasUsed the maximum gas used for rebate
+    /// @param _baseFee the maximum base fee for rebate
     /// @return GovernanceCreator this contract
-    function withGasRefund(uint256 _gasUsed, uint256 _baseFee) external returns (GovernanceCreator) {
+    function withGasRebate(uint256 _gasUsed, uint256 _baseFee) external returns (GovernanceCreator) {
         GovernanceProperties storage _properties = _buildMap[msg.sender];
         _properties.maxGasUsed = _gasUsed;
         _properties.maxBaseFee = _baseFee;
@@ -248,7 +249,7 @@ contract GovernanceBuilder is GovernanceCreator, ERC165 {
     }
 
     function createTimelock(Storage _storage) private returns (TimeLocker) {
-        uint256 _timeLockDelay = max(_storage.minimumVoteDuration(), Constant.TIMELOCK_MINIMUM_DELAY);
+        uint256 _timeLockDelay = Math.max(_storage.minimumVoteDuration(), Constant.TIMELOCK_MINIMUM_DELAY);
         TimeLocker _timeLock = new TimeLock(_timeLockDelay);
         emit TimeLockCreated(address(_timeLock), _timeLockDelay);
         return _timeLock;
@@ -274,17 +275,10 @@ contract GovernanceBuilder is GovernanceCreator, ERC165 {
         _properties.minimumVoteDelay = Constant.MINIMUM_VOTE_DELAY;
         _properties.minimumVoteDuration = Constant.MINIMUM_VOTE_DURATION;
         _properties.minimumProjectQuorum = Constant.MINIMUM_PROJECT_QUORUM;
-        _properties.maxGasUsed = Constant.MAXIMUM_REFUND_GAS_USED;
-        _properties.maxBaseFee = Constant.MAXIMUM_REFUND_BASE_FEE;
+        _properties.maxGasUsed = Constant.MAXIMUM_REBATE_GAS_USED;
+        _properties.maxBaseFee = Constant.MAXIMUM_REBATE_BASE_FEE;
         _properties.name = "";
         _properties.url = "";
         _properties.description = "";
     }
-
-    function max(uint256 a, uint256 b) internal pure returns (uint256) {
-        if (a > b) {
-            return a;
-        }
-        return b;
-    }
 }

contracts/GovernanceCreator.sol

@@ -92,9 +92,9 @@ interface GovernanceCreator is IERC165 {
         uint256 minimumVoteDuration;
         /// @notice minimum allowed quorum
         uint256 minimumProjectQuorum;
-        /// @notice max gas used for refund
+        /// @notice max gas used for rebate
         uint256 maxGasUsed;
-        /// @notice max base fee for refund
+        /// @notice max base fee for rebate
         uint256 maxBaseFee;
         /// @notice array of supervisors
         address[] supervisorList;
@@ -163,11 +163,11 @@ interface GovernanceCreator is IERC165 {
     /// @return GovernanceCreator this contract
     function withDescription(string memory _descritpion) external returns (GovernanceCreator);
 
-    /// @notice setup gas refund parameters
-    /// @param _gasUsed the maximum gas used for refund
-    /// @param _baseFee the maximum base fee for refund
+    /// @notice setup gas rebate parameters
+    /// @param _gasUsed the maximum gas used for rebate
+    /// @param _baseFee the maximum base fee for rebate
     /// @return GovernanceCreator this contract
-    function withGasRefund(uint256 _gasUsed, uint256 _baseFee) external returns (GovernanceCreator);
+    function withGasRebate(uint256 _gasUsed, uint256 _baseFee) external returns (GovernanceCreator);
 
     /// @notice build the specified contract
     /// @dev Contructs a new contract and may require a large gas fee.  Build does not reinitialize context.

test/CollectiveGovernance.t.sol

@@ -1376,7 +1376,7 @@ contract CollectiveGovernanceTest is Test {
         vm.prank(_VOTER1, _VOTER1);
         governance.voteFor(PROPOSAL_ID, TOKEN_ID1);
         assertTrue(_VOTER1.balance > 0);
-        assertEq(_VOTER1.balance, 8599396 gwei);
+        assertApproxEqAbs(_VOTER1.balance, 8604908 gwei, 500 gwei);
     }
 
     function testCastAgainstVoteWithRefund() public {
@@ -1393,10 +1393,10 @@ contract CollectiveGovernanceTest is Test {
         vm.prank(_VOTER1, _VOTER1);
         governance.voteAgainst(PROPOSAL_ID, TOKEN_ID1);
         assertTrue(_VOTER1.balance > 0);
-        assertEq(_VOTER1.balance, 7504536 gwei);
+        assertApproxEqAbs(_VOTER1.balance, 7510048 gwei, 500 gwei);
     }
 
-    function testCastAbstainVoteWithRefund() public {
+    function testAbstainWithRefund() public {
         vm.fee(50 gwei);
 
         vm.deal(_OWNER, 1 ether);
@@ -1410,7 +1410,7 @@ contract CollectiveGovernanceTest is Test {
         vm.prank(_VOTER1, _VOTER1);
         governance.abstainFrom(PROPOSAL_ID, TOKEN_ID1);
         assertTrue(_VOTER1.balance > 0);
-        assertEq(_VOTER1.balance, 8522748 gwei);
+        assertApproxEqAbs(_VOTER1.balance, 8528260 gwei, 500 gwei);
     }
 
     function testVoteAndUndoWithRefund() public {
@@ -1430,7 +1430,7 @@ contract CollectiveGovernanceTest is Test {
         governance.undoVote(PROPOSAL_ID, TOKEN_ID1);
         vm.stopPrank();
         assertTrue(_VOTER1.balance > 0);
-        assertEq(_VOTER1.balance, 11905244 gwei);
+        assertApproxEqAbs(_VOTER1.balance, 11916268 gwei, 500 gwei);
     }
 
     function testCastVoteWithMaximumRefund() public {
@@ -1447,9 +1447,9 @@ contract CollectiveGovernanceTest is Test {
         vm.prank(_VOTER1, _VOTER1);
         governance.voteFor(PROPOSAL_ID, TOKEN_ID1);
         assertTrue(_VOTER1.balance > 0);
-        uint256 expectRefund = 16702673 gwei;
-        assertEq(_VOTER1.balance, expectRefund);
-        assertEq(_governanceAddress.balance, 1 ether - expectRefund);
+        uint256 expectRefund = 16713379 gwei;
+        assertApproxEqAbs(_VOTER1.balance, expectRefund, 500 gwei);
+        assertApproxEqAbs(_governanceAddress.balance, 1 ether - expectRefund, 500 gwei);
     }
 
     function mintTokens() private returns (IERC721) {