feat!: completed transaction use bytes for transaction protocol (not …

…hex string) in wallet database (#5906)

Description
---
Changed the transaction protocol in the wallet database to use bytes
instead of a hex string, as the underlying data type is encrypted bytes.
This issue was highlighted due to the `to_hex` function in
`tari_utilities` not being able to convert large transactions into hex
strings (returned `**String to large**`) for saving in the wallet
database during system-level coin-split stress testing.

Motivation and Context
---
See above.

How Has This Been Tested?
---
Existing unit tests and cucumber tests passed
Added two new unit tests and a cucumber test (long-running)
System-level test creating a coin-split transaction with 499 outputs
passed

What process can a PR reviewer use to test or verify this change?
---
Code walk-through
Run the new unit tests
Perform a system-level test creating a coin-split transaction with 499
outputs and wait for it to be mined and confirmed,

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [ ] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [X] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
BREAKING CHANGE: All completed transactions in the wallet database will
be deleted at startup, while subsequent output validation will
re-allocate outputs that were encumbered to be spent as available and
invalidate outputs that were encumbered to be received.

base_layer/core/src/mempool/service/inbound_handlers.rs

@@ -93,10 +93,15 @@ impl MempoolInboundHandlers {
         tx: Transaction,
         source_peer: Option<NodeId>,
     ) -> Result<(), MempoolServiceError> {
+        let first_tx_kernel_excess_sig = tx
+            .first_kernel_excess_sig()
+            .ok_or(MempoolServiceError::TransactionNoKernels)?
+            .get_signature()
+            .to_hex();
         debug!(
             target: LOG_TARGET,
             "Transaction ({}) received from {}.",
-            tx.body.kernels()[0].excess_sig.get_signature().to_hex(),
+            first_tx_kernel_excess_sig,
             source_peer
                 .as_ref()
                 .map(|p| format!("remote peer: {}", p))

base_layer/core/src/transactions/transaction_components/transaction_output.rs

@@ -293,7 +293,7 @@ impl TransactionOutput {
         let e = PrivateKey::from_uniform_bytes(&e_bytes).unwrap();
         let value_as_private_key = PrivateKey::from(self.minimum_value_promise.as_u64());
         let commit_nonce_a = PrivateKey::default(); // This is the deterministic nonce `r_a` of zero
-        if self.metadata_signature.u_a().to_hex() == (commit_nonce_a + e * value_as_private_key).to_hex() {
+        if self.metadata_signature.u_a() == &(commit_nonce_a + e * value_as_private_key) {
             Ok(())
         } else {
             Err(RangeProofError::InvalidRangeProof {

base_layer/wallet/migrations/2023-11-03-161500_transaction_protocol/down.sql

@@ -0,0 +1 @@
+-- This file should undo anything in `up.sql`

base_layer/wallet/migrations/2023-11-03-161500_transaction_protocol/up.sql

@@ -0,0 +1,27 @@
+-- Any old 'completed_transactions' will not be valid due to the change in 'transaction_protocol' to
+-- 'BLOB', so we drop and recreate the table.
+
+DROP TABLE completed_transactions;
+CREATE TABLE completed_transactions
+(
+    tx_id                       BIGINT PRIMARY KEY NOT NULL,
+    source_address              BLOB               NOT NULL,
+    destination_address         BLOB               NOT NULL,
+    amount                      BIGINT             NOT NULL,
+    fee                         BIGINT             NOT NULL,
+    transaction_protocol        BLOB               NOT NULL,
+    status                      INTEGER            NOT NULL,
+    message                     TEXT               NOT NULL,
+    timestamp                   DATETIME           NOT NULL,
+    cancelled                   INTEGER            NULL,
+    direction                   INTEGER            NULL,
+    coinbase_block_height       BIGINT             NULL,
+    send_count                  INTEGER DEFAULT 0  NOT NULL,
+    last_send_timestamp         DATETIME           NULL,
+    confirmations               BIGINT             NULL,
+    mined_height                BIGINT             NULL,
+    mined_in_block              BLOB               NULL,
+    mined_timestamp             DATETIME           NULL,
+    transaction_signature_nonce BLOB    DEFAULT 0  NOT NULL,
+    transaction_signature_key   BLOB    DEFAULT 0  NOT NULL
+);

base_layer/wallet/src/schema.rs

@@ -1,6 +1,4 @@
-// @generated automatically by Diesel CLI.
-
-diesel::table! {
+table! {
     burnt_proofs (id) {
         id -> Integer,
         reciprocal_claim_public_key -> Text,
@@ -9,21 +7,21 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     client_key_values (key) {
         key -> Text,
         value -> Text,
     }
 }
 
-diesel::table! {
+table! {
     completed_transactions (tx_id) {
         tx_id -> BigInt,
         source_address -> Binary,
         destination_address -> Binary,
         amount -> BigInt,
         fee -> BigInt,
-        transaction_protocol -> Text,
+        transaction_protocol -> Binary,
         status -> Integer,
         message -> Text,
         timestamp -> Timestamp,
@@ -41,7 +39,7 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     inbound_transactions (tx_id) {
         tx_id -> BigInt,
         source_address -> Binary,
@@ -56,7 +54,7 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     known_one_sided_payment_scripts (script_hash) {
         script_hash -> Binary,
         private_key -> Text,
@@ -66,7 +64,7 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     outbound_transactions (tx_id) {
         tx_id -> BigInt,
         destination_address -> Binary,
@@ -82,7 +80,7 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     outputs (id) {
         id -> Integer,
         commitment -> Binary,
@@ -122,7 +120,7 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     scanned_blocks (header_hash) {
         header_hash -> Binary,
         height -> BigInt,
@@ -132,14 +130,14 @@ diesel::table! {
     }
 }
 
-diesel::table! {
+table! {
     wallet_settings (key) {
         key -> Text,
         value -> Text,
     }
 }
 
-diesel::allow_tables_to_appear_in_same_query!(
+allow_tables_to_appear_in_same_query!(
     burnt_proofs,
     client_key_values,
     completed_transactions,

base_layer/wallet/src/transaction_service/error.rs

@@ -229,6 +229,8 @@ pub enum TransactionStorageError {
     ValueNotFound(DbKey),
     #[error("Unexpected result: `{0}`")]
     UnexpectedResult(String),
+    #[error("Bincode error: `{0}`")]
+    BincodeSerialize(String),
     #[error("This write operation is not supported for provided DbKey")]
     OperationNotSupported,
     #[error("Could not find all values specified for batch operation")]

base_layer/wallet/src/transaction_service/storage/sqlite_db.rs

@@ -1705,7 +1705,7 @@ pub struct CompletedTransactionSql {
     destination_address: Vec<u8>,
     amount: i64,
     fee: i64,
-    transaction_protocol: String,
+    transaction_protocol: Vec<u8>,
     status: i32,
     message: String,
     timestamp: NaiveDateTime,
@@ -2015,13 +2015,16 @@ impl CompletedTransactionSql {
     }
 
     fn try_from(c: CompletedTransaction, cipher: &XChaCha20Poly1305) -> Result<Self, TransactionStorageError> {
+        let transaction_bytes =
+            bincode::serialize(&c.transaction).map_err(|e| TransactionStorageError::BincodeSerialize(e.to_string()))?;
+
         let output = Self {
             tx_id: c.tx_id.as_u64() as i64,
             source_address: c.source_address.to_bytes().to_vec(),
             destination_address: c.destination_address.to_bytes().to_vec(),
             amount: u64::from(c.amount) as i64,
             fee: u64::from(c.fee) as i64,
-            transaction_protocol: serde_json::to_string(&c.transaction)?,
+            transaction_protocol: transaction_bytes.to_vec(),
             status: c.status as i32,
             message: c.message,
             timestamp: c.timestamp,
@@ -2057,26 +2060,15 @@ impl Encryptable<XChaCha20Poly1305> for CompletedTransactionSql {
         self.transaction_protocol = encrypt_bytes_integral_nonce(
             cipher,
             self.domain("transaction_protocol"),
-            Hidden::hide(self.transaction_protocol.as_bytes().to_vec()),
-        )?
-        .to_hex();
+            Hidden::hide(self.transaction_protocol),
+        )?;
 
         Ok(self)
     }
 
     fn decrypt(mut self, cipher: &XChaCha20Poly1305) -> Result<Self, String> {
-        let mut decrypted_protocol = decrypt_bytes_integral_nonce(
-            cipher,
-            self.domain("transaction_protocol"),
-            &from_hex(self.transaction_protocol.as_str()).map_err(|e| e.to_string())?,
-        )?;
-
-        self.transaction_protocol = from_utf8(decrypted_protocol.as_slice())
-            .map_err(|e| e.to_string())?
-            .to_string();
-
-        // zeroize sensitive data
-        decrypted_protocol.zeroize();
+        self.transaction_protocol =
+            decrypt_bytes_integral_nonce(cipher, self.domain("transaction_protocol"), &self.transaction_protocol)?;
 
         Ok(self)
     }
@@ -2094,6 +2086,8 @@ pub enum CompletedTransactionConversionError {
     KeyError(#[from] TransactionKeyError),
     #[error("Aead Error: {0}")]
     AeadError(String),
+    #[error("Bincode error: `{0}`")]
+    BincodeDeserialize(String),
 }
 
 impl CompletedTransaction {
@@ -2126,7 +2120,8 @@ impl CompletedTransaction {
                 .map_err(TransactionKeyError::Destination)?,
             amount: MicroMinotari::from(c.amount as u64),
             fee: MicroMinotari::from(c.fee as u64),
-            transaction: serde_json::from_str(&c.transaction_protocol.clone())?,
+            transaction: bincode::deserialize(&c.transaction_protocol)
+                .map_err(|e| CompletedTransactionConversionError::BincodeDeserialize(e.to_string()))?,
             status: TransactionStatus::try_from(c.status)?,
             message: c.message,
             timestamp: c.timestamp,
@@ -2158,7 +2153,7 @@ pub struct UpdateCompletedTransactionSql {
     timestamp: Option<NaiveDateTime>,
     cancelled: Option<Option<i32>>,
     direction: Option<i32>,
-    transaction_protocol: Option<String>,
+    transaction_protocol: Option<Vec<u8>>,
     send_count: Option<i32>,
     last_send_timestamp: Option<Option<NaiveDateTime>>,
     confirmations: Option<Option<i64>>,

base_layer/wallet/tests/output_manager_service_tests/service.rs

@@ -62,7 +62,7 @@ use tari_core::{
     transactions::{
         fee::Fee,
         key_manager::{TransactionKeyManagerBranch, TransactionKeyManagerInterface},
-        tari_amount::{uT, MicroMinotari},
+        tari_amount::{uT, MicroMinotari, T},
         test_helpers::{
             create_test_core_key_manager_with_memory_db,
             create_wallet_output_with_data,
@@ -1235,6 +1235,28 @@ async fn coin_split_no_change() {
     assert_eq!(amount, val1 + val2 + val3);
 }
 
+#[tokio::test]
+async fn it_handles_large_coin_splits() {
+    let (connection, _tempdir) = get_temp_sqlite_database_connection();
+    let backend = OutputManagerSqliteDatabase::new(connection.clone());
+    let mut oms = setup_output_manager_service(backend, true).await;
+
+    let val = 20 * T;
+    let uo = make_input(&mut OsRng, val, &OutputFeatures::default(), &oms.key_manager_handle).await;
+    assert!(oms.output_manager_handle.add_output(uo, None).await.is_ok());
+
+    let fee_per_gram = MicroMinotari::from(1);
+    let split_count = 499;
+
+    let (_tx_id, coin_split_tx, _amount) = oms
+        .output_manager_handle
+        .create_coin_split(vec![], 10000.into(), split_count, fee_per_gram)
+        .await
+        .unwrap();
+    assert_eq!(coin_split_tx.body.inputs().len(), 1);
+    assert_eq!(coin_split_tx.body.outputs().len(), split_count + 1);
+}
+
 #[tokio::test]
 async fn handle_coinbase_with_bulletproofs_rewinding() {
     let (connection, _tempdir) = get_temp_sqlite_database_connection();

base_layer/wallet/tests/transaction_service_tests/service.rs

@@ -741,6 +741,87 @@ async fn single_transaction_to_self() {
     );
 }
 
+#[tokio::test]
+async fn large_coin_split_transaction() {
+    let network = Network::LocalNet;
+    let consensus_manager = ConsensusManager::builder(network).build().unwrap();
+    let factories = CryptoFactories::default();
+    // Alice's parameters
+    let alice_node_identity = Arc::new(NodeIdentity::random(
+        &mut OsRng,
+        get_next_memory_address(),
+        PeerFeatures::COMMUNICATION_NODE,
+    ));
+
+    let base_node_identity = Arc::new(NodeIdentity::random(
+        &mut OsRng,
+        get_next_memory_address(),
+        PeerFeatures::COMMUNICATION_NODE,
+    ));
+
+    log::info!(
+        "large_coin_split_transaction: Alice: '{}', Base: '{}'",
+        alice_node_identity.node_id().short_str(),
+        base_node_identity.node_id().short_str()
+    );
+
+    let temp_dir = tempdir().unwrap();
+    let database_path = temp_dir.path().to_str().unwrap().to_string();
+
+    let (db_connection, _tempdir) = make_wallet_database_connection(Some(database_path.clone()));
+
+    let shutdown = Shutdown::new();
+    let (mut alice_ts, mut alice_oms, _alice_comms, _alice_connectivity, key_manager_handle) =
+        setup_transaction_service(
+            alice_node_identity.clone(),
+            vec![],
+            consensus_manager,
+            factories.clone(),
+            db_connection,
+            database_path,
+            Duration::from_secs(0),
+            shutdown.to_signal(),
+        )
+        .await;
+
+    let initial_wallet_value = 20 * T;
+    let uo1 = make_input(
+        &mut OsRng,
+        initial_wallet_value,
+        &OutputFeatures::default(),
+        &key_manager_handle,
+    )
+    .await;
+
+    alice_oms.add_output(uo1, None).await.unwrap();
+
+    let fee_per_gram = MicroMinotari::from(1);
+    let split_count = 499;
+    let (tx_id, coin_split_tx, amount) = alice_oms
+        .create_coin_split(vec![], 10000.into(), split_count, fee_per_gram)
+        .await
+        .unwrap();
+    assert_eq!(coin_split_tx.body.inputs().len(), 1);
+    assert_eq!(coin_split_tx.body.outputs().len(), split_count + 1);
+
+    alice_ts
+        .submit_transaction(tx_id, coin_split_tx, amount, "large coin-split".to_string())
+        .await
+        .expect("Alice sending coin-split tx");
+
+    let completed_tx = alice_ts
+        .get_completed_transaction(tx_id)
+        .await
+        .expect("Could not find tx");
+
+    let fees = completed_tx.fee;
+
+    assert_eq!(
+        alice_oms.get_balance().await.unwrap().pending_incoming_balance,
+        initial_wallet_value - fees
+    );
+}
+
 #[tokio::test]
 async fn single_transaction_burn_tari() {
     // let _ = env_logger::builder().is_test(true).try_init(); // Need `$env:RUST_LOG = "trace"` for this to work

integration_tests/tests/features/WalletCli.feature

@@ -105,6 +105,21 @@ Feature: Wallet CLI
         Then wallet WALLET has at least 1 transactions that are all TRANSACTION_STATUS_MINED_CONFIRMED and not cancelled
         Then I get count of utxos of wallet WALLET and it's at least 10 via command line
 
+    @long-running
+    Scenario: As a user I want to large coin-split via command line
+        Given I have a seed node SEED
+        When I have a base node BASE connected to seed SEED
+        When I have wallet WALLET connected to base node BASE
+        When I have mining node MINE connected to base node BASE and wallet WALLET
+        When mining node MINE mines 4 blocks
+        Then I wait for wallet WALLET to have at least 1100000 uT
+        When I wait 30 seconds
+        When I do coin split on wallet WALLET to 10000 uT 499 coins via command line
+        Then wallet WALLET has at least 1 transactions that are all TRANSACTION_STATUS_BROADCAST and not cancelled
+        When mining node MINE mines 5 blocks
+        Then wallet WALLET has at least 1 transactions that are all TRANSACTION_STATUS_MINED_CONFIRMED and not cancelled
+        Then I get count of utxos of wallet WALLET and it's at least 499 via command line
+
     Scenario: As a user I want to count utxos via command line
         Given I have a base node BASE
         When I have wallet WALLET connected to base node BASE