Skip to content

Commit d2edd5a

Browse files
0o-de-lallyVale McUrchinIsa McAccelerandoGabi Sforzando
authored
Exclude aborted txs (#21)
* test excluding unsuccessful transactions * info prints * patch tests * fix: vendor read_tx_chunk to resolve CI clippy failure * fix: run rust-tests on all PR branches and include reopened event * fix: enable workflows on all pull requests * fix: restore env indentation in cleanliness workflow * fix: restrict PR workflows to main branch on open/sync * fix: apply cargo fmt * feat: separate cargo check into distinct job before clippy * refactor: run format and clippy jobs in parallel * fix: resolve clippy errors and limit workflows to main branch * fix: apply cargo fmt --------- Co-authored-by: Vale McUrchin <vale_mcurchin@github.com> Co-authored-by: Isa McAccelerando <isa_mcaccelerando@github.com> Co-authored-by: Gabi Sforzando <gabi_sforzando@github.com> Co-authored-by: 0o-de-lally <>
1 parent fada7c4 commit d2edd5a

14 files changed

Lines changed: 196 additions & 69 deletions

‎.github/workflows/cleanliness.yaml‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,18 @@ on:
33
push:
44
tags: # only on releases, not RC, since we've tested already
55
- "[0-9]+.[0-9]+.[0-9]+"
6-
branches: ["**"] # glob pattern to allow slash /
6+
branches: ["main"]
77
pull_request:
8-
types:
9-
- opened
10-
- synchronize
11-
branches:
12-
- "release**"
13-
- "main**"
8+
types: [opened, synchronize]
9+
branches: [main]
10+
1411
env:
1512
DIEM_FORGE_NODE_BIN_PATH: ${{github.workspace}}/diem-node
1613
LIBRA_CI: 1
1714
MODE_0L: "TESTNET"
1815

1916
jobs:
20-
clippy:
17+
format:
2118
runs-on: ubuntu-latest
2219
steps:
2320
- uses: actions/checkout@v3
@@ -37,6 +34,20 @@ jobs:
3734
command: fmt
3835
args: --all -- --check
3936

37+
clippy:
38+
runs-on: ubuntu-latest
39+
steps:
40+
- uses: actions/checkout@v3
41+
42+
- name: setup env
43+
uses: ./.github/actions/build_env
44+
45+
- uses: Swatinem/rust-cache@v2
46+
with:
47+
shared-key: "forensic-db"
48+
cache-all-crates: true
49+
cache-on-failure: true
50+
4051
# TODO: clippy can share cache if build for tests is done prior
4152
# - name: build for cache
4253
# run: cargo build --tests --workspace

‎.github/workflows/rust-tests.yaml‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,10 @@ on:
44
push:
55
tags: # only on releases, not RC, since we've tested already
66
- "[0-9]+.[0-9]+.[0-9]+"
7-
branches: ["**"] # glob pattern to allow slash /
7+
branches: ["main"]
88
pull_request:
9-
types:
10-
- opened
11-
- synchronize
12-
branches:
13-
- "release**"
14-
- "main**"
9+
types: [opened, synchronize]
10+
branches: [main]
1511
schedule:
1612
- cron: "30 00 * * *"
1713

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ chrono = { version = "0.4.19", features = ["clock", "serde"] }
1616
clap = { version = "4.3.5", features = ["derive", "unstable-styles"] }
1717
diem-temppath = { git = "https://github.com/0LNetworkCommunity/diem.git", branch = "release" }
1818
diem-types = { git = "https://github.com/0LNetworkCommunity/diem.git", branch = "release" }
19+
diem-backup-cli = { git = "https://github.com/0LNetworkCommunity/diem.git", branch = "release" }
1920
diem-crypto = { git = "https://github.com/0LNetworkCommunity/diem.git", branch = "release" }
2021
env_logger = "^0.11"
2122
flate2 = "^1.0"

‎src/extract_transactions.rs‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::decode_entry_function::decode_entry_function_all_versions;
2+
use crate::read_tx_chunk::{load_chunk, load_tx_chunk_manifest};
23
use crate::scan::FrameworkVersion;
34
use crate::schema_transaction::{RelationLabel, UserEventTypes, WarehouseEvent, WarehouseTxMaster};
45
use anyhow::Result;
@@ -7,7 +8,6 @@ use diem_crypto::HashValue;
78
use diem_types::account_config::{NewBlockEvent, WithdrawEvent};
89
use diem_types::contract_event::ContractEvent;
910
use diem_types::{account_config::DepositEvent, transaction::SignedTransaction};
10-
use libra_storage::read_tx_chunk::{load_chunk, load_tx_chunk_manifest};
1111
use libra_types::move_resource::coin_register_event::CoinRegisterEvent;
1212
use log::{error, info, warn};
1313
use serde_json::json;
@@ -33,30 +33,14 @@ pub async fn extract_current_transactions(
3333
let mut user_txs: Vec<WarehouseTxMaster> = vec![];
3434
let mut events: Vec<WarehouseEvent> = vec![];
3535

36+
let mut count_excluded = 0;
37+
3638
for each_chunk_manifest in manifest.chunks {
3739
let chunk = load_chunk(archive_path, each_chunk_manifest).await?;
3840

3941
for (i, tx) in chunk.txns.iter().enumerate() {
40-
// TODO: unsure if this is off by one
41-
// perhaps reverse the vectors before transforming
42-
43-
// first increment the block metadata. This assumes the vector is sequential.
42+
// first collect the block metadata. This assumes the vector is sequential.
4443
if let Some(block) = tx.try_as_block_metadata() {
45-
// // check the epochs are incrementing or not
46-
// if epoch > block.epoch()
47-
// && round > block.round()
48-
// && timestamp > block.timestamp_usecs()
49-
// {
50-
// dbg!(
51-
// epoch,
52-
// block.epoch(),
53-
// round,
54-
// block.round(),
55-
// timestamp,
56-
// block.timestamp_usecs()
57-
// );
58-
// }
59-
6044
epoch = block.epoch();
6145
round = block.round();
6246
timestamp = block.timestamp_usecs();
@@ -66,6 +50,13 @@ pub async fn extract_current_transactions(
6650
.txn_infos
6751
.get(i)
6852
.expect("could not index on tx_info chunk, vectors may not be same length");
53+
54+
// only process successful transactions
55+
if !tx_info.status().is_success() {
56+
count_excluded += 1;
57+
continue;
58+
};
59+
6960
let tx_hash_info = tx_info.transaction_hash();
7061

7162
let tx_events = chunk
@@ -104,6 +95,8 @@ pub async fn extract_current_transactions(
10495
}
10596
}
10697

98+
info!("Excluding {} unsuccessful transactions", count_excluded);
99+
107100
Ok((user_txs, events))
108101
}
109102

‎src/json_rescue_v5_extract.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use libra_backwards_compatibility::{
2121
use anyhow::{anyhow, Context, Result};
2222
use diem_temppath::TempPath;
2323
use diem_types::account_address::AccountAddress;
24-
use log::trace;
24+
use log::{info, trace};
2525
use std::path::{Path, PathBuf};
2626

2727
/// The canonical transaction archives for V5 were kept in a different format as in v6 and v7.
@@ -32,9 +32,17 @@ pub fn extract_v5_json_rescue(
3232
) -> Result<(Vec<WarehouseTxMaster>, Vec<WarehouseEvent>, Vec<String>)> {
3333
let json = std::fs::read_to_string(one_json_file).context("could not read file")?;
3434

35-
let txs: Vec<TransactionViewV5> = serde_json::from_str(&json)
35+
let mut txs: Vec<TransactionViewV5> = serde_json::from_str(&json)
3636
.map_err(|e| anyhow!("could not parse JSON to TransactionViewV5, {:?}", e))?;
3737

38+
// remove any aborted txs
39+
let orig_len = txs.len();
40+
txs.retain(|t| t.vm_status.is_executed());
41+
let new_len = txs.len();
42+
if orig_len > new_len {
43+
info!("Excluding {} unsuccessful transactions", orig_len - new_len);
44+
};
45+
3846
decode_transaction_dataview_v5(&txs)
3947
}
4048

‎src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ pub mod load_exchange_orders;
1515
pub mod load_tx_cypher;
1616
pub mod neo4j_init;
1717
pub mod queue;
18+
pub mod read_tx_chunk;
1819
pub mod scan;
1920
pub mod schema_account_state;
2021
pub mod schema_exchange_orders;

‎src/read_tx_chunk.rs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use std::path::Path;
2+
3+
use anyhow::{anyhow, Context, Result};
4+
5+
use diem_backup_cli::backup_types::transaction::manifest::TransactionBackup;
6+
use diem_backup_cli::backup_types::transaction::manifest::TransactionChunk;
7+
use diem_backup_cli::utils::read_record_bytes::ReadRecordBytes;
8+
use diem_types::contract_event::ContractEvent;
9+
use diem_types::transaction::Transaction;
10+
use diem_types::transaction::TransactionInfo;
11+
use diem_types::write_set::WriteSet;
12+
use libra_backwards_compatibility::version_five::state_snapshot_v5::open_for_read;
13+
14+
/// read snapshot manifest file into object
15+
pub fn load_tx_chunk_manifest(path: &Path) -> anyhow::Result<TransactionBackup> {
16+
let s =
17+
std::fs::read_to_string(path).context(format!("Error: cannot read file at {:?}", path))?;
18+
19+
let map: TransactionBackup = serde_json::from_str(&s)?;
20+
21+
Ok(map)
22+
}
23+
24+
// similar to Loaded Chunk
25+
// diem/storage/backup/backup-cli/src/backup_types/transaction/restore.rs
26+
// The vectors below are OF THE SAME LENGTH
27+
// It is a table where for example, a tx without events will be an empty slot in the vector.
28+
pub struct TransactionArchiveChunk {
29+
pub manifest: TransactionChunk,
30+
pub txns: Vec<Transaction>,
31+
pub txn_infos: Vec<TransactionInfo>,
32+
pub event_vecs: Vec<Vec<ContractEvent>>,
33+
pub write_sets: Vec<WriteSet>,
34+
}
35+
36+
pub async fn load_chunk(
37+
archive_path: &Path,
38+
manifest: TransactionChunk,
39+
) -> Result<TransactionArchiveChunk> {
40+
let full_handle = archive_path
41+
.parent()
42+
.expect("could not read archive path")
43+
.join(&manifest.transactions);
44+
let handle_str = full_handle.to_str().unwrap();
45+
assert!(full_handle.exists(), "file does not exist");
46+
47+
let mut file = open_for_read(handle_str)
48+
.await
49+
.map_err(|e| anyhow!("snapshot chunk {:?}, {:?}", &handle_str, e))?;
50+
51+
let mut txns = Vec::new();
52+
let mut txn_infos = Vec::new();
53+
let mut event_vecs = Vec::new();
54+
let mut write_sets = Vec::new();
55+
56+
while let Some(record_bytes) = file.read_record_bytes().await? {
57+
let (txn, txn_info, events, write_set): (_, _, _, WriteSet) =
58+
bcs::from_bytes(&record_bytes)?;
59+
txns.push(txn);
60+
txn_infos.push(txn_info);
61+
event_vecs.push(events);
62+
write_sets.push(write_set);
63+
}
64+
65+
// the chunk is a table implements with vectors,
66+
// they should have the same length
67+
assert!(
68+
txns.len() == txn_infos.len()
69+
&& txn_infos.len() == event_vecs.len()
70+
&& event_vecs.len() == write_sets.len(),
71+
"transactions chunk have different vector length for txs, events, and writesets"
72+
);
73+
74+
// TODO: for purposes of explorer/warehouse do we want to do the full tx restore controller verifications
75+
76+
Ok(TransactionArchiveChunk {
77+
manifest,
78+
txns,
79+
txn_infos,
80+
event_vecs,
81+
write_sets,
82+
})
83+
}

‎src/unzip_temp.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use anyhow::{Context, Result};
22
use diem_temppath::TempPath;
33
use flate2::read::GzDecoder;
44
use glob::glob;
5-
// use libra_storage::read_tx_chunk::load_tx_chunk_manifest;
5+
// use crate::read_tx_chunk::load_tx_chunk_manifest;
66
use log::{info, warn};
77
use std::{
88
fs::File,

‎src/warehouse_cli.rs‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
json_rescue_v5_load,
1313
load::{ingest_all, try_load_one_archive},
1414
load_exchange_orders,
15-
neo4j_init::{self, get_credentials_from_env, PASS_ENV, URI_ENV, USER_ENV},
15+
neo4j_init::{self, get_credentials_from_env},
1616
scan::{scan_dir_archive, BundleContent, ManifestInfo},
1717
unzip_temp, util,
1818
};
@@ -294,22 +294,19 @@ pub async fn try_db_connection_pool(cli: &WarehouseCli) -> Result<Graph> {
294294
let db = match get_credentials_from_env() {
295295
Ok((uri, user, password)) => Graph::new(uri, user, password).await?,
296296
Err(_) => {
297-
if cli.db_uri.is_some() && cli.db_username.is_some() && cli.db_password.is_some() {
298-
Graph::new(
299-
cli.db_uri.as_ref().unwrap(),
300-
cli.db_username.as_ref().unwrap(),
301-
cli.db_password.as_ref().unwrap(),
302-
)
303-
.await?
304-
} else {
305-
println!("Must pass DB credentials, either with CLI args or environment variable");
306-
println!("call with --db-uri, --db-user, and --db-password");
307-
println!(
308-
"Alternatively export credentials to env variables: {}, {}, {}",
309-
URI_ENV, USER_ENV, PASS_ENV
310-
);
311-
bail!("could not get a db instance with credentials");
312-
}
297+
let uri = cli
298+
.db_uri
299+
.as_ref()
300+
.expect("Must pass --db-uri or set URI_ENV");
301+
let user = cli
302+
.db_username
303+
.as_ref()
304+
.expect("Must pass --db-user or set USER_ENV");
305+
let password = cli
306+
.db_password
307+
.as_ref()
308+
.expect("Must pass --db-password or set PASS_ENV");
309+
Graph::new(uri, user, password).await?
313310
}
314311
};
315312
Ok(db)

‎tests/test_extract_transactions.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ async fn test_extract_tx_from_archive() -> anyhow::Result<()> {
1818
async fn test_extract_v6_tx_from_archive() -> anyhow::Result<()> {
1919
let archive_path = support::fixtures::v6_tx_manifest_fixtures_path();
2020
let list = extract_current_transactions(&archive_path, &FrameworkVersion::V6).await?;
21-
22-
assert!(list.0.len() == 27);
21+
assert!(list.0.len() == 25);
2322
assert!(list.1.len() == 52);
2423

2524
Ok(())

0 commit comments

Comments
 (0)