diff --git a/script/deploys/DeployEtherFiOperationParameters.s.sol b/script/deploys/DeployEtherFiOperationParameters.s.sol index 06070bb1..15080939 100644 --- a/script/deploys/DeployEtherFiOperationParameters.s.sol +++ b/script/deploys/DeployEtherFiOperationParameters.s.sol @@ -23,16 +23,24 @@ contract Deploy is Script { vm.startBroadcast(deployerPrivateKey); - EtherFiOperationParameters impl = new EtherFiOperationParameters(); - UUPSProxy proxy = new UUPSProxy(address(impl), ""); - - EtherFiOperationParameters instance = EtherFiOperationParameters(payable(proxy)); - - instance.updateTagAdmin("ORACLE", 0x566E58ac0F2c4BCaF6De63760C56cC3f825C48f5, true); - instance.updateTagAdmin("EIGENPOD", 0x566E58ac0F2c4BCaF6De63760C56cC3f825C48f5, true); - // instance.updateTagKeyValue("ORACLE", "WITHDRAWAL_TARGET_GAS_PRICE", "12.5"); // 12.5 gwei - // instance.updateTagKeyValue("ORACLE", "TARGET_LIQUIDITY_IN_PERCENT_OF_TVL", "2.0"); // 2.0 % of TVL - // instance.updateTagKeyValue("EIGENPOD", "PROOF_SUBMIT_TARGET_GAS_PRICE", "12.5"); // 12.5 gwei + bool to_upgrade = true; + + if (to_upgrade) { + EtherFiOperationParameters instance = EtherFiOperationParameters(0xD0Ff8996DB4bDB46870b7E833b7532f484fEad1A); + EtherFiOperationParameters impl = new EtherFiOperationParameters(); + instance.upgradeTo(address(impl)); + } else { + EtherFiOperationParameters impl = new EtherFiOperationParameters(); + UUPSProxy proxy = new UUPSProxy(address(impl), ""); + + EtherFiOperationParameters instance = EtherFiOperationParameters(payable(proxy)); + + instance.updateTagAdmin("ORACLE", 0x566E58ac0F2c4BCaF6De63760C56cC3f825C48f5, true); + instance.updateTagAdmin("EIGENPOD", 0x566E58ac0F2c4BCaF6De63760C56cC3f825C48f5, true); + // instance.updateTagKeyValue("ORACLE", "WITHDRAWAL_TARGET_GAS_PRICE", "12.5"); // 12.5 gwei + // instance.updateTagKeyValue("ORACLE", "TARGET_LIQUIDITY_IN_PERCENT_OF_TVL", "2.0"); // 2.0 % of TVL + // instance.updateTagKeyValue("EIGENPOD", "PROOF_SUBMIT_TARGET_GAS_PRICE", "12.5"); // 12.5 gwei + } vm.stopBroadcast(); } diff --git a/src/helpers/EtherFiOperationParameters.sol b/src/helpers/EtherFiOperationParameters.sol index a52bdb05..dc2309d1 100644 --- a/src/helpers/EtherFiOperationParameters.sol +++ b/src/helpers/EtherFiOperationParameters.sol @@ -16,18 +16,18 @@ contract EtherFiOperationParameters is UUPSUpgradeable, OwnableUpgradeable { _disableInitializers(); } - function initialize() public initializer { + function initialize() external initializer { __Ownable_init(); __UUPSUpgradeable_init(); } - function updateTagAdmin(string memory tag, address admin, bool allowed) public { + function updateTagAdmin(string memory tag, address admin, bool allowed) external onlyOwner { tagAdmins[tag][admin] = allowed; emit UpdatedAdmin(tag, admin, allowed); } - function updateTagKeyValue(string memory tag, string memory key, string memory value) public { + function updateTagKeyValue(string memory tag, string memory key, string memory value) external onlyAdmin(tag) { string memory old_value = tagKeyValues[tag][key]; tagKeyValues[tag][key] = value; @@ -39,4 +39,9 @@ contract EtherFiOperationParameters is UUPSUpgradeable, OwnableUpgradeable { function getImplementation() external view returns (address) { return _getImplementation(); } + + modifier onlyAdmin(string memory tag) { + require(tagAdmins[tag][msg.sender], "Only admin can call"); + _; + } } \ No newline at end of file diff --git a/test/EtherFiOperationParameters.t.sol b/test/EtherFiOperationParameters.t.sol index a2a9073f..20d88ac1 100644 --- a/test/EtherFiOperationParameters.t.sol +++ b/test/EtherFiOperationParameters.t.sol @@ -10,6 +10,8 @@ import "../src/UUPSProxy.sol"; contract EtherFiOperationParametersTest is TestSetup { EtherFiOperationParameters operationParameters; + address rando1 = address(0x1); // Non-owner account for testing + address rando2 = address(0x2); // Another non-owner account function setUp() public { EtherFiOperationParameters impl = new EtherFiOperationParameters(); @@ -29,26 +31,98 @@ contract EtherFiOperationParametersTest is TestSetup { } function testFuzz_updateTagKeyValue( + string calldata tag, + string calldata key, + string calldata value1, + string calldata value2 + ) public { + vm.expectRevert(); + operationParameters.updateTagKeyValue(tag, key, value1); + + vm.prank(operationParameters.owner()); + operationParameters.updateTagAdmin(tag, admin, true); + + vm.prank(admin); + operationParameters.updateTagKeyValue(tag, key, value1); + assertEq(operationParameters.tagKeyValues(tag, key), value1); + + vm.prank(admin); + operationParameters.updateTagKeyValue(tag, key, value2); + assertEq(operationParameters.tagKeyValues(tag, key), value2); + } + + // Test that only the owner can call updateTagAdmin + function testFuzz_updateTagAdmin_onlyOwner( + string calldata tag, + address admin, + bool allowed + ) public { + vm.prank(rando1); // Attempt to call from a non-owner account + vm.expectRevert("Ownable: caller is not the owner"); + operationParameters.updateTagAdmin(tag, admin, allowed); + + // Owner can successfully call the function + vm.prank(operationParameters.owner()); + operationParameters.updateTagAdmin(tag, admin, allowed); + assertEq(operationParameters.tagAdmins(tag, admin), allowed); + } + + // Test that only an admin can call updateTagKeyValue + function testFuzz_updateTagKeyValue_onlyAdmin( string calldata tag, string calldata key, string calldata value ) public { + // Attempt to call without being an admin + vm.prank(rando1); + vm.expectRevert("Only admin can call"); + operationParameters.updateTagKeyValue(tag, key, value); + + // Assign admin role to rando1 for the tag + vm.prank(operationParameters.owner()); + operationParameters.updateTagAdmin(tag, rando1, true); + + // Alice can now update the key value + vm.prank(rando1); operationParameters.updateTagKeyValue(tag, key, value); assertEq(operationParameters.tagKeyValues(tag, key), value); } - function testFuzz_updateTagKeyValue( + // Test the updateTagKeyValue function with multiple updates by an admin + function testFuzz_updateTagKeyValue_asAdmin( string calldata tag, string calldata key, string calldata value1, string calldata value2 ) public { + // Assign admin role to rando2 for the tag + operationParameters.updateTagAdmin(tag, rando2, true); + + // Bob updates the key value + vm.prank(rando2); operationParameters.updateTagKeyValue(tag, key, value1); assertEq(operationParameters.tagKeyValues(tag, key), value1); + + // Bob updates the key value again + vm.prank(rando2); operationParameters.updateTagKeyValue(tag, key, value2); assertEq(operationParameters.tagKeyValues(tag, key), value2); } + // Test that only the owner can upgrade the contract + function test_upgrade_onlyOwner() public { + address newImplementation = address(new EtherFiOperationParameters()); + + // Attempt to upgrade from a non-owner account + vm.prank(rando1); + vm.expectRevert("Ownable: caller is not the owner"); + operationParameters.upgradeTo(newImplementation); + + // Owner upgrades successfully + operationParameters.upgradeTo(newImplementation); + assertEq(operationParameters.getImplementation(), newImplementation); + } + function test_upgrade() public { address newImplementation = address(new EtherFiOperationParameters()); operationParameters.upgradeTo(newImplementation);