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

test(grpc): GetMempoolTx and GetMempoolStream test #4537

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
34 changes: 34 additions & 0 deletions zebrad/tests/common/lightwalletd/send_transaction_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,40 @@ pub async fn run() -> Result<()> {
assert_eq!(response, expected_response);
}

// We test mempool grpc calls here as we know we have mempool transactions

// Transactions can be in the RPC queue, lets wait until they are retired
// before checking the mempool.
tokio::time::sleep(tokio::time::Duration::from_secs(120)).await;

// Call `GetMempoolStream`
let mut response = rpc_client
.get_mempool_stream(wallet_grpc::Empty {})
.await?
.into_inner();

// Make sure transactions are returned from the mmepool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Make sure transactions are returned from the mmepool
// Make sure transactions are returned from the mempool

let mut counter = 0;
while let Some(_txs) = response.message().await? {
counter += 1;
}
assert!(counter > 0);
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@teor2345 teor2345 Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion might be failing because lightwalletd is still syncing when the transactions are sent. When lightwalletd finishes syncing, it looks like it clears its internal mempool cache, so transactions are no longer available to clients.

(It should eventually re-check the mempool and get the transactions again. But the test will still be unreliable, because it doesn't wait.)

{"app":"lightwalletd","level":"info","msg":"Latest Block changed, clearing everything","time":"2022-08-01T05:30:30Z"}

https://github.com/ZcashFoundation/zebra/runs/7605046338?check_suite_focus=true#step:6:269

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened ticket #4894 to fix this.


// Call `GetMempoolTx`
let mut response = rpc_client
.get_mempool_tx(wallet_grpc::Exclude { txid: vec![] })
.await?
.into_inner();

// Make sure transactions are returned from the mmepool
let mut counter = 0;
while let Some(_txs) = response.message().await? {
counter += 1;
}
// TODO: It seems to be a bug in `GetMempoolTx` as the grpc is not returning transactions
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
// but we know there are transactions in the mempool as they are returned by `GetMempoolStream` test above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried calling the RPCs in the opposite order?
It's possible that they only provide new mempool transactions.

assert!(counter == 0);

Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions zebrad/tests/common/lightwalletd/wallet_grpc_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
//! - `GetBlockRange`: Covered.
//!
//! - `GetTransaction`: Covered.
//! - `SendTransaction`: Not covered and it will never be, it has its own test.
//! - `SendTransaction`: Not covered and it will never be, it has its own test in `send_transactions_tests.rs` file.
//!
//! - `GetTaddressTxids`: Covered.
//! - `GetTaddressBalance`: Covered.
//! - `GetTaddressBalanceStream`: Covered.
//!
//! - `GetMempoolTx`: Not covered.
//! - `GetMempoolStream`: Not covered.
//! - `GetMempoolTx`: Covered in `send_transactions_tests.rs` file
//! - `GetMempoolStream`: Covered in `send_transactions_tests.rs` file
//!
//! - `GetTreeState`: Covered.
//!
Expand Down