Skip to content

fix!: calculate default request cost before sending#256

Merged
lpahlavi merged 5 commits intomainfrom
lpahlavi/defi-2446-calculate-default-cycles-before-sending
Jan 12, 2026
Merged

fix!: calculate default request cost before sending#256
lpahlavi merged 5 commits intomainfrom
lpahlavi/defi-2446-calculate-default-cycles-before-sending

Conversation

@lpahlavi
Copy link
Contributor

@lpahlavi lpahlavi commented Dec 9, 2025

Instead of initializing RequestBuilder instances with a default cycles cost estimate, calculate the cycles cost estimate when sending the corresponding request. This ensures all request parameters are correctly taken into account for the cycles cost estimation.

BREAKING CHANGE: This change is breaking for the sol_rpc_client crate. The Request::cycles_mut method now returns an &mut Option<u128> instead of &mut u128 and the obsolete GetBlockRequestBuilder::update_cycles method is removed.

@lpahlavi lpahlavi marked this pull request as ready for review December 10, 2025 05:56
@lpahlavi lpahlavi requested a review from a team December 10, 2025 05:56
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 @lpahlavi ! Just a couple of nits, but otherwise LGTM!


impl<R> DefaultRequestCycles for GetSignaturesForAddressRequestBuilder<R> {
fn default_request_cycles(&self) -> u128 {
2_000_000_000 // TODO XC-338: Check heuristic
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is the TODO still relevant (also below)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not 100% certain so I would still maybe double-check this to be sure. I will do this in another PR so I can merge this one and include it in today's release.

Co-authored-by: gregorydemay <112856886+gregorydemay@users.noreply.github.com>
@lpahlavi lpahlavi changed the title fix: calculate default request cost before sending fix!: calculate default request cost before sending Jan 12, 2026
@lpahlavi lpahlavi merged commit c4612ad into main Jan 12, 2026
18 checks passed
@lpahlavi lpahlavi deleted the lpahlavi/defi-2446-calculate-default-cycles-before-sending branch January 12, 2026 11:04
@github-actions github-actions bot 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>
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