From a9b3951f1f6a9dd84ebf88b3b5becc387d20f624 Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Thu, 1 Feb 2024 17:07:56 +0200 Subject: [PATCH] Move balance tracker inside accumulator --- .../src/accounts_balances_tracker.rs | 88 ++++++++++++++ .../src/constraints_accumulator.rs | 10 +- .../src/error.rs | 6 +- .../constraints-value-accumulator/src/lib.rs | 1 + chainstate/src/detail/ban_score.rs | 6 +- .../test-suite/src/tests/delegation_tests.rs | 58 ++++++--- .../src/tests/pos_processing_tests.rs | 4 +- .../accounts_balances_check.rs | 115 ------------------ .../input_output_policy/mod.rs | 27 +--- common/src/chain/config/builder.rs | 36 ++---- mempool/src/error/ban_score.rs | 4 - 11 files changed, 164 insertions(+), 191 deletions(-) create mode 100644 chainstate/constraints-value-accumulator/src/accounts_balances_tracker.rs delete mode 100644 chainstate/tx-verifier/src/transaction_verifier/input_output_policy/accounts_balances_check.rs diff --git a/chainstate/constraints-value-accumulator/src/accounts_balances_tracker.rs b/chainstate/constraints-value-accumulator/src/accounts_balances_tracker.rs new file mode 100644 index 0000000000..626e0dc23b --- /dev/null +++ b/chainstate/constraints-value-accumulator/src/accounts_balances_tracker.rs @@ -0,0 +1,88 @@ +// Copyright (c) 2024 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::collections::{btree_map::Entry, BTreeMap}; + +use common::{ + chain::{AccountSpending, AccountType, DelegationId}, + primitives::Amount, +}; + +use crate::Error; + +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] +enum GenericAccountId { + Delegation(DelegationId), +} + +impl From for GenericAccountId { + fn from(account: AccountSpending) -> Self { + match account { + AccountSpending::DelegationBalance(id, _) => Self::Delegation(id), + } + } +} + +impl From for AccountType { + fn from(value: GenericAccountId) -> Self { + match value { + GenericAccountId::Delegation(id) => AccountType::Delegation(id), + } + } +} + +pub struct AccountsBalancesTracker<'a, DelegationBalanceGetterFn> { + balances: BTreeMap, + + delegation_balance_getter: &'a DelegationBalanceGetterFn, +} + +impl<'a, DelegationBalanceGetterFn> AccountsBalancesTracker<'a, DelegationBalanceGetterFn> +where + DelegationBalanceGetterFn: Fn(DelegationId) -> Result, Error>, +{ + pub fn new(delegation_balance_getter: &'a DelegationBalanceGetterFn) -> Self { + Self { + balances: BTreeMap::new(), + delegation_balance_getter, + } + } + + pub fn spend_from_account(&mut self, account: AccountSpending) -> Result<(), Error> { + match self.balances.entry(account.clone().into()) { + Entry::Vacant(e) => { + let (balance, spending) = match account { + AccountSpending::DelegationBalance(id, spending) => { + let balance = (self.delegation_balance_getter)(id)? + .ok_or(Error::AccountBalanceNotFound(account.clone().into()))?; + (balance, spending) + } + }; + let new_balance = (balance - spending) + .ok_or(Error::NegativeAccountBalance(account.clone().into()))?; + e.insert(new_balance); + } + Entry::Occupied(mut e) => { + let balance = e.get_mut(); + let spending = match account { + AccountSpending::DelegationBalance(_, spending) => spending, + }; + *balance = (*balance - spending) + .ok_or(Error::NegativeAccountBalance(account.clone().into()))?; + } + }; + Ok(()) + } +} diff --git a/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs b/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs index 8abb2e09ca..a8cf75a0bd 100644 --- a/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs +++ b/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs @@ -25,6 +25,8 @@ use common::{ }; use utils::ensure; +use crate::accounts_balances_tracker::AccountsBalancesTracker; + use super::{accumulated_fee::AccumulatedFee, insert_or_increase, Error}; /// `ConstrainedValueAccumulator` helps avoiding messy inputs/outputs combinations analysis by @@ -73,6 +75,8 @@ impl ConstrainedValueAccumulator { let mut accumulator = Self::new(); let mut total_fee_deducted = Amount::ZERO; + let mut accounts_balances_tracker = + AccountsBalancesTracker::new(&delegation_balance_getter); for (input, input_utxo) in inputs.iter().zip(inputs_utxos.iter()) { match input { @@ -92,6 +96,7 @@ impl ConstrainedValueAccumulator { chain_config, block_height, outpoint.account(), + &mut accounts_balances_tracker, &delegation_balance_getter, )?; } @@ -195,6 +200,7 @@ impl ConstrainedValueAccumulator { chain_config: &ChainConfig, block_height: BlockHeight, account: &AccountSpending, + accounts_balances_tracker: &mut AccountsBalancesTracker, delegation_balance_getter: &DelegationBalanceGetterFn, ) -> Result<(), Error> where @@ -216,7 +222,9 @@ impl ConstrainedValueAccumulator { Error::AttemptToPrintMoney(CoinOrTokenId::Coin) ); } - AccountsBalancesCheckVersion::V1 => {} + AccountsBalancesCheckVersion::V1 => { + accounts_balances_tracker.spend_from_account(account.clone())?; + } } let maturity_distance = diff --git a/chainstate/constraints-value-accumulator/src/error.rs b/chainstate/constraints-value-accumulator/src/error.rs index f278ef1fe9..03371dd2f0 100644 --- a/chainstate/constraints-value-accumulator/src/error.rs +++ b/chainstate/constraints-value-accumulator/src/error.rs @@ -14,7 +14,7 @@ // limitations under the License. use common::{ - chain::{DelegationId, PoolId, UtxoOutPoint}, + chain::{AccountType, DelegationId, PoolId, UtxoOutPoint}, primitives::CoinOrTokenId, }; @@ -44,4 +44,8 @@ pub enum Error { SpendingNonSpendableOutput(UtxoOutPoint), #[error("Balance not found for delegation `{0}`")] DelegationBalanceNotFound(DelegationId), + #[error("Account balance not found for `{0:?}`")] + AccountBalanceNotFound(AccountType), + #[error("Negative account balance for `{0:?}`")] + NegativeAccountBalance(AccountType), } diff --git a/chainstate/constraints-value-accumulator/src/lib.rs b/chainstate/constraints-value-accumulator/src/lib.rs index 57afcc0fcd..79cf068ef1 100644 --- a/chainstate/constraints-value-accumulator/src/lib.rs +++ b/chainstate/constraints-value-accumulator/src/lib.rs @@ -17,6 +17,7 @@ use std::collections::BTreeMap; use common::primitives::Amount; +mod accounts_balances_tracker; mod accumulated_fee; mod constraints_accumulator; mod error; diff --git a/chainstate/src/detail/ban_score.rs b/chainstate/src/detail/ban_score.rs index 99806704cd..2da7da23eb 100644 --- a/chainstate/src/detail/ban_score.rs +++ b/chainstate/src/detail/ban_score.rs @@ -477,7 +477,6 @@ impl BanScore for EpochSealError { impl BanScore for IOPolicyError { fn ban_score(&self) -> u32 { match self { - IOPolicyError::PoSAccountingError(err) => err.ban_score(), IOPolicyError::InvalidInputTypeInReward => 100, IOPolicyError::InvalidOutputTypeInReward => 100, IOPolicyError::InvalidInputTypeInTx => 100, @@ -486,9 +485,6 @@ impl BanScore for IOPolicyError { IOPolicyError::ProduceBlockInTx => 100, IOPolicyError::MultipleAccountCommands => 100, IOPolicyError::AttemptToUseAccountInputInReward => 100, - IOPolicyError::AccountBalanceNotFound(_) => 0, - IOPolicyError::NegativeAccountBalance(_) => 100, - IOPolicyError::AccountBalanceOverflow(_) => 100, } } } @@ -507,6 +503,8 @@ impl BanScore for constraints_value_accumulator::Error { constraints_value_accumulator::Error::SpendingNonSpendableOutput(_) => 100, constraints_value_accumulator::Error::AttemptToViolateFeeRequirements => 100, constraints_value_accumulator::Error::DelegationBalanceNotFound(_) => 0, + constraints_value_accumulator::Error::AccountBalanceNotFound(_) => 0, + constraints_value_accumulator::Error::NegativeAccountBalance(_) => 100, } } } diff --git a/chainstate/test-suite/src/tests/delegation_tests.rs b/chainstate/test-suite/src/tests/delegation_tests.rs index 38ad7c94ab..99d68a83e2 100644 --- a/chainstate/test-suite/src/tests/delegation_tests.rs +++ b/chainstate/test-suite/src/tests/delegation_tests.rs @@ -1007,7 +1007,17 @@ fn delegate_and_spend_share_same_tx( ); } AccountsBalancesCheckVersion::V1 => { - res.unwrap(); + assert_eq!( + res.unwrap_err(), + ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( + AccountType::Delegation(delegation_id) + ), + tx_id.into() + ) + )) + ); } } }); @@ -1114,8 +1124,28 @@ fn delegate_and_spend_share_same_tx_no_overspend_per_input( )) .add_output(TxOutput::DelegateStaking(change, delegation_id)) .build(); + let tx_id = tx.transaction().get_id(); - tf.make_block_builder().add_transaction(tx).build_and_process().unwrap(); + let res = tf.make_block_builder().add_transaction(tx).build_and_process(); + + match accumulator_version { + AccountsBalancesCheckVersion::V0 => { + res.unwrap(); + } + AccountsBalancesCheckVersion::V1 => { + assert_eq!( + res.unwrap_err(), + ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( + AccountType::Delegation(delegation_id) + ), + tx_id.into() + ) + )) + ); + } + } }); } @@ -1234,10 +1264,10 @@ fn delegate_and_spend_share_same_block( assert_eq!( res.unwrap_err(), ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( - ConnectTransactionError::IOPolicyError( - IOPolicyError::NegativeAccountBalance(AccountType::Delegation( - delegation_id - )), + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( + AccountType::Delegation(delegation_id) + ), tx1_id.into() ) )) @@ -1363,10 +1393,10 @@ fn try_overspend_delegation( assert_eq!( res.unwrap_err(), ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( - ConnectTransactionError::IOPolicyError( - IOPolicyError::NegativeAccountBalance(AccountType::Delegation( - delegation_id - )), + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( + AccountType::Delegation(delegation_id) + ), tx_id.into() ) )) @@ -1531,10 +1561,10 @@ fn delegate_and_spend_share_same_block_multiple_delegations( assert_eq!( res.unwrap_err(), ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( - ConnectTransactionError::IOPolicyError( - IOPolicyError::NegativeAccountBalance(AccountType::Delegation( - delegation_id_1 - )), + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( + AccountType::Delegation(delegation_id_1) + ), tx_id_1.into() ) )) diff --git a/chainstate/test-suite/src/tests/pos_processing_tests.rs b/chainstate/test-suite/src/tests/pos_processing_tests.rs index 2398cca006..db2126b507 100644 --- a/chainstate/test-suite/src/tests/pos_processing_tests.rs +++ b/chainstate/test-suite/src/tests/pos_processing_tests.rs @@ -1891,8 +1891,8 @@ fn spend_from_delegation_with_reward(#[case] seed: Seed) { assert_eq!( res, ChainstateError::ProcessBlockError(BlockError::StateUpdateFailed( - ConnectTransactionError::IOPolicyError( - chainstate::IOPolicyError::NegativeAccountBalance( + ConnectTransactionError::ConstrainedValueAccumulatorError( + constraints_value_accumulator::Error::NegativeAccountBalance( common::chain::AccountType::Delegation(delegation_id) ), tx_id.into() diff --git a/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/accounts_balances_check.rs b/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/accounts_balances_check.rs deleted file mode 100644 index c012fcc462..0000000000 --- a/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/accounts_balances_check.rs +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) 2022 RBB S.r.l -// opensource@mintlayer.org -// SPDX-License-Identifier: MIT -// Licensed under the MIT License; -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use std::collections::{btree_map::Entry, BTreeMap}; - -use common::{ - chain::{AccountSpending, AccountType, DelegationId, Transaction, TxInput, TxOutput}, - primitives::Amount, -}; -use pos_accounting::PoSAccountingView; - -use super::IOPolicyError; - -#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] -enum GenericAccountId { - Delegation(DelegationId), -} - -impl From for GenericAccountId { - fn from(account: AccountSpending) -> Self { - match account { - AccountSpending::DelegationBalance(id, _) => Self::Delegation(id), - } - } -} - -impl From for AccountType { - fn from(value: GenericAccountId) -> Self { - match value { - GenericAccountId::Delegation(id) => AccountType::Delegation(id), - } - } -} - -pub fn check_accounts_balances_overspend( - tx: &Transaction, - pos_accounting_view: &impl PoSAccountingView, -) -> Result<(), IOPolicyError> { - let mut balances = BTreeMap::::new(); - - // if an outputs top up account's balance it should be accounted calculation - for output in tx.outputs() { - match output { - TxOutput::Transfer(_, _) - | TxOutput::LockThenTransfer(_, _, _) - | TxOutput::Burn(_) - | TxOutput::CreateStakePool(_, _) - | TxOutput::ProduceBlockFromStake(_, _) - | TxOutput::CreateDelegationId(_, _) - | TxOutput::IssueFungibleToken(_) - | TxOutput::IssueNft(_, _, _) - | TxOutput::DataDeposit(_) => { /* skip */ } - TxOutput::DelegateStaking(delegated_amount, delegation_id) => { - let account = GenericAccountId::Delegation(*delegation_id); - match balances.entry(account) { - Entry::Vacant(e) => { - let balance = pos_accounting_view - .get_delegation_balance(*delegation_id) - .map_err(|_| pos_accounting::Error::ViewFail)? - .unwrap_or(Amount::ZERO); - let new_balance = (balance + *delegated_amount) - .ok_or(IOPolicyError::AccountBalanceOverflow(account.into()))?; - e.insert(new_balance); - } - Entry::Occupied(mut e) => { - let balance = e.get_mut(); - *balance = (*balance + *delegated_amount) - .ok_or(IOPolicyError::AccountBalanceOverflow(account.into()))?; - } - }; - } - } - } - - for input in tx.inputs() { - match input { - TxInput::Utxo(_) | TxInput::AccountCommand(_, _) => { /* skip */ } - TxInput::Account(account_outpoint) => match account_outpoint.account() { - AccountSpending::DelegationBalance(delegation_id, spend_amount) => { - let account = account_outpoint.account().clone().into(); - match balances.entry(account) { - Entry::Vacant(e) => { - let balance = pos_accounting_view - .get_delegation_balance(*delegation_id) - .map_err(|_| pos_accounting::Error::ViewFail)? - .ok_or(IOPolicyError::AccountBalanceNotFound(account.into()))?; - let new_balance = (balance - *spend_amount) - .ok_or(IOPolicyError::NegativeAccountBalance(account.into()))?; - e.insert(new_balance); - } - Entry::Occupied(mut e) => { - let balance = e.get_mut(); - *balance = (*balance - *spend_amount) - .ok_or(IOPolicyError::NegativeAccountBalance(account.into()))?; - } - }; - } - }, - } - } - - Ok(()) -} diff --git a/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/mod.rs b/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/mod.rs index c6958c41be..1dcd97159f 100644 --- a/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/mod.rs +++ b/chainstate/tx-verifier/src/transaction_verifier/input_output_policy/mod.rs @@ -19,8 +19,8 @@ use common::{ output_value::OutputValue, signature::Signable, tokens::{get_tokens_issuance_count, TokenId}, - AccountType, AccountsBalancesCheckVersion, Block, ChainConfig, DelegationId, PoolId, - TokenIssuanceVersion, Transaction, TxInput, TxOutput, + Block, ChainConfig, DelegationId, PoolId, TokenIssuanceVersion, Transaction, TxInput, + TxOutput, }, primitives::{Amount, BlockHeight, Fee, Id, Idable, Subsidy}, }; @@ -33,13 +33,10 @@ use crate::error::{SpendStakeError, TokensError}; use super::error::ConnectTransactionError; -mod accounts_balances_check; mod purposes_check; #[derive(Error, Debug, PartialEq, Eq, Clone)] pub enum IOPolicyError { - #[error("PoS accounting error: `{0}`")] - PoSAccountingError(#[from] pos_accounting::Error), #[error("Attempted to use a invalid input type in block reward")] InvalidInputTypeInReward, #[error("Attempted to use a invalid output type in block reward")] @@ -56,12 +53,6 @@ pub enum IOPolicyError { MultipleAccountCommands, #[error("Attempt to use account input in block reward")] AttemptToUseAccountInputInReward, - #[error("Account balance not found for `{0:?}`")] - AccountBalanceNotFound(AccountType), - #[error("Negative account balance for `{0:?}`")] - NegativeAccountBalance(AccountType), - #[error("Account balance overflow `{0:?}`")] - AccountBalanceOverflow(AccountType), } pub fn calculate_tokens_burned_in_outputs( @@ -204,20 +195,6 @@ pub fn check_tx_inputs_outputs_policy( .map_err(|_| pos_accounting::Error::ViewFail)?) }; - // check if account inputs/outputs do not overspend account balances - match chain_config - .chainstate_upgrades() - .version_at_height(block_height) - .1 - .accounts_balances_version() - { - AccountsBalancesCheckVersion::V0 => { /* skip */ } - AccountsBalancesCheckVersion::V1 => { - accounts_balances_check::check_accounts_balances_overspend(tx, pos_accounting_view) - .map_err(|e| ConnectTransactionError::IOPolicyError(e, tx.get_id().into()))?; - } - } - let inputs_accumulator = ConstrainedValueAccumulator::from_inputs( chain_config, block_height, diff --git a/common/src/chain/config/builder.rs b/common/src/chain/config/builder.rs index 10c097c8a7..94745892a6 100644 --- a/common/src/chain/config/builder.rs +++ b/common/src/chain/config/builder.rs @@ -46,7 +46,6 @@ const TESTNET_TOKEN_FORK_HEIGHT: BlockHeight = BlockHeight::new(78440); // The fork, at which we upgrade chainstate to distribute reward to staker proportionally to its balance, // changed various tokens fees and also increased max ticker length for tokens const TESTNET_STAKER_REWARD_AND_TOKENS_FEE_FORK_HEIGHT: BlockHeight = BlockHeight::new(138244); -const CONSTRAINTS_ACCUMULATOR_FORK_HEIGHT: BlockHeight = BlockHeight::new(99999999999999); impl ChainType { fn default_genesis_init(&self) -> GenesisBlockInit { @@ -153,30 +152,17 @@ impl ChainType { fn default_chainstate_upgrades(&self) -> NetUpgrades { match self { ChainType::Mainnet => { - let upgrades = vec![ - ( - BlockHeight::new(0), - ChainstateUpgrade::new( - TokenIssuanceVersion::V1, - RewardDistributionVersion::V1, - TokensFeeVersion::V1, - TokensTickerMaxLengthVersion::V1, - NftIdMismatchCheck::Yes, - AccountsBalancesCheckVersion::V0, - ), - ), - ( - CONSTRAINTS_ACCUMULATOR_FORK_HEIGHT, - ChainstateUpgrade::new( - TokenIssuanceVersion::V1, - RewardDistributionVersion::V1, - TokensFeeVersion::V1, - TokensTickerMaxLengthVersion::V1, - NftIdMismatchCheck::Yes, - AccountsBalancesCheckVersion::V1, - ), + let upgrades = vec![( + BlockHeight::new(0), + ChainstateUpgrade::new( + TokenIssuanceVersion::V1, + RewardDistributionVersion::V1, + TokensFeeVersion::V1, + TokensTickerMaxLengthVersion::V1, + NftIdMismatchCheck::Yes, + AccountsBalancesCheckVersion::V0, ), - ]; + )]; NetUpgrades::initialize(upgrades).expect("net upgrades") } ChainType::Regtest | ChainType::Signet => { @@ -225,7 +211,7 @@ impl ChainType { TokensFeeVersion::V1, TokensTickerMaxLengthVersion::V1, NftIdMismatchCheck::Yes, - AccountsBalancesCheckVersion::V1, + AccountsBalancesCheckVersion::V0, ), ), ]; diff --git a/mempool/src/error/ban_score.rs b/mempool/src/error/ban_score.rs index 2420af9af9..abf836837f 100644 --- a/mempool/src/error/ban_score.rs +++ b/mempool/src/error/ban_score.rs @@ -344,7 +344,6 @@ impl MempoolBanScore for accounting::Error { impl MempoolBanScore for IOPolicyError { fn mempool_ban_score(&self) -> u32 { match self { - IOPolicyError::PoSAccountingError(err) => err.mempool_ban_score(), IOPolicyError::InvalidInputTypeInReward => 100, IOPolicyError::InvalidOutputTypeInReward => 100, IOPolicyError::InvalidInputTypeInTx => 100, @@ -353,9 +352,6 @@ impl MempoolBanScore for IOPolicyError { IOPolicyError::MultipleAccountCommands => 100, IOPolicyError::ProduceBlockInTx => 100, IOPolicyError::AttemptToUseAccountInputInReward => 100, - IOPolicyError::AccountBalanceOverflow(_) => 100, - IOPolicyError::AccountBalanceNotFound(_) => 0, - IOPolicyError::NegativeAccountBalance(_) => 0, } } }