Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: merge development into tx-validation2 and fix issues #3417

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2410,6 +2410,6 @@ impl<'a> OutputKey<'a> {
}

pub fn get_key(&self) -> String {
format!("{}-{:010}", to_hex(&self.header_hash), self.mmr_position)
format!("{}-{:010}", to_hex(self.header_hash), self.mmr_position)
}
}
50 changes: 31 additions & 19 deletions base_layer/wallet/src/output_manager_service/storage/sqlite_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase {
} else {
OutputStatus::UnspentMinedUnconfirmed as i32
};
error!(
target: LOG_TARGET,
"`set_received_output_mined_height` status: {}", status
);
// Only allow updating of non-deleted utxos
diesel::update(outputs::table.filter(outputs::hash.eq(hash).and(outputs::marked_deleted_at_height.is_null())))
.set((
Expand Down Expand Up @@ -489,25 +493,34 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase {
fn set_coinbase_abandoned(&self, tx_id: TxId, abandoned: bool) -> Result<(), OutputManagerStorageError> {
let conn = self.database_connection.acquire_lock();

debug!(target: LOG_TARGET, "set_coinbase_abandoned(TxID: {})", tx_id);

let status = if abandoned {
OutputStatus::AbandonedCoinbase as i32
if abandoned {
debug!(
target: LOG_TARGET,
"set_coinbase_abandoned(TxID: {}) as {}", tx_id, abandoned
);
diesel::update(
outputs::table.filter(
outputs::received_in_tx_id
.eq(Some(tx_id as i64))
.and(outputs::coinbase_block_height.is_not_null()),
),
)
.set((outputs::status.eq(OutputStatus::AbandonedCoinbase as i32),))
.execute(&(*conn))
.num_rows_affected_or_not_found(1)?;
} else {
OutputStatus::EncumberedToBeReceived as i32
let output = OutputSql::find_by_tx_id_and_status(tx_id, OutputStatus::AbandonedCoinbase, &conn)?;
for o in output.into_iter() {
o.update(
UpdateOutput {
status: Some(OutputStatus::EncumberedToBeReceived),
..Default::default()
},
&conn,
)?;
}
};

diesel::update(
outputs::table.filter(
outputs::received_in_tx_id
.eq(Some(tx_id as i64))
.and(outputs::coinbase_block_height.is_not_null()),
),
)
.set((outputs::status.eq(status),))
.execute(&(*conn))
.num_rows_affected_or_not_found(1)?;

Ok(())
}

Expand Down Expand Up @@ -1481,9 +1494,8 @@ impl Encryptable<Aes256Gcm> for KeyManagerStateSql {

impl Encryptable<Aes256Gcm> for NewKeyManagerStateSql {
fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), Error> {
let encrypted_master_key = encrypt_bytes_integral_nonce(&cipher, self.master_key.clone())?;
let encrypted_branch_seed =
encrypt_bytes_integral_nonce(&cipher, self.branch_seed.clone().as_bytes().to_vec())?;
let encrypted_master_key = encrypt_bytes_integral_nonce(cipher, self.master_key.clone())?;
let encrypted_branch_seed = encrypt_bytes_integral_nonce(cipher, self.branch_seed.clone().as_bytes().to_vec())?;
self.master_key = encrypted_master_key;
self.branch_seed = encrypted_branch_seed.to_hex();
Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ where
.for_protocol(self.operation_id)?;
debug!(
target: LOG_TARGET,
"Base node returned {} as mined and {} as unmined",
"Base node returned {} outputs as mined and {} outputs as unmined",
mined.len(),
unmined.len()
);
Expand All @@ -242,7 +242,7 @@ where
mined_height,
tip_height
);
self.update_output_as_mined(&output, mined_in_block, *mined_height, *mmr_position, tip_height)
self.update_output_as_mined(output, mined_in_block, *mined_height, *mmr_position, tip_height)
.await?;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ where
if let Some((tip_height, tip_block)) = tip_info {
for tx in &unmined {
// Treat coinbases separately
if tx.is_coinbase_transaction() {
if tx.is_coinbase() {
if tx.coinbase_block_height.unwrap_or_default() <= tip_height {
info!(target: LOG_TARGET, "Updated coinbase {} as abandoned", tx.tx_id);
self.update_coinbase_as_abandoned(
Expand All @@ -146,7 +146,7 @@ where
}
} else {
info!(target: LOG_TARGET, "Updated transaction {} as unmined", tx.tx_id);
self.update_transaction_as_unmined(&tx).await?;
self.update_transaction_as_unmined(tx).await?;
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions base_layer/wallet/src/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ where
{
return Err(TransactionServiceError::InvalidCompletedTransaction);
}
if completed_tx.is_coinbase_transaction() {
if completed_tx.is_coinbase() {
return Err(TransactionServiceError::AttemptedToBroadcastCoinbaseTransaction(
completed_tx.tx_id,
));
Expand Down Expand Up @@ -1685,7 +1685,7 @@ where
if completed_tx.valid &&
(completed_tx.status == TransactionStatus::Completed ||
completed_tx.status == TransactionStatus::Broadcast) &&
!completed_tx.is_coinbase_transaction()
!completed_tx.is_coinbase()
{
self.broadcast_completed_transaction(completed_tx, join_handles).await?;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl CompletedTransaction {
}
}

pub fn is_coinbase_transaction(&self) -> bool {
pub fn is_coinbase(&self) -> bool {
self.coinbase_block_height.is_some()
}
}
Expand Down
14 changes: 9 additions & 5 deletions base_layer/wallet/src/utxo_scanner_service/utxo_scanning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,10 +716,18 @@ where TBackend: WalletBackend + 'static
//we make sure the flag is set to false here
running_flag.store(false, Ordering::Relaxed);
});
if self.mode == UtxoScannerMode::Recovery {
return Ok(());
}
}
},
_ = self.resources.current_base_node_watcher.changed() => {
trace!(target: LOG_TARGET, "Base node change detected.");
debug!(target: LOG_TARGET, "Base node change detected.");
let peer = self.resources.current_base_node_watcher.borrow().as_ref().cloned();
if let Some(peer) = peer {
self.peer_seeds = vec![peer.public_key];
}

self.is_running.store(false, Ordering::Relaxed);
},
_ = shutdown.wait() => {
Expand All @@ -729,10 +737,6 @@ where TBackend: WalletBackend + 'static
return Ok(());
}
}

if self.mode == UtxoScannerMode::Recovery {
return Ok(());
}
}
}
}
Expand Down
81 changes: 0 additions & 81 deletions base_layer/wallet/tests/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5094,87 +5094,6 @@ fn broadcast_all_completed_transactions_on_startup() {
});
}

#[test]
fn only_start_one_tx_broadcast_protocol_at_a_time() {
let mut runtime = Runtime::new().unwrap();
let factories = CryptoFactories::default();

let temp_dir = tempdir().unwrap();
let db_name = format!("{}.sqlite3", random::string(8).as_str());
let db_path = format!("{}/{}", temp_dir.path().to_str().unwrap(), db_name);
let connection = run_migration_and_create_sqlite_connection(&db_path).unwrap();
let backend = TransactionServiceSqliteDatabase::new(connection.clone(), None);

let kernel = KernelBuilder::new()
.with_excess(&factories.commitment.zero())
.with_signature(&Signature::default())
.build()
.unwrap();

let tx = Transaction::new(
vec![],
vec![],
vec![kernel],
PrivateKey::random(&mut OsRng),
PrivateKey::random(&mut OsRng),
);

let completed_tx1 = CompletedTransaction {
tx_id: 1,
source_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)),
destination_public_key: PublicKey::from_secret_key(&PrivateKey::random(&mut OsRng)),
amount: 5000 * uT,
fee: MicroTari::from(100),
transaction: tx,
status: TransactionStatus::Completed,
message: "Yo!".to_string(),
timestamp: Utc::now().naive_utc(),
cancelled: false,
direction: TransactionDirection::Outbound,
coinbase_block_height: None,
send_count: 0,
last_send_timestamp: None,
valid: true,
confirmations: None,
mined_height: None,
mined_in_block: None,
};

backend
.write(WriteOperation::Insert(DbKeyValuePair::CompletedTransaction(
completed_tx1.tx_id,
Box::new(completed_tx1),
)))
.unwrap();

let (
mut alice_ts,
_,
_,
_,
_,
_,
_,
_,
_shutdown,
_mock_rpc_server,
server_node_identity,
rpc_service_state,
_,
mut alice_connectivity,
_rpc_server_connection,
) = setup_transaction_service_no_comms(&mut runtime, factories, connection, None);

alice_connectivity.set_base_node(server_node_identity.to_peer());

assert!(runtime.block_on(alice_ts.restart_broadcast_protocols()).is_ok());
assert!(runtime.block_on(alice_ts.restart_broadcast_protocols()).is_ok());

let tx_submit_calls =
runtime.block_on(rpc_service_state.wait_pop_submit_transaction_calls(2, Duration::from_secs(2)));
assert!(tx_submit_calls.is_err(), "Should not be 2 calls made");
}

#[test]
fn dont_broadcast_invalid_transactions() {
let mut runtime = Runtime::new().unwrap();
Expand Down
3 changes: 0 additions & 3 deletions common/src/configuration/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,6 @@ fn convert_node_config(
let key = "wallet.output_manager_event_channel_size";
let output_manager_event_channel_size = optional(cfg.get_int(key))?.unwrap_or(250) as usize;

let key = "wallet.base_node_update_publisher_channel_size";
let base_node_update_publisher_channel_size = optional(cfg.get_int(key))?.unwrap_or(50) as usize;

let key = "wallet.prevent_fee_gt_amount";
let prevent_fee_gt_amount = cfg
.get_bool(key)
Expand Down
5 changes: 3 additions & 2 deletions integration_tests/features/WalletFFI.feature
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ Feature: Wallet FFI
And mining node MINER mines 10 blocks
Then I wait for wallet RECEIVER to have at least 1000000 uT
And I have 1 received and 1 send transaction in ffi wallet FFI_WALLET
And I start TXO validation on ffi wallet FFI_WALLET
And I start TX validation on ffi wallet FFI_WALLET
Then I wait for ffi wallet FFI_WALLET to receive 1 mined
Then I want to view the transaction kernels for completed transactions in ffi wallet FFI_WALLET
And I start STXO validation on ffi wallet FFI_WALLET
And I start UTXO validation on ffi wallet FFI_WALLET
And I stop ffi wallet FFI_WALLET

Scenario: As a client I want to receive Tari via my Public Key sent while I am offline when I come back online
Expand Down
35 changes: 20 additions & 15 deletions integration_tests/features/WalletTransactions.feature
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Feature: Wallet Transactions
# for imported UTXO's anyway so until that is decided we will just check that the imported output becomes Spent
#Then I check if last imported transactions are invalid in wallet WALLET_C

@broken #Currently there is not handling for detecting that a reorged imported output is invalid
@critical @flakey
Scenario: Wallet imports reorged outputs that become invalidated
# Chain 1
Given I have a seed node SEED_B
Expand All @@ -105,6 +105,8 @@ Feature: Wallet Transactions
When I have wallet WALLET_IMPORTED connected to base node B
Then I import WALLET_RECEIVE_TX unspent outputs to WALLET_IMPORTED
Then I wait for wallet WALLET_IMPORTED to have at least 1000000 uT
# This triggers a validation of the imported outputs
Then I restart wallet WALLET_IMPORTED
# Chain 2
Given I have a seed node SEED_C
And I have a base node C connected to seed SEED_C
Expand All @@ -120,10 +122,13 @@ Feature: Wallet Transactions
And node C is at height 10
Then I restart wallet WALLET_IMPORTED
Then I wait for wallet WALLET_IMPORTED to have less than 1 uT
And mining node CM mines 1 blocks with min difficulty 1000 and max difficulty 9999999999
And node B is at height 11
And node C is at height 11
# TODO Either remove the check for invalid Faux tx and change the test name or implement a new way to invalidate Faux Tx
# The concept of invalidating the Faux transaction doesn't exist in this branch anymore. There has been talk of removing the Faux transaction
# for imported UTXO's anyway so until that is decided we will just check that the imported output becomes invalid
Then I check if last imported transactions are invalid in wallet WALLET_IMPORTED
# Then I check if last imported transactions are invalid in wallet WALLET_IMPORTED

@critical
Scenario: Wallet imports faucet UTXO
Expand Down Expand Up @@ -157,19 +162,19 @@ Feature: Wallet Transactions
When I merge mine 10 blocks via PROXY
Then all nodes are at height 10
Then I wait for wallet WALLET_A to have at least 10000000000 uT
Then I have wallet WALLET_B connected to all seed nodes
And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
When wallet WALLET_A detects all transactions are at least Broadcast
Then I merge mine 5 blocks via PROXY
Then all nodes are at height 15
Then I wait for wallet WALLET_B to have at least 500000 uT
Then I check if wallet WALLET_B has 5 transactions
Then I restart wallet WALLET_B
Then I check if wallet WALLET_B has 5 transactions
# Then I have wallet WALLET_B connected to all seed nodes
# And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
# And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
# And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
# And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
# And I send 100000 uT from wallet WALLET_A to wallet WALLET_B at fee 100
# When wallet WALLET_A detects all transactions are at least Broadcast
# Then I merge mine 5 blocks via PROXY
# Then all nodes are at height 15
# Then I wait for wallet WALLET_B to have at least 500000 uT
# Then I check if wallet WALLET_B has 5 transactions
# Then I restart wallet WALLET_B
# Then I check if wallet WALLET_B has 5 transactions

# runs 8mins on circle ci
@critical @long-running
Expand Down
12 changes: 6 additions & 6 deletions integration_tests/features/support/ffi_steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,22 +538,22 @@ When("I stop ffi wallet {word}", function (walletName) {
});

Then(
"I start STXO validation on ffi wallet {word}",
"I start TXO validation on ffi wallet {word}",
async function (wallet_name) {
const wallet = this.getWallet(wallet_name);
await wallet.startStxoValidation();
while (!wallet.getStxoValidationStatus().stxo_validation_complete) {
await wallet.startTxoValidation();
while (!wallet.getTxoValidationStatus().txo_validation_complete) {
await sleep(1000);
}
}
);

Then(
"I start UTXO validation on ffi wallet {word}",
"I start TX validation on ffi wallet {word}",
async function (wallet_name) {
const wallet = this.getWallet(wallet_name);
await wallet.startUtxoValidation();
while (!wallet.getUtxoValidationStatus().utxo_validation_complete) {
await wallet.startTxValidation();
while (!wallet.getTxValidationStatus().tx_validation_complete) {
await sleep(1000);
}
}
Expand Down
Loading