Send `Response::Nil` instead of sending empty `Message`s (#2791)

* Send `Response::Nil` instead of sending empty `Message`s

This matches `zcashd`'s behaviour more closely.

In most cases, the network layer filters these out already.
But this change makes the the inbound service code clearer.

* revert changes made to `AdvertiseTransactionIds` and `PushTransaction`

* remove newline

Co-authored-by: Conrado Gouvea <conrado@zfnd.org>
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
This commit is contained in:
teor 2021-09-24 05:58:00 +10:00 committed by GitHub
parent 30c9618207
commit 07e8926fd5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 27 additions and 6 deletions

View File

@ -260,7 +260,13 @@ impl Service<zn::Request> for Inbound {
let mut peers = peers.sanitized();
const MAX_ADDR: usize = 1000; // bitcoin protocol constant
peers.truncate(MAX_ADDR);
async { Ok(zn::Response::Peers(peers)) }.boxed()
if !peers.is_empty() {
async { Ok(zn::Response::Peers(peers)) }.boxed()
} else {
info!("ignoring `Peers` request from remote peer because our address book is empty");
async { Ok(zn::Response::Nil) }.boxed()
}
} else {
info!("ignoring `Peers` request from remote peer during network setup");
async { Ok(zn::Response::Nil) }.boxed()
@ -292,19 +298,30 @@ impl Service<zn::Request> for Inbound {
})
})
.try_collect::<Vec<_>>()
.map_ok(zn::Response::Blocks)
.map_ok(|blocks| {
if blocks.is_empty() {
zn::Response::Nil
} else {
zn::Response::Blocks(blocks)
}
})
.boxed()
}
zn::Request::TransactionsById(transactions) => {
if let Setup::Initialized { mempool, .. } = &mut self.network_setup {
let request = mempool::Request::TransactionsById(transactions);
mempool.clone().oneshot(request).map_ok(|resp| match resp {
mempool::Response::Transactions(transactions) if transactions.is_empty() => zn::Response::Nil,
mempool::Response::Transactions(transactions) => zn::Response::Transactions(transactions),
_ => unreachable!("Mempool component should always respond to a `TransactionsById` request with a `Transactions` response"),
})
.boxed()
} else {
async { Ok(zn::Response::Transactions(Default::default())) }.boxed()
info!(
transaction_hash_count = ?transactions.len(),
"ignoring `TransactionsById` request from remote peer during network setup"
);
async { Ok(zn::Response::Nil) }.boxed()
}
}
zn::Request::FindBlocks { known_blocks, stop } => {
@ -338,7 +355,7 @@ impl Service<zn::Request> for Inbound {
?transaction.id,
"ignoring `PushTransaction` request from remote peer during network setup"
);
async { Ok(zn::Response::TransactionIds(Default::default())) }.boxed()
async { Ok(zn::Response::Nil) }.boxed()
}
}
zn::Request::AdvertiseTransactionIds(transactions) => {
@ -354,7 +371,7 @@ impl Service<zn::Request> for Inbound {
info!(
"ignoring `AdvertiseTransactionIds` request from remote peer during network setup"
);
async { Ok(zn::Response::TransactionIds(Default::default())) }.boxed()
async { Ok(zn::Response::Nil) }.boxed()
}
}
zn::Request::AdvertiseBlock(hash) => {
@ -374,12 +391,16 @@ impl Service<zn::Request> for Inbound {
zn::Request::MempoolTransactionIds => {
if let Setup::Initialized { mempool, .. } = &mut self.network_setup {
mempool.clone().oneshot(mempool::Request::TransactionIds).map_ok(|resp| match resp {
mempool::Response::TransactionIds(transaction_ids) if transaction_ids.is_empty() => zn::Response::Nil,
mempool::Response::TransactionIds(transaction_ids) => zn::Response::TransactionIds(transaction_ids),
_ => unreachable!("Mempool component should always respond to a `TransactionIds` request with a `TransactionIds` response"),
})
.boxed()
} else {
async { Ok(zn::Response::TransactionIds(Default::default())) }.boxed()
info!(
"ignoring `MempoolTransactionIds` request from remote peer during network setup"
);
async { Ok(zn::Response::Nil) }.boxed()
}
}
zn::Request::Ping(_) => {