From 000d939a2c8fcb1d0fa229acb2ca3c21dfa037e3 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Wed, 3 Aug 2022 12:36:25 +0100 Subject: [PATCH 1/3] clear pending coinbase transactions now rely on utxo hashes --- base_layer/core/src/transactions/coinbase_builder.rs | 2 +- .../wallet/src/output_manager_service/service.rs | 4 ++-- .../output_manager_service/storage/database/backend.rs | 4 ++-- .../src/output_manager_service/storage/database/mod.rs | 6 +++--- .../output_manager_service/storage/sqlite_db/mod.rs | 8 ++++---- .../storage/sqlite_db/output_sql.rs | 10 ++++++++++ 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index 6363831374..ecdce58e26 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -203,7 +203,7 @@ impl CoinbaseBuilder { let sig = Signature::sign(spending_key.clone(), nonce, &challenge) .map_err(|_| CoinbaseBuildError::BuildError("Challenge could not be represented as a scalar".into()))?; - let sender_offset_private_key = PrivateKey::random(&mut OsRng); + let sender_offset_private_key = PrivateKey::from_bytes(Blake256::digest(spending_key)); // H(spending_key) <- Blake256 let sender_offset_public_key = PublicKey::from_secret_key(&sender_offset_private_key); let covenant = self.covenant; diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index cc16d39468..c50c7da0aa 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -1012,12 +1012,12 @@ where match self .resources .db - .clear_pending_coinbase_transaction_at_block_height(block_height) + .clear_pending_coinbase_transaction_with_hash(output.hash.as_slice()) { Ok(_) => { debug!( target: LOG_TARGET, - "An existing pending coinbase was cleared for block height {}", block_height + "An existing pending coinbase was cleared with hash {}", output.hash.to_hex() ) }, Err(e) => match e { diff --git a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs index 4527e1b561..e3a7cad923 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs @@ -97,9 +97,9 @@ pub trait OutputManagerBackend: Send + Sync + Clone { /// Get the output that was most recently spent, ordered descending by mined height fn get_last_spent_output(&self) -> Result, OutputManagerStorageError>; /// Check if there is a pending coinbase transaction at this block height, if there is clear it. - fn clear_pending_coinbase_transaction_at_block_height( + fn clear_pending_coinbase_transaction_with_hash( &self, - block_height: u64, + hash: &[u8], ) -> Result<(), OutputManagerStorageError>; /// Set if a coinbase output is abandoned or not fn set_coinbase_abandoned(&self, tx_id: TxId, abandoned: bool) -> Result<(), OutputManagerStorageError>; diff --git a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs index 91c15e1bbb..d5af1bcbae 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs @@ -220,11 +220,11 @@ where T: OutputManagerBackend + 'static } /// Check if there is a pending coinbase transaction at this block height, if there is clear it. - pub fn clear_pending_coinbase_transaction_at_block_height( + pub fn clear_pending_coinbase_transaction_with_hash( &self, - block_height: u64, + hash: &[u8], ) -> Result<(), OutputManagerStorageError> { - self.db.clear_pending_coinbase_transaction_at_block_height(block_height) + self.db.clear_pending_coinbase_transaction_with_hash(hash) } pub fn fetch_all_unspent_outputs(&self) -> Result, OutputManagerStorageError> { diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs index 09fdafe15d..f2d0a21e03 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs @@ -1074,21 +1074,21 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn clear_pending_coinbase_transaction_at_block_height( + fn clear_pending_coinbase_transaction_with_hash( &self, - block_height: u64, + hash: &[u8], ) -> Result<(), OutputManagerStorageError> { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let output = OutputSql::find_pending_coinbase_at_block_height(block_height, &conn)?; + let output = OutputSql::find_pending_coinbase_with_hash(hash, &conn)?; output.delete(&conn)?; if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, - "sqlite profile - clear_pending_coinbase_transaction_at_block_height: lock {} + db_op {} = {} ms", + "sqlite profile - clear_pending_coinbase_transaction_with_hash: lock {} + db_op {} = {} ms", acquire_lock.as_millis(), (start.elapsed() - acquire_lock).as_millis(), start.elapsed().as_millis() diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs index 63480ee86c..804681ab59 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs @@ -576,6 +576,16 @@ impl OutputSql { .first::(conn)?) } + pub fn find_pending_coinbase_with_hash( + hash: &[u8], + conn: &SqliteConnection, + ) -> Result { + Ok(outputs::table + .filter(outputs::status.ne(OutputStatus::Unspent as i32)) + .filter(outputs::hash.eq(Some(hash))) + .first::(conn)?) + } + /// Find a particular Output, if it exists and is in the specified Spent state pub fn find_pending_coinbase_at_block_height( block_height: u64, From aa225a474a69c0cf0ad02f203c1de853063742b2 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 12:15:51 +0100 Subject: [PATCH 2/3] sync with dev --- base_layer/core/src/transactions/coinbase_builder.rs | 2 +- .../wallet/src/output_manager_service/service.rs | 4 ++-- .../output_manager_service/storage/database/backend.rs | 4 ++-- .../src/output_manager_service/storage/database/mod.rs | 6 +++--- .../output_manager_service/storage/sqlite_db/mod.rs | 8 ++++---- .../storage/sqlite_db/output_sql.rs | 10 ---------- 6 files changed, 12 insertions(+), 22 deletions(-) diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index 5b984352cf..0b55b78e6d 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -205,7 +205,7 @@ impl CoinbaseBuilder { let sig = Signature::sign(spending_key.clone(), nonce, &challenge) .map_err(|_| CoinbaseBuildError::BuildError("Challenge could not be represented as a scalar".into()))?; - let sender_offset_private_key = PrivateKey::from_bytes(Blake256::digest(spending_key)); // H(spending_key) <- Blake256 + let sender_offset_private_key = PrivateKey::random(&mut OsRng); let sender_offset_public_key = PublicKey::from_secret_key(&sender_offset_private_key); let covenant = self.covenant; diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 9e8395c57a..97bdfd64a3 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -1014,12 +1014,12 @@ where match self .resources .db - .clear_pending_coinbase_transaction_with_hash(output.hash.as_slice()) + .clear_pending_coinbase_transaction_at_block_height(block_height) { Ok(_) => { debug!( target: LOG_TARGET, - "An existing pending coinbase was cleared with hash {}", output.hash.to_hex() + "An existing pending coinbase was cleared for block height {}", block_height ) }, Err(e) => match e { diff --git a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs index e3a7cad923..4527e1b561 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs @@ -97,9 +97,9 @@ pub trait OutputManagerBackend: Send + Sync + Clone { /// Get the output that was most recently spent, ordered descending by mined height fn get_last_spent_output(&self) -> Result, OutputManagerStorageError>; /// Check if there is a pending coinbase transaction at this block height, if there is clear it. - fn clear_pending_coinbase_transaction_with_hash( + fn clear_pending_coinbase_transaction_at_block_height( &self, - hash: &[u8], + block_height: u64, ) -> Result<(), OutputManagerStorageError>; /// Set if a coinbase output is abandoned or not fn set_coinbase_abandoned(&self, tx_id: TxId, abandoned: bool) -> Result<(), OutputManagerStorageError>; diff --git a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs index d5af1bcbae..91c15e1bbb 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs @@ -220,11 +220,11 @@ where T: OutputManagerBackend + 'static } /// Check if there is a pending coinbase transaction at this block height, if there is clear it. - pub fn clear_pending_coinbase_transaction_with_hash( + pub fn clear_pending_coinbase_transaction_at_block_height( &self, - hash: &[u8], + block_height: u64, ) -> Result<(), OutputManagerStorageError> { - self.db.clear_pending_coinbase_transaction_with_hash(hash) + self.db.clear_pending_coinbase_transaction_at_block_height(block_height) } pub fn fetch_all_unspent_outputs(&self) -> Result, OutputManagerStorageError> { diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs index 9fab53d2d1..73af04cd9c 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs @@ -1074,21 +1074,21 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn clear_pending_coinbase_transaction_with_hash( + fn clear_pending_coinbase_transaction_at_block_height( &self, - hash: &[u8], + block_height: u64, ) -> Result<(), OutputManagerStorageError> { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let output = OutputSql::find_pending_coinbase_with_hash(hash, &conn)?; + let output = OutputSql::find_pending_coinbase_at_block_height(block_height, &conn)?; output.delete(&conn)?; if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, - "sqlite profile - clear_pending_coinbase_transaction_with_hash: lock {} + db_op {} = {} ms", + "sqlite profile - clear_pending_coinbase_transaction_at_block_height: lock {} + db_op {} = {} ms", acquire_lock.as_millis(), (start.elapsed() - acquire_lock).as_millis(), start.elapsed().as_millis() diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs index cfedab1288..8e6cbdd476 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs @@ -576,16 +576,6 @@ impl OutputSql { .first::(conn)?) } - pub fn find_pending_coinbase_with_hash( - hash: &[u8], - conn: &SqliteConnection, - ) -> Result { - Ok(outputs::table - .filter(outputs::status.ne(OutputStatus::Unspent as i32)) - .filter(outputs::hash.eq(Some(hash))) - .first::(conn)?) - } - /// Find a particular Output, if it exists and is in the specified Spent state pub fn find_pending_coinbase_at_block_height( block_height: u64, From 8d9aa740adbd7fdddf68ccb12559bcc294cb29c5 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Tue, 30 Aug 2022 16:26:19 +0100 Subject: [PATCH 3/3] resolve tests --- .../wallet/tests/output_manager_service_tests/service.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/base_layer/wallet/tests/output_manager_service_tests/service.rs b/base_layer/wallet/tests/output_manager_service_tests/service.rs index efab1a6d0f..43b64159bd 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/service.rs @@ -390,7 +390,6 @@ async fn fee_estimate() { assert!(matches!(err, OutputManagerError::NotEnoughFunds)); } -#[ignore] #[allow(clippy::identity_op)] #[tokio::test] async fn test_utxo_selection_no_chain_metadata() { @@ -492,7 +491,7 @@ async fn test_utxo_selection_no_chain_metadata() { let (_, tx, utxos_total_value) = oms.create_coin_split(vec![], amount, 5, fee_per_gram).await.unwrap(); let expected_fee = fee_calc.calculate(fee_per_gram, 1, 1, 6, default_metadata_byte_size() * 6); assert_eq!(tx.body.get_total_fee(), expected_fee); - assert_eq!(utxos_total_value, MicroTari::from(10_000)); + assert_eq!(utxos_total_value, MicroTari::from(5_000)); // test that largest utxo was encumbered let utxos = oms.get_unspent_outputs().await.unwrap(); @@ -507,7 +506,6 @@ async fn test_utxo_selection_no_chain_metadata() { #[tokio::test] #[allow(clippy::identity_op)] #[allow(clippy::too_many_lines)] -#[ignore] async fn test_utxo_selection_with_chain_metadata() { let factories = CryptoFactories::default(); let (connection, _tempdir) = get_temp_sqlite_database_connection(); @@ -576,7 +574,7 @@ async fn test_utxo_selection_with_chain_metadata() { // test coin split is maturity aware let (_, tx, utxos_total_value) = oms.create_coin_split(vec![], amount, 5, fee_per_gram).await.unwrap(); - assert_eq!(utxos_total_value, MicroTari::from(6_000)); + assert_eq!(utxos_total_value, MicroTari::from(5_000)); let expected_fee = fee_calc.calculate(fee_per_gram, 1, 1, 6, default_metadata_byte_size() * 6); assert_eq!(tx.body.get_total_fee(), expected_fee); @@ -1113,7 +1111,6 @@ async fn sending_transaction_persisted_while_offline() { } #[tokio::test] -#[ignore] async fn coin_split_with_change() { let factories = CryptoFactories::default(); let (connection, _tempdir) = get_temp_sqlite_database_connection();