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

drop txo field from txos table in db #534

Merged
merged 42 commits into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
afbc7e5
Turn code line into code block
pouneh Oct 11, 2022
d61f99f
Explicitly gitignore the sgx libraries downloaded when developing for…
pouneh Oct 11, 2022
7df9ac7
First draft of migration
pouneh Oct 11, 2022
f2fcf0d
Update README diesel hash to match the one we depend on formally in C…
pouneh Oct 11, 2022
4a057f9
Probably don't need the down path
pouneh Oct 17, 2022
796c4bc
DOCS: update upgrade scenario notes
pouneh Oct 17, 2022
d3b189b
Revert "Probably don't need the down path"
pouneh Oct 17, 2022
d526755
DOCS: update diesel migration instructions
pouneh Oct 17, 2022
0f2922f
WIP: Remove TXO from schema.rs
pouneh Oct 17, 2022
aad0685
WIP: remove txo field from db related structs
pouneh Oct 17, 2022
ce757ff
DOCS: update readme
pouneh Oct 17, 2022
1293dd5
WIP: Use the txo id to get the txo from the ledger service
pouneh Oct 18, 2022
c8da0c0
WIP:get ledger.rs to get a txo from the txo id
pouneh Oct 18, 2022
2a40b69
WIP: use ledger to get the txo
pouneh Oct 18, 2022
c40e690
Fix some unwrapping issues
pouneh Oct 18, 2022
09b43d2
REFACTOR: inline variables
pouneh Oct 18, 2022
f3e9535
Fix compiler errors for tests
pouneh Oct 18, 2022
c77e65e
Fix one test
pouneh Oct 20, 2022
cd64fae
Move get_tx_out_by_public_key to ledger.rs
pouneh Oct 20, 2022
a4fb223
Update helper function uses
pouneh Oct 20, 2022
2811897
Make get_tx_out_by_index return a result instead of using a bunch of …
pouneh Oct 20, 2022
d9aee34
Unwrap the txout from receipt
pouneh Oct 20, 2022
e082b9c
Remove uneeded function
pouneh Oct 20, 2022
43e7c34
Update db::transaction_log::tests::test_log_submitted to use the tx_p…
pouneh Oct 20, 2022
da2791f
Update service::transaction::tests::test_send_transaction to use the …
pouneh Oct 20, 2022
1c6ad09
oops: make sure we get the input_txos from the proposal
pouneh Oct 20, 2022
40aae23
Update test_list_transaction_logs_for_account_with_min_and_max_block_…
pouneh Oct 20, 2022
463f069
Remove unused import
pouneh Oct 20, 2022
8e1e4a3
fix typos
pouneh Oct 20, 2022
2ce7e95
Don't need to decode the keyimage
pouneh Oct 21, 2022
9044fab
refactor: rename
pouneh Oct 21, 2022
1f19758
Change signature
pouneh Oct 21, 2022
478d604
Remove because our db no loner has the actual txos in it
pouneh Oct 21, 2022
0e248e9
remove unused var
pouneh Oct 21, 2022
5d7df6d
Finish fixing tests
pouneh Oct 21, 2022
709339d
cargo fmt
pouneh Oct 21, 2022
1e7ed64
Remove transaction_log version of the create_test_minted_and_change_txos
pouneh Oct 21, 2022
9a72b69
Remove tx_proposal version of the create_test_minted_and_change_txos
pouneh Oct 21, 2022
ca09433
Remove unused test helper
pouneh Oct 21, 2022
8c6d789
reformat: rustfmt
pouneh Oct 21, 2022
91ece70
Merge branch 'develop' into pouneh/#431_drop_txo_field_from_txos_tabl…
briancorbin Oct 22, 2022
0b3aabc
Merge branch 'develop' into pouneh/#431_drop_txo_field_from_txos_tabl…
briancorbin Oct 26, 2022
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,4 @@ dbml-error.log

__pycache__

/tools/*.bin
*sgx_linux_x64_sdk_2.9.101.2.bin
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,5 @@ x25519-dalek = { git = "https://github.com/mobilecoinfoundation/x25519-dalek.git

# Override diesel dependency with our fork, in order to use a version of libsqlite3-sys that has bundled-sqlcipher. This allows us to
# statically link SQLite.
# If updating here, also update in README
diesel = { git = "https://github.com/mobilecoinofficial/diesel", rev = "026f6379715d27c8be48396e5ca9059f4a263198" }
27 changes: 24 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,33 @@ See [CONTRIBUTING](./CONTRIBUTING.md).
To add or edit tables:

1. Ensure that you have `diesel_cli` installed and that it is using the current sqlite
version: `cargo install --git="https://github.com/mobilecoinofficial/diesel" --rev="22a4a4b973db2b7aadaf088b3279dbbe52176896" diesel_cli --no-default-features --features sqlite`
version:

```
cargo install --git="https://github.com/mobilecoinofficial/diesel" --rev="026f6379715d27c8be48396e5ca9059f4a263198" diesel_cli --no-default-features --features sqlite
```

1. `cd full-service`

1. Create an empty version of the base db `diesel migration run --database-url $MIGRATION_TEST_DB`

1. Create a migration with `diesel migration generate <migration_name>`

1. Edit the migrations/<migration_name>/up.sql and down.sql.
1. Run the migration with `diesel migration run --database-url /tmp/db.db`, and test delete
with `diesel migration redo --database-url /tmp/db.db`

1. Run the migration with `diesel migration run --database-url $MIGRATION_TEST_DB`, and test the
inverse operation with `diesel migration redo --database-url $MIGRATION_TEST_DB`

Make sure that the following is still present in `schema.rs` before commiting changes.
```
table! {
__diesel_schema_migrations(version) {
version -> Text,
run_on -> Timestamp,
}
}
```


Note that full-service/diesel.toml provides the path to the schema.rs which will be updated in a migration.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE txos ADD COLUMN txo BLOB NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE txos DROP COLUMN txo;
3 changes: 0 additions & 3 deletions full-service/src/db/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ pub struct Txo {
pub public_key: Vec<u8>,
/// The serialized e_fog_hint of the TxOut.
pub e_fog_hint: Vec<u8>,
/// The serialized TxOut.
pub txo: Vec<u8>,
/// The receiving subaddress, if known.
pub subaddress_index: Option<i64>,
/// Pre-computed key image for this Txo, or None if the Txo is orphaned.
Expand Down Expand Up @@ -104,7 +102,6 @@ pub struct NewTxo<'a> {
pub target_key: &'a [u8],
pub public_key: &'a [u8],
pub e_fog_hint: &'a [u8],
pub txo: &'a [u8],
pub subaddress_index: Option<i64>,
pub key_image: Option<&'a [u8]>,
pub received_block_index: Option<i64>,
Expand Down
1 change: 0 additions & 1 deletion full-service/src/db/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ table! {
target_key -> Binary,
public_key -> Binary,
e_fog_hint -> Binary,
txo -> Binary,
subaddress_index -> Nullable<BigInt>,
key_image -> Nullable<Binary>,
received_block_index -> Nullable<BigInt>,
Expand Down
28 changes: 19 additions & 9 deletions full-service/src/db/transaction_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ mod tests {
use mc_account_keys::{PublicAddress, CHANGE_SUBADDRESS_INDEX};
use mc_common::logger::{test_with_logger, Logger};
use mc_ledger_db::Ledger;
use mc_transaction_core::{tokens::Mob, Token};
use mc_transaction_core::{ring_signature::KeyImage, tokens::Mob, Token};
use rand::{rngs::StdRng, SeedableRng};

use crate::{
Expand All @@ -588,9 +588,9 @@ mod tests {
transaction_builder::WalletTransactionBuilder,
},
test_utils::{
add_block_from_transaction_log, add_block_with_tx_outs, builder_for_random_recipient,
get_resolver_factory, get_test_ledger, manually_sync_account,
random_account_with_seed_values, WalletDbTestContext, MOB,
add_block_with_tx_outs, builder_for_random_recipient, get_resolver_factory,
get_test_ledger, manually_sync_account, random_account_with_seed_values,
WalletDbTestContext, MOB,
},
util::b58::b58_encode_public_address,
};
Expand Down Expand Up @@ -719,10 +719,20 @@ mod tests {
// The subaddress will also be set once received.
assert_eq!(change_details.subaddress_index, None,);

add_block_from_transaction_log(
let key_images: Vec<KeyImage> = tx_proposal
.input_txos
.iter()
.map(|txo| txo.key_image.clone())
.collect();

// Note: This block doesn't contain the fee output.
add_block_with_tx_outs(
&mut ledger_db,
&wallet_db.get_conn().unwrap(),
&tx_log,
&[
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&key_images,
&mut rng,
);

Expand Down Expand Up @@ -1128,8 +1138,8 @@ mod tests {
add_block_with_tx_outs(
&mut ledger_db,
&[
mc_util_serial::decode(&change_details.txo).unwrap(),
mc_util_serial::decode(&output_details.txo).unwrap(),
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&[
mc_util_serial::decode(&input_details0.key_image.unwrap()).unwrap(),
Expand Down
33 changes: 17 additions & 16 deletions full-service/src/db/txo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ impl TxoModel for Txo {
target_key: &mc_util_serial::encode(&txo.target_key),
public_key: &mc_util_serial::encode(&txo.public_key),
e_fog_hint: &mc_util_serial::encode(&txo.e_fog_hint),
txo: &mc_util_serial::encode(&txo),
subaddress_index: subaddress_index.map(|i| i as i64),
key_image: key_image_bytes.as_deref(),
received_block_index: Some(received_block_index as i64),
Expand Down Expand Up @@ -411,7 +410,6 @@ impl TxoModel for Txo {
target_key: &mc_util_serial::encode(&output_txo.tx_out.target_key),
public_key: &mc_util_serial::encode(&output_txo.tx_out.public_key),
e_fog_hint: &mc_util_serial::encode(&output_txo.tx_out.e_fog_hint),
txo: &mc_util_serial::encode(&output_txo.tx_out),
subaddress_index: None,
key_image: None,
received_block_index: None,
Expand Down Expand Up @@ -1430,10 +1428,10 @@ mod tests {
transaction_builder::WalletTransactionBuilder,
},
test_utils::{
add_block_with_db_txos, add_block_with_tx, add_block_with_tx_outs,
create_test_minted_and_change_txos, create_test_received_txo,
create_test_txo_for_recipient, get_resolver_factory, get_test_ledger,
manually_sync_account, random_account_with_seed_values, WalletDbTestContext, MOB,
add_block_with_tx, add_block_with_tx_outs, create_test_minted_and_change_txos,
create_test_received_txo, create_test_txo_for_recipient, get_resolver_factory,
get_test_ledger, manually_sync_account, random_account_with_seed_values,
WalletDbTestContext, MOB,
},
WalletDb,
};
Expand Down Expand Up @@ -1509,7 +1507,6 @@ mod tests {
target_key: mc_util_serial::encode(&for_alice_txo.target_key),
public_key: mc_util_serial::encode(&for_alice_txo.public_key),
e_fog_hint: mc_util_serial::encode(&for_alice_txo.e_fog_hint),
txo: mc_util_serial::encode(&for_alice_txo),
subaddress_index: Some(0),
key_image: Some(mc_util_serial::encode(&for_alice_key_image)),
received_block_index: Some(12),
Expand Down Expand Up @@ -1538,7 +1535,7 @@ mod tests {
// have not yet assigned. At the DB layer, we accomplish this by
// constructing the output txos, then logging sent and received for this
// account.
let transaction_log = create_test_minted_and_change_txos(
let (transaction_log, tx_proposal) = create_test_minted_and_change_txos(
alice_account_key.clone(),
alice_account_key.subaddress(4),
33 * MOB,
Expand All @@ -1556,10 +1553,12 @@ mod tests {
assert_eq!(minted_txo.value as u64, 33 * MOB);
assert_eq!(change_txo.value as u64, 967 * MOB - Mob::MINIMUM_FEE);

add_block_with_db_txos(
add_block_with_tx_outs(
&mut ledger_db,
&wallet_db,
&[minted_txo.id.clone(), change_txo.id.clone()],
&[
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&[KeyImage::from(for_alice_key_image)],
&mut rng,
);
Expand Down Expand Up @@ -1754,7 +1753,7 @@ mod tests {
)
.unwrap();

let transaction_log = create_test_minted_and_change_txos(
let (transaction_log, tx_proposal) = create_test_minted_and_change_txos(
alice_account_key.clone(),
bob_account_key.subaddress(0),
72 * MOB,
Expand All @@ -1773,10 +1772,12 @@ mod tests {
assert_eq!(change_txo.value as u64, 928 * MOB - (2 * Mob::MINIMUM_FEE));

// Add the minted Txos to the ledger
add_block_with_db_txos(
add_block_with_tx_outs(
&mut ledger_db,
&wallet_db,
&[minted_txo.id.clone(), change_txo.id.clone()],
&[
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&[KeyImage::from(for_bob_key_image)],
&mut rng,
);
Expand Down Expand Up @@ -2093,7 +2094,7 @@ mod tests {

assert_eq!(txos.len(), 12);

let transaction_log = create_test_minted_and_change_txos(
let (transaction_log, _) = create_test_minted_and_change_txos(
src_account.clone(),
recipient,
1 * MOB,
Expand Down
16 changes: 14 additions & 2 deletions full-service/src/json_rpc/v2/api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,21 @@ where
)
.map_err(format_error)?;

let unverified_txos_encoded: Vec<String> = unverified_txos
let unverified_tx_results: Result<Vec<mc_transaction_core::tx::TxOut>, JsonRPCError> =
unverified_txos
.iter()
.map(
|(txo, _)| -> Result<mc_transaction_core::tx::TxOut, JsonRPCError> {
service.get_txo_object(&txo.id).map_err(format_error)
},
)
.collect::<Result<Vec<_>, JsonRPCError>>();

let unverified_tx_results = unverified_tx_results?;

let unverified_txos_encoded: Vec<String> = unverified_tx_results
.iter()
.map(|(txo, _)| hex::encode(&txo.txo))
.map(|txo_obj| hex::encode(&mc_util_serial::encode(txo_obj)))
.collect();

JsonCommandResponse::create_view_only_account_sync_request {
Expand Down
17 changes: 14 additions & 3 deletions full-service/src/service/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use mc_connection::{
};
use mc_crypto_keys::CompressedRistrettoPublic;
use mc_fog_report_validation::FogPubkeyResolver;
use mc_ledger_db::Ledger;
use mc_ledger_db::{Ledger, LedgerDB};
use mc_ledger_sync::NetworkState;
use mc_transaction_core::{
ring_signature::KeyImage,
Expand Down Expand Up @@ -176,8 +176,10 @@ where
fn get_txo_object(&self, txo_id_hex: &str) -> Result<TxOut, LedgerServiceError> {
let conn = self.get_conn()?;
let txo_details = Txo::get(txo_id_hex, &conn)?;

let txo: TxOut = mc_util_serial::decode(&txo_details.txo)?;
let txo = self.ledger_db.get_tx_out_by_index(
self.ledger_db
.get_tx_out_index_by_public_key(&txo_details.public_key()?)?,
)?;
Ok(txo)
}

Expand Down Expand Up @@ -295,3 +297,12 @@ where
Ok(self.ledger_db.get_block_index_by_tx_out_index(index)?)
}
}

pub fn get_tx_out_by_public_key(
ledger_db: &LedgerDB,
public_key: &CompressedRistrettoPublic,
) -> Result<TxOut, LedgerServiceError> {
let txo_index = ledger_db.get_tx_out_index_by_public_key(public_key)?;
let txo = ledger_db.get_tx_out_by_index(txo_index)?;
Ok(txo)
}
9 changes: 7 additions & 2 deletions full-service/src/service/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ mod tests {
account::AccountService,
address::AddressService,
confirmation_number::ConfirmationService,
ledger::get_tx_out_by_public_key,
transaction::{TransactionMemo, TransactionService},
transaction_log::TransactionLogService,
txo::TxoService,
Expand Down Expand Up @@ -477,8 +478,12 @@ mod tests {
.expect("Could not decode pubkey");
assert_eq!(receipt.public_key, txo_pubkey);
assert_eq!(receipt.tombstone_block, 23); // Ledger seeded with 12 blocks at tx construction, then one appended + 10
let txo: TxOut =
mc_util_serial::decode(&txos_and_statuses[0].0.txo).expect("Could not decode txo");
let public_key = txos_and_statuses[0]
.0
.public_key()
.expect("Could not get CompressedRistrettoPublic from txo");
let txo: TxOut = get_tx_out_by_public_key(&ledger_db, &public_key)
.expect("Could not get the txo from the ledger.");
assert_eq!(&receipt.amount, txo.get_masked_amount().unwrap());
assert_eq!(receipt.confirmation, confirmations[0].confirmation);
}
Expand Down
46 changes: 37 additions & 9 deletions full-service/src/service/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ mod tests {
transaction_log::TransactionLogService,
},
test_utils::{
add_block_from_transaction_log, add_block_to_ledger_db, get_test_ledger,
manually_sync_account, setup_wallet_service, MOB,
add_block_to_ledger_db, add_block_with_tx_outs, get_test_ledger, manually_sync_account,
setup_wallet_service, MOB,
},
util::b58::b58_encode_public_address,
};
Expand Down Expand Up @@ -770,7 +770,7 @@ mod tests {
.unwrap();

// Send a transaction from Alice to Bob
let (transaction_log, _associated_txos, _value_map, _tx_proposal) = service
let (transaction_log, _associated_txos, _value_map, tx_proposal) = service
.build_sign_and_submit_transaction(
&alice.id,
&[(
Expand All @@ -793,8 +793,22 @@ mod tests {
// workaround.
{
log::info!(logger, "Adding block from transaction log");
let conn = service.get_conn().unwrap();
add_block_from_transaction_log(&mut ledger_db, &conn, &transaction_log, &mut rng);
let key_images: Vec<KeyImage> = tx_proposal
.input_txos
.iter()
.map(|txo| txo.key_image.clone())
.collect();

// Note: This block doesn't contain the fee output.
add_block_with_tx_outs(
&mut ledger_db,
&[
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&key_images,
&mut rng,
);
}

manually_sync_account(
Expand Down Expand Up @@ -853,7 +867,7 @@ mod tests {
assert_eq!(bob_balance_pmob.unspent, 42000000000000);

// Bob should now be able to send to Alice
let (transaction_log, _associated_txos, _value_map, _tx_proposal) = service
let (_, _, _, tx_proposal) = service
.build_sign_and_submit_transaction(
&bob.id,
&[(
Expand All @@ -875,9 +889,23 @@ mod tests {
// workaround.

{
log::info!(logger, "Adding block from transaction log");
let conn = service.get_conn().unwrap();
add_block_from_transaction_log(&mut ledger_db, &conn, &transaction_log, &mut rng);
log::info!(logger, "Adding block from transaction proposal");
let key_images: Vec<KeyImage> = tx_proposal
.input_txos
.iter()
.map(|txo| txo.key_image.clone())
.collect();

// Note: This block doesn't contain the fee output.
add_block_with_tx_outs(
&mut ledger_db,
&[
tx_proposal.change_txos[0].tx_out.clone(),
tx_proposal.payload_txos[0].tx_out.clone(),
],
&key_images,
&mut rng,
);
}

manually_sync_account(
Expand Down
Loading