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.

Sign up for GitHub

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

Fix transaction logs incorrectly marked successful #1027

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Fix transaction logs incorrectly marked successful #1027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8470bf7
Fix transaction logs incorrectly marked successful
nick-mobilecoin Dec 2, 2024
e4b40bf
Clarify name of the ouput transaction id
nick-mobilecoin Dec 3, 2024
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
8 changes: 4 additions & 4 deletions full-service/src/db/transaction_log.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,16 +777,16 @@ impl TransactionLogModel for TransactionLog {
}

fn update_pending_associated_with_txo_to_succeeded(
txo_id_hex: &str,
transaction_output_txo_id_hex: &str,
finalized_block_index: u64,
conn: Conn,
) -> Result<(), WalletDbError> {
use crate::db::schema::{transaction_input_txos, transaction_logs};
use crate::db::schema::{transaction_logs, transaction_output_txos};
// Find all submitted transaction logs associated with this txo that have not
// yet been finalized (there should only ever be one).
let transaction_log_ids: Vec<String> = transaction_logs::table
.inner_join(transaction_input_txos::table)
.filter(transaction_input_txos::txo_id.eq(txo_id_hex))
.inner_join(transaction_output_txos::table)
.filter(transaction_output_txos::txo_id.eq(transaction_output_txo_id_hex))
.filter(transaction_logs::submitted_block_index.is_not_null()) // we actually sent this transaction
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved
.filter(transaction_logs::failed.eq(false)) // non-failed transactions
.filter(transaction_logs::finalized_block_index.is_null()) // non-completed transactions
Expand Down
218 changes: 100 additions & 118 deletions full-service/src/json_rpc/v2/e2e_tests/transaction/build_submit/build_then_submit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod e2e_transaction {
json_rpc::v2::{
api::test_utils::{dispatch, dispatch_expect_error, setup},
models::{
amount::Amount as AmountJSON, transaction_log::TransactionLog,
account::Account, amount::Amount as AmountJSON, transaction_log::TransactionLog,
tx_proposal::TxProposal as TxProposalJSON,
},
},
Expand Down Expand Up @@ -717,10 +717,18 @@ mod e2e_transaction {

#[test_with_logger]
fn test_reuse_input_txo_then_submit_transaction(logger: Logger) {
// This test mimics a situation where multiple services are using the same
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved
// Full-Service instance. Both services are able to `build_transaction`s,
// which consume the same input txo(s). Both of the services are able to submit
// the transactions prior to either transaction landing on the blockchain. The
// blockchain will only allow one of these transactions to succeed.
// Previous versions of Full-Service had a bug that would mark both transactions
// as succeeded. This was because Full-Service would mark a transaction as
// succeeded if the inputs were consumed. This test ensures that this
// bug has been fixed and only the successful transaction is marked as such.
let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]);
let (client, mut ledger_db, db_ctx, _network_state) = setup(&mut rng, logger.clone());

// Add an account
let body = json!({
"jsonrpc": "2.0",
"id": 1,
Expand All @@ -731,16 +739,14 @@ mod e2e_transaction {
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let account_obj = result.get("account").unwrap();
let account_id = account_obj.get("id").unwrap().as_str().unwrap();
let b58_public_address = account_obj.get("main_address").unwrap().as_str().unwrap();
let public_address = b58_decode_public_address(b58_public_address).unwrap();
let account: Account =
serde_json::from_value(result.get("account").unwrap().clone()).unwrap();
let moving_funds = 100000000000;
let starting_funds = 10 * moving_funds;

add_block_to_ledger_db(
&mut ledger_db,
&vec![public_address],
&vec![b58_decode_public_address(&account.main_address).unwrap()],
starting_funds,
&[KeyImage::from(rng.next_u64())],
&mut rng,
Expand All @@ -749,126 +755,102 @@ mod e2e_transaction {
manually_sync_account(
&ledger_db,
&db_ctx.get_db_instance(logger.clone()),
&AccountID(account_id.to_string()),
&AccountID(account.id.clone()),
&logger,
);

// get the txo we're going to use for the two transaction proposals
let body = json!({
"method": "get_txos",
"params": {
"account_id": account_id,
},
"jsonrpc": "2.0",
"id": 1
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let txo_id = (result.get("txo_ids").unwrap())[0].as_str().unwrap();

// Create a tx proposal to ourselves
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "build_transaction",
"params": {
"account_id": account_id,
"recipient_public_address": b58_public_address,
"amount": { "value": moving_funds.to_string(), "token_id": "0"},
"input_txo_ids" : [txo_id.to_string()],
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let tx_proposal_a: TxProposalJSON =
serde_json::from_value(result.get("tx_proposal").unwrap().clone()).unwrap();
let txlog_id_a = result.get("transaction_log_id").unwrap().as_str().unwrap();

// Create a 2nd tx proposal to ourselves
let moving_funds = moving_funds * 2;
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "build_transaction",
"params": {
"account_id": account_id,
"recipient_public_address": b58_public_address,
"amount": { "value": moving_funds.to_string(), "token_id": "0"},
"input_txo_ids" : [txo_id.to_string()],
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
// let tx_proposal_b: TxProposalJSON =
// serde_json::from_value(result.get("tx_proposal").unwrap().clone()).
// unwrap();
let txlog_id_b = result.get("transaction_log_id").unwrap().as_str().unwrap();

assert_ne!(txlog_id_a, txlog_id_b);

// Submit the tx_proposal
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "submit_transaction",
"params": {
"tx_proposal": tx_proposal_a,
"account_id": account_id,
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let submitted_transaction_id = result
.get("transaction_log")
.unwrap()
.get("id")
.unwrap()
.as_str()
.unwrap();

assert_eq!(txlog_id_a, submitted_transaction_id);
let payments_tx_proposal_a = TxProposal::try_from(&tx_proposal_a).unwrap();
let tx_log_id_and_proposals = (0..2)
.into_iter()
.map(|_| {
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "build_transaction",
"params": {
"account_id": account.id,
"recipient_public_address": account.main_address,
"amount": { "value": moving_funds.to_string(), "token_id": "0"},
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let tx_log_id =
serde_json::from_value(result.get("transaction_log_id").unwrap().clone())
.unwrap();
let tx_proposal =
serde_json::from_value(result.get("tx_proposal").unwrap().clone()).unwrap();
(tx_log_id, tx_proposal)
})
.collect::<Vec<(String, TxProposalJSON)>>();

// In order for this test to prove the point the transaction logs should be
// different while the input txos are the same.
assert_ne!(tx_log_id_and_proposals[0].0, tx_log_id_and_proposals[1].0);
assert_eq!(
tx_log_id_and_proposals[0].1.input_txos,
tx_log_id_and_proposals[1].1.input_txos
);

// The MockBlockchainConnection does not write to the ledger_db
add_block_with_tx(&mut ledger_db, payments_tx_proposal_a.tx, &mut rng);
tx_log_id_and_proposals
.iter()
.for_each(|(tx_log_id, tx_proposal)| {
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "submit_transaction",
"params": {
"tx_proposal": tx_proposal,
"account_id": account.id,
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let submitted_tx_log_id = result
.get("transaction_log")
.unwrap()
.get("id")
.unwrap()
.as_str()
.unwrap();
assert_eq!(tx_log_id, submitted_tx_log_id);
});

let tx_proposal = TxProposal::try_from(&tx_log_id_and_proposals[0].1).unwrap();

add_block_with_tx(&mut ledger_db, tx_proposal.tx, &mut rng);
manually_sync_account(
&ledger_db,
&db_ctx.get_db_instance(logger.clone()),
&AccountID(account_id.to_string()),
&AccountID(account.id),
&logger,
);

// Get the transaction_id and verify it contains what we expect
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "get_transaction_log",
"params": {
"transaction_log_id": submitted_transaction_id,
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let transaction_log: TransactionLog =
serde_json::from_value(result.get("transaction_log").unwrap().clone()).unwrap();

assert_eq!(transaction_log.status, TxStatus::Succeeded.to_string());
assert_eq!(transaction_log.id, submitted_transaction_id);

// Get the transaction_id and verify it contains what we expect
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "get_transaction_log",
"params": {
"transaction_log_id": txlog_id_b,
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let transaction_log: TransactionLog =
serde_json::from_value(result.get("transaction_log").unwrap().clone()).unwrap();
assert_eq!(transaction_log.id, txlog_id_b);
assert_eq!(transaction_log.status, TxStatus::Built.to_string());
// The first transaction log will have succeeded, because it's outputs
// show up on the ledger.
// The second transaction log will still be pending, because it's outputs
// don't exist in the ledger. This second transaction log will
// eventually fail once the tombstone block is reached.
let expected_statuses = [TxStatus::Succeeded, TxStatus::Pending];
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved

tx_log_id_and_proposals
.iter()
.zip(expected_statuses)
.for_each(|((tx_log_id, _), status)| {
let body = json!({
"jsonrpc": "2.0",
"id": 1,
"method": "get_transaction_log",
"params": {
"transaction_log_id": tx_log_id,
}
});
let res = dispatch(&client, body, &logger);
let result = res.get("result").unwrap();
let tx_log: TransactionLog =
serde_json::from_value(result.get("transaction_log").unwrap().clone()).unwrap();

assert_eq!(tx_log.status, status.to_string());
assert_eq!(&tx_log.id, tx_log_id);
});
}
}
16 changes: 11 additions & 5 deletions full-service/src/service/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ pub fn sync_account_next_chunk(
(*account_key.view_private_key(), Some(account_key))
};

tx_outs.iter().try_for_each(|(block_index, tx_out)| {
nick-mobilecoin marked this conversation as resolved.
Show resolved Hide resolved
let txos = Txo::select_by_public_key(&[&tx_out.public_key], conn).unwrap();
txos.iter().try_for_each(|txo| {
TransactionLog::update_pending_associated_with_txo_to_succeeded(
&txo.id,
*block_index,
conn,
)
})
})?;

// Attempt to decode each transaction as received by this account.
let received_txos: Vec<_> = tx_outs
.into_par_iter()
Expand Down Expand Up @@ -288,11 +299,6 @@ pub fn sync_account_next_chunk(

for (block_index, txo_id_hex) in &spent_txos {
Txo::update_spent_block_index(txo_id_hex, *block_index, conn)?;
TransactionLog::update_pending_associated_with_txo_to_succeeded(
txo_id_hex,
*block_index,
conn,
)?;
}

TransactionLog::update_pending_exceeding_tombstone_block_index_to_failed(
Expand Down