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.

Sign up for GitHub

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

Jump to bottom

suggestion to don't clone self in get_block #9044

Open
wants to merge 1 commit into
base: get-block
Choose a base branch
from
Open

suggestion to don't clone self in get_block #9044

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 12 additions & 7 deletions zebra-rpc/src/methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,16 @@ where

let mut state = self.state.clone();
let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY);
let self_clone = self.clone();

let network = self.network.clone();
let original_hash_or_height = hash_or_height.clone();

// If verbosity requires a call to `get_block_header`, resolve it here
let get_block_header_future = if matches!(verbosity, 1 | 2) {
Some(self.get_block_header(original_hash_or_height.clone(), Some(true)))
} else {
None
};
Comment on lines +757 to +761
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional/Nitpick/Subjective: This may actually be a little less readable, but it's tempting to use then() here.

Suggested change
let get_block_header_future = if matches!(verbosity, 1 | 2) {
Some(self.get_block_header(original_hash_or_height.clone(), Some(true)))
} else {
None
};
let get_block_header_future = matches!(verbosity, 1 | 2)
.then(|| self.get_block_header(original_hash_or_height.clone(), Some(true)));


async move {
let hash_or_height: HashOrHeight = hash_or_height.parse().map_server_error()?;

Expand All @@ -779,10 +786,8 @@ where
}),
_ => unreachable!("unmatched response to a block request"),
}
} else if matches!(verbosity, 1 | 2) {
let get_block_header_result: Result<GetBlockHeader> = self_clone
.get_block_header(original_hash_or_height, Some(true))
.await;
} else if let Some(get_block_header_future) = get_block_header_future {
let get_block_header_result: Result<GetBlockHeader> = get_block_header_future.await;

let GetBlockHeader::Object(block_header) = get_block_header_result? else {
panic!("must return Object")
Expand Down Expand Up @@ -846,7 +851,7 @@ where
unreachable!("unmatched response to a OrchardTree request");
};

let nu5_activation = NetworkUpgrade::Nu5.activation_height(&self_clone.network);
let nu5_activation = NetworkUpgrade::Nu5.activation_height(&network);

// This could be `None` if there's a chain reorg between state queries.
let orchard_tree =
Expand Down
Loading