Skip to content

feat: replace String with IndexerInfo#2259

Draft
dorianvp wants to merge 3 commits intodevfrom
chore/indexer-info-upgrades
Draft

feat: replace String with IndexerInfo#2259
dorianvp wants to merge 3 commits intodevfrom
chore/indexer-info-upgrades

Conversation

@dorianvp
Copy link
Copy Markdown
Member

Fixes #2257.

@dorianvp dorianvp requested review from Oscar-Pepper and zancas March 16, 2026 18:30
"latest_block_height" => i.block_height
};
o.pretty(2)
let branch_id: u32 = i.consensus_branch_id.parse().unwrap();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you replace with an expect with a message to explain why this should not panic?

pub supports_transparent: bool,
pub chain_name: String,
pub sapling_activation_height: BlockHeight,
pub consensus_branch_id: BranchId,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

work is undergoing to remove zcash_protocol from the public API. I think this is ok for now and we can revisit this later when the replacement internal types and modules are in place.

vendor: i.vendor,
supports_transparent: i.taddr_support,
chain_name: i.chain_name,
sapling_activation_height: i.sapling_activation_height.try_into().unwrap(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we replace unwrap with atleast an expect or preferably returning an error now this fn returns a result?

supports_transparent: i.taddr_support,
chain_name: i.chain_name,
sapling_activation_height: i.sapling_activation_height.try_into().unwrap(),
consensus_branch_id: BranchId::try_from(branch_id).unwrap(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

chain_name: i.chain_name,
sapling_activation_height: i.sapling_activation_height.try_into().unwrap(),
consensus_branch_id: BranchId::try_from(branch_id).unwrap(),
latest_block_height: i.block_height.try_into().unwrap(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

pub sapling_activation_height: BlockHeight,
pub consensus_branch_id: BranchId,
pub latest_block_height: BlockHeight,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we add an impl:
impl From for json::JsonValue {
...
}

"sapling_activation_height" => info.sapling_activation_height.to_string(),
"consensus_branch_id" => u32::from(info.consensus_branch_id).to_string(),
"latest_block_height" => info.latest_block_height.to_string()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we move this code into the impl i mentioned below and call it here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i.e. Ok(info) => json::JsonValue::from(info).pretty(2)

@dorianvp dorianvp marked this pull request as draft March 26, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LightClient::do_info returns JSON-formatted String

2 participants