Skip to content

fix: do not ignore response_size_estimate for getBlock#236

Merged
lpahlavi merged 1 commit intomainfrom
lpahlavi/do-not-ignore-response-size-estimate-for-get-block
Sep 26, 2025
Merged

fix: do not ignore response_size_estimate for getBlock#236
lpahlavi merged 1 commit intomainfrom
lpahlavi/do-not-ignore-response-size-estimate-for-get-block

Conversation

@lpahlavi
Copy link
Contributor

Previously, the response_size_estimate parameter in the RPC config was ignored when calling getBlock on the SOL RPC canister. This PR ensures that the value is properly respected.

@lpahlavi lpahlavi marked this pull request as ready for review September 23, 2025 10:31
@lpahlavi lpahlavi requested a review from a team as a code owner September 23, 2025 10:31
let providers = Providers::new(rpc_sources, consensus_strategy.clone(), now)?;
let max_response_bytes = Self::response_size_estimate(&params);
let max_response_bytes = config
.response_size_estimate
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we had a test for this, that explicitly configured parameters will be taken into account. We should extend this one or add a new one if there is none.

Copy link
Contributor Author

@lpahlavi lpahlavi Sep 24, 2025

Choose a reason for hiding this comment

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

@gregorydemay No, currently we don't have such a test (hence this bug going unnoticed). This also made me think there is a test missing. The tricky thing is we need to do this for each endpoint individually...

I was generally thinking of adding integration tests to ensure the response_size_estimate field in the RPC config is respected (e.g. by checking how many retries are performed in case of max_response_bytes exceeded errors), and potentially for the response_consensus field as well (e.g. by comparing the response between equality and threshold consensus strategies).

For special RPC config fields (i.e. GetSlotRpcConfig#rounding_error, GetRecentPrioritizationFeesRpcConfig#max_slot_rounding_error and GetRecentPrioritizationFeesRpcConfig#max_length), we should already have tests.

Since adding the tests is a bit of effort though, I did it in a separate PR. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Separate PR sounds good!

@lpahlavi lpahlavi merged commit f7d6f8c into main Sep 26, 2025
21 of 22 checks passed
@lpahlavi lpahlavi deleted the lpahlavi/do-not-ignore-response-size-estimate-for-get-block branch September 26, 2025 13:10
@lpahlavi lpahlavi mentioned this pull request Jan 12, 2026
lpahlavi added a commit that referenced this pull request Jan 12, 2026
## 🤖 New release

* `sol_rpc_types`: 3.0.0 -> 4.0.0
* `sol_rpc_canister`: 1.3.0
* `sol_rpc_client`: 3.0.0 -> 3.1.0

<details><summary><i><b>Changelog</b></i></summary><p>

## `sol_rpc_types`

<blockquote>

## [3.1.0] - 2026-01-12

### Changed

- Bump `ic-cdk` to v0.19.0
([#251](#251))
- Upgrade various dependencies
([#260](#260))

[3.1.0]:
https://github.com/dfinity/sol-rpc-canister/compare/3.0.0..3.1.0
</blockquote>

## `sol_rpc_canister`

<blockquote>

## [1.3.0] - 2026-01-12

### Changed

- Bump `ic-cdk` to v0.19.0
([#251](#251))
- Upgrade various dependencies
([#260](#260))

### Fixed

- Do not ignore `response_size_estimate` for `getBlock` endpoint
([#236](#236))

[1.3.0]:
sol_rpc_canister-v1.2.0...sol_rpc_canister-v1.3.0
</blockquote>

## `sol_rpc_client`

<blockquote>

## [4.0.0] - 2026-01-12

### Changed

- Use `ic_canister_runtime::Runtime` instead of local `Runtime` trait
([#248](#248))
- **BREAKING:** Bump `ic-cdk` to v0.19.0. See PR description for more
details on the breaking changes.
([#251](#251))
- Upgrade various dependencies
([#260](#260))

### Fixed

- **BREAKING:** Calculate default request cost before sending. See PR
description for more details on the breaking changes.
([#256](#256))

[4.0.0]:
https://github.com/dfinity/sol-rpc-canister/compare/3.0.0..4.0.0
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Louis Pahlavi <louis.pahlavi@dfinity.org>
Co-authored-by: Louis Pahlavi <louis.pahlavi@gmail.com>
@github-actions github-actions bot mentioned this pull request Feb 18, 2026
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