fix: Address audit findings for Allowlist contract

- Add two-step process enforcement for weight decrease (Issue #1)
  - Introduce decreasePending flag to track valid decrease requests
  - Prevent bypassing the intended authorization flow

- Add zero address validation (Issue #3)
  - Validate walletRegistry in initialize()
  - Validate stakingProvider in addStakingProvider()

- Add zero weight validation (Issue #5)
  - Prevent adding staking providers with zero weight
  - Avoid potential duplicate additions

- Restrict seize function access (Issue #8)
  - Only allow WalletRegistry to call seize()
  - Prevent event spam from unauthorized callers

- Add comprehensive test coverage for all security fixes

Author: piotr-roslaniec

solidity/ecdsa/contracts/Allowlist.sol

@@ -31,6 +31,7 @@ contract Allowlist is Ownable2StepUpgradeable {
     struct StakingProviderInfo {
         uint96 weight;
         uint96 pendingNewWeight;
+        bool decreasePending;
     }
 
     /// @notice Mapping between the staking provider address and a struct
@@ -59,13 +60,20 @@ contract Allowlist is Ownable2StepUpgradeable {
     error StakingProviderUnknown();
     error RequestedWeightNotBelowCurrentWeight();
     error NotWalletRegistry();
+    error NoDecreasePending();
+    error ZeroAddress();
+    error ZeroWeight();
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
         _disableInitializers();
     }
 
     function initialize(address _walletRegistry) external initializer {
+        if (_walletRegistry == address(0)) {
+            revert ZeroAddress();
+        }
+
         __Ownable2Step_init();
 
         walletRegistry = WalletRegistry(_walletRegistry);
@@ -80,6 +88,14 @@ contract Allowlist is Ownable2StepUpgradeable {
         external
         onlyOwner
     {
+        if (stakingProvider == address(0)) {
+            revert ZeroAddress();
+        }
+
+        if (weight == 0) {
+            revert ZeroWeight();
+        }
+
         StakingProviderInfo storage info = stakingProviders[stakingProvider];
 
         if (info.weight != 0) {
@@ -124,6 +140,7 @@ contract Allowlist is Ownable2StepUpgradeable {
         emit WeightDecreaseRequested(stakingProvider, currentWeight, newWeight);
 
         info.pendingNewWeight = newWeight;
+        info.decreasePending = true;
         walletRegistry.authorizationDecreaseRequested(
             stakingProvider,
             currentWeight,
@@ -151,10 +168,15 @@ contract Allowlist is Ownable2StepUpgradeable {
             revert StakingProviderUnknown();
         }
 
+        if (!info.decreasePending) {
+            revert NoDecreasePending();
+        }
+
         emit WeightDecreaseFinalized(stakingProvider, currentWeight, newWeight);
 
         info.weight = newWeight;
         info.pendingNewWeight = 0;
+        info.decreasePending = false;
         return newWeight;
     }
 
@@ -181,6 +203,10 @@ contract Allowlist is Ownable2StepUpgradeable {
         address notifier,
         address[] memory _stakingProviders
     ) external {
+        if (msg.sender != address(walletRegistry)) {
+            revert NotWalletRegistry();
+        }
+
         emit MaliciousBehaviorIdentified(notifier, _stakingProviders);
     }
 

solidity/ecdsa/test/Allowlist.test.ts

@@ -1,10 +1,9 @@
 /* eslint-disable @typescript-eslint/no-unused-expressions */
-import { ethers, helpers } from "hardhat"
+import { ethers } from "hardhat"
 import { smock } from "@defi-wonderland/smock"
 import { expect } from "chai"
 
 import type { FakeContract } from "@defi-wonderland/smock"
-import type { ContractTransaction } from "ethers"
 import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers"
 import type { Allowlist, WalletRegistry } from "../typechain"
 
@@ -51,6 +50,15 @@ describe("Allowlist", () => {
         allowlist.initialize(walletRegistry.address)
       ).to.be.revertedWith("Initializable: contract is already initialized")
     })
+
+    it("should revert if initialized with zero address", async () => {
+      const AllowlistFactory = await ethers.getContractFactory("Allowlist")
+      const newAllowlist = await AllowlistFactory.deploy()
+
+      await expect(newAllowlist.initialize(ZERO_ADDRESS)).to.be.revertedWith(
+        "ZeroAddress"
+      )
+    })
   })
 
   describe("addStakingProvider", () => {
@@ -100,6 +108,22 @@ describe("Allowlist", () => {
             .addStakingProvider(stakingProvider1.address, weight)
         ).to.be.reverted
       })
+
+      it("should revert if staking provider is zero address", async () => {
+        const weight = ethers.utils.parseEther("40000")
+
+        await expect(
+          allowlist.connect(governance).addStakingProvider(ZERO_ADDRESS, weight)
+        ).to.be.revertedWith("ZeroAddress")
+      })
+
+      it("should revert if weight is zero", async () => {
+        await expect(
+          allowlist
+            .connect(governance)
+            .addStakingProvider(stakingProvider1.address, 0)
+        ).to.be.revertedWith("ZeroWeight")
+      })
     })
 
     context("when called by non-owner", () => {
@@ -278,6 +302,20 @@ describe("Allowlist", () => {
             .approveAuthorizationDecrease(stakingProvider2.address)
         ).to.be.reverted
       })
+
+      it("should revert if no decrease was requested (bypass protection)", async () => {
+        // Add a staking provider but don't request decrease
+        await allowlist
+          .connect(governance)
+          .addStakingProvider(stakingProvider2.address, initialWeight)
+
+        // Trying to approve decrease without requesting it first should fail
+        await expect(
+          allowlist
+            .connect(walletRegistry.wallet)
+            .approveAuthorizationDecrease(stakingProvider2.address)
+        ).to.be.revertedWith("NoDecreasePending")
+      })
     })
 
     context("when called by non-WalletRegistry", () => {
@@ -330,14 +368,14 @@ describe("Allowlist", () => {
   })
 
   describe("seize", () => {
-    it("should emit MaliciousBehaviorIdentified event without seizing tokens", async () => {
+    it("should emit MaliciousBehaviorIdentified event when called by WalletRegistry", async () => {
       const stakingProviders = [
         stakingProvider1.address,
         stakingProvider2.address,
       ]
 
       await expect(
-        allowlist.seize(
+        allowlist.connect(walletRegistry.wallet).seize(
           ethers.utils.parseEther("1000"), // amount (ignored)
           100, // rewardMultiplier (ignored)
           thirdParty.address, // notifier
@@ -348,7 +386,7 @@ describe("Allowlist", () => {
         .withArgs(thirdParty.address, stakingProviders)
     })
 
-    it("should be callable by anyone", async () => {
+    it("should revert when called by non-WalletRegistry", async () => {
       const stakingProviders = [stakingProvider1.address]
 
       await expect(
@@ -360,7 +398,7 @@ describe("Allowlist", () => {
             governance.address,
             stakingProviders
           )
-      ).to.not.be.reverted
+      ).to.be.revertedWith("NotWalletRegistry")
     })
   })