BLOCKCHAIN-236 - Bridge - min value for the setVotesRequired

bin/node/runtime/src/lib.rs

@@ -1581,6 +1581,7 @@ parameter_types! {
 	pub const LLMMinimumTransfer: Balance = 10 * GRAINS_IN_LLM;
 	pub const BridgeMinimumFee: Balance = 1 * DOLLARS;
 	pub const BridgeMaximumFee: Balance = 10 * DOLLARS;
+	pub const BridgeMinimumVotesRequired: u32 = 3;
 }
 
 type EthLLDBridgeInstance = pallet_federated_bridge::Instance1;
@@ -1598,6 +1599,7 @@ impl pallet_federated_bridge::Config<EthLLDBridgeInstance> for Runtime {
 	type MinimumTransfer = LLDMinimumTransfer;
 	type MinimumFee = BridgeMinimumFee;
 	type MaximumFee = BridgeMaximumFee;
+	type MinimumVotesRequired = BridgeMinimumVotesRequired;
 	type WeightInfo = ();
 }
 
@@ -1616,6 +1618,7 @@ impl pallet_federated_bridge::Config<EthLLMBridgeInstance> for Runtime {
 	type MinimumTransfer = LLMMinimumTransfer;
 	type MinimumFee = BridgeMinimumFee;
 	type MaximumFee = BridgeMaximumFee;
+	type MinimumVotesRequired = BridgeMinimumVotesRequired;
 	type WeightInfo = ();
 }
 

eth-bridge/contracts/.solhint.json

@@ -7,6 +7,7 @@
     "modifier-name-mixedcase": "error",
     "named-parameters-mapping": "error",
     "private-vars-leading-underscore": ["error",{"strict":true}],
-    "func-visibility": ["warn",{"ignoreConstructors":true}]
+    "func-visibility": ["warn",{"ignoreConstructors":true}],
+    "max-states-count": "off"
   }
 }

eth-bridge/contracts/script/Deploy.s.sol

@@ -44,7 +44,8 @@ contract Deploy is Script {
                     30_000_000_000_000, // min transfer
                     3_000_000_000_000_000_000, // max supply limit
                     1_000_000 gwei, // min fee
-                    100_000_000 gwei // max fee
+                    100_000_000 gwei, // max fee
+                    3 // min votes required
                 )
             )
         );
@@ -74,7 +75,8 @@ contract Deploy is Script {
                     10_000_000_000_000, // min transfer
                     1_000_000_000_000_000_000, // max supply limit
                     1_000_000 gwei, // min fee
-                    100_000_000 gwei // max fee
+                    100_000_000 gwei, // max fee
+                    3 // min votes required
                 )
             )
         );

eth-bridge/contracts/script/UpgradeBridgesToV2.s.sol

@@ -19,7 +19,8 @@ contract UpgradeBridgesToV2 is Script {
                 (
                     3_000_000_000_000_000_000, // max supply limit of 3M LLD, admin can lower it
                     1_000_000 gwei, // min fee
-                    100_000_000 gwei // max fee
+                    100_000_000 gwei, // max fee
+                    3 // min votes required
                 )
             )
         );
@@ -30,7 +31,8 @@ contract UpgradeBridgesToV2 is Script {
                 (
                     1_000_000_000_000_000_000, // max supply limit of 3M LLD, admin can lower it
                     1_000_000 gwei, // min fee
-                    100_000_000 gwei // max fee
+                    100_000_000 gwei, // max fee
+                    3 // min votes required
                 )
             )
         );

eth-bridge/contracts/src/Bridge.sol

@@ -100,6 +100,8 @@ contract Bridge is Initializable, AccessControlUpgradeable, UUPSUpgradeable, Bri
     uint256 public minFee;
     /// Maximum fee that admin can set
     uint256 public maxFee;
+    /// Minimum votes required that admin can set
+    uint256 public minVotesRequired;
 
     constructor() {
         _disableInitializers();
@@ -128,10 +130,11 @@ contract Bridge is Initializable, AccessControlUpgradeable, UUPSUpgradeable, Bri
         uint256 minTransfer_,
         uint256 maxSupplyLimit_,
         uint256 minFee_,
-        uint256 maxFee_
+        uint256 maxFee_,
+        uint256 minVotesRequired_
     ) external {
         _initializeV1(token_, votesRequired_, mintDelay_, fee_, counterLimit, decayRate, supplyLimit_, minTransfer_);
-        initializeV2(maxSupplyLimit_, minFee_, maxFee_);
+        initializeV2(maxSupplyLimit_, minFee_, maxFee_, minVotesRequired_);
     }
 
     function _initializeV1(
@@ -162,10 +165,14 @@ contract Bridge is Initializable, AccessControlUpgradeable, UUPSUpgradeable, Bri
 
     /// Reinitializer from v1 to v2. Should be used in the same tx as upgrade
     /// @param maxSupplyLimit_ maximum `supplyLimit` that can be set by admin
-    function initializeV2(uint256 maxSupplyLimit_, uint256 minFee_, uint256 maxFee_) public reinitializer(2) {
+    function initializeV2(uint256 maxSupplyLimit_, uint256 minFee_, uint256 maxFee_, uint256 minVotesRequired_)
+        public
+        reinitializer(2)
+    {
         maxSupplyLimit = maxSupplyLimit_;
         minFee = minFee_;
         maxFee = maxFee_;
+        minVotesRequired = minVotesRequired_;
     }
 
     /// Adding special users. See role docs on info who can grant each role
@@ -342,7 +349,7 @@ contract Bridge is Initializable, AccessControlUpgradeable, UUPSUpgradeable, Bri
     /// @dev Only addresses with SUPER_ADMIN_ROLE can call this
     /// @dev Reverts with `InvalidArgument` if `votesRequired_` is 0
     function setVotesRequired(uint32 votesRequired_) public onlyRole(SUPER_ADMIN_ROLE) {
-        if (votesRequired_ == 0) revert InvalidArgument();
+        if (votesRequired_ < minVotesRequired) revert InvalidArgument();
         votesRequired = votesRequired_;
     }
 

eth-bridge/contracts/test/Bridge.t.sol

@@ -42,7 +42,7 @@ contract BridgeTest is Test, BridgeEvents {
                     address(bridgeImpl),
                     abi.encodeCall(
                         Bridge.initialize,
-                        (token, 2, 10, 4, 1000, 10, 1_000_000, 0, 1_000_000, 2, 100)
+                        (token, 2, 10, 4, 1000, 10, 1_000_000, 0, 1_000_000, 2, 100, 2)
                     )
                 )
             )
@@ -428,12 +428,17 @@ contract BridgeTest is Test, BridgeEvents {
     }
 
     function testSetVotesRequiredWorks() public {
-        bridge.setVotesRequired(1);
-        assertEq(bridge.votesRequired(), 1);
+        bridge.setVotesRequired(5);
+        assertEq(bridge.votesRequired(), 5);
         bridge.setVotesRequired(10);
         assertEq(bridge.votesRequired(), 10);
     }
 
+    function testSetVotesRequiredRespectsMin() public {
+        vm.expectRevert(InvalidArgument.selector);
+        bridge.setVotesRequired(1);
+    }
+
     function testSetVotesRequiredRequiresSuperAdmin() public {
         vm.prank(alice);
         vm.expectRevert(
@@ -762,10 +767,11 @@ contract BridgeTest is Test, BridgeEvents {
 
     function testMaxTotalSupplyIsRespectedAfterBurns() public {
         bridge.setSupplyLimit(500);
-        bridge.setVotesRequired(1);
 
         vm.prank(alice);
         bridge.voteMint(receipt1, 1, 100, alice);
+        vm.prank(bob);
+        bridge.voteMint(receipt1, 1, 100, alice);
 
         vm.roll(11);
         vm.expectRevert(TooMuchSupply.selector);
@@ -775,13 +781,6 @@ contract BridgeTest is Test, BridgeEvents {
         bridge.mint{value: 4}(receipt1);
     }
 
-    function testVotesRequiredCantBeZero() public {
-        vm.expectRevert(InvalidArgument.selector);
-        bridge.setVotesRequired(0);
-
-        bridge.setVotesRequired(1);
-    }
-
     function testOnlyUpgraderCanUpgrade() public {
         Bridge impl2 = new Bridge();
 

eth-bridge/contracts/test/VoteFeeCompare.t.sol

@@ -55,7 +55,8 @@ contract VoteFeeCompare is Test, BridgeEvents {
                             0,
                             650,
                             2,
-                            100
+                            100,
+                            2
                         )
                     )
                 )

frame/federated-bridge/src/benchmarking.rs

@@ -114,7 +114,7 @@ benchmarks_instance_pallet! {
 	}
 
 	vote_withdraw {
-		let r in 1 .. T::MaxRelays::get() => add_relays::<T, I>(r)?;
+		let r in 2 .. T::MaxRelays::get() => add_relays::<T, I>(r)?;
 		activate::<T, I>();
 
 		let admin: T::AccountId = account("admin", 0, SEED);
@@ -137,7 +137,7 @@ benchmarks_instance_pallet! {
 	}
 
 	withdraw {
-		let r in 1 .. T::MaxRelays::get() => add_relays::<T, I>(r)?;
+		let r in 2 .. T::MaxRelays::get() => add_relays::<T, I>(r)?;
 
 		let admin: T::AccountId = account("admin", 0, SEED);
 		let origin = RawOrigin::Signed(admin.clone());

frame/federated-bridge/src/lib.rs

@@ -274,6 +274,10 @@ pub mod pallet {
 		/// Maximum fee that admin can set.
 		type MaximumFee: Get<BalanceOf<Self, I>>;
 
+		#[pallet::constant]
+		/// Maximum value of votes required that admin can set.
+		type MinimumVotesRequired: Get<u32>;
+
 		/// Origin that's authorized to set Admin and SuperAdmin
 		type ForceOrigin: EnsureOrigin<Self::RuntimeOrigin>;
 
@@ -660,6 +664,7 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::set_votes_required())]
 		pub fn set_votes_required(origin: OriginFor<T>, votes_required: u32) -> DispatchResult {
 			Self::ensure_super_admin(origin)?;
+			ensure!(votes_required >= T::MinimumVotesRequired::get(), Error::<T, I>::InvalidValue);
 			VotesRequired::<T, I>::set(votes_required);
 			Ok(())
 		}

frame/federated-bridge/src/mock.rs

@@ -97,6 +97,7 @@ impl pallet_federated_bridge::Config for Test {
 	type MinimumTransfer = ConstU64<2>;
 	type MinimumFee = ConstU64<10>;
 	type MaximumFee = ConstU64<100>;
+	type MinimumVotesRequired = ConstU32<2>;
 	type WeightInfo = ();
 }
 

frame/federated-bridge/src/tests.rs

@@ -686,6 +686,16 @@ fn set_votes_required_works() {
 	});
 }
 
+#[test]
+fn set_votes_required_respects_minimum() {
+	new_test_ext().execute_with(|| {
+		assert_noop!(
+			Bridge::set_votes_required(RuntimeOrigin::signed(101), 1),
+			Error::<Test>::InvalidValue
+		);
+	});
+}
+
 #[test]
 fn add_relay_checks_origin() {
 	new_test_ext().execute_with(|| {