From 000d939a2c8fcb1d0fa229acb2ca3c21dfa037e3 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Wed, 3 Aug 2022 12:36:25 +0100 Subject: [PATCH 01/17] 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 814773d3df8ebb2d1fdb22e2744bb18acf45ecd7 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Wed, 3 Aug 2022 15:42:40 +0100 Subject: [PATCH 02/17] add changes --- base_layer/core/src/transactions/coinbase_builder.rs | 10 +++++++++- base_layer/core/src/transactions/types.rs | 4 ++++ .../wallet/src/output_manager_service/service.rs | 3 ++- .../output_manager_service/storage/database/backend.rs | 5 +---- .../src/output_manager_service/storage/database/mod.rs | 5 +---- .../output_manager_service/storage/sqlite_db/mod.rs | 5 +---- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index ecdce58e26..5c803f7ad3 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -25,6 +25,9 @@ use rand::rngs::OsRng; use tari_common_types::types::{BlindingFactor, PrivateKey, PublicKey, Signature}; use tari_crypto::{ commitment::HomomorphicCommitmentFactory, + hash::blake2::Blake256, + hash_domain, + hashing::DomainSeparatedHasher, keys::{PublicKey as PK, SecretKey}, }; use tari_script::{inputs, script, TariScript}; @@ -51,6 +54,7 @@ use crate::{ UnblindedOutput, }, transaction_protocol::{build_challenge, RewindData, TransactionMetadata}, + types::WalletServiceHashingDomain, }, }; @@ -203,7 +207,11 @@ 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 hasher = + DomainSeparatedHasher::::new_with_label("sender_offset_private_key"); + let spending_key_hash_bytes = hasher.chain(spending_key).finalize().as_ref(); + + let sender_offset_private_key = PrivateKey::from_bytes(hspending_key_hash_bytes); let sender_offset_public_key = PublicKey::from_secret_key(&sender_offset_private_key); let covenant = self.covenant; diff --git a/base_layer/core/src/transactions/types.rs b/base_layer/core/src/transactions/types.rs index d051788bca..1ae2ff9d67 100644 --- a/base_layer/core/src/transactions/types.rs +++ b/base_layer/core/src/transactions/types.rs @@ -19,3 +19,7 @@ // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use tari_crypto::hash_domain; + +hash_domain!(WalletServiceHashingDomain, "base_layer.wallet.service"); diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index c50c7da0aa..54520d3f68 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -1017,7 +1017,8 @@ where Ok(_) => { debug!( target: LOG_TARGET, - "An existing pending coinbase was cleared with hash {}", output.hash.to_hex() + "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 e3a7cad923..2159162b58 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,10 +97,7 @@ 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( - &self, - hash: &[u8], - ) -> Result<(), OutputManagerStorageError>; + fn clear_pending_coinbase_transaction_with_hash(&self, 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>; /// Reinstate a cancelled inbound output 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..ff4037b5a8 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,10 +220,7 @@ 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( - &self, - hash: &[u8], - ) -> Result<(), OutputManagerStorageError> { + pub fn clear_pending_coinbase_transaction_with_hash(&self, hash: &[u8]) -> Result<(), OutputManagerStorageError> { self.db.clear_pending_coinbase_transaction_with_hash(hash) } 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 f2d0a21e03..aca0b4efe0 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,10 +1074,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn clear_pending_coinbase_transaction_with_hash( - &self, - hash: &[u8], - ) -> Result<(), OutputManagerStorageError> { + fn clear_pending_coinbase_transaction_with_hash(&self, hash: &[u8]) -> Result<(), OutputManagerStorageError> { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); From 4c31cb5cbbdcea04d9dec3388a5a3119e6943946 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 4 Aug 2022 07:57:13 +0100 Subject: [PATCH 03/17] refactor test --- .../core/src/transactions/coinbase_builder.rs | 13 ++++++++----- .../wallet/src/output_manager_service/service.rs | 4 ++-- .../tests/output_manager_service_tests/service.rs | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index 5c803f7ad3..3df278927e 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -21,16 +21,15 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // -use rand::rngs::OsRng; use tari_common_types::types::{BlindingFactor, PrivateKey, PublicKey, Signature}; use tari_crypto::{ commitment::HomomorphicCommitmentFactory, hash::blake2::Blake256, - hash_domain, hashing::DomainSeparatedHasher, - keys::{PublicKey as PK, SecretKey}, + keys::{PublicKey as PK}, }; use tari_script::{inputs, script, TariScript}; +use tari_utilities::ByteArray; use thiserror::Error; use crate::{ @@ -76,6 +75,8 @@ pub enum CoinbaseBuildError { BuildError(String), #[error("Some inconsistent data was given to the builder. This transaction is not valid")] InvalidTransaction, + #[error("Unable to produce a spender offset key from spend key hash")] + InvalidSenderOffsetKey, } pub struct CoinbaseBuilder { @@ -209,9 +210,11 @@ impl CoinbaseBuilder { let hasher = DomainSeparatedHasher::::new_with_label("sender_offset_private_key"); - let spending_key_hash_bytes = hasher.chain(spending_key).finalize().as_ref(); + let spending_key_hash = hasher.chain(spending_key.as_bytes()).finalize(); + let spending_key_hash_bytes = spending_key_hash.as_ref(); - let sender_offset_private_key = PrivateKey::from_bytes(hspending_key_hash_bytes); + let sender_offset_private_key = + PrivateKey::from_bytes(spending_key_hash_bytes).map_err(|_| CoinbaseBuildError::InvalidSenderOffsetKey)?; 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 54520d3f68..d66e694c01 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -960,7 +960,7 @@ where } /// Request a Coinbase transaction for a specific block height. All existing pending transactions with - /// this blockheight will be cancelled. + /// the corresponding output hash will be cancelled. /// The key will be derived from the coinbase specific keychain using the blockheight as an index. The coinbase /// keychain is based on the wallets master_key and the "coinbase" branch. async fn get_coinbase_transaction( @@ -1008,7 +1008,7 @@ where None, )?; - // Clear any existing pending coinbase transactions for this blockheight if they exist + // Clear any existing pending coinbase transactions having this output hash, if they exist match self .resources .db 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 963b5a12c6..d975e80a3e 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/service.rs @@ -1324,7 +1324,7 @@ async fn handle_coinbase_with_bulletproofs_rewinding() { .await .unwrap() .pending_incoming_balance, - value2 + value1 + value2 ); let tx3 = oms .output_manager_handle @@ -1338,7 +1338,7 @@ async fn handle_coinbase_with_bulletproofs_rewinding() { .await .unwrap() .pending_incoming_balance, - value2 + value3 + value1 + value2 + value3 ); let output = tx3.body.outputs()[0].clone(); From 8cc7b0c1656128056631705cbc3cb878d1c96e5d Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 4 Aug 2022 08:18:12 +0100 Subject: [PATCH 04/17] run cargo fmt --- base_layer/core/src/transactions/coinbase_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index 3df278927e..2369dbf3a9 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -26,7 +26,7 @@ use tari_crypto::{ commitment::HomomorphicCommitmentFactory, hash::blake2::Blake256, hashing::DomainSeparatedHasher, - keys::{PublicKey as PK}, + keys::PublicKey as PK, }; use tari_script::{inputs, script, TariScript}; use tari_utilities::ByteArray; From 5a847254780549ed5dee29d743dc59a55e76f681 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 4 Aug 2022 08:51:11 +0100 Subject: [PATCH 05/17] add duplicate transaction to assert that coinbase values are replaced --- .../output_manager_service_tests/service.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 d975e80a3e..8336f90a9d 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/service.rs @@ -1312,6 +1312,24 @@ async fn handle_coinbase_with_bulletproofs_rewinding() { .pending_incoming_balance, value1 ); + + // duplicate the same transaction, creates the same utxo, does previous coinbase should be removed + // and we should get value1 for the balance + let _tx1 = oms + .output_manager_handle + .get_coinbase_transaction(1u64.into(), reward1, fees1, 1) + .await + .unwrap(); + assert_eq!(oms.output_manager_handle.get_unspent_outputs().await.unwrap().len(), 0); + assert_eq!( + oms.output_manager_handle + .get_balance() + .await + .unwrap() + .pending_incoming_balance, + value1 + ); + let _tx2 = oms .output_manager_handle .get_coinbase_transaction(2u64.into(), reward2, fees2, 1) From b140b9b07631cebc03d10d396fd70961a1f39c59 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 4 Aug 2022 10:49:53 +0100 Subject: [PATCH 06/17] add cucumber tests for wallet having two connected miners --- .../features/WalletTransactions.feature | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/integration_tests/features/WalletTransactions.feature b/integration_tests/features/WalletTransactions.feature index b2feea999e..b54968ba91 100644 --- a/integration_tests/features/WalletTransactions.feature +++ b/integration_tests/features/WalletTransactions.feature @@ -83,6 +83,18 @@ Feature: Wallet Transactions Then I wait for wallet WALLET_C to have at least 1000000 uT Then I check if last imported transactions are valid in wallet WALLET_C + Scenario: Wallet has two connected miners, coin bases are computed correctly + Given I have a seed node NODE + And I have 1 base nodes connected to all seed nodes + And I have wallet WALLET_A connected to all seed nodes + And I have mining node MINER connected to base node NODE and wallet WALLET_A + When mining node MINER mines 2 blocks + When mining node MINER2 mines 2 blocks + When mining node MINER mines 3 blocks + When mining node MINER2 mines 3 blocks + Then all nodes are at height 10 + Then I wait for wallet WALLET_A to have at least 20000000000 uT + @flaky Scenario: Wallet imports spent outputs that become invalidated Given I have a seed node NODE From 7b215c84bfeef182c2330ecad03d6555f7bdd550 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 4 Aug 2022 10:52:50 +0100 Subject: [PATCH 07/17] resolve multi addr --- comms/core/src/utils/multiaddr.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/comms/core/src/utils/multiaddr.rs b/comms/core/src/utils/multiaddr.rs index bc1b3669db..76d8547a0a 100644 --- a/comms/core/src/utils/multiaddr.rs +++ b/comms/core/src/utils/multiaddr.rs @@ -89,15 +89,16 @@ mod test { #[test] fn multiaddr_to_socketaddr_ok() { - fn expect_success(addr: &str, expected_ip: &str) { + fn expect_success(addr: &str, expected_ips: &[&str]) { let addr = Multiaddr::from_str(addr).unwrap(); let sock_addr = super::multiaddr_to_socketaddr(&addr).unwrap(); - assert_eq!(sock_addr.ip().to_string(), expected_ip); + assert!(expected_ips.iter().any(|ip| *ip == sock_addr.ip().to_string())); } - expect_success("/ip4/254.0.1.2/tcp/1234", "254.0.1.2"); - expect_success("/ip6/::1/tcp/1234", "::1"); - expect_success("/dns4/taridns.dyn-ip.me/tcp/1234", "127.0.0.1"); + expect_success("/ip4/254.0.1.2/tcp/1234", &["254.0.1.2"]); + expect_success("/ip6/::1/tcp/1234", &["::1"]); + // Test DNS name resolution + expect_success("/dns4/localhost/tcp/1234", &["127.0.0.1", "::1"]); } #[test] From 41c35d42881bb1a809167c03f2e28cb2c392e2ef Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Sun, 7 Aug 2022 11:42:27 +0100 Subject: [PATCH 08/17] refactor some tests --- .../transaction_service_tests/service.rs | 47 +++++++++++++++---- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/base_layer/wallet/tests/transaction_service_tests/service.rs b/base_layer/wallet/tests/transaction_service_tests/service.rs index 74518321c0..02a7a55505 100644 --- a/base_layer/wallet/tests/transaction_service_tests/service.rs +++ b/base_layer/wallet/tests/transaction_service_tests/service.rs @@ -3059,7 +3059,7 @@ async fn test_restarting_transaction_protocols() { } #[tokio::test] -async fn test_coinbase_transactions_rejection_same_height() { +async fn test_coinbase_transactions_rejection_same_hash_but_accept_on_same_height() { let factories = CryptoFactories::default(); let (connection, _temp_dir) = make_wallet_database_connection(None); @@ -3105,7 +3105,35 @@ async fn test_coinbase_transactions_rejection_same_height() { fees1 + reward1 ); - // Create another coinbase Txn at the same block height; the previous one will be cancelled + // Create a second coinbase txn at the first block height, with same output hash as the previous one + // the previous one should be cancelled + let _tx1b = alice_ts_interface + .transaction_service_handle + .generate_coinbase_transaction(reward1, fees1, block_height_a) + .await + .unwrap(); + let transactions = alice_ts_interface + .transaction_service_handle + .get_completed_transactions() + .await + .unwrap(); + assert_eq!(transactions.len(), 1); + let _tx_id1b = transactions + .values() + .find(|tx| tx.amount == fees1 + reward1) + .unwrap() + .tx_id; + assert_eq!( + alice_ts_interface + .output_manager_service_handle + .get_balance() + .await + .unwrap() + .pending_incoming_balance, + fees1 + reward1 + ); + + // Create another coinbase Txn at the same block height; the previous one should not be cancelled let _tx2 = alice_ts_interface .transaction_service_handle .generate_coinbase_transaction(reward2, fees2, block_height_a) @@ -3129,10 +3157,10 @@ async fn test_coinbase_transactions_rejection_same_height() { .await .unwrap() .pending_incoming_balance, - fees2 + reward2 + fees1 + reward1 + fees2 + reward2 ); - // Create a third coinbase Txn at the second block height; only the last two will be valid + // Create a third coinbase Txn at the second block height; all the three should be valid let _tx3 = alice_ts_interface .transaction_service_handle .generate_coinbase_transaction(reward3, fees3, block_height_b) @@ -3156,7 +3184,7 @@ async fn test_coinbase_transactions_rejection_same_height() { .await .unwrap() .pending_incoming_balance, - fees2 + reward2 + fees3 + reward3 + fees1 + reward1 + fees2 + reward2 + fees3 + reward3 ); assert!(!transactions.values().any(|tx| tx.amount == fees1 + reward1)); @@ -3241,7 +3269,7 @@ async fn test_coinbase_generation_and_monitoring() { fees1 + reward1 + fees2 + reward2 ); - // Take out a second one at the second height which should overwrite the initial one + // Take out a second one at the second height which should not overwrite the initial one let _tx2b = alice_ts_interface .transaction_service_handle .generate_coinbase_transaction(reward2, fees2b, block_height_b) @@ -3265,7 +3293,7 @@ async fn test_coinbase_generation_and_monitoring() { .await .unwrap() .pending_incoming_balance, - fees1 + reward1 + fees2b + reward2 + fees1 + reward1 + fees2b + reward2 + fees2 + reward2 ); assert!(transactions.values().any(|tx| tx.amount == fees1 + reward1)); @@ -3914,6 +3942,7 @@ async fn test_coinbase_transaction_reused_for_same_height() { for tx in transactions.values() { assert_eq!(tx.amount, fees1 + reward1); } + // balance should be fees1 + reward1, not double assert_eq!( ts_interface .output_manager_service_handle @@ -3948,7 +3977,7 @@ async fn test_coinbase_transaction_reused_for_same_height() { .await .unwrap() .pending_incoming_balance, - fees2 + reward2 + fees1 + reward1 + fees2 + reward2 ); // a requested coinbase transaction for a new height should be different @@ -3975,7 +4004,7 @@ async fn test_coinbase_transaction_reused_for_same_height() { .await .unwrap() .pending_incoming_balance, - 2 * (fees2 + reward2) + fees1 + reward1 + 2 * (fees2 + reward2) ); } From 50cfb15decfdd2c05fd488fa304e4e1a47d072a7 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Sun, 7 Aug 2022 12:17:57 +0100 Subject: [PATCH 09/17] add chagnes --- integration_tests/features/WalletTransactions.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_tests/features/WalletTransactions.feature b/integration_tests/features/WalletTransactions.feature index b54968ba91..72dada6b01 100644 --- a/integration_tests/features/WalletTransactions.feature +++ b/integration_tests/features/WalletTransactions.feature @@ -83,7 +83,7 @@ Feature: Wallet Transactions Then I wait for wallet WALLET_C to have at least 1000000 uT Then I check if last imported transactions are valid in wallet WALLET_C - Scenario: Wallet has two connected miners, coin bases are computed correctly + Scenario: Wallet has two connected miners, coinbase's are computed correctly Given I have a seed node NODE And I have 1 base nodes connected to all seed nodes And I have wallet WALLET_A connected to all seed nodes From aa225a474a69c0cf0ad02f203c1de853063742b2 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 12:15:51 +0100 Subject: [PATCH 10/17] 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 6bb39cbbb66756ddfd3b95764f7761e1221e6749 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 12:32:30 +0100 Subject: [PATCH 11/17] refactor --- base_layer/wallet/src/output_manager_service/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 13773e4123..0f764397fb 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -1014,7 +1014,7 @@ 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!( From 776a6ff4e0d2c48f936737d209b1858a2e1cd827 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 12:43:47 +0100 Subject: [PATCH 12/17] refactoring --- .../storage/sqlite_db/output_sql.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 8e6cbdd476..a8770fa799 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,17 @@ impl OutputSql { .first::(conn)?) } + // Find a particular Output, if it exists and is in the specified Spent state + 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 46e45cf8586f15c4351c6c9343b827a8eb4b4ea4 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 15:06:30 +0100 Subject: [PATCH 13/17] remove clear pending coinbase transactions --- .../src/output_manager_service/service.rs | 41 +++++-------------- .../storage/database/backend.rs | 2 - .../storage/database/mod.rs | 5 --- .../storage/sqlite_db/mod.rs | 21 ---------- .../storage/sqlite_db/output_sql.rs | 11 ----- .../features/WalletTransactions.feature | 1 + 6 files changed, 12 insertions(+), 69 deletions(-) diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 0f764397fb..fc8229b7a0 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -93,6 +93,7 @@ use crate::{ storage::{ database::{OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, models::{DbUnblindedOutput, KnownOneSidedPaymentScript, SpendingPriority}, + sqlite_db::*, OutputStatus, }, tasks::TxoValidationTask, @@ -1010,38 +1011,18 @@ where None, )?; - // Clear any existing pending coinbase transactions having this output hash, if they exist - match self - .resources - .db - .clear_pending_coinbase_transaction_with_hash(output.hash.as_slice()) - { - Ok(_) => { - debug!( - target: LOG_TARGET, - "An existing pending coinbase was cleared with hash {}", - output.hash.to_hex() - ) - }, - Err(e) => match e { - OutputManagerStorageError::DieselError(DieselError::NotFound) => {}, - _ => return Err(OutputManagerError::from(e)), - }, - }; - - // Clear any matching outputs for this commitment. Even if the older output is valid - // we are losing no information as this output has the same commitment. - match self.resources.db.remove_output_by_commitment(output.commitment.clone()) { + // If there is no existing output available, we store the one we produced. + match self.resources.db.find_by_commitment(output.commitment.clone()) { Ok(_) => {}, - Err(OutputManagerStorageError::ValueNotFound) => {}, - Err(e) => return Err(e.into()), - } + Err(OutputManagerStorageError::ValueNotFound) => { + self.resources + .db + .add_output_to_be_received(tx_id, output, Some(block_height))?; - self.resources - .db - .add_output_to_be_received(tx_id, output, Some(block_height))?; - - self.confirm_encumberance(tx_id)?; + self.confirm_encumberance(tx_id)?; + }, + Err(e) => return Err(e.into()), + }; Ok(tx) } 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 2159162b58..e0576b57f8 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 @@ -96,8 +96,6 @@ pub trait OutputManagerBackend: Send + Sync + Clone { fn get_last_mined_output(&self) -> Result, OutputManagerStorageError>; /// 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(&self, 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>; /// Reinstate a cancelled inbound output 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 ff4037b5a8..7712093173 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 @@ -219,11 +219,6 @@ where T: OutputManagerBackend + 'static self.db.cancel_pending_transaction(tx_id) } - /// 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(&self, hash: &[u8]) -> Result<(), OutputManagerStorageError> { - self.db.clear_pending_coinbase_transaction_with_hash(hash) - } - pub fn fetch_all_unspent_outputs(&self) -> Result, OutputManagerStorageError> { let result = match self.db.fetch(&DbKey::UnspentOutputs)? { Some(DbValue::UnspentOutputs(outputs)) => outputs, 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 4d155d00a4..cf8f6df4cf 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,27 +1074,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn clear_pending_coinbase_transaction_with_hash(&self, 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_with_hash(hash, &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", - acquire_lock.as_millis(), - (start.elapsed() - acquire_lock).as_millis(), - start.elapsed().as_millis() - ); - } - - Ok(()) - } - fn set_coinbase_abandoned(&self, tx_id: TxId, abandoned: bool) -> Result<(), OutputManagerStorageError> { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; 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 a8770fa799..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,17 +576,6 @@ impl OutputSql { .first::(conn)?) } - // Find a particular Output, if it exists and is in the specified Spent state - 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, diff --git a/integration_tests/features/WalletTransactions.feature b/integration_tests/features/WalletTransactions.feature index 72dada6b01..ec66c0fcdc 100644 --- a/integration_tests/features/WalletTransactions.feature +++ b/integration_tests/features/WalletTransactions.feature @@ -88,6 +88,7 @@ Feature: Wallet Transactions And I have 1 base nodes connected to all seed nodes And I have wallet WALLET_A connected to all seed nodes And I have mining node MINER connected to base node NODE and wallet WALLET_A + And I have mining node MINER2 connected to base node NODE and wallet WALLET_A When mining node MINER mines 2 blocks When mining node MINER2 mines 2 blocks When mining node MINER mines 3 blocks From b3d04fbd3cd6f91149a1ed1584f6b22c538b0a63 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 16:52:29 +0100 Subject: [PATCH 14/17] add fetch by commitment to output manager database and match in get coinbase transaction --- .../wallet/src/output_manager_service/service.rs | 14 +++++++++++--- .../output_manager_service/storage/database/mod.rs | 9 +++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index fc8229b7a0..0e1903959a 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -93,7 +93,7 @@ use crate::{ storage::{ database::{OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, models::{DbUnblindedOutput, KnownOneSidedPaymentScript, SpendingPriority}, - sqlite_db::*, + database::{DbKey, backend::*}, OutputStatus, }, tasks::TxoValidationTask, @@ -1012,8 +1012,16 @@ where )?; // If there is no existing output available, we store the one we produced. - match self.resources.db.find_by_commitment(output.commitment.clone()) { - Ok(_) => {}, + match self.resources.db.fetch_by_commitment(output.commitment.clone()) { + Ok(outs) => { + if outs.is_empty() { + self.resources + .db + .add_output_to_be_received(tx_id, output, Some(block_height))?; + + self.confirm_encumberance(tx_id)?; + } + }, Err(OutputManagerStorageError::ValueNotFound) => { self.resources .db 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 7712093173..1703f309ff 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 @@ -228,6 +228,15 @@ where T: OutputManagerBackend + 'static Ok(result) } + pub fn fetch_by_commitment(&self, commitment: Commitment) -> Result, OutputManagerStorageError> { + let result = match self.db.fetch(&DbKey::AnyOutputByCommitment(commitment))? { + Some(DbValue::UnspendOutputs(outputs)) => outputs, + Some(other) => return unexpected_result(DbKey::UnspentOutputs, other), + None => vec![], + }; + Ok(result) + } + pub fn fetch_with_features( &self, feature: OutputType, From c6fc285673c2f7548fbb77572cee4da9cc178d0e Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Mon, 8 Aug 2022 16:53:30 +0100 Subject: [PATCH 15/17] add cargo fmt --- base_layer/wallet/src/output_manager_service/service.rs | 3 +-- .../src/output_manager_service/storage/database/mod.rs | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 0e1903959a..2e765f4a6d 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -91,9 +91,8 @@ use crate::{ recovery::StandardUtxoRecoverer, resources::{OutputManagerKeyManagerBranch, OutputManagerResources}, storage::{ - database::{OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, + database::{backend::*, DbKey, OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, models::{DbUnblindedOutput, KnownOneSidedPaymentScript, SpendingPriority}, - database::{DbKey, backend::*}, OutputStatus, }, tasks::TxoValidationTask, 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 1703f309ff..430b8f33d0 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 @@ -228,7 +228,10 @@ where T: OutputManagerBackend + 'static Ok(result) } - pub fn fetch_by_commitment(&self, commitment: Commitment) -> Result, OutputManagerStorageError> { + pub fn fetch_by_commitment( + &self, + commitment: Commitment, + ) -> Result, OutputManagerStorageError> { let result = match self.db.fetch(&DbKey::AnyOutputByCommitment(commitment))? { Some(DbValue::UnspendOutputs(outputs)) => outputs, Some(other) => return unexpected_result(DbKey::UnspentOutputs, other), From 7907b49006865fa315a5d0ec1258e3a6c9584fd4 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Tue, 9 Aug 2022 08:41:58 +0100 Subject: [PATCH 16/17] resolve clippy; --- base_layer/wallet/src/output_manager_service/service.rs | 2 +- .../wallet/src/output_manager_service/storage/database/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/base_layer/wallet/src/output_manager_service/service.rs b/base_layer/wallet/src/output_manager_service/service.rs index 2e765f4a6d..ff231bf84f 100644 --- a/base_layer/wallet/src/output_manager_service/service.rs +++ b/base_layer/wallet/src/output_manager_service/service.rs @@ -91,7 +91,7 @@ use crate::{ recovery::StandardUtxoRecoverer, resources::{OutputManagerKeyManagerBranch, OutputManagerResources}, storage::{ - database::{backend::*, DbKey, OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, + database::{OutputBackendQuery, OutputManagerBackend, OutputManagerDatabase}, models::{DbUnblindedOutput, KnownOneSidedPaymentScript, SpendingPriority}, OutputStatus, }, 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 430b8f33d0..d205ec0c8c 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 @@ -233,7 +233,7 @@ where T: OutputManagerBackend + 'static commitment: Commitment, ) -> Result, OutputManagerStorageError> { let result = match self.db.fetch(&DbKey::AnyOutputByCommitment(commitment))? { - Some(DbValue::UnspendOutputs(outputs)) => outputs, + Some(DbValue::UnspentOutputs(outputs)) => outputs, Some(other) => return unexpected_result(DbKey::UnspentOutputs, other), None => vec![], }; From 8b0d1eb618dcb441df8aefcd66594abee9ca9a79 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Tue, 9 Aug 2022 09:07:45 +0100 Subject: [PATCH 17/17] refactor test --- .../output_manager_service_tests/service.rs | 17 ----------------- 1 file changed, 17 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 c448038dea..20b34de2b6 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/service.rs @@ -1313,23 +1313,6 @@ async fn handle_coinbase_with_bulletproofs_rewinding() { value1 ); - // duplicate the same transaction, creates the same utxo, does previous coinbase should be removed - // and we should get value1 for the balance - let _tx1 = oms - .output_manager_handle - .get_coinbase_transaction(1u64.into(), reward1, fees1, 1) - .await - .unwrap(); - assert_eq!(oms.output_manager_handle.get_unspent_outputs().await.unwrap().len(), 0); - assert_eq!( - oms.output_manager_handle - .get_balance() - .await - .unwrap() - .pending_incoming_balance, - value1 - ); - let _tx2 = oms .output_manager_handle .get_coinbase_transaction(2u64.into(), reward2, fees2, 1)