Skip to content

feat: add client method to estimate recent blockhash#121

Merged
lpahlavi merged 23 commits intomainfrom
lpahlavi/XC-317-client-method-to-estimate-blockhash
Jun 11, 2025
Merged

feat: add client method to estimate recent blockhash#121
lpahlavi merged 23 commits intomainfrom
lpahlavi/XC-317-client-method-to-estimate-blockhash

Conversation

@lpahlavi
Copy link
Contributor

@lpahlavi lpahlavi commented May 22, 2025

(XC-317): Add client method to estimate recent blockhash by repeatedly fetching getSlot and getBlock until either the maximum number of retries is reached, or a blockhash is obtained.

The new method is tested in end-to-end tests and solana_test_validator.rs, however further integration tests will follow in another PR due to the mock runtime currently not supporting mocking responses for sequential calls.

@lpahlavi lpahlavi force-pushed the lpahlavi/XC-317-client-method-to-estimate-blockhash branch from c3cc025 to c3d1ca3 Compare May 28, 2025 13:46
@lpahlavi lpahlavi force-pushed the lpahlavi/XC-317-client-method-to-estimate-blockhash branch from 1f569d1 to 9ba9444 Compare June 4, 2025 13:43
@lpahlavi lpahlavi force-pushed the lpahlavi/XC-317-client-method-to-estimate-blockhash branch from 407dedc to 01d568a Compare June 6, 2025 05:51
@lpahlavi lpahlavi marked this pull request as ready for review June 6, 2025 05:54
@lpahlavi lpahlavi requested a review from a team as a code owner June 6, 2025 05:54
@lpahlavi lpahlavi requested a review from gregorydemay June 6, 2025 05:55
Copy link
Contributor

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

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

thanks a lot for this PR @lpahlavi ! Only some minor comments from my side.

RpcError::from(e),
)),
}
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: continue appears quite often and some times it's easy to miss that one is missing. I would rewrite the code a bit to be safer. Something like

pub async fn send(self) -> Result<Hash, Vec<EstimateRecentBlockhashError>> {
        let mut errors = Vec::with_capacity(self.num_tries.into());
        while errors.len() <= self.num_tries.into() {
            match self.get_slot_then_get_block().await {
                Ok(hash) => return Ok(hash),
                Err(e) => {
                    errors.push(e);
                    continue;
                }
            }
        }
        Err(errors)
    }

    async fn get_slot_then_get_block(&self) -> Result<Hash, EstimateRecentBlockhashError> {
        let slot = self.get_slot().await?;
        match self.get_block(slot).await? {
            Some(block) => match Hash::from_str(&block.blockhash) {
                Ok(blockhash) => return Ok(blockhash),
                Err(e) => Err(EstimateRecentBlockhashError::GetBlockRpcError(
                    RpcError::from(e),
                )),
            },
            None => Err(EstimateRecentBlockhashError::MissingBlock(slot)),
        }
    }

    async fn get_slot(&self) -> Result<Slot, EstimateRecentBlockhashError> {
        todo!()
    }

    async fn get_block(
        &self,
        slot: Slot,
    ) -> Result<Option<UiConfirmedBlock>, EstimateRecentBlockhashError> {
        todo!()
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Done.

@lpahlavi lpahlavi requested a review from gregorydemay June 10, 2025 10:01
Copy link
Contributor

@gregorydemay gregorydemay left a comment

Choose a reason for hiding this comment

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

Very cool PR, thanks a lot for all the improvements @lpahlavi !

@lpahlavi lpahlavi merged commit 919477a into main Jun 11, 2025
17 of 18 checks passed
@lpahlavi lpahlavi deleted the lpahlavi/XC-317-client-method-to-estimate-blockhash branch June 11, 2025 14:39
@github-actions github-actions bot mentioned this pull request Jul 31, 2025
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.

2 participants