Skip to content

Commit

Permalink
Expose error details when block production fails
Browse files Browse the repository at this point in the history
  • Loading branch information
styppo committed Aug 30, 2024
1 parent 7b12ce6 commit dc399ca
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 40 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions blockchain-interface/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ serde = "1.0"
thiserror = "1.0"
tokio-stream = { version = "0.1", features = ["sync"] }

nimiq-account = { workspace = true, features = [] }
nimiq-block = { workspace = true }
nimiq-collections = { workspace = true }
nimiq-database-value = { workspace = true }
Expand Down
5 changes: 3 additions & 2 deletions blockchain-interface/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use nimiq_account::AccountsError;
use nimiq_block::{Block, BlockError, EquivocationProofError, ForkProof};
use nimiq_hash::Blake2bHash;
use nimiq_primitives::{account::AccountError, networks::NetworkId};
Expand Down Expand Up @@ -74,8 +75,8 @@ pub enum PushError {
DuplicateTransaction,
#[error("Equivocation proof: {0}")]
InvalidEquivocationProof(#[from] EquivocationProofError),
#[error("Account error: {0}")]
AccountsError(#[from] AccountError),
#[error("Accounts error: {0}")]
AccountsError(#[from] AccountsError),
#[error("Invalid fork")]
InvalidFork,
#[error("Blockchain error: {0}")]
Expand Down
53 changes: 43 additions & 10 deletions blockchain/src/block_production/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use nimiq_account::BlockState;
use nimiq_account::{Account, AccountsError, BlockState};
use nimiq_block::{
EquivocationProof, MacroBlock, MacroBody, MacroHeader, MicroBlock, MicroBody, MicroHeader,
MicroJustification, SkipBlockInfo, SkipBlockProof,
Expand All @@ -8,7 +8,7 @@ use nimiq_bls::KeyPair as BlsKeyPair;
use nimiq_database::{mdbx::MdbxReadTransaction as DBTransaction, traits::WriteTransaction};
use nimiq_hash::{Blake2bHash, Blake2sHash, Hash};
use nimiq_keys::KeyPair as SchnorrKeyPair;
use nimiq_primitives::{account::AccountError, policy::Policy};
use nimiq_primitives::policy::Policy;
use nimiq_transaction::{
historic_transaction::HistoricTransaction, inherent::Inherent, Transaction,
};
Expand All @@ -19,14 +19,37 @@ use crate::{interface::HistoryInterface, Blockchain};

#[derive(Debug, Error)]
pub enum BlockProducerError {
#[error("Failed to commit accounts: {0}")]
AccountError(#[from] AccountError),
#[error("Failed to commit: error={0}, account={1:?}, transactions={2:?}, inherents={3:?}")]
AccountsError(AccountsError, Account, Vec<Transaction>, Vec<Inherent>),
#[error("Failed to add to history")]
HistoryError,
#[error("Accounts are incomplete")]
AccountsIncomplete,
}

impl BlockProducerError {
fn accounts_error(
blockchain: &Blockchain,
error: AccountsError,
transactions: Vec<Transaction>,
inherents: Vec<Inherent>,
) -> Self {
let Some(account) = (match &error {
AccountsError::InvalidTransaction(_, transaction) => {
blockchain.get_account_if_complete(&transaction.sender)
}
AccountsError::InvalidInherent(_, inherent) => {
blockchain.get_account_if_complete(inherent.target())
}
AccountsError::InvalidDiff(_) => unreachable!(),
}) else {
return BlockProducerError::AccountsIncomplete;
};

BlockProducerError::AccountsError(error, account, transactions, inherents)
}
}

/// Struct that contains all necessary information to actually produce blocks.
/// It has the validator keys for this validator.
#[derive(Clone)]
Expand Down Expand Up @@ -140,7 +163,15 @@ impl BlockProducer {
let (state_root, diff_root, executed_txns) = blockchain
.state
.accounts
.exercise_transactions(&transactions, &inherents, &block_state)?;
.exercise_transactions(&transactions, &inherents, &block_state)
.map_err(|error| {
BlockProducerError::accounts_error(
blockchain,
error,
transactions,
inherents.clone(),
)
})?;

// Calculate the historic transactions from the transactions and the inherents.
let hist_txs = HistoricTransaction::from(
Expand Down Expand Up @@ -331,11 +362,13 @@ impl BlockProducer {

// Update the state and add the state root to the header.
let block_state = BlockState::new(block_number, timestamp);
let (state_root, diff_root, _) =
blockchain
.state
.accounts
.exercise_transactions(&[], &inherents, &block_state)?;
let (state_root, diff_root, _) = blockchain
.state
.accounts
.exercise_transactions(&[], &inherents, &block_state)
.map_err(|error| {
BlockProducerError::accounts_error(blockchain, error, vec![], inherents.clone())
})?;

macro_block.header.state_root = state_root;
macro_block.header.diff_root = diff_root;
Expand Down
54 changes: 30 additions & 24 deletions primitives/account/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ use nimiq_trie::{trie::MerkleRadixTrie, WriteTransactionProxy};

use crate::{
Account, AccountInherentInteraction, AccountPruningInteraction, AccountReceipt,
AccountTransactionInteraction, BlockLogger, BlockState, DataStore, InherentLogger,
InherentOperationReceipt, OperationReceipt, Receipts, ReservedBalance, RevertInfo,
TransactionLog, TransactionOperationReceipt, TransactionReceipt,
AccountTransactionInteraction, AccountsError, BlockLogger, BlockState, DataStore,
InherentLogger, InherentOperationReceipt, OperationReceipt, Receipts, ReservedBalance,
RevertInfo, TransactionLog, TransactionOperationReceipt, TransactionReceipt,
};

declare_table!(AccountsTrieTable, "AccountsTrie", KeyNibbles => TrieNode);
Expand Down Expand Up @@ -269,7 +269,7 @@ impl Accounts {
transactions: &[Transaction],
inherents: &[Inherent],
block_state: &BlockState,
) -> Result<(Blake2bHash, Blake2bHash, Vec<ExecutedTransaction>), AccountError> {
) -> Result<(Blake2bHash, Blake2bHash, Vec<ExecutedTransaction>), AccountsError> {
let mut raw_txn = self.env.write_transaction();
let mut txn: WriteTransactionProxy = (&mut raw_txn).into();
assert!(self.is_complete(Some(&txn)), "Tree must be complete");
Expand Down Expand Up @@ -310,7 +310,7 @@ impl Accounts {
inherents: &[Inherent],
block_state: &BlockState,
block_logger: &mut BlockLogger,
) -> Result<Receipts, AccountError> {
) -> Result<Receipts, AccountsError> {
let receipts =
self.commit_batch(txn, transactions, inherents, block_state, block_logger)?;
self.tree.update_root(txn).expect("Tree must be complete");
Expand All @@ -321,7 +321,7 @@ impl Accounts {
&self,
txn: &mut WriteTransactionProxy,
diff: TrieDiff,
) -> Result<RevertTrieDiff, AccountError> {
) -> Result<RevertTrieDiff, AccountsError> {
let diff = self.tree.apply_diff(txn, diff)?;
self.tree.update_root(txn).ok();
Ok(diff)
Expand All @@ -334,27 +334,31 @@ impl Accounts {
inherents: &[Inherent],
block_state: &BlockState,
block_logger: &mut BlockLogger,
) -> Result<Receipts, AccountError> {
) -> Result<Receipts, AccountsError> {
assert!(self.is_complete(Some(txn)), "Tree must be complete");
let mut receipts = Receipts::default();

for transaction in transactions {
let receipt = self.commit_transaction(
txn,
transaction,
block_state,
block_logger.new_tx_log(transaction.hash()),
)?;
let receipt = self
.commit_transaction(
txn,
transaction,
block_state,
block_logger.new_tx_log(transaction.hash()),
)
.map_err(|error| AccountsError::InvalidTransaction(error, transaction.clone()))?;
receipts.transactions.push(receipt);
}

for inherent in inherents {
let receipt = self.commit_inherent(
txn,
inherent,
block_state,
&mut block_logger.inherent_logger(),
)?;
let receipt = self
.commit_inherent(
txn,
inherent,
block_state,
&mut block_logger.inherent_logger(),
)
.map_err(|error| AccountsError::InvalidInherent(error, inherent.clone()))?;
receipts.inherents.push(receipt);
}

Expand Down Expand Up @@ -560,7 +564,7 @@ impl Accounts {
block_state: &BlockState,
revert_info: RevertInfo,
block_logger: &mut BlockLogger,
) -> Result<(), AccountError> {
) -> Result<(), AccountsError> {
match revert_info {
RevertInfo::Receipts(receipts) => {
self.revert_batch(
Expand All @@ -584,7 +588,7 @@ impl Accounts {
&self,
txn: &mut WriteTransactionProxy,
diff: RevertTrieDiff,
) -> Result<(), AccountError> {
) -> Result<(), AccountsError> {
self.tree.revert_diff(txn, diff)?;
Ok(())
}
Expand All @@ -597,7 +601,7 @@ impl Accounts {
block_state: &BlockState,
receipts: Receipts,
block_logger: &mut BlockLogger,
) -> Result<(), AccountError> {
) -> Result<(), AccountsError> {
// Revert inherents in reverse order.
assert_eq!(inherents.len(), receipts.inherents.len());
let iter = inherents.iter().zip(receipts.inherents).rev();
Expand All @@ -608,7 +612,8 @@ impl Accounts {
block_state,
receipt,
&mut block_logger.inherent_logger(),
)?;
)
.map_err(|error| AccountsError::InvalidInherent(error, inherent.clone()))?;
}

// Revert transactions in reverse order.
Expand All @@ -621,7 +626,8 @@ impl Accounts {
block_state,
receipt,
block_logger.new_tx_log(transaction.hash()),
)?;
)
.map_err(|error| AccountsError::InvalidTransaction(error, transaction.clone()))?;
}

Ok(())
Expand Down
14 changes: 14 additions & 0 deletions primitives/account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
#[macro_use]
extern crate log;

use nimiq_primitives::{account::AccountError, trie::error::MerkleRadixTrieError};
use nimiq_transaction::{inherent::Inherent, Transaction};
use thiserror::Error;

#[cfg(feature = "accounts")]
pub use crate::accounts::{Accounts, AccountsTrie};
#[cfg(feature = "interaction-traits")]
Expand Down Expand Up @@ -30,3 +34,13 @@ mod interaction_traits;
mod logs;
mod receipts;
mod reserved_balance;

#[derive(Debug, Error, PartialEq, Eq)]
pub enum AccountsError {
#[error("Invalid transaction: error={0}, transaction={1:?}")]
InvalidTransaction(AccountError, Transaction),
#[error("Invalid inherent: error={0}, inherent={1:?}")]
InvalidInherent(AccountError, Inherent),
#[error("Invalid diff: {0}")]
InvalidDiff(#[from] MerkleRadixTrieError),
}
1 change: 0 additions & 1 deletion primitives/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ pub enum AccountType {
Vesting = 1,
HTLC = 2,
Staking = 3,
// HistoricTransaction = 0xff,
}

impl fmt::Display for AccountType {
Expand Down
7 changes: 4 additions & 3 deletions test-utils/src/accounts_revert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use std::{fmt::Debug, ops::Deref};

use nimiq_account::{
Account, AccountInherentInteraction, AccountReceipt, AccountTransactionInteraction, Accounts,
BlockLog, BlockLogger, BlockState, DataStore, InherentLogger, Receipts, TransactionLog,
AccountsError, BlockLog, BlockLogger, BlockState, DataStore, InherentLogger, Receipts,
TransactionLog,
};
use nimiq_database::{
mdbx::MdbxDatabase,
Expand Down Expand Up @@ -132,7 +133,7 @@ impl TestCommitRevert {
inherents: &[Inherent],
block_state: &BlockState,
block_logger: &mut BlockLogger,
) -> Result<Receipts, AccountError> {
) -> Result<Receipts, AccountsError> {
self.commit_revert(transactions, inherents, block_state, block_logger, true)
}

Expand Down Expand Up @@ -161,7 +162,7 @@ impl TestCommitRevert {
block_state: &BlockState,
block_logger: &mut BlockLogger,
keep_commit: bool,
) -> Result<Receipts, AccountError> {
) -> Result<Receipts, AccountsError> {
let mut raw_txn = self.accounts.env.write_transaction();
let mut txn: WriteTransactionProxy = (&mut raw_txn).into();

Expand Down

0 comments on commit dc399ca

Please sign in to comment.