From 5c690674544220e2512f74ca13b233246b7375da Mon Sep 17 00:00:00 2001 From: David Edey Date: Fri, 26 Jul 2024 21:50:43 +0100 Subject: [PATCH] tweak: Improve formatting of the test_stake_reconciliation failure --- .../tests/application/stake_reconciliation.rs | 56 +++-- .../src/transaction/transaction_receipt.rs | 192 ++++++++++-------- 2 files changed, 146 insertions(+), 102 deletions(-) diff --git a/radix-engine-tests/tests/application/stake_reconciliation.rs b/radix-engine-tests/tests/application/stake_reconciliation.rs index 2c186f9a845..c329ab3c774 100644 --- a/radix-engine-tests/tests/application/stake_reconciliation.rs +++ b/radix-engine-tests/tests/application/stake_reconciliation.rs @@ -2,6 +2,7 @@ use radix_common::prelude::*; use radix_engine::updates::*; use radix_substate_store_interface::db_key_mapper::{DatabaseKeyMapper, SpreadPrefixKeyMapper}; use scrypto_test::prelude::*; +use core::fmt::Write; #[test] fn test_stake_reconciliation() { @@ -29,8 +30,7 @@ fn test_stake_reconciliation() { receipt.expect_commit_success(); // Store current DB substate value hashes for comparision after staking execution - let mut pre_transaction_substates: HashMap<(DbPartitionKey, DbSortKey), Vec> = - HashMap::new(); + let mut pre_transaction_substates: IndexMap<(DbPartitionKey, DbSortKey), Vec> = IndexMap::new(); let db = ledger.substate_db(); let old_keys: Vec = db.list_partition_keys().collect(); for key in old_keys { @@ -210,20 +210,49 @@ fn test_stake_reconciliation() { }; let post_transaction_partitions: Vec<_> = post_transaction_partitions.collect(); + let encoder = AddressBech32Encoder::for_simulator(); + let receipt_display_context = TransactionReceiptDisplayContextBuilder::new() + .encoder(&encoder) + .display_state_updates(true) + .use_ansi_colors(false) + .schema_lookup_from_db(ledger.substate_db()) + .build(); + + let mut error_message = String::new(); for (full_key, (expected_old_value, _)) in expected_updated_substates.iter() { let database_value = &pre_transaction_substates[full_key]; + let (node_id, partition) = SpreadPrefixKeyMapper::from_db_partition_key(&full_key.0); + // Luckily they're all fields + let substate_key = SpreadPrefixKeyMapper::from_db_sort_key::(&full_key.1); let address = AddressBech32Encoder::for_simulator() - .encode( - &SpreadPrefixKeyMapper::from_db_partition_key(&full_key.0) - .0 - .0, - ) + .encode(&node_id.0) .unwrap(); - assert_eq!( - database_value, expected_old_value, - "The pre-transaction value of updated substate under {} is not expected: {:?}", - address, full_key - ); + if database_value != expected_old_value { + write!(&mut error_message, "\nThe pre-transaction value of updated substate under {address} {partition:?} has changed.").unwrap(); + write!(&mut error_message, "\n\nEXPECTED:").unwrap(); + display_substate_change( + &mut error_message, + "", + &commit_result.system_structure, + &receipt_display_context, + &node_id, + &partition, + &substate_key, + SubstateChange::Upsert(expected_old_value) + ).unwrap(); + write!(&mut error_message, "\nACTUAL:").unwrap(); + display_substate_change( + &mut error_message, + "", + &commit_result.system_structure, + &receipt_display_context, + &node_id, + &partition, + &substate_key, + SubstateChange::Upsert(database_value) + ).unwrap(); + write!(&mut error_message, "\n").unwrap(); + } // For printing: // let (db_partition_key, db_sort_key) = full_key; // println!( @@ -246,6 +275,9 @@ fn test_stake_reconciliation() { // hex::encode(new_value) // ); } + if error_message.len() > 0 { + panic!("{}", error_message); + } for key in post_transaction_partitions { let partition_entries = ledger.substate_db().list_entries(&key); diff --git a/radix-engine/src/transaction/transaction_receipt.rs b/radix-engine/src/transaction/transaction_receipt.rs index 55328a8c628..ca7a70f66b7 100644 --- a/radix-engine/src/transaction/transaction_receipt.rs +++ b/radix-engine/src/transaction/transaction_receipt.rs @@ -1030,95 +1030,7 @@ impl<'a> ContextualDisplay> for Transaction } if context.display_state_updates { - // The state updates are bizarrely full of lots of partitions with empty updates, - // so we remove them to avoid polluting the output. - let state_updates = c.state_updates.rebuild_without_empty_entries(); - context.format_top_level_title_with_detail( - f, - "State Updates", - format!( - "{} {}", - state_updates.by_node.len(), - if state_updates.by_node.len() == 1 { - "entity" - } else { - "entities" - }, - ), - )?; - for (i, (node_id, node_updates)) in state_updates.by_node.iter().enumerate() { - let by_partition = match node_updates { - NodeStateUpdates::Delta { by_partition } => by_partition, - }; - write!( - f, - "\n{} {} across {} partitions", - prefix!(i, c.state_updates.by_node), - node_id.display(address_display_context), - by_partition.len(), - )?; - - for (j, (partition_number, partition_updates)) in - by_partition.iter().enumerate() - { - // NOTE: This could be improved by mapping partition numbers back to a system-focused name - // This would require either adding partition descriptions into SystemStructure, or - // having some inverse entity-type specific descriptors. - match partition_updates { - PartitionStateUpdates::Delta { by_substate } => { - write!( - f, - "\n {} Partition({}): {} {}", - prefix!(j, by_partition), - partition_number.0, - by_substate.len(), - if by_substate.len() == 1 { - "change" - } else { - "changes" - }, - )?; - for (k, (substate_key, update)) in by_substate.iter().enumerate() { - display_substate_change( - f, - prefix!(k, by_substate), - &c.system_structure, - context, - node_id, - partition_number, - substate_key, - update.as_change(), - )?; - } - } - PartitionStateUpdates::Batch(BatchPartitionStateUpdate::Reset { - new_substate_values, - }) => { - write!( - f, - "\n {} Partition({}): RESET ({} new values)", - prefix!(j, by_partition), - partition_number.0, - new_substate_values.len() - )?; - for (k, (substate_key, value)) in - new_substate_values.iter().enumerate() - { - display_substate_change( - f, - prefix!(k, new_substate_values), - &c.system_structure, - context, - node_id, - partition_number, - substate_key, - SubstateChange::Upsert(value), - )?; - } - } - } - } - } + (&c.state_updates, &c.system_structure).contextual_format(f, context)?; } if let TransactionOutcome::Success(outputs) = &c.outcome { @@ -1209,7 +1121,107 @@ impl<'a> ContextualDisplay> for Transaction } } -fn display_substate_change<'a, F: fmt::Write>( +impl<'a, 'b> ContextualDisplay> + for (&'b StateUpdates, &'b SystemStructure) +{ + type Error = fmt::Error; + + fn contextual_format( + &self, + f: &mut F, + context: &TransactionReceiptDisplayContext<'a>, + ) -> Result<(), Self::Error> { + // The state updates are bizarrely full of lots of partitions with empty updates, + // so we remove them to avoid polluting the output. + let state_updates = self.0.rebuild_without_empty_entries(); + let system_structure = self.1; + context.format_top_level_title_with_detail( + f, + "State Updates", + format!( + "{} {}", + state_updates.by_node.len(), + if state_updates.by_node.len() == 1 { + "entity" + } else { + "entities" + }, + ), + )?; + for (i, (node_id, node_updates)) in state_updates.by_node.iter().enumerate() { + let by_partition = match node_updates { + NodeStateUpdates::Delta { by_partition } => by_partition, + }; + write!( + f, + "\n{} {} across {} partitions", + prefix!(i, state_updates.by_node), + node_id.display(context.address_display_context()), + by_partition.len(), + )?; + + for (j, (partition_number, partition_updates)) in by_partition.iter().enumerate() { + // NOTE: This could be improved by mapping partition numbers back to a system-focused name + // This would require either adding partition descriptions into SystemStructure, or + // having some inverse entity-type specific descriptors. + match partition_updates { + PartitionStateUpdates::Delta { by_substate } => { + write!( + f, + "\n {} Partition({}): {} {}", + prefix!(j, by_partition), + partition_number.0, + by_substate.len(), + if by_substate.len() == 1 { + "change" + } else { + "changes" + }, + )?; + for (k, (substate_key, update)) in by_substate.iter().enumerate() { + display_substate_change( + f, + prefix!(k, by_substate), + system_structure, + context, + node_id, + partition_number, + substate_key, + update.as_change(), + )?; + } + } + PartitionStateUpdates::Batch(BatchPartitionStateUpdate::Reset { + new_substate_values, + }) => { + write!( + f, + "\n {} Partition({}): RESET ({} new values)", + prefix!(j, by_partition), + partition_number.0, + new_substate_values.len() + )?; + for (k, (substate_key, value)) in new_substate_values.iter().enumerate() { + display_substate_change( + f, + prefix!(k, new_substate_values), + system_structure, + context, + node_id, + partition_number, + substate_key, + SubstateChange::Upsert(value), + )?; + } + } + } + } + } + Ok(()) + } +} + +pub fn display_substate_change<'a, F: fmt::Write>( f: &mut F, prefix: &str, system_structure: &SystemStructure,