diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92fdf4e9..6f96b4b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,23 @@ jobs: components: rustfmt - run: cargo fmt --all -- --check + test: + name: Run Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - id: toolchain + uses: dtolnay/rust-toolchain@1.70 + - name: Cache dependencies + uses: actions/cache@v3 + with: # test cache key is different (adding test cfg is a re-compile) + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-test-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('**/Cargo.lock') }} + - run: cargo test --package electrs --lib --all-features + clippy: name: Linter runs-on: ubuntu-latest @@ -63,4 +80,4 @@ jobs: target key: ${{ runner.os }}-cargo-${{ steps.toolchain.outputs.cachekey }}-${{ hashFiles('**/Cargo.lock') }} - name: Clippy with Features = ${{ matrix.features }} - run: cargo clippy ${{ matrix.features }} + run: cargo clippy ${{ matrix.features }} -- -D warnings diff --git a/.gitignore b/.gitignore index 8dbb109c..4031dabb 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ target *.sublime* *~ *.pyc +.vscode diff --git a/scripts/checks.sh b/scripts/checks.sh index 92559fb9..aeda1261 100755 --- a/scripts/checks.sh +++ b/scripts/checks.sh @@ -56,3 +56,7 @@ cargo clippy $@ -q -F liquid TESTNAME="Running cargo clippy check electrum-discovery + liquid" echo "$TESTNAME" cargo clippy $@ -q -F electrum-discovery,liquid + +TESTNAME="Running cargo test with all features" +echo "$TESTNAME" +cargo test $@ -q --package electrs --lib --all-features diff --git a/src/electrum/discovery.rs b/src/electrum/discovery.rs index dfe9f5a1..09374870 100644 --- a/src/electrum/discovery.rs +++ b/src/electrum/discovery.rs @@ -531,20 +531,26 @@ mod tests { const PROTOCOL_VERSION: ProtocolVersion = ProtocolVersion::new(1, 4); #[test] + #[ignore = "This test requires external connection to server that no longer exists"] fn test() -> Result<()> { stderrlog::new().verbosity(4).init().unwrap(); + #[cfg(feature = "liquid")] + let testnet = Network::LiquidTestnet; + #[cfg(not(feature = "liquid"))] + let testnet = Network::Testnet; + let features = ServerFeatures { hosts: serde_json::from_str("{\"test.foobar.example\":{\"tcp_port\":60002}}").unwrap(), server_version: VERSION_STRING.clone(), - genesis_hash: genesis_hash(Network::Testnet), + genesis_hash: genesis_hash(testnet), protocol_min: PROTOCOL_VERSION, protocol_max: PROTOCOL_VERSION, hash_function: "sha256".into(), pruning: None, }; let discovery = Arc::new(DiscoveryManager::new( - Network::Testnet, + testnet, features, PROTOCOL_VERSION, false, diff --git a/src/new_index/mempool.rs b/src/new_index/mempool.rs index f393eb2b..ccd50fe1 100644 --- a/src/new_index/mempool.rs +++ b/src/new_index/mempool.rs @@ -422,7 +422,7 @@ impl Mempool { for txid in txids { let tx = self.txstore.get(&txid).expect("missing tx from txstore"); - let prevouts = match extract_tx_prevouts(tx, &txos, false) { + let prevouts = match extract_tx_prevouts(tx, &txos) { Ok(v) => v, Err(e) => { warn!("Skipping tx {txid} missing parent error: {e}"); diff --git a/src/new_index/query.rs b/src/new_index/query.rs index 604dc61f..99ff3d34 100644 --- a/src/new_index/query.rs +++ b/src/new_index/query.rs @@ -127,6 +127,7 @@ impl Query { .or_else(|| self.mempool().lookup_raw_txn(txid)) } + /// Not all OutPoints from mempool transactions are guaranteed to be included in the result pub fn lookup_txos(&self, outpoints: &BTreeSet) -> HashMap { // the mempool lookup_txos() internally looks up confirmed txos as well self.mempool().lookup_txos(outpoints) diff --git a/src/rest.rs b/src/rest.rs index 625f7643..d25da2e8 100644 --- a/src/rest.rs +++ b/src/rest.rs @@ -155,9 +155,9 @@ impl TransactionValue { blockid: Option, txos: &HashMap, config: &Config, - ) -> Self { - let prevouts = - extract_tx_prevouts(&tx, txos, true).expect("Cannot Err when allow_missing is true"); + ) -> Result { + let prevouts = extract_tx_prevouts(&tx, txos)?; + let vins: Vec = tx .input .iter() @@ -174,9 +174,9 @@ impl TransactionValue { let fee = get_tx_fee(&tx, &prevouts, config.network_type); - TransactionValue { + #[allow(clippy::unnecessary_cast)] + Ok(TransactionValue { txid: tx.txid(), - #[allow(clippy::unnecessary_cast)] version: tx.version as u32, locktime: tx.lock_time, vin: vins, @@ -185,7 +185,7 @@ impl TransactionValue { weight: tx.weight() as u32, fee, status: Some(TransactionStatus::from(blockid)), - } + }) } } @@ -519,6 +519,9 @@ fn ttl_by_depth(height: Option, query: &Query) -> u32 { }) } +/// Prepare transactions to be serialized in a JSON response +/// +/// Any transactions with missing prevouts will be filtered out of the response, rather than returned with incorrect data. fn prepare_txs( txs: Vec<(Transaction, Option)>, query: &Query, @@ -537,7 +540,7 @@ fn prepare_txs( let prevouts = query.lookup_txos(&outpoints); txs.into_iter() - .map(|(tx, blockid)| TransactionValue::new(tx, blockid, &prevouts, config)) + .filter_map(|(tx, blockid)| TransactionValue::new(tx, blockid, &prevouts, config).ok()) .collect() } @@ -1003,7 +1006,15 @@ fn handle_request( let mut tx = prepare_txs(vec![(tx, blockid)], query, config); - json_response(tx.remove(0), ttl) + if tx.is_empty() { + http_message( + StatusCode::INTERNAL_SERVER_ERROR, + "Transaction missing prevouts", + 0, + ) + } else { + json_response(tx.remove(0), ttl) + } } (&Method::GET, Some(&"tx"), Some(hash), Some(out_type @ &"hex"), None, None) | (&Method::GET, Some(&"tx"), Some(hash), Some(out_type @ &"raw"), None, None) => { diff --git a/src/util/transaction.rs b/src/util/transaction.rs index 09561967..5a4aacf1 100644 --- a/src/util/transaction.rs +++ b/src/util/transaction.rs @@ -85,25 +85,21 @@ pub fn is_spendable(txout: &TxOut) -> bool { pub fn extract_tx_prevouts<'a>( tx: &Transaction, txos: &'a HashMap, - allow_missing: bool, ) -> Result, errors::Error> { tx.input .iter() .enumerate() .filter(|(_, txi)| has_prevout(txi)) - .filter_map(|(index, txi)| { - Some(Ok(( + .map(|(index, txi)| { + Ok(( index as u32, - match (allow_missing, txos.get(&txi.previous_output)) { - (_, Some(txo)) => txo, - (true, None) => return None, - (false, None) => { - return Some(Err( - format!("missing outpoint {:?}", txi.previous_output).into() - )); + match txos.get(&txi.previous_output) { + Some(txo) => txo, + None => { + return Err(format!("missing outpoint {:?}", txi.previous_output).into()); } }, - ))) + )) }) .collect() }