From fcd7596405fa859665de474dfdf9332ea998d230 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Fri, 3 May 2024 15:24:25 +0200 Subject: [PATCH] Adding checks for newVal == oldVal Verified that a new val is not the same as the old one during parameter updates. --- solidity/contracts/PausableOwnable.sol | 7 +++++++ solidity/contracts/stBTC.sol | 12 ++++++++++++ solidity/test/stBTC.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/solidity/contracts/PausableOwnable.sol b/solidity/contracts/PausableOwnable.sol index 7e888475b..00f8a36ec 100644 --- a/solidity/contracts/PausableOwnable.sol +++ b/solidity/contracts/PausableOwnable.sol @@ -35,6 +35,10 @@ abstract contract PausableOwnable is /// mechanism. error PausableUnauthorizedAccount(address account); + /// @notice Reverts when the new pause admin address is the same as the + /// current pause admin address. + error SamePauseAdmin(); + /// @notice Reverts if called by any account other than the pause admin /// or the contract owner. modifier onlyPauseAdminOrOwner() { @@ -95,6 +99,9 @@ abstract contract PausableOwnable is if (newPauseAdmin == address(0)) { revert ZeroAddress(); } + if (newPauseAdmin == pauseAdmin) { + revert SamePauseAdmin(); + } emit PauseAdminUpdated(newPauseAdmin, pauseAdmin); diff --git a/solidity/contracts/stBTC.sol b/solidity/contracts/stBTC.sol index a8065cde1..0019aaf55 100644 --- a/solidity/contracts/stBTC.sol +++ b/solidity/contracts/stBTC.sol @@ -74,6 +74,12 @@ contract stBTC is ERC4626Fees, PausableOwnable { /// Reverts if the address is disallowed. error DisallowedAddress(); + /// Reverts if the treasury address is the same. + error SameTreasury(); + + /// Reverts if the dispatcher address is the same. + error SameDispatcher(); + /// @custom:oz-upgrades-unsafe-allow constructor constructor() { _disableInitializers(); @@ -104,6 +110,9 @@ contract stBTC is ERC4626Fees, PausableOwnable { if (newTreasury == address(this)) { revert DisallowedAddress(); } + if (newTreasury == treasury) { + revert SameTreasury(); + } emit TreasuryUpdated(treasury, newTreasury); @@ -128,6 +137,9 @@ contract stBTC is ERC4626Fees, PausableOwnable { if (address(newDispatcher) == address(0)) { revert ZeroAddress(); } + if (address(newDispatcher) == address(dispatcher)) { + revert SameDispatcher(); + } address oldDispatcher = address(dispatcher); diff --git a/solidity/test/stBTC.test.ts b/solidity/test/stBTC.test.ts index 37e8a04ad..560bb904f 100644 --- a/solidity/test/stBTC.test.ts +++ b/solidity/test/stBTC.test.ts @@ -1554,6 +1554,14 @@ describe("stBTC", () => { }) }) + context("when a new dispatcher is the same as the old one", () => { + it("should revert", async () => { + await expect( + stbtc.connect(governance).updateDispatcher(mezoAllocator), + ).to.be.revertedWithCustomError(stbtc, "SameDispatcher") + }) + }) + context("when a new dispatcher is non-zero address", () => { let newDispatcher: string let stbtcAddress: string @@ -1616,6 +1624,14 @@ describe("stBTC", () => { }) }) + context("when a new treasury is same as the old one", () => { + it("should revert", async () => { + await expect( + stbtc.connect(governance).updateTreasury(treasury), + ).to.be.revertedWithCustomError(stbtc, "SameTreasury") + }) + }) + context("when a new treasury is Acre address", () => { it("should revert", async () => { await expect( @@ -1693,6 +1709,16 @@ describe("stBTC", () => { .withArgs(pauseAdmin.address) }) }) + + context("when updating pause admin to the same address", () => { + beforeAfterSnapshotWrapper() + + it("should revert", async () => { + await expect( + stbtc.connect(governance).updatePauseAdmin(pauseAdmin.address), + ).to.be.revertedWithCustomError(stbtc, "SamePauseAdmin") + }) + }) }) context("when the unauthorized account tries to pause contract", () => {