Skip to content

feat(ckbtc): Add get_utxos_cache to reduce latency of update_balance calls#4788

Merged
ninegua merged 12 commits intomasterfrom
paulliu/ckbtc-utxo-cache
Apr 23, 2025
Merged

feat(ckbtc): Add get_utxos_cache to reduce latency of update_balance calls#4788
ninegua merged 12 commits intomasterfrom
paulliu/ckbtc-utxo-cache

Conversation

@ninegua
Copy link
Member

@ninegua ninegua commented Apr 11, 2025

XC-314: ckBTC: Cache update_balance results

The call to the bitcoin canister's get_utxos method takes up the majority time of a update_balance call.
Sometimes a client may call update_balance repeatedly, which adds to the workload of the ckBTC minter canister.
The result of get_utxos would remain unchanged until the bitcoin canister receives a new block, which makes it a good candidate for caching.

This PR implements an internal get_utxos_cache that will reuse previous call results (if available and not expired) without making new get_utxos calls. Entries are expired based on a pre-configured get_utxos_cache_expiration setting.

Caching get_utxos results is safe for update_balance because:

  1. UTXOs are immutable data, and their values would never change.
  2. The set of UTXOs are unique to an address, and never shared between addresses.
  3. The update_balance call is looking for new UTXOs from the result, so re-using older results only delays the finding. In particular, cached results may include transaction outputs that could have already been spent, but this won't affect update_balance because only the minter could spend them, which means they are already known and not new.

@ninegua ninegua requested a review from a team as a code owner April 11, 2025 13:39
@github-actions github-actions bot added the feat label Apr 11, 2025
@ninegua ninegua marked this pull request as draft April 11, 2025 13:39
@ninegua ninegua removed the request for review from a team April 11, 2025 13:40
@ninegua ninegua added the CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 label Apr 15, 2025
@ninegua ninegua marked this pull request as ready for review April 15, 2025 06:11
@ninegua ninegua requested a review from a team April 15, 2025 06:11
@ninegua ninegua requested review from a team as code owners April 15, 2025 06:14
@ninegua ninegua requested a review from a team April 15, 2025 06:14
@ninegua ninegua requested review from a team as code owners April 15, 2025 06:14
@ninegua ninegua requested a review from a team April 15, 2025 06:14
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

If this pull request affects the behavior of any canister owned by
the Governance team, remember to update the corresponding
unreleased_changes.md file(s).

To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, and
supply one of the following reasons:

  1. Done.

  2. No canister behavior changes.

github-actions[bot]

This comment was marked as duplicate.

@ninegua ninegua changed the base branch from paulliu/ckbtc-cleanup-types to master April 15, 2025 06:15
@ninegua ninegua removed request for a team April 15, 2025 06:15
@ninegua ninegua removed the request for review from a team April 15, 2025 06:15
@ninegua ninegua force-pushed the paulliu/ckbtc-utxo-cache branch from c5fde3e to f5079ba Compare April 15, 2025 07:11
@ninegua ninegua dismissed github-actions[bot]’s stale review April 15, 2025 07:17

No NNS governance canister behavior changes.

@ninegua ninegua changed the title feat(ckbtc): Add a get_utxos_cache to reduce latency of update_balance calls feat(ckbtc): Add get_utxos_cache to reduce latency of update_balance calls Apr 15, 2025
Copy link
Contributor

@mducroux mducroux left a comment

Choose a reason for hiding this comment

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

Very nice PR @ninegua! Some minor comments on my side.

Copy link
Member Author

@ninegua ninegua left a comment

Choose a reason for hiding this comment

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

Thanks @mducroux for a careful review! I should have addressed all the concerns, please let me know if the changes look good to you. Thanks!

Copy link
Contributor

@mducroux mducroux left a comment

Choose a reason for hiding this comment

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

LGTM @ninegua! Thanks for addressing my comments

@ninegua ninegua requested review from a team, gregorydemay and lpahlavi April 15, 2025 15:28
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 @ninegua for this PR! Mostly some minor comments and understanding questions, otherwise code looks very good!

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 @ninegua for this PR!

@ninegua ninegua enabled auto-merge April 23, 2025 08:33
@ninegua ninegua added this pull request to the merge queue Apr 23, 2025
Merged via the queue into master with commit 9204403 Apr 23, 2025
21 checks passed
@ninegua ninegua deleted the paulliu/ckbtc-utxo-cache branch April 23, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 @cross-chain-team feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants