From 0350db42a9881c5304273b4af917064d6f099fe6 Mon Sep 17 00:00:00 2001 From: drsk Date: Mon, 16 Dec 2024 14:32:42 +0100 Subject: [PATCH 1/5] Extend ValidatorAdd with a suspended field --- concordium-consensus/src/Concordium/GlobalState/BakerInfo.hs | 5 +++-- .../src/Concordium/GlobalState/Persistent/BlockState.hs | 2 +- concordium-consensus/src/Concordium/Scheduler.hs | 1 + .../tests/globalstate/GlobalStateTests/ConfigureValidator.hs | 3 ++- .../tests/globalstate/GlobalStateTests/Updates.hs | 3 ++- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/concordium-consensus/src/Concordium/GlobalState/BakerInfo.hs b/concordium-consensus/src/Concordium/GlobalState/BakerInfo.hs index 8bbac188f2..abff2ab303 100644 --- a/concordium-consensus/src/Concordium/GlobalState/BakerInfo.hs +++ b/concordium-consensus/src/Concordium/GlobalState/BakerInfo.hs @@ -219,8 +219,9 @@ data ValidatorAdd = ValidatorAdd -- | The metadata URL for the validator. vaMetadataURL :: !UrlText, -- | The commission rates for the validator. - vaCommissionRates :: !CommissionRates - -- TODO (drsk) Github issue #1246. Support suspend/resume for ValidatorAdd. + vaCommissionRates :: !CommissionRates, + -- | Whether the validator should be added as suspended. + vaSuspended :: !Bool } deriving (Eq, Show) diff --git a/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs b/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs index d96f50a699..5a4996e38f 100644 --- a/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs +++ b/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs @@ -1632,7 +1632,7 @@ newAddValidator pbs ai va@ValidatorAdd{..} = do BaseAccounts.BakerInfoExV1 { _bieBakerPoolInfo = poolInfo, _bieBakerInfo = bakerInfo, - _bieIsSuspended = conditionally hasValidatorSuspension False + _bieIsSuspended = conditionally hasValidatorSuspension vaSuspended } -- The precondition guaranties that the account exists acc <- fromJust <$> Accounts.indexedAccount ai (bspAccounts bsp) diff --git a/concordium-consensus/src/Concordium/Scheduler.hs b/concordium-consensus/src/Concordium/Scheduler.hs index eedd34c331..4d99ca264b 100644 --- a/concordium-consensus/src/Concordium/Scheduler.hs +++ b/concordium-consensus/src/Concordium/Scheduler.hs @@ -2114,6 +2114,7 @@ handleConfigureBaker let vaCommissionRates = CommissionRates{..} vaOpenForDelegation <- cbOpenForDelegation vaMetadataURL <- cbMetadataURL + let vaSuspended = fromMaybe False cbSuspend return CBCAdd { cbcRemoveDelegator = removeDelegator, diff --git a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs index a3b7fddb0c..cc532787a7 100644 --- a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs +++ b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs @@ -142,7 +142,8 @@ testAddValidatorAllCases spv = describe "bsoAddValidator" $ do { _finalizationCommission = makeAmountFraction $ if vcFinalizationRewardNotInRange then 100 else 300, _bakingCommission = makeAmountFraction $ if vcBakingRewardNotInRange then 100 else 500, _transactionCommission = makeAmountFraction $ if vcTransactionFeeNotInRange then 300 else 100 - } + }, + vaSuspended = False } initialAccounts <- mapM makeDummyAccount (addValidatorTestAccounts withCooldown) initialBS <- mkInitialState initialAccounts diff --git a/concordium-consensus/tests/globalstate/GlobalStateTests/Updates.hs b/concordium-consensus/tests/globalstate/GlobalStateTests/Updates.hs index a4115215c9..acaa5e7161 100644 --- a/concordium-consensus/tests/globalstate/GlobalStateTests/Updates.hs +++ b/concordium-consensus/tests/globalstate/GlobalStateTests/Updates.hs @@ -143,7 +143,8 @@ addBakerWith am (bs, ai) = do { _transactionCommission = makeAmountFraction 0, _finalizationCommission = makeAmountFraction 0, _bakingCommission = makeAmountFraction 0 - } + }, + vaSuspended = False } res <- bsoAddValidator bs ai conf return ((,ai) <$> res) From 5b06b6249a256b84e35e7d30f7e6478d6922b768 Mon Sep 17 00:00:00 2001 From: drsk Date: Tue, 7 Jan 2025 12:51:12 +0100 Subject: [PATCH 2/5] test for suspended ValidatorAdd --- .../GlobalStateTests/ConfigureValidator.hs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs index cc532787a7..e40bcd13a1 100644 --- a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs +++ b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs @@ -105,10 +105,12 @@ testAddValidatorAllCases :: Spec testAddValidatorAllCases spv = describe "bsoAddValidator" $ do forM_ validatorConditions $ \vc -> do - it (show vc) $ runTest False vc - when supportCooldown $ it (show vc <> " with cooldown") $ runTest True vc + it (show vc) $ runTest False False vc + when supportCooldown $ it (show vc <> " with cooldown") $ runTest True False vc + when supportSuspension $ it (show vc <> " with suspended validator") $ runTest True True vc where supportCooldown = supportsFlexibleCooldown $ accountVersionFor $ demoteProtocolVersion (protocolVersion @pv) + supportSuspension = supportsValidatorSuspension $ accountVersionFor $ demoteProtocolVersion (protocolVersion @pv) minEquity = 1_000_000_000 chainParams = DummyData.dummyChainParameters @(ChainParametersVersionFor pv) @@ -129,7 +131,7 @@ testAddValidatorAllCases spv = describe "bsoAddValidator" $ do DummyData.dummyArs (withIsAuthorizationsVersionForPV spv DummyData.dummyKeyCollection) chainParams - runTest withCooldown ValidatorConditions{..} = runTestBlockState @pv $ do + runTest withCooldown suspended ValidatorConditions{..} = runTestBlockState @pv $ do let va = ValidatorAdd { vaKeys = if vcAggregationKeyDuplicate then badKeys else goodKeys, @@ -143,7 +145,7 @@ testAddValidatorAllCases spv = describe "bsoAddValidator" $ do _bakingCommission = makeAmountFraction $ if vcBakingRewardNotInRange then 100 else 500, _transactionCommission = makeAmountFraction $ if vcTransactionFeeNotInRange then 300 else 100 }, - vaSuspended = False + vaSuspended = suspended } initialAccounts <- mapM makeDummyAccount (addValidatorTestAccounts withCooldown) initialBS <- mkInitialState initialAccounts @@ -190,7 +192,7 @@ testAddValidatorAllCases spv = describe "bsoAddValidator" $ do _poolMetadataUrl = vaMetadataURL va, _poolCommissionRates = vaCommissionRates va }, - _bieIsSuspended = conditionally (sSupportsValidatorSuspension (accountVersion @(AccountVersionFor pv))) False + _bieIsSuspended = conditionally (sSupportsValidatorSuspension (accountVersion @(AccountVersionFor pv))) suspended } } bkr <- getAccountBaker (fromJust acc) @@ -578,3 +580,7 @@ tests lvl = parallel $ describe "Validator" $ do testAddValidatorAllCases SP7 testUpdateValidator SP7 (lvl > 1) testUpdateValidatorOverlappingCommissions SP7 + describe "P8" $ do + testAddValidatorAllCases SP8 + testUpdateValidator SP8 (lvl > 1) + testUpdateValidatorOverlappingCommissions SP8 From 266d7f09a0cba7134d0df5d528b1d1ef1e775bb4 Mon Sep 17 00:00:00 2001 From: drsk Date: Wed, 8 Jan 2025 14:43:49 +0100 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 661f3e793f..1dce0b7021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Automatically suspend validators from the consensus that missed too many rounds in the previous payday. - Add support for suspend/resume to validator configuration updates. +- Add support to add a validator in a suspended state. - Validators that are suspended are paused from participating in the consensus algorithm. - Add suspension info to `BakerPoolStatus` / `CurrentPaydayBakerPoolStatus` query results. - Add `GetConsensusDetailedStatus` gRPC endpoint for getting detailed information on the status From 59975e0144a84c7acd97eae073823299c6e39f59 Mon Sep 17 00:00:00 2001 From: drsk Date: Wed, 8 Jan 2025 19:22:24 +0100 Subject: [PATCH 4/5] fix tests --- .../GlobalState/Persistent/BlockState.hs | 19 +++++++++++-------- .../GlobalStateTests/ConfigureDelegator.hs | 3 +++ .../GlobalStateTests/ConfigureValidator.hs | 10 ++++++++++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs b/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs index 5a4996e38f..0b2a562424 100644 --- a/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs +++ b/concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs @@ -1783,7 +1783,13 @@ updateValidatorChecks bsp baker ValidatorUpdate{..} = do -- -- (3) append @BakerConfigureFinalizationRewardCommission frc@ to @events@. -- --- 8. If the capital is supplied: if there is a pending change to the baker's capital, return +-- 8. (>= P8) If the suspended/resumed flag is set: + +-- (1) Suspend/resume the validator according to the flag. + +-- (2) Append @BakerConfigureSuspended@ or @BakerConfigureResumed@ accordingly to @events@. +-- +-- 9. If the capital is supplied: if there is a pending change to the baker's capital, return -- @VCFChangePending@; otherwise: -- -- * if the capital is 0 @@ -1820,12 +1826,6 @@ updateValidatorChecks bsp baker ValidatorUpdate{..} = do -- index by adding the difference between the new and old capital) and append -- @BakerConfigureStakeIncreased capital@ to @events@. --- 9. (>= P8) If the suspended/resumed flag is set: - --- (1) Suspend/resume the validator according to the flag. - --- (2) Append @BakerConfigureSuspended@ or @BakerConfigureResumed@ accordingly to @events@. --- -- 10. Return @events@ with the updated block state. newUpdateValidator :: forall pv m. @@ -1853,8 +1853,11 @@ newUpdateValidator pbs curTimestamp ai vu@ValidatorUpdate{..} = do updateKeys existingBaker (bsp, acc) >>= updateRestakeEarnings >>= updatePoolInfo existingBaker - >>= updateCapital existingBaker + -- NOTE: updateSuspend needs to be executed before updateCapital. + -- Because if we update the stake to 0, the validator gets + -- removed. After this, a call to updateSuspend will error. >>= updateSuspend + >>= updateCapital existingBaker newAccounts <- Accounts.setAccountAtIndex ai newAcc (bspAccounts newBSP) return newBSP{bspAccounts = newAccounts} (events,) <$> storePBS pbs newBSP diff --git a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureDelegator.hs b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureDelegator.hs index 4c15c3ecac..75650487d3 100644 --- a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureDelegator.hs +++ b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureDelegator.hs @@ -531,3 +531,6 @@ tests = parallel $ describe "Configure delegator" $ do describe "P7" $ do it "bsoAddDelegator" $ testAddDelegator SP7 it "bsoUpdateDelegator" $ testUpdateDelegator SP7 + describe "P8" $ do + it "bsoAddDelegator" $ testAddDelegator SP8 + it "bsoUpdateDelegator" $ testUpdateDelegator SP8 diff --git a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs index e40bcd13a1..82f9cebbb9 100644 --- a/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs +++ b/concordium-consensus/tests/globalstate/GlobalStateTests/ConfigureValidator.hs @@ -491,6 +491,11 @@ runUpdateValidatorTest spv commissionRanges ValidatorUpdateConfig{vucValidatorUp forM_ (vuTransactionFeeCommission vu) $ \fee -> tell [BakerConfigureTransactionFeeCommission fee] forM_ (vuBakingRewardCommission vu) $ \fee -> tell [BakerConfigureBakingRewardCommission fee] forM_ (vuFinalizationRewardCommission vu) $ \fee -> tell [BakerConfigureFinalizationRewardCommission fee] + forM_ (vuSuspend vu) $ \suspended -> + tell + [ if suspended then BakerConfigureSuspended else BakerConfigureResumed + | STrue <- [hasValidatorSuspension] + ] forM_ (vuCapital vu) $ \capital -> tell $ if capital >= initialStakedAmount @@ -529,6 +534,9 @@ runUpdateValidatorTest spv commissionRanges ValidatorUpdateConfig{vucValidatorUp .~ (if newCapital == 0 then RemoveStake else ReduceStake newCapital) (PendingChangeEffectiveV1 (24 * 60 * 60 * 1000 + 1000)) | otherwise = stakedAmount .~ newCapital + let updateSuspended suspend + | STrue <- hasValidatorSuspension = accountBakerInfo . bieIsSuspended .~ suspend + | otherwise = id let expectedAccountBaker | STrue <- flexibleCooldown, vuCapital vu == Just 0 = Nothing | otherwise = @@ -549,10 +557,12 @@ runUpdateValidatorTest spv commissionRanges ValidatorUpdateConfig{vucValidatorUp & maybe id (poolCommissionRates . finalizationCommission .~) (vuFinalizationRewardCommission vu) & maybe id (poolCommissionRates . bakingCommission .~) (vuBakingRewardCommission vu) & maybe id (poolCommissionRates . transactionCommission .~) (vuTransactionFeeCommission vu) + & maybe id updateSuspended (vuSuspend vu) actualAccountBaker <- getAccountBaker acc0 liftIO $ actualAccountBaker `shouldBe` expectedAccountBaker where flexibleCooldown = sSupportsFlexibleCooldown (accountVersion @(AccountVersionFor pv)) + hasValidatorSuspension = sSupportsValidatorSuspension (accountVersion @(AccountVersionFor pv)) minEquity = 1_000_000_000 chainParams = DummyData.dummyChainParameters @(ChainParametersVersionFor pv) From 412ae235e75fafefd56108592a186316e8b98374 Mon Sep 17 00:00:00 2001 From: drsk Date: Thu, 9 Jan 2025 15:44:45 +0100 Subject: [PATCH 5/5] update docs --- .../src/Concordium/GlobalState/BlockState.hs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/concordium-consensus/src/Concordium/GlobalState/BlockState.hs b/concordium-consensus/src/Concordium/GlobalState/BlockState.hs index 7c81152df5..4c3a7da4e4 100644 --- a/concordium-consensus/src/Concordium/GlobalState/BlockState.hs +++ b/concordium-consensus/src/Concordium/GlobalState/BlockState.hs @@ -1084,8 +1084,15 @@ class (BlockStateQuery m) => BlockStateOperations m where -- (2) update the account's finalization reward commission rate to the the supplied value @frc@; -- -- (3) append @BakerConfigureFinalizationRewardCommission frc@ to @events@. + + -- 7. (>= P8) If the suspended/resumed flag is set: + + -- (1) Suspend/resume the validator according to the flag. + + -- (2) Append @BakerConfigureSuspended@ or @BakerConfigureResumed@ accordingly to @events@. + -- -- - -- 7. If the capital is supplied: if there is a pending change to the baker's capital, return + -- 8. If the capital is supplied: if there is a pending change to the baker's capital, return -- @VCFChangePending@; otherwise: -- -- * if the capital is 0 @@ -1121,12 +1128,6 @@ class (BlockStateQuery m) => BlockStateOperations m where -- is (preferentially) reactivated from the inactive stake, updating the global indices -- accordingly. -- - -- 8. (>= P8) If the suspended/resumed flag is set: - - -- (1) Suspend/resume the validator according to the flag. - - -- (2) Append @BakerConfigureSuspended@ or @BakerConfigureResumed@ accordingly to @events@. - -- -- 9. Return @events@ with the updated block state. bsoUpdateValidator :: (PVSupportsDelegation (MPV m)) =>